[PATCH] D85994: [LLD][PowerPC] Add check in LLD to produce an error for missing TLSGD/TLSLD

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp marked 2 inline comments as not done.
stefanp added inline comments.


================
Comment at: lld/ELF/Relocations.cpp:1330
+        // start.
+        const RelTy &prevRel = *(i - 2);
+        RelType prevType = prevRel.getType(config->isMips64EL);
----------------
MaskRay wrote:
> i-2 may go out of bounds.
Sorry, I'm probably missing something here.
I have checked that `(i-1)` is not `array.begin()`.  I assumed that was the way this could go out of bounds (by iterating before the start of the array). How else can this go out of bounds?


================
Comment at: lld/ELF/Relocations.cpp:1332
+        RelType prevType = prevRel.getType(config->isMips64EL);
+        if(prevRel.r_offset != rel.r_offset ||
+           (prevType != R_PPC64_TLSGD && prevType != R_PPC64_TLSLD))
----------------
NeHuang wrote:
> MaskRay wrote:
> > `if `
> Why do we also need `prevRel.r_offset != rel.r_offset`? Is it for the case that previous relocation type is `R_PPC64_TLSGD` or `R_PPC64_TLSLD` but with a wrong relocation offset?
> 
Yes, that is why we need it. Both of the relocations need to be in the same place as the call to `__tls_get_addr` and if we don't check the offsets we can technically have a situation where the TLSGD/TLSLD does come before the call but is not on the call.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85994/new/

https://reviews.llvm.org/D85994