Bug 201664 - [JSC] CodeBlock::calleeSaveRegisters should not see half-baked JITData
Summary: [JSC] CodeBlock::calleeSaveRegisters should not see half-baked JITData
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-10 16:01 PDT by Yusuke Suzuki
Modified: 2019-09-10 21:24 PDT (History)
7 users (show)

See Also:

Attachments
Patch (3.30 KB, patch)
2019-09-10 16:07 PDT, Yusuke Suzuki 		tzagallo: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-09-10 16:01:41 PDT 
[JSC] CodeBlock::calleeSaveRegisters should not see half-baked JITData
Comment 1 Yusuke Suzuki 2019-09-10 16:07:34 PDT 
Created attachment 378505 [details]
Patch
Comment 2 Yusuke Suzuki 2019-09-10 16:07:36 PDT 
<rdar://problem/52126927>
Comment 3 Yusuke Suzuki 2019-09-10 16:09:18 PDT 
Comment on attachment 378505 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:25
> +        (JSC::CodeBlock::ensureJITDataSlow):

This crash exists so long time since previously we are seeing half-baked CodeBlock::m_calleeSaveRegisters instead of JITData.
Comment 4 Yusuke Suzuki 2019-09-10 16:18:19 PDT 
Committed r249740: <https://trac.webkit.org/changeset/249740>
Comment 5 Saam Barati 2019-09-10 20:58:50 PDT 
Comment on attachment 378505 [details]
Patch

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

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1349
> +    // But we should not see garbage pointer in that case. We ensure JITData::m_calleeSaveRegisters is initialized as nullptr before exposing it to BaselineJIT by store-store-fence.

do the compiler threads check for nullptr?

We're seeing this crash on ARM only?
Comment 6 Yusuke Suzuki 2019-09-10 21:24:47 PDT 
(In reply to Saam Barati from comment #5)
> Comment on attachment 378505 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=378505&action=review
> 
> > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1349
> > +    // But we should not see garbage pointer in that case. We ensure JITData::m_calleeSaveRegisters is initialized as nullptr before exposing it to BaselineJIT by store-store-fence.
> 
> do the compiler threads check for nullptr?

Yes, compiler thread is checking nullptr.

> 
> We're seeing this crash on ARM only?

Yes, this crash is happening only on ARM devices, because x86 offers TSO.
Theoretically, we can see x86 crash if clang emits the code storing JITData pointer to CodeBlock before null-ing that field.