Skip to content

Conversation

LocutusOfBorg
Copy link

@LocutusOfBorg LocutusOfBorg commented Jun 15, 2024

    build/src/njs_diyfp.o build/src/njs_dtoa.o build/src/njs_dtoa_fixed.o build/src/njs_str.o build/src/njs_strtod.o build/src/njs_murmur_hash.o build/src/njs_djb_hash.o build/src/njs_utf8.o build/src/njs_utf16.o build/src/njs_arr.o build/src/njs_rbtree.o build/src/njs_flathsh.o build/src/njs_trace.o build/src/njs_random.o build/src/njs_malloc.o build/src/njs_mp.o build/src/njs_sprintf.o build/src/njs_utils.o build/src/njs_chb.o build/src/njs_value.o build/src/njs_vm.o build/src/njs_vmcode.o build/src/njs_lexer.o build/src/njs_lexer_keyword.o build/src/njs_parser.o build/src/njs_variable.o build/src/njs_scope.o build/src/njs_generator.o build/src/njs_disassembler.o build/src/njs_module.o build/src/njs_extern.o build/src/njs_boolean.o build/src/njs_number.o build/src/njs_symbol.o build/src/njs_string.o build/src/njs_object.o build/src/njs_object_prop.o build/src/njs_array.o build/src/njs_json.o build/src/njs_function.o build/src/njs_regexp.o build/src/njs_date.o build/src/njs_error.o build/src/njs_math.o build/src/njs_array_buffer.o build/src/njs_typed_array.o build/src/njs_promise.o build/src/njs_encoding.o build/src/njs_iterator.o build/src/njs_async.o build/src/njs_builtin.o build/external/njs_regex.o build/src/njs_buffer.o build/external/njs_crypto_module.o build/external/njs_md5.o build/external/njs_sha1.o build/external/njs_sha2.o build/external/njs_webcrypto_module.o build/external/njs_fs_module.o build/external/njs_query_string_module.o build/build/njs_modules.o

cc -Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -Wl,-z,relro -Wl,-z,now -specs=/usr/share/dpkg/elf-package-metadata.specs -o build/njs -Isrc -Ibuild
-pipe -fPIC -fvisibility=hidden -O -W -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wmissing-prototypes -Werror -g -fexcess-precision=standard -g -O3 -ffile-prefix-map=/<>=. -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -fno-stack-clash-protection -fdebug-prefix-map=/<>=/usr/src/libnginx-mod-js-0.8.4-1 -fPIC -Wdate-time -D_FORTIFY_SOURCE=3 -g -O3 -ffile-prefix-map=/<>=. -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -fno-stack-clash-protection -fdebug-prefix-map=/<>=/usr/src/libnginx-mod-js-0.8.4-1
external/njs_shell.c
build/libnjs.a
-Wl,-Bsymbolic-functions -flto=auto -ffat-lto-objects -Wl,-z,relro -Wl,-z,now -specs=/usr/share/dpkg/elf-package-metadata.specs -lm -lpcre2-8 -lcrypto -ledit
In function ‘njs_utf8_decode’,
inlined from ‘njs_text_encoder_encode_into’ at src/njs_encoding.c:214:14:
src/njs_utf8.c:191:42: error: ‘ctx.codepoint’ may be used uninitialized [-Werror=maybe-uninitialized]
191 | ctx->codepoint = (ctx->codepoint << 6) | (c & 0x3F);
| ^
src/njs_encoding.c: In function ‘njs_text_encoder_encode_into’:
src/njs_encoding.c:169:27: note: ‘ctx.codepoint’ was declared here
169 | njs_unicode_decode_t ctx;
| ^
In function ‘njs_utf8_length’,
inlined from ‘njs_error_new’ at src/njs_error.c:39:14,
inlined from ‘njs_throw_error_va’ at src/njs_error.c:69:5:
src/njs_utf8.h:141:12: error: ‘buf’ may be used uninitialized [-Werror=maybe-uninitialized]
141 | return njs_utf8_stream_length(&ctx, p, len, 1, 1, NULL);
| ^
src/njs_utf8.c: In function ‘njs_throw_error_va’:
src/njs_utf8.c:354:1: note: by argument 2 of type ‘const u_char *’ to ‘njs_utf8_stream_length.constprop’ declared here
354 | njs_utf8_stream_length(njs_unicode_decode_t *ctx, const u_char *p, size_t len,
| ^
src/njs_error.c:61:14: note: ‘buf’ declared here
61 | u_char buf[NJS_MAX_ERROR_STR], *p;
| ^
lto1: all warnings being treated as errors
make[3]: *** [/tmp/ccCzf1An.mk:32: /tmp/cc3U6GC7.ltrans10.ltrans.o] Error 1
make[3]: *** Waiting for unfinished jobs....
lto1: all warnings being treated as errors
make[3]: *** [/tmp/ccCzf1An.mk:26: /tmp/cc3U6GC7.ltrans8.ltrans.o] Error 1
lto-wrapper: fatal error: make returned 2 exit status
compilation terminated.
/usr/bin/ld: error: lto-wrapper failed
collect2: error: ld returned 1 exit status

Proposed changes

I found on Ubuntu ppc64el, where lto and O3 are used, gcc to fail with some missing initialization issues.

Checklist

Before creating a PR, run through this checklist and mark each as complete:

  • I have read the CONTRIBUTING document
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes

@xeioex
Copy link
Contributor

xeioex commented Jun 20, 2024

Hi @LocutusOfBorg,

Thanks for the patch. What version of gcc it is? Older compilers sometimes tend to generate false positive warnings.
Because the memory zeroing is not free we prefer to avoid fixing the code in this way just to please a compiler.
But if I will be able to reproduce the issue I will see what we can do about it.

@xeioex
Copy link
Contributor

xeioex commented Jun 21, 2024

Hi @LocutusOfBorg,

Could you please check that xeioex@74e1349 fixes at least the part of the problem? It should fix issue reported within njs_throw_error_va()

@LocutusOfBorg
Copy link
Author

https://launchpadlibrarian.net/735312991/buildlog_ubuntu-oracular-ppc64el.libnginx-mod-js_0.8.4-1ubuntu3_BUILDING.txt.gz
Get:171 http://ftpmaster.internal/ubuntu oracular-proposed/main ppc64el gcc-13 ppc64el 13.2.0-25ubuntu1 [488 kB]
Get:172 http://ftpmaster.internal/ubuntu oracular-proposed/main ppc64el libgcc-13-dev ppc64el 13.2.0-25ubuntu1 [1580 kB]

This is the gcc-13 version

@LocutusOfBorg
Copy link
Author

Yes, one failure is gone now.

In function ‘njs_utf8_decode’,
    inlined from ‘njs_text_encoder_encode_into’ at src/njs_encoding.c:214:14:
src/njs_utf8.c:191:42: error: ‘ctx.codepoint’ may be used uninitialized [-Werror=maybe-uninitialized]
  191 |         ctx->codepoint = (ctx->codepoint << 6) | (c & 0x3F);
      |                                          ^
src/njs_encoding.c: In function ‘njs_text_encoder_encode_into’:
src/njs_encoding.c:169:27: note: ‘ctx.codepoint’ was declared here
  169 |     njs_unicode_decode_t  ctx;
      |                           ^
lto1: all warnings being treated as errors
@LocutusOfBorg
Copy link
Author

(and rebased my merge request without the content you fixed in a different way)

@xeioex
Copy link
Contributor

xeioex commented Jul 2, 2024

@LocutusOfBorg the problem should be fixed by now.

@xeioex
Copy link
Contributor

xeioex commented Jul 2, 2024

Committed as 8c7ade4.

@xeioex xeioex closed this Jul 2, 2024
@LocutusOfBorg
Copy link
Author

Hello, the patch in xeioex@74e1349 was not applied?

@xeioex
Copy link
Contributor

xeioex commented Sep 16, 2024

Hi @LocutusOfBorg,

Hello, the patch in xeioex@74e1349 was not applied?

It was committed as ed631d2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants