Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: fix up GOT based relative relocations

["Liming Gao" <liming.gao@intel.com>]

Ack-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ard Biesheuvel
> Sent: Wednesday, September 4, 2019 10:22 PM
> To: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: fix up GOT based relative relocations
> 
> On Wed, 4 Sep 2019 at 05:01, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > On Wed, 4 Sep 2019 at 04:49, Leif Lindholm <leif.lindholm@linaro.org> 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 <ard.biesheuvel@linaro.org>
> > >
> > > 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.
> 
>