'use strict' is ignored on mips64el/riscv64 when compiled with gcc 12

[kapouer]

Version

18.6.0

Platform

SMP Debian 5.10.103-1 (2022-03-07) mips64 GNU/Linux

Subsystem

build

What steps will reproduce the bug?

Build node 18.6.0 with gcc 11.2.0 and 12.1.0 on mips64el (or riscv64 too), and compare results.

How often does it reproduce? Is there a required condition?

Always. Only gcc change between the two builds.

What is the expected behavior?

Same tests pass.

What do you see instead?

Several tests fail: parallel/test-crypto-sign-verify parallel/test-freeze-intrinsics parallel/test-tls-root-certificates parallel/test-vm-global-non-writable-properties parallel/test-vm-module-synthetic parallel/test-vm-strict-assign

Additional information

Attached are the log files.

gcc11.log.gz gcc12.log.gz

Note several things:

• node built on debian using several shared libs and some patches, not pristine source
• the only change between those two log files is the gcc version
• previously on a build machine with gcc 11, all tests were passing
• on the qemu mips64el machine, debugger and some network tests fail (in both cases, so only differences of failures are relevant).

There are some new warnings

In file included from /usr/include/c++/12/atomic:41,
                 from ../deps/v8/src/base/atomic-utils.h:10,
                 from ../deps/v8/src/common/globals.h:15,
                 from ../deps/v8/src/builtins/builtins.h:10,
                 from ../deps/v8/src/builtins/builtins-utils.h:9,
                 from ../deps/v8/src/builtins/builtins-utils-inl.h:8,
                 from ../deps/v8/src/builtins/builtins-arraybuffer.cc:5:
In member function 'std::__atomic_base<_IntTp>::__int_type std::__atomic_base<_IntTp>::load(std::memory_order) const [with _ITp = long unsigned int]',
    inlined from 'size_t v8::internal::BackingStore::byte_length(std::memory_order) const' at ../deps/v8/src/objects/backing-store.h:91:29,
    inlined from 'size_t v8::internal::JSArrayBuffer::GetByteLength() const' at ../deps/v8/src/objects/js-array-buffer-inl.h:59:42,
    inlined from 'size_t v8::internal::JSArrayBuffer::GetByteLength() const' at ../deps/v8/src/objects/js-array-buffer-inl.h:53:8,
    inlined from 'v8::internal::Object v8::internal::Builtin_Impl_SharedArrayBufferPrototypeGetByteLength(BuiltinArguments, Isolate*)' at ../deps/v8/src/builtins/builtins-arraybuffer.cc:478:51,
    inlined from 'v8::internal::Address v8::internal::Builtin_SharedArrayBufferPrototypeGetByteLength(int, Address*, Isolate*)' at ../deps/v8/src/builtins/builtins-arraybuffer.cc:465:1:
/usr/include/c++/12/bits/atomic_base.h:488:31: warning: 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
  488 |         return __atomic_load_n(&_M_i, int(__m));
      |                ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
In member function 'std::__atomic_base<_IntTp>::__int_type std::__atomic_base<_IntTp>::load(std::memory_order) const [with _ITp = long unsigned int]',
    inlined from 'size_t v8::internal::BackingStore::byte_length(std::memory_order) const' at ../deps/v8/src/objects/backing-store.h:91:29,
    inlined from 'size_t v8::internal::JSArrayBuffer::GetByteLength() const' at ../deps/v8/src/objects/js-array-buffer-inl.h:59:42,
    inlined from 'size_t v8::internal::JSArrayBuffer::GetByteLength() const' at ../deps/v8/src/objects/js-array-buffer-inl.h:53:8:
/usr/include/c++/12/bits/atomic_base.h:488:31: warning: 'long unsigned int __atomic_load_8(const volatile void*, int)' writing 8 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
  488 |         return __atomic_load_n(&_M_i, int(__m));
      |                ~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~

That some crypto fails points to a openssl runtime bug maybe. However that some vm tests fail too is confusing.

[kvakil]

I coudln't reproduce on amd64 with gcc 12, so I tagged this as mips/risc-v specific.

[kapouer]

log for riscv64

[bnoordhuis]

Looking at the test failures, it's either a gcc bug (unlikely) or UB in V8 that causes miscompiles with gcc-12.

The mips64el and riscv ports are maintained externally and don't get the same coverage as the upstream maintained ports so bugs like these are not too surprising.

Turning on sanitizers (ASan, TSan, etc.) may help track down the cause.

[kapouer]

Maybe those failures have nothing to do with gcc...

This doesn't throw on mips64el:

node --frozen-intrinsics -e "'use strict'; globalThis.globalThis = null;"

but does throw on amd64.

More precisely, on mips64el, it doesn't throw but doesn't set globalThis either. The flag is honored, just not the fact it should throw with "use strict".

[kapouer]

Changed the title: all the failing tests fail the same way on amd64 when removing "use strict".

[bnoordhuis]

Can you report that upstream? That's a bug in V8's mips port.

[kapouer]

https://bugs.chromium.org/p/v8/issues/detail?id=13437

[luyahan]

Hi～ I am maintain v8 for riscv. I have a question about this issue. I don't understand the function of this parameter frozen_intrinsics. It isn't parameter for v8 and I can't find any code in the src of node except its declaration and definition

src/node_options.cc

  AddOption("--frozen-intrinsics",
            "experimental frozen intrinsics support",
            &EnvironmentOptions::frozen_intrinsics,
            kAllowedInEnvvar);

src/node_options.h

  bool frozen_intrinsics = false;

[RaisinTen]

@luyahan it's true that it's not used inside src/ but we do have some code in lib/ that runs internal/freeze_intrinsics if the option is passed, see - https://github.com/nodejs/node/blob/d5d7a416c72a46a91dbe2daf14239a55143a52f8/lib/internal/process/pre_execution.js#L611-L615

[luyahan]

test on commit d5d7a416c72a46a91dbe2daf14239a55143a52f8 It's is ok

luyahan@plct-dev-7:~/source/node/out/Release $ qemu-riscv64 -L /home/luyahan/riscv64/sysroot/ ./node --frozen-intrinsics -e "'use strict'; globalThis.globalThis = null;"
[eval]:1
'use strict'; globalThis.globalThis = null;
                                    ^

TypeError: Cannot assign to read only property 'globalThis' of object '[object global]'
    at [eval]:1:37
    at Script.runInThisContext (node:vm:129:12)
    at Object.runInThisContext (node:vm:307:38)
    at node:internal/process/execution:83:21
    at [eval]-wrapper:6:24
    at runScript (node:internal/process/execution:82:62)
    at evalScript (node:internal/process/execution:104:10)
    at node:internal/main/eval_string:50:3

Node.js v20.0.0-pre

Maybe when V8 Release , the RV does not PORT upstream of a certain patch in time.

@kapouer Can you double check it? Thanks

[kapouer]

Using commit 78954846 and testing with ./out/Release/node --frozen-intrinsics -e "'use strict'; globalThis.globalThis = null;",

Correct error is thrown:

./configure --shared-zlib --shared-cares --shared-nghttp2 --shared-brotli --shared-openssl --openssl-use-def-ca-store --shared-libuv --with-intl=system-icu --dest-cpu=riscv64

No error thrown:

./configure --shared-zlib --shared-cares --shared-nghttp2 --shared-brotli --shared-openssl --openssl-use-def-ca-store --shared-libuv --shared --with-intl=system-icu --dest-cpu=riscv64

However, building node 18.13.0 with or without the --shared flag gives the same result.

[kapouer]

Or, maybe it's something with how "use strict" is activated: starting from https://github.com/nodejs/node/blob/e7504f2f5949c677fddb9b30efba85e03cdea2d9/deps/v8/include/v8-function-callback.h#L223-L230 hinting at IntToSmi in https://github.com/nodejs/node/blob/e7504f2f5949c677fddb9b30efba85e03cdea2d9/deps/v8/include/v8-internal.h#L154-L157 and then V8_31BIT_SMIS_ON_64BIT_ARCH and corresponding configure flag v8_enable_31bit_smis_on_64bit_arch, which is defined here: https://github.com/nodejs/node/blob/e7504f2f5949c677fddb9b30efba85e03cdea2d9/common.gypi#L105-L109

That would explain very well why I have an issue with two unrelated architectures.

[kapouer]

Building node 18 on riscv64 with --experimental-enable-pointer-compression, to check if that works around the issue.

[kapouer]

Result with node 18 is

g++-12 -o /home/kapouer/nodejs->18.13.0+dfsg1/out/Release/obj.target/v8_base_without_compiler/deps/v8/src/baseline/bytecode-offset-iterator.o >../deps/v8/src/baseline/bytecode-offset-iterator.cc '-D_GLIBCXX_USE_CXX11_ABI=1' '->DNODE_OPENSSL_CONF_NAME=nodejs_conf' '-DNODE_OPENSSL_CERT_STORE' '->DNODE_SHARED_BUILTIN_CJS_MODULE_LEXER_LEXER_PATH=/usr/share/nodejs/cjs-module-lexer/lexer.js' '->DNODE_SHARED_BUILTIN_CJS_MODULE_LEXER_DIST_LEXER_PATH=/usr/share/nodejs/cjs-module-lexer/dist/lexer.js' '->DNODE_SHARED_BUILTIN_UNDICI_UNDICI_PATH=/usr/share/nodejs/undici/undici-fetch.js' '->DNODE_SHARED_BUILTIN_ACORN_PATH=/usr/share/nodejs/acorn/dist/acorn.js' '->DNODE_SHARED_BUILTIN_ACORN_WALK_PATH=/usr/share/nodejs/acorn-walk/dist/walk.js' '-DV8_GYP_BUILD' '->DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64' '-DV8_COMPRESS_POINTERS' '->DV8_COMPRESS_POINTERS_IN_ISOLATE_CAGE' '-DV8_31BIT_SMIS_ON_64BIT_ARCH' '-D__STDC_FORMAT_MACROS' '->DV8_TARGET_ARCH_RISCV64' '-D__riscv_xlen=64' '-DCAN_USE_FPU_INSTRUCTIONS' '-DV8_HAVE_TARGET_OS' '->DV8_TARGET_OS_LINUX' '-DV8_EMBEDDER_STRING="-node.21"' '-DENABLE_DISASSEMBLER' '->DV8_PROMISE_INTERNAL_FIELD_COUNT=1' '-DOBJECT_PRINT' '-DV8_INTL_SUPPORT' '->DV8_ATOMIC_OBJECT_FIELD_WRITES' '-DV8_ENABLE_LAZY_SOURCE_POSITIONS' '-DV8_USE_SIPHASH' '->DV8_WIN64_UNWINDING_INFO' '-DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH' '->DV8_ENABLE_WEBASSEMBLY' '-DV8_ENABLE_JAVASCRIPT_PROMISE_HOOKS' '-DV8_ALLOCATION_FOLDING' '->DV8_ALLOCATION_SITE_TRACKING' '-DV8_SCRIPTORMODULE_LEGACY_LIFETIME' '->DV8_ADVANCED_BIGINT_ALGORITHMS' '-DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_STATIC' -I../deps/v8 ->I../deps/v8/include -I/home/kapouer/nodejs-18.13.0+dfsg1/out/Release/obj/gen/inspector-generated-output-root ->I../deps/v8/third_party/inspector_protocol -I/home/kapouer/nodejs-18.13.0+dfsg1/out/Release/obj/gen ->I/home/kapouer/nodejs-18.13.0+dfsg1/out/Release/obj/gen/generate-bytecode-output-root ->I../deps/v8/third_party/zlib -I../deps/v8/third_party/zlib/google -pthread -Wno-unused-parameter -fPIC -Wno-return->type -fno-strict-aliasing -O3 -fno-omit-frame-pointer -fdata-sections -ffunction-sections -O3 -fno-rtti -fno-exceptions ->std=gnu++17 -MMD -MF /home/kapouer/nodejs-18.13.0+dfsg1/out/Release/.deps//home/kapouer/nodejs->18.13.0+dfsg1/out/Release/obj.target/v8_base_without_compiler/deps/v8/src/baseline/bytecode-offset->iterator.o.d.raw -fPIC -g -fPIC -g -fPIC -g -fPIC -g -fPIC -g -c In file included from ../deps/v8/src/baseline/baseline->compiler.cc:51: ../deps/v8/src/baseline/riscv64/baseline-compiler-riscv64-inl.h: In member function 'void >v8::internal::baseline::BaselineCompiler::PrologueFillFrame()': ../deps/v8/src/baseline/riscv64/baseline-compiler-riscv64-inl.h:42:40: error: 'kPointerSize' was not declared in this scope 42 | __ masm()->Add64(sp, sp, Operand(-(kPointerSize * new_target_index))); | ^~~~~~~~~~~~ ../deps/v8/src/baseline/riscv64/baseline->compiler-riscv64-inl.h:52:40: error: 'kPointerSize' was not declared in this scope 52 | __ masm()->Add64(sp, sp, >Operand(-(kPointerSize * register_count))); | ^~~~~~~~~~~~ >../deps/v8/src/baseline/riscv64/baseline-compiler-riscv64-inl.h:57:40: error: 'kPointerSize' was not declared in this >scope 57 | __ masm()->Add64(sp, sp, Operand(-(kPointerSize * register_count))); | >^~~~~~~~~~~~

[kapouer]

@luyahan same tests are still failing with nodejs 20.12.2 on mips64el and loong64 https://buildd.debian.org/status/fetch.php?pkg=nodejs&arch=mips64el&ver=20.12.2%2Bdfsg-1&stamp=1713937710&raw=0 https://buildd.debian.org/status/fetch.php?pkg=nodejs&arch=loong64&ver=20.12.2%2Bdfsg-1&stamp=1713910684&raw=0

(for comparison, armhf build gives https://buildd.debian.org/status/fetch.php?pkg=nodejs&arch=armhf&ver=20.12.2%2Bdfsg-1&stamp=1713920235&raw=0)

[mhdawson]

@shipujin is this something you could look at from the loong64 perspective?

[shipujin]

@shipujin is this something you could look at from the loong64 perspective?

ok, i test this locally

Best regards

[shipujin]

https://buildd.debian.org/status/fetch.php?pkg=nodejs&arch=loong64&ver=20.12.2%2Bdfsg-1&stamp=1713910684&raw=0

Hi @kapouer @mhdawson , I looked at the debian loong64 nodejs compilation log and found:

cat build.log| grep "not ok"

not ok 456 parallel/test-crypto-sign-verify # TODO : Fix flaky test
not ok 794 parallel/test-freeze-intrinsics # TODO : Fix flaky test
not ok 1167 parallel/test-debugger-preserve-breaks # TODO : Fix flaky test
not ok 2879 parallel/test-tls-root-certificates # TODO : Fix flaky test
not ok 3054 parallel/test-vm-global-non-writable-properties # TODO : Fix flaky test
not ok 3056 parallel/test-vm-global-setter # TODO : Fix flaky test
not ok 3076 parallel/test-vm-module-synthetic # TODO : Fix flaky test
not ok 3093 parallel/test-vm-strict-assign # TODO : Fix flaky test
not ok 3426 sequential/test-cpu-prof-dir-worker # TODO : Fix flaky test

I guess the failed test item seems to be caused by the use of --openssl-no-asm, It is recommended to temporarily "PASS, FLAKY" in debian loong64

--- a/test/parallel/parallel.status                                                                                                                                                   [2/8]
+++ b/test/parallel/parallel.status
                                              
+[$arch==riscv64]                 
+test-crypto-sign-verify: PASS, FLAKY
+test-freeze-intrinsics: PASS, FLAKY
+test-debugger-preserve-breaks: PASS, FLAKY
+test-tls-root-certificates: PASS, FLAKY
+test-vm-global-non-writable-properties: PASS, FLAKY
+test-vm-global-setter: PASS, FLAKY   
+test-vm-module-synthetic: PASS, FLAKY
+test-vm-strict-assign: PASS, FLAKY
+
+
--- a/test/sequential/sequential.status
+++ b/test/sequential/sequential.status

+test-cpu-prof-dir-worker: PASS, FLAKY
+

[kapouer]

(Yes, I have marked them as PASS, FLAKY because it's easier to keep nodejs in loong64, even with test failures)

How is --openssl-no-asm enabled here ? openssl 3.2.1-3, the one used as shared lib in that case, seems to be compiled with asm support: https://buildd.debian.org/status/fetch.php?pkg=openssl&arch=loong64&ver=3.2.1-3&stamp=1712268679&raw=0

[shipujin]

(Yes, I have marked them as PASS, FLAKY because it's easier to keep nodejs in loong64, even with test failures)

How is --openssl-no-asm enabled here ? openssl 3.2.1-3, the one used as shared lib in that case, seems to be compiled with asm support: https://buildd.debian.org/status/fetch.php?pkg=openssl&arch=loong64&ver=3.2.1-3&stamp=1712268679&raw=0

I'm sorry, but I realize that debian-nodejs uses --shared-openssl, so it's probably not openssl-no-asm

Best regards

[shipujin]

Look the failed items are related to ssl, but the debian-nodejs build uses --shared-openssl, which is weird

Best regards