Bug 129947 – Fix bugs in 32-bit Structure implementation

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Lam]

There were some bugs in the 32-bit Structures implementation landed for https://bugs.webkit.org/show_bug.cgi?id=123195 mostly in the form of not loading the Structure from JSCells before using them.  In some cases, this results in crashes.  In other cases, the bug results in incorrect behavior but has gone quietly undetected thus far.  This patch searches for uses of "Structure::" in the llint/, hit/, and dfg/ directories and ensure that the Structure was loaded from the JSCell in use.

[Mark Lam]

(In reply to comment #0)
> There were some bugs in the 32-bit Structures implementation landed for https://bugs.webkit.org/show_bug.cgi?id=123195 mostly in the form of not loading the Structure from JSCells before using them.  In some cases, this results in crashes.  In other cases, the bug results in incorrect behavior but has gone quietly undetected thus far.  This patch searches for uses of "Structure::" in the llint/, hit/, and dfg/ directories and ensure that the Structure was loaded from the JSCell in use.

"hit/" ==> "jit/".  Darn spelling auto-correct.

[Mark Lam]

<rdar://problem/16268223>

[Mark Lam]

Created attachment 226195 [details]
the patch: still needs a lot of testing.

[Mark Hahnenberg]

Comment on attachment 226195 [details]
the patch: still needs a lot of testing.

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

r=me with required changes.

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:4415
> +            GPRTemporary scratch(this);

This is not correct. You can't start allocating registers after you've already started doing control flow. You need to allocate this GPR up where the local/remote global object GPRs are allocated.

[Mark Lam]

Thanks.  Fix has been applied as suggested.  Landed in r165325: <http://trac.webkit.org/r165325>.