Ack-by: Liming Gao <[email protected]> > -----Original Message----- > From: [email protected] [mailto:[email protected]] On Behalf Of Ard > Biesheuvel > Sent: Wednesday, September 4, 2019 10:22 PM > To: Leif Lindholm <[email protected]> > Cc: edk2-devel-groups-io <[email protected]>; Gao, Liming > <[email protected]> > Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: fix up GOT based > relative relocations > > On Wed, 4 Sep 2019 at 05:01, Ard Biesheuvel <[email protected]> wrote: > > > > On Wed, 4 Sep 2019 at 04:49, Leif Lindholm <[email protected]> wrote: > > > > > > On Tue, Sep 03, 2019 at 09:17:33PM -0700, Ard Biesheuvel wrote: > > > > We take great care to avoid GOT based relocations in EDK2 executables, > > > > primarily because they are pointless - we don't care about things like > > > > the CoW footprint or relocations that target read-only sections, and so > > > > GOT entries only bloat the binary. > > > > > > > > However, in some cases (e.g., when building the relocatable PrePi SEC > > > > module in ArmVirtPkg with the CLANG38 toolchain), we may end up with > > > > some GOT based relocations nonetheless, which break the build since > > > > GenFw does not know how to deal with them. > > > > > > > > The relocations emitted in this case are ADRP/LDR instruction pairs > > > > that are annotated as GOT based, which means that it is the linker's > > > > job to emit the GOT entry and tag it with an appropriate dynamic > > > > relocation that ensures that the correct absolute value is stored into > > > > the GOT entry when the executable is loaded. This dynamic relocation > > > > not visible to GenFw, and so populating the PE/COFF relocation section > > > > for these entries is non-trivial. > > > > > > > > Since each ADRP/LDR pair refers to a single symbol that is local to the > > > > binary (given that shared libraries are not supported), we can actually > > > > convert the ADRP/LDR pair into an ADRP/ADD pair that produces the symbol > > > > address directly rather than loading it from memory. This leaves the > > > > GOT entry in the binary, but since it is now unused, it is no longer > > > > necessary to emit a PE/COFF relocation entry for it. > > > > > > > > Signed-off-by: Ard Biesheuvel <[email protected]> > > > > > > This is a very neat fix. My only concern is that I am not able to > > > reproduce the issue on my local Buster with clang7 (default). Is it > > > reproducible with clang8? > > > > > > > I managed to reproduce it on Ubuntu Bionic with clang 6. It may also > > be related to the version of ld.gold or the LLVM gold plugin. > > > > You should be able to test this patch for correctness by stripping the > > no-pie/no-pic options from the GCC5 command line, and checking any > > produced .dll with readelf -r to see whether any GOT based relocations > > were emitted, and whether the resulting binary still runs. I will do > > the same locally. > > > > This worked for me, so I am fairly confident that this change is correct. > > Once we have this change in, I'll double check whether the produced > CLANG38 ArmVirtQemuKernel images work as expected. > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#46815): https://edk2.groups.io/g/devel/message/46815 Mute This Topic: https://groups.io/mt/33134949/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
