Bug 187362

Summary: [32-bit JSC tests] ASSERTION FAILED: !butterfly->propertyStorage()[-I - 1].get() under JSC::ObjectInitializationScope::verifyPropertiesAreInitialized
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, realdawei, saam, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=186692
Attachments:
Description Flags
proposed patch. saam: review+

Description Ryan Haddad 2018-07-05 14:10:27 PDT 
The following assertion failure is seen with 858 tests on the 32-bit JSC bot:

ASSERTION FAILED: !butterfly->propertyStorage()[-i - 1].get()
./runtime/ObjectInitializationScope.cpp(89) : void JSC::ObjectInitializationScope::verifyPropertiesAreInitialized(JSC::JSObject *)
1   0x2a104b WTFCrash
2   0x134540e JSC::ObjectInitializationScope::verifyPropertiesAreInitialized(JSC::JSObject*)
3   0x134501a JSC::ObjectInitializationScope::~ObjectInitializationScope()
4   0x1345537 JSC::ObjectInitializationScope::~ObjectInitializationScope()
5   0x9ce262 JSC::createRegExpMatchesArray(JSC::VM&, JSC::JSGlobalObject*, JSC::JSString*, WTF::String const&, JSC::RegExp*, unsigned int, JSC::MatchResult&)
6   0x9cceb4 JSC::RegExpObject::execInline(JSC::ExecState*, JSC::JSGlobalObject*, JSC::JSString*)
7   0x139354e JSC::RegExpObject::exec(JSC::ExecState*, JSC::JSGlobalObject*, JSC::JSString*)
8   0x1395ebb JSC::regExpProtoFuncExec(JSC::ExecState*)
9   0xdbddb0e1
10  0x3a0238 llint_entry
11  0x3a0238 llint_entry
12  0x3a0238 llint_entry
13  0x39a100 vmEntryToJavaScript
14  0xe46c19 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
15  0xe460b6 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
16  0x1159e12 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
17  0x7309f runWithOptions(GlobalObject*, CommandLine&, bool&)
18  0x43d0a jscmain(int, char**)::$_3::operator()(JSC::VM&, GlobalObject*, bool&) const
19  0x290ea int runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
20  0x27880 jscmain(int, char**)
21  0x277a7 main
22  0xa73f4611 start

https://build.webkit.org/builders/Apple%20High%20Sierra%2032-bit%20JSC%20%28BuildAndTest%29/builds/2238
Comment 1 Ryan Haddad 2018-07-05 14:12:40 PDT 
This assert was added with https://trac.webkit.org/changeset/232951/webkit
Comment 2 Alexey Proskuryakov 2018-07-09 20:46:01 PDT 
Is this still happening?
Comment 3 Ryan Haddad 2018-07-10 09:21:31 PDT 
(In reply to Alexey Proskuryakov from comment #2)
> Is this still happening?
Yes.
Comment 4 Radar WebKit Bug Importer 2018-07-10 09:30:16 PDT 
<rdar://problem/42027210>
Comment 5 Mark Lam 2018-07-10 13:09:04 PDT 
Created attachment 344717 [details]
proposed patch.
Comment 6 Saam Barati 2018-07-10 13:29:53 PDT 
Comment on attachment 344717 [details]
proposed patch.

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

> Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp:91
> +#else
> +        return !value || !JSValue::encode(value);
> +#endif

Why are we even pretending this assertion does anything on 32-bit? Can't we just always return true, since we don't have a concurrentJIT/concurrentGC?
Comment 7 Mark Lam 2018-07-10 13:35:13 PDT 
(In reply to Saam Barati from comment #6)
> Comment on attachment 344717 [details]
> proposed patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=344717&action=review
> 
> > Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp:91
> > +#else
> > +        return !value || !JSValue::encode(value);
> > +#endif
> 
> Why are we even pretending this assertion does anything on 32-bit? Can't we
> just always return true, since we don't have a concurrentJIT/concurrentGC?

Good point. Let me rework the patch.
Comment 8 Mark Lam 2018-07-10 13:43:06 PDT 
(In reply to Mark Lam from comment #7)
> (In reply to Saam Barati from comment #6)
> > Comment on attachment 344717 [details]
> > proposed patch.
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=344717&action=review
> > 
> > > Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp:91
> > > +#else
> > > +        return !value || !JSValue::encode(value);
> > > +#endif
> > 
> > Why are we even pretending this assertion does anything on 32-bit? Can't we
> > just always return true, since we don't have a concurrentJIT/concurrentGC?
> 
> Good point. Let me rework the patch.

On second thought, I'll leave it as is.  Reasons:
1. This way, this code has parity with the 64-bit version (more or less).
2. While it is true that this assertion is a non-issue on 32-bit because it doesn't currently support the concurrent GC, I don't think we need to make it more difficult for anyone to implement / add support for the concurrent GC on 32-bit in the future if they wish.
Comment 9 Mark Lam 2018-07-10 13:49:14 PDT 
Thanks for the review.  Landed in r233697: <http://trac.webkit.org/r233697>.