13.2.0 crashes when running uglify-js fuzzer

[alexlamsl]

• Version: 13.2.0
• Platform: Ubuntu & Windows

Node.js silently crashes when running the following:

$ git clone https://github.com/mishoo/UglifyJS2.git
$ cd UglifyJS2
$ node --max-old-space-size=2048 test/ufuzz
581 of Infinity
$

The process is expected to run indefinitely, as with 13.1.0 and all previous versions of Node.js. Instead with 13.2.0 it aborts without even returning a non-zero code.

[alexlamsl]

For completeness I've just tried 12.13.1 (was on 12.13.0) and it also works fine without crashing.

[targos]

I think this is fixed by #30582

No it's not fixed by that PR. It just took longer to crash.

[targos]

So that's probably a bug in V8 7.9. I'll try to run it with GDB.

[targos]

(gdb) bt
#0  0x00000009df5d3ede in ?? ()
#1  0x0000000000e041e6 in v8::internal::Runtime_StorePropertyWithInterceptor(int, unsigned long*, v8::internal::Isolate*) ()
#2  0x00000000013bc319 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit () at ../../deps/v8/../../deps/v8/src/builtins/base.tq:3629
#3  0x0000000001434f22 in Builtins_StaNamedPropertyWideHandler () at ../../deps/v8/../../deps/v8/src/builtins/base.tq:385
#4  0x00000000013483fb in Builtins_InterpreterEntryTrampoline () at ../../deps/v8/../../deps/v8/src/builtins/base.tq:412

/cc @nodejs/v8

[targos]

#
# Fatal error in ../../deps/v8/src/ic/ic.cc, line 2739
# Debug check failed: interceptor_holder->HasNamedInterceptor().
#
#
#
#FailureMessage Object: 0x7ffdb10211b0
Thread 1 "node" received signal SIGILL, Illegal instruction.
v8::base::OS::Abort () at ../../deps/v8/src/base/platform/platform-posix.cc:407
407         V8_IMMEDIATE_CRASH();
#2  0x000000000272d203 in v8::base::(anonymous namespace)::DefaultDcheckHandler (file=<optimized out>, line=<optimized out>, message=<optimized out>) at ../../deps/v8/src/base/logging.cc:57
57        V8_Fatal(file, line, "Debug check failed: %s.", message);
Missing separate debuginfos, use: dnf debuginfo-install libgcc-9.2.1-1.fc31.x86_64 libstdc++-9.2.1-1.fc31.x86_64
(gdb) bt
#0  v8::base::OS::Abort () at ../../deps/v8/src/base/platform/platform-posix.cc:407
#1  0x000000000272d1e9 in V8_Fatal (file=0x2de6720 "../../deps/v8/src/ic/ic.cc", line=2739, format=format@entry=0x4eac23a "Debug check failed: %s.") at ../../deps/v8/src/base/logging.cc:182
#2  0x000000000272d203 in v8::base::(anonymous namespace)::DefaultDcheckHandler (file=<optimized out>, line=<optimized out>, message=<optimized out>) at ../../deps/v8/src/base/logging.cc:57
#3  0x00000000014c8021 in v8::internal::__RT_impl_Runtime_StorePropertyWithInterceptor (args=..., isolate=isolate@entry=0x5d2e850) at ../../deps/v8/src/ic/ic.cc:2739
#4  0x00000000014c8547 in v8::internal::Runtime_StorePropertyWithInterceptor (args_length=5, args_object=0x7ffdb10216a0, isolate=0x5d2e850) at ../../deps/v8/src/ic/ic.cc:2719
#5  0x0000000001e6d720 in Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_NoBuiltinExit () at ../../deps/v8/src/builtins/builtins-async-iterator-gen.cc:267
#6  0x00000000020d7c94 in Builtins_StaNamedPropertyWideHandler () at ../../deps/v8/src/interpreter/interpreter-generator.cc:778

[targos]

Possibly introduced by v8/v8@2f1bc98

/cc @danelphic

[targos]

@danelphick

[verwaest]

Does your build include https://chromium-review.googlesource.com/c/v8/v8/+/1807356 ? If not, that might be the issue.

[targos]

@verwaest yes, it does include that commit. I can reproduce the crash with V8 master.

[verwaest]

Alternatively you try to revert that CL as well as https://chromium-review.googlesource.com/c/v8/v8/+/1696285. If that works there could be another bug in that CL.

Or did you bisect the issue to Dan's CL?

[targos]

No, I did not bisect.
Reverting just https://chromium-review.googlesource.com/c/v8/v8/+/1696285 is not trivial, there are conflicts in src/ic/accessor-assembler.cc and src/ic/ic.cc.

[verwaest]

https://chromium-review.googlesource.com/c/v8/v8/+/1930903 may fix it. (Took me a while to repro in debug mode :))

[targos]

I confirm the fix seems to work (no crash in over 10 minutes). Will it be backmerged to 7.9 ?

[bricss]

Still no-signal on this? 🙄

[verwaest]

Hey sorry for the late reply, but this fix only affects node so it made sense that you locally applied it. Thanks!

[alexlamsl]

@verwaest @targos thanks for sorting this out!