[PATCH] D28571: RuntimeDyldELF/AArch64: Implement basic support for PIC relocations

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 added inline comments.


================
Comment at: lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp:1111
+    } else if (RelType == ELF::R_AARCH64_LD64_GOT_LO12_NC) {
+      // Use GOT entry already created for us after processing
+      // R_AARCH64_ADR_GOT_PAGE. So we get current GOT offset and
----------------
peter.smith wrote:
> Am I correct in thinking that this is assuming that all R_AARCH64_LD64_GOT_LO12_NC to symbol S relocations immediately follow a R_AARCH64_ADR_GOT_PAGE to symbol S?
> 
> There isn't anything in the AArch64 ABI that guarantees this and compilers can generate code that doesn't. For example when compiled with the Linaro gcc 5.3.1 20160113 the source file
> 
> ```
> struct S {
>     int a;
>     int b;
> };
> int array[1024];
> struct S s;
> 
> int func(void)
> {
>     return s.a + s.b + array[0] + array[1023];
> }
> ```
> produces with aarch64-linux-gnu-gcc -c t.c -fpie -O3
> `
> Disassembly of section .text:
> 
> 0000000000000000 <func>:
>    0:	90000002 	adrp	x2, 8 <func+0x8>
> 			0: R_AARCH64_ADR_GOT_PAGE	s
>    4:	90000001 	adrp	x1, 1000 <func+0x1000>
> 			4: R_AARCH64_ADR_GOT_PAGE	array
>    8:	f9400042 	ldr	x2, [x2]
> 			8: R_AARCH64_LD64_GOT_LO12_NC	s
>    c:	f9400021 	ldr	x1, [x1]
> 			c: R_AARCH64_LD64_GOT_LO12_NC	array
>   10:	29400043 	ldp	w3, w0, [x2]
>   14:	b9400022 	ldr	w2, [x1]
>   18:	b94ffc21 	ldr	w1, [x1,#4092]
>   1c:	0b000060 	add	w0, w3, w0
>   20:	0b020000 	add	w0, w0, w2
>   24:	0b010000 	add	w0, w0, w1
>   28:	d65f03c0 	ret
>   2c:	d503201f 	nop
> `
> The LO12_NC relocations do not immediately follow GOT_PAGE
>  
> I don't know enough about the use cases of dlydelf to know if this is a problem, for example it may only need to link code that is guaranteed to have L012_NC following a  GOT_PAGE.
Thanks for looking at it. Yes, you're right about the assumption: I expect LO12_NC to immediately follow GOT_PAGE to keep things as simple as possible for the very first patch, otherwise I'd have to implement a mapping between symbol (section) and corresponding GOT entry. I'm using clang and I thought this is not an issue. However, now it looks like such mapping is worth doing.



Repository:
  rL LLVM

https://reviews.llvm.org/D28571