Bug 130156

Summary: Allocate the data section on the heap again for FTL on ARM64
Product: WebKit Reporter: Juergen Ributzka <juergen>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Critical CC: dbates, fpizlo, ggaren, oliver
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: iPhone / iPad   
OS: iOS 7.0   
Bug Depends on:    
Bug Blocks: 112840    
Attachments:
Description Flags
Patch ggaren: review+

Description Juergen Ributzka 2014-03-12 13:36:38 PDT
Revert the temporary workaround that allocated data section in executable memory. This is no longer required, because the MCJIT supports now the large code model for ARM64.
Comment 1 Juergen Ributzka 2014-03-12 13:38:17 PDT
Created attachment 226549 [details]
Patch
Comment 2 Geoffrey Garen 2014-03-12 23:42:26 PDT
Comment on attachment 226549 [details]
Patch

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

r=me

> Source/JavaScriptCore/ftl/FTLCompile.cpp:84
> +    // Allocate the GOT in the code section to make it reachable for all code.
> +    if (!strcmp(sectionName, "__got"))
> +        return mmAllocateCodeSection(opaqueState, size, alignment, sectionID, sectionName);

As a follow-up, we'll need to allocate the GOT outside executable memory -- otherwise we're still subject to "JIT spray" attacks.
Comment 3 Filip Pizlo 2014-03-13 07:12:42 PDT
(In reply to comment #2)
> (From update of attachment 226549 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=226549&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/ftl/FTLCompile.cpp:84
> > +    // Allocate the GOT in the code section to make it reachable for all code.
> > +    if (!strcmp(sectionName, "__got"))
> > +        return mmAllocateCodeSection(opaqueState, size, alignment, sectionID, sectionName);
> 
> As a follow-up, we'll need to allocate the GOT outside executable memory -- otherwise we're still subject to "JIT spray" attacks.

No it won't. The GOT is just a table of pointers. We control it entirely.
Comment 4 Filip Pizlo 2014-03-13 10:43:52 PDT
Comment on attachment 226549 [details]
Patch

Let's not land this yet.  We want to first verify the LLVM changes and give everyone time to start building with the new LLVM that has the code model changes.
Comment 5 Filip Pizlo 2014-04-16 16:30:50 PDT
Landed in http://trac.webkit.org/changeset/167397
Comment 6 Filip Pizlo 2014-04-29 14:45:05 PDT
*** Bug 129756 has been marked as a duplicate of this bug. ***