Bug 187255

Summary: [32-bit JSC tests] ASSERTION FAILED: !getDirect(offset) || !JSValue::encode(getDirect(offset))
Product: WebKit Reporter: Dawei Fenton (:realdawei) <realdawei>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, lforschler, mark.lam, msaboff, ryanhaddad, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=186989
Attachments:
Description Flags
proposed patch. saam: review+

Description Dawei Fenton (:realdawei) 2018-07-02 10:19:56 PDT 
The 32-bit JSC bot has been seeing 3900+ regressions since around June 23rd (r233121 - r233122)  Previously had been 25000+ regressions starting around June 18th (r232953 - r232954) 

Sample run:
https://build.webkit.org/builders/Apple%20High%20Sierra%2032-bit%20JSC%20%28BuildAndTest%29/builds/2220/steps/webkit-32bit-jsc-test/logs/stdio

slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: ASSERTION FAILED: !getDirect(offset) || !JSValue::encode(getDirect(offset))
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: /Volumes/Data/slave/highsierra-32bitJSC-debug/build/Source/JavaScriptCore/runtime/JSObjectInlines.h(335) : bool JSC::JSObject::putDirectInternal(JSC::VM &, JSC::PropertyName, JSC::JSValue, unsigned int, JSC::PutPropertySlot &)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 1   0x28e51b WTFCrash
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 2   0x3cc740 bool JSC::JSObject::putDirectInternal<(JSC::JSObject::PutMode)1>(JSC::VM&, JSC::PropertyName, JSC::JSValue, unsigned int, JSC::PutPropertySlot&)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 3   0x9d51be JSC::JSObject::putDirect(JSC::VM&, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 4   0xeb8d74 JSC::CommonSlowPaths::putDirectWithReify(JSC::VM&, JSC::ExecState*, JSC::JSObject*, JSC::PropertyName, JSC::JSValue, JSC::PutPropertySlot&, JSC::Structure**)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 5   0xeb9cca operationPutByIdDirectStrictOptimize
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 6   0x3149f2a3
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 7   0x3149f7f6
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 8   0x38d708 llint_entry
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 9   0x38d6b1 llint_entry
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 10  0x38d708 llint_entry
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 11  0x3875d0 vmEntryToJavaScript
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 12  0xe34089 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 13  0xe33526 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 14  0x1147132 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 15  0x12909f runWithOptions(GlobalObject*, CommandLine&, bool&)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 16  0xf9d0a jscmain(int, char**)::$_3::operator()(JSC::VM&, GlobalObject*, bool&) const
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 17  0xdf0ea int runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 18  0xdd880 jscmain(int, char**)
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 19  0xdd7a7 main
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: 20  0xa73f4611 start
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: test_script_14: line 2: 36453 Segmentation fault: 11  ( "$@" ../../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --validateExceptionChecks\=true --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --validateBytecode\=true --validateGraph\=true --useConcurrentJIT\=false --thresholdForJITAfterWarmUp\=100 --scribbleFreeCells\=true rest-parameter-allocation-elimination.js )
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.no-cjit: ERROR: Unexpected exit code: 139
Comment 1 Radar WebKit Bug Importer 2018-07-03 11:50:28 PDT 
<rdar://problem/41785257>
Comment 2 Mark Lam 2018-07-03 12:03:38 PDT 
Created attachment 344201 [details]
proposed patch.
Comment 3 Saam Barati 2018-07-03 12:15:57 PDT 
Comment on attachment 344201 [details]
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=344201&action=review

> Source/JavaScriptCore/ChangeLog:9
> +        The 32-bit JIT::emit_op_create_this() needs to initialize uninitialized properties

Do we really care about this assert on 32-bit since we don’t run concurrent JIT/GC?
Comment 4 Mark Lam 2018-07-03 12:19:15 PDT 
Thanks for the review.

(In reply to Saam Barati from comment #3)
> Comment on attachment 344201 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=344201&action=review
> 
> > Source/JavaScriptCore/ChangeLog:9
> > +        The 32-bit JIT::emit_op_create_this() needs to initialize uninitialized properties
> 
> Do we really care about this assert on 32-bit since we don’t run concurrent
> JIT/GC?

Good point.  I guess we don't care then, but it doesn't hurt to just have the code in parity with the 64-bit i.e. I won't make the change conditional on asserts being enabled.  I'll land the patch shortly.
Comment 5 Mark Lam 2018-07-03 12:21:35 PDT 
(In reply to Mark Lam from comment #4)
> (In reply to Saam Barati from comment #3)
> > Do we really care about this assert on 32-bit since we don’t run concurrent
> > JIT/GC?
> 
> Good point.  I guess we don't care then, but it doesn't hurt to just have
> the code in parity with the 64-bit i.e. I won't make the change conditional
> on asserts being enabled.  I'll land the patch shortly.

I'll also add a ChangeLog comment that this is only needed for an assertion, and not strictly needed because we son't useConcurrentGC on 32-bit.
Comment 6 Mark Lam 2018-07-03 12:26:28 PDT 
Landed in r233473: <http://trac.webkit.org/r233473>.