Re: [Tarantool-patches] [PATCH luajit 1/2] c: disable strcmp optimization in Valgrind build

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 4375 bytes --]

Hi, Maxim,

thanks for the patch!

The patch below is quite similar to the patch in OpenResty [1].

Have you used it?

1. 
https://github.com/openresty/luajit2/commit/6315a752274f3a4db6827b64788173f40733204e

On 26.06.2024 15:27, mandesero--- via Tarantool-patches wrote:
> From: mandesero<mandesero@gmail.com>
>
> ---
>   src/lj_str.c | 43 +++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 41 insertions(+), 2 deletions(-)
>
> diff --git a/src/lj_str.c b/src/lj_str.c
> index 321e8c4f..00cced03 100644
> --- a/src/lj_str.c
> +++ b/src/lj_str.c
> @@ -26,10 +26,29 @@ static LJ_AINLINE int str_fastcmp(const char *a, const char *b, MSize len)
>   
>   /* -- String helpers ------------------------------------------------------ */
>   
> +#if LUAJIT_USE_VALGRIND
> +int lj_str_cmp_no_opt(const char *a, const char *b, MSize len)
> +{
> +  for (MSize i = 0; i < len; i++)
> +    if (a[i] != b[i])
> +      return 1;
> +  return 0;
> +}
> +#endif
> +
>   /* Ordered compare of strings. Assumes string data is 4-byte aligned. */
>   int32_t LJ_FASTCALL lj_str_cmp(GCstr *a, GCstr *b)
>   {
>     MSize i, n = a->len > b->len ? b->len : a->len;
> +#if LUAJIT_USE_VALGRIND
> +  const uint8_t *sa = (const uint8_t *)strdata(a);
> +  const uint8_t *sb = (const uint8_t *)strdata(b);
> +
> +  for (i = 0; i < n; i++) {
> +    if (sa[i] != sb[i])
> +      return (uint8_t)sa[i] < (uint8_t)sb[i] ? -1 : 1;
> +  }
> +#else
>     for (i = 0; i < n; i += 4) {
>       /* Note: innocuous access up to end of string + 3. */
>       uint32_t va = *(const uint32_t *)(strdata(a)+i);
> @@ -46,6 +65,7 @@ int32_t LJ_FASTCALL lj_str_cmp(GCstr *a, GCstr *b)
>         return va < vb ? -1 : 1;
>       }
>     }
> +#endif
>     return (int32_t)(a->len - b->len);
>   }
>   
> @@ -53,6 +73,12 @@ int32_t LJ_FASTCALL lj_str_cmp(GCstr *a, GCstr *b)
>   static LJ_AINLINE int str_fastcmp(const char *a, const char *b, MSize len)
>   {
>     MSize i = 0;
> +#if LUAJIT_USE_VALGRIND
> +  for (i = 0; i < len; i++) {
> +    if (a[i] != b[i])
> +      return (uint8_t)a[i] < (uint8_t)b[i] ? -1 : 1;
> +  }
> +#else
>     lj_assertX(len > 0, "fast string compare with zero length");
>     lj_assertX((((uintptr_t)a+len-1) & (LJ_PAGESIZE-1)) <= LJ_PAGESIZE-4,
>   	     "fast string compare crossing page boundary");
> @@ -68,6 +94,7 @@ static LJ_AINLINE int str_fastcmp(const char *a, const char *b, MSize len)
>       }
>       i += 4;
>     } while (i < len);
> +#endif
>     return 0;
>   }
>   
> @@ -83,7 +110,11 @@ const char *lj_str_find(const char *s, const char *p, MSize slen, MSize plen)
>         while (slen) {
>   	const char *q = (const char *)memchr(s, c, slen);
>   	if (!q) break;
> +#if LUAJIT_USE_VALGRIND
> +  if (lj_str_cmp_no_opt(q+1, p, plen) == 0) return q;
> +#else
>   	if (memcmp(q+1, p, plen) == 0) return q;
> +#endif
>   	q++; slen -= (MSize)(q-s); s = q;
>         }
>       }
> @@ -232,8 +263,13 @@ GCstr *lj_str_new(lua_State *L, const char *str, size_t lenx)
>     } else {  /* Slow path: end of string is too close to a page boundary. */
>       while (o != NULL) {
>         GCstr *sx = gco2str(o);
> +#if LUAJIT_USE_VALGRIND
> +      if (sx->hash == h && sx->len == len && inc_collision_hard() &&
> +                      lj_str_cmp_no_opt(str, strdata(sx), len) == 0) {
> +#else
>         if (sx->hash == h && sx->len == len && inc_collision_hard() &&
>                         memcmp(str, strdata(sx), len) == 0) {
> +#endif
>   	/* Resurrect if dead. Can only happen with fixstring() (keywords). */
>   	if (isdead(g, o)) flipwhite(o);
>   	g->strhash_hit++;
> @@ -277,7 +313,11 @@ GCstr *lj_str_new(lua_State *L, const char *str, size_t lenx)
>   	} else {  /* Slow path: end of string is too close to a page boundary. */
>   	  while (o != NULL) {
>   	    GCstr *sx = gco2str(o);
> +#if LUAJIT_USE_VALGRIND
> +	    if (sx->hash == fh && sx->len == len && lj_str_cmp_no_opt(str, strdata(sx), len) == 0) {
> +#else
>   	    if (sx->hash == fh && sx->len == len && memcmp(str, strdata(sx), len) == 0) {
> +#endif
>   	      /* Resurrect if dead. Can only happen with fixstring() (keywords). */
>   	      if (isdead(g, o)) flipwhite(o);
>   	      g->strhash_hit++;
> @@ -323,5 +363,4 @@ void LJ_FASTCALL lj_str_free(global_State *g, GCstr *s)
>   {
>     g->strnum--;
>     lj_mem_free(g, s, sizestring(s));
> -}
> -
> +}
> \ No newline at end of file

[-- Attachment #2: Type: text/html, Size: 4975 bytes --]