'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 -

node/lib/internal/process/pre_execution.js

Lines 611 to 615 in d5d7a41

	if (getOptionValue('--frozen-intrinsics')) {
	process.emitWarning('The --frozen-intrinsics flag is experimental',
	'ExperimentalWarning');
	require('internal/freeze_intrinsics')();
	}

[luyahan]

test on commit d5d7a41
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 7895484 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.