Skip to content

Remove trailing zero-size arrays from JSString#930

Merged
bnoordhuis merged 2 commits intoquickjs-ng:masterfrom
bnoordhuis:fix928
Feb 25, 2025
Merged

Remove trailing zero-size arrays from JSString#930
bnoordhuis merged 2 commits intoquickjs-ng:masterfrom
bnoordhuis:fix928

Conversation

@bnoordhuis
Copy link
Copy Markdown
Contributor

It's been reported that UBSan's -fsanitize=bounds-strict does not like empty arrays. Remove them and replace their uses with old school pointer arithmetic.

Fixes: #928


I had to remove function argument const-ness in a number of places. It was either that or introduce additional str8_c and str16_c functions, but that's both more work and more to remember.

If you think it's a good approach, I'll run benchmarks to see if it doesn't regress performance. (It shouldn't, but hey, compilers.)

It's been reported that UBSan's `-fsanitize=bounds-strict` does not
like empty arrays. Remove them and replace their uses with old school
pointer arithmetic.

Fixes: quickjs-ng#928
Copy link
Copy Markdown
Contributor

@saghul saghul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@bnoordhuis
Copy link
Copy Markdown
Contributor Author

For posterity, the build error is a false positive (buf is not uninitialized) but I don't understand why it would pop up now all of a sudden. Compilers, man.

Details
 In function ‘js_free_rt’,
    inlined from ‘js_free’ at /home/runner/work/quickjs/quickjs/quickjs.c:1521:5,
    inlined from ‘js_string_normalize’ at /home/runner/work/quickjs/quickjs/quickjs.c:42717:13:
/home/runner/work/quickjs/quickjs/quickjs.c:1427:23: error: ‘buf’ may be used uninitialized [-Werror=maybe-uninitialized]
 1427 |     s->malloc_size -= rt->mf.js_malloc_usable_size(ptr) + MALLOC_OVERHEAD;
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/quickjs/quickjs/quickjs.c: In function ‘js_string_normalize’:
/home/runner/work/quickjs/quickjs/quickjs.c:42683:15: note: ‘buf’ was declared here
42683 |     uint32_t *buf, *out_buf;
@bnoordhuis bnoordhuis merged commit 9d6e372 into quickjs-ng:master Feb 25, 2025
59 checks passed
@bnoordhuis bnoordhuis deleted the fix928 branch February 25, 2025 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants