Bug 199684

Summary: Assigning object property twice in a loop breaks JSCore on powerpc64 and s390x
Product: WebKit Reporter: Dmitry Shachnev <mitya57>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: berto, bugs-noreply, cgarcia, mcatanzaro
Priority: P2    
Version: Other   
Hardware: PC   
OS: Linux   

Description Dmitry Shachnev 2019-07-10 13:58:29 PDT
Test case with WebKitGTK+:

#include <jsc/jsc.h>

int main() {
    JSCContext *context = jsc_context_new();
    const char *script =
        "for (var i = 0; i < 2; i++) {\n"
        "    var foo = {};\n"
        "    foo.bar = null;\n"
        "}\n"
        "888\n";
    JSCValue *value = jsc_context_evaluate(context, script, -1);
    gint32 value_int = jsc_value_to_int32(value);
    g_message("Value is %d\n", value_int);
    return 0;
}

Expected result: "Value is 888" is printed.

Instead it crashes on powerpc64 (little endian) and s390x. Here is a stacktrace from s390x:

#0  0x000003fffd8739f6 in JSC::LLInt::CLoop::execute(JSC::OpcodeID, void*, JSC::VM*, JSC::ProtoCallFrame*, bool) ()
    at DerivedSources/JavaScriptCore/LLIntAssembly.h:6044
#1  0x000003fffd88dd10 in vmEntryToJavaScript() ()
    at ../Source/JavaScriptCore/llint/LLIntThunks.cpp:108
#2  0x000003fffd85cb44 in JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) () at ../Source/JavaScriptCore/jit/JITCodeInlines.h:38
#3  0x000003fffd85cb44 in JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*) ()
    at ../Source/JavaScriptCore/interpreter/Interpreter.cpp:834
#4  0x000003fffd9fac82 in JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) ()
    at ../Source/JavaScriptCore/runtime/Completion.cpp:137
#5  0x000003fffd9fae50 in JSC::profiledEvaluate(JSC::ExecState*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
    () at ../Source/JavaScriptCore/runtime/Completion.cpp:153
#6  0x000003fffd649278 in JSEvaluateScript() ()
    at ../Source/JavaScriptCore/API/JSBase.cpp:70
#7  0x000003fffd613910 in evaluateScriptInContext() ()
    at ../Source/JavaScriptCore/API/glib/JSCContext.cpp:824
#8  0x000003fffd614e2a in jsc_context_evaluate_with_source_uri() ()
    at ../Source/JavaScriptCore/API/glib/JSCContext.cpp:847
#9  0x000003fffd614efa in jsc_context_evaluate() ()
    at ../Source/JavaScriptCore/API/glib/JSCContext.cpp:817
#10 0x0000000100000936 in main () at test_jscore.c:12

This stacktrace is from Debian unstable, WebKitGTK version is 2.24.3.

According to our CI logs, it regressed somewhere between 2.23.91 and 2.23.92.

See https://bugs.debian.org/931807 for my original report that has a more complicate test case.
Comment 1 Alberto Garcia 2019-07-12 07:11:03 PDT
Yeah, I can reproduce this simply by saving the following piece of code to a test.js file and running it with /usr/bin/jsc.

for (var i = 0; i < 2; i++) {
    var foo = {};
    foo.bar = null;
}
Comment 2 Alberto Garcia 2019-07-12 07:32:34 PDT
A slightly more detailed backtrace (using the test case from my previous comment):

JSC::LLInt::CLoop::execute (entryOpcodeID=JSC::llint_vm_entry_to_javascript, 
    executableAddress=0x3fffb6db9fc0 <JSC::LLInt::CLoop::execute(JSC::OpcodeID, void*, JSC::VM*, JSC::ProtoCallFrame*, bool)+26076>, vm=0x3fffb2b10010, protoCallFrame=0x3fffffffe1b0, 
    isInitializationPass=false) at DerivedSources/JavaScriptCore/LLIntAssembly.h:6047
6047        t2 = *CAST<int64_t*>(t2.i8p() + 32);                     // Source/JavaScriptCore/llint/LowLevelInterpreter64.asm:1368
(gdb) bt
#0  0x00003fffb6def1e8 in JSC::LLInt::CLoop::execute(JSC::OpcodeID, void*, JSC::VM*, JSC::ProtoCallFrame*, bool)
    (entryOpcodeID=JSC::llint_vm_entry_to_javascript, executableAddress=0x3fffb6db9fc0 <JSC::LLInt::CLoop::execute(JSC::OpcodeID, void*, JSC::VM*, JSC::ProtoCallFrame*, bool)+26076>, vm=0x3fffb2b10010, protoCallFrame=0x3fffffffe1b0, isInitializationPass=false) at DerivedSources/JavaScriptCore/LLIntAssembly.h:6047
#1  0x00003fffb6eb58dc in JSC::vmEntryToJavaScript(void*, JSC::VM*, JSC::ProtoCallFrame*)
    (executableAddress=0x3fffb6db9fc0 <JSC::LLInt::CLoop::execute(JSC::OpcodeID, void*, JSC::VM*, JSC::ProtoCallFrame*, bool)+26076>, vm=0x3fffb2b10010, protoCallFrame=0x3fffffffe1b0)
    at ../Source/JavaScriptCore/llint/LLIntThunks.cpp:108
#2  0x00003fffb6d8981c in JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) (protoCallFrame=0x3fffffffe1b0, vm=0x3fffb2b10010, this=0x1001569e0)
    at ../Source/JavaScriptCore/jit/JITCodeInlines.h:38
#3  0x00003fffb6d8981c in JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
    (this=0x10012aba0, source=..., callFrame=0x3fffb2690048, thisObj=0x3fffb2570280) at ../Source/JavaScriptCore/interpreter/Interpreter.cpp:834
#4  0x00003fffb7215b34 in JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
    (exec=0x3fffb2690048, source=..., thisValue=..., returnedException=...) at ../Source/JavaScriptCore/runtime/Completion.cpp:137
#5  0x000000010005fed0 in runWithOptions(GlobalObject*, CommandLine&, bool&) (globalObject=0x3fffb2690000, options=..., success=@0x3fffffffef03: true)
    at ../Source/JavaScriptCore/jsc.cpp:2584
#6  0x0000000100061e54 in <lambda(JSC::VM&, GlobalObject*, bool&)>::operator()(JSC::VM &, GlobalObject *, bool &) const (__closure=0x3ffffffff000, 
    vm=..., globalObject=0x3fffb2690000, success=@0x3fffffffef03: true) at ../Source/JavaScriptCore/jsc.cpp:3050
#7  0x0000000100063f08 in runJSC<jscmain(int, char**)::<lambda(JSC::VM&, GlobalObject*, bool&)> >(const CommandLine &, bool, const <lambda(JSC::VM&, GlobalObject*, bool&)> &)
    (options=..., isWorker=false, func=...) at ../Source/JavaScriptCore/jsc.cpp:2908
#8  0x0000000100061f38 in jscmain(int, char**) (argc=2, argv=0x3ffffffff508) at ../Source/JavaScriptCore/jsc.cpp:3043
#9  0x00000001000541d8 in main(int, char**) (argc=2, argv=0x3ffffffff508) at ../Source/JavaScriptCore/jsc.cpp:2408
Comment 3 Alberto Garcia 2019-07-17 01:57:39 PDT
I bisected this problem, jsc started to crash in r242469 (which is a cherry-pick of r242096 for the WebKitGTK 2.24 branch):

https://trac.webkit.org/changeset/242096/webkit
Comment 4 Carlos Garcia Campos 2019-07-17 02:58:35 PDT
(In reply to Alberto Garcia from comment #3)
> I bisected this problem, jsc started to crash in r242469 (which is a
> cherry-pick of r242096 for the WebKitGTK 2.24 branch):
> 
> https://trac.webkit.org/changeset/242096/webkit

hmm, but does it happen in trunk too?
Comment 5 Alberto Garcia 2019-07-17 05:14:53 PDT
(In reply to Carlos Garcia Campos from comment #4)
> hmm, but does it happen in trunk too?

WebKitGTK 2.25.2 does not build in ppc64el so I supposed that trunk
does not build either, but fortunately jsc does build fine.

So it seems that the problem was already fixed in r242215. I confirm
that if I cherry-pick that patch to the webkitgtk 2.24 branch the
problem disappears (in ppc64el at least).
Comment 6 Carlos Garcia Campos 2019-07-17 05:43:37 PDT
(In reply to Alberto Garcia from comment #5)
> (In reply to Carlos Garcia Campos from comment #4)
> > hmm, but does it happen in trunk too?
> 
> WebKitGTK 2.25.2 does not build in ppc64el so I supposed that trunk
> does not build either, but fortunately jsc does build fine.
> 
> So it seems that the problem was already fixed in r242215. I confirm
> that if I cherry-pick that patch to the webkitgtk 2.24 branch the
> problem disappears (in ppc64el at least).

Great, please add that one to the wiki for 2.24.4
Comment 7 Alberto Garcia 2019-07-17 05:50:24 PDT
(In reply to Carlos Garcia Campos from comment #6)
> Great, please add that one to the wiki for 2.24.4

Done, thanks!
Comment 8 Alberto Garcia 2019-07-17 05:58:56 PDT

*** This bug has been marked as a duplicate of bug 195181 ***
Comment 9 Michael Catanzaro 2019-07-17 08:26:09 PDT
Good job Berto!
Comment 10 Dmitry Shachnev 2019-07-17 10:54:00 PDT
> So it seems that the problem was already fixed in r242215. I confirm
> that if I cherry-pick that patch to the webkitgtk 2.24 branch the
> problem disappears (in ppc64el at least).

Nice to know, thanks a lot for digging that!