[PATCH] RuntimeDyldMachO should handle RIP-relative relocations correctly, and the corresponding tests should have coverage for this

[Filip Pizlo]

On May 14, 2013, at 5:13 PM, "Kaylor, Andrew" <andrew.kaylor at intel.com> wrote:

> Hi Filip,
> 
> You've got a cut-and-paste error in the ArchSupportsMCJITWithSmallCodeModel function.  It's searching the old data structure.

Ooops!  I will fix.

> 
> More importantly, I would have said that MCJIT doesn't exactly support small code on x86_64.  Strictly speaking, MCJIT does support it, but it relies on the memory manager to allocate memory for code and data in the lower 2GB of user address space.  At least on Linux and FreeBSD, that doesn't usually happen unless you've done something special to make it happen.

I don't think that's true.  I'm never seeing it allocate anything in the low 2GB, and yet it works.  I think it just relies on code and data being close enough to each other.  Currently, SectionMemoryManager appears to make this be true.

WebKit is definitely going to use the small memory model on X86_64: we ensure that all code on X86_64 is always allocated within a common 1GB pool of virtual memory.  We currently don't have the notion of "data" in WebKit - at least not what LLVM calls data - since global variables in JavaScript will be GC-allocated by us, and the LLVM IR will refer to them directly (i.e. a pointer immediate in the IR itself).  But we will add support for this to ensure that constant pools work right - though this will require some other work, since currently RuntimeDyldMachO thinks that constant pools are code (since they are __text sections).  Once this is fixed though, we will then ensure that data sections we allocate for LLVM are in an adjacent 1GB pool.

This is what I'm trying to get right.  The current WebKit JITs benefit from things like never having to use anything bigger than a 32-bit offset to access anything JIT-related.  It would be a regression if LLVM couldn't do it; and it appears that LLVM's MCJIT can do it with the small code model already.

If you think it's better, I would be happy to hack up a RTDyldMemoryManager subclass that provides a strong guarantee that code and data are within 2GB of each other, and then use that for my tests.  The point is, I'd like to make sure that behavior that WebKit relies on, that LLVM happens to provide, is covered by tests inside LLVM.

-Filip

> 
> -Andy
> 
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Filip Pizlo
> Sent: Sunday, May 12, 2013 6:04 PM
> To: llvm-commits at cs.uiuc.edu
> Subject: [PATCH] RuntimeDyldMachO should handle RIP-relative relocations correctly, and the corresponding tests should have coverage for this
> 
> The small code model on X86_64 involves using RIP-relative relocations, particularly for the constant pool.
> 
> The RuntimeDyldMachO doesn't understand these correctly.  In particular, it doesn't understand what offset immediate will be used in a RIP-relative reference to a local symbol as in:
> 
> 	.section	__TEXT,__literal8,8byte_literals
> 	.align	3
> LABEL:
>     .quad <blah>
> 	.section	__TEXT,__text,regular,pure_instructions
> # .... bunch of things things
> addsd LABEL(%rip), %xmm0
> 
> In MachO, the "LABEL(%rip)" will be transformed into an offset from the addsd instruction to where the compiler thought the __literal8 section will be placed (plus the offset of the immediate within the __literal8 section, but that offset is zero, in this example).  For example, the compiler may place the __literal8 section right after the __text section, in which case the offset immediate in the addsd instruction will correspond to the size of the __text section, minus the offset of the addsd instruction, and then plus whatever padding/alignment for the __literal8 section.
> 
> So, for example, if we then allocated the __literal8 section exactly after the __text section in the MCJIT, then we wouldn't want to change the offset in the addsd instruction.  More broadly, we would want to:
> 
> 1) subtract out the offset to the __literal8 section according to the MachO file (i.e. the offset from the referencing instruction - the addsd in this example - to the __literal8 section)
> 2) add in the true offset to the __literal8 section after both __text and __literal8 have addresses in target memory.
> 
> Note that after (1) we will have an offset from the start of  __literal8 to the constant that the code is actually referencing. Then after (2) we have the correct offset that the code should actually be using.
> 
> This allows for the __literal8 section to be placed anywhere we want, and also allows the compiler to convey specific offsets inside of constant pools.
> 
> This is different from other relocations; for example if we had used the large code model instead, then we would have a load immediate (for example "movabsq $LABEL, %rax"), where the $LABEL that is encoded in the MachO file will be an offset of the __literal8 section relative to the *start* of the module, rather than relative to the instruction that had the reference.
> 
> The RuntimeDyldMachO now only gets this part-way right.  It knows how to handle non-PC-relative relocations just fine.  For PC-relative ones, it knows how to do (2) above, but it doesn't understand how to do (1): it assumes that the code has an offset from the start of the module, rather than an offset from the referencing instruction.
> 
> The included patch fixes this bug.
> 
> I also include a separate patch that does some stuff to the tests, including adding coverage for small code model on x86_64.  It makes sense for a lot of JIT clients to use the small code model, and I believe it's good for LLVM to have test coverage for this.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130514/c0d28bbd/attachment.html>