From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web11.1404.1662793177771833277 for ; Fri, 09 Sep 2022 23:59:38 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=QfTiZAE2; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 3D87060BED for ; Sat, 10 Sep 2022 06:59:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A7B1AC433D7 for ; Sat, 10 Sep 2022 06:59:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1662793176; bh=jYKzX/M6N50Z7K5Gku0RvxaDT/07/5l/NnRKaUJqbh0=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=QfTiZAE2VrN3uUiA/TnperOGOCM9NDmQZJq5ChCINwasZ1gEeh8rrYM4+TUkLWv2w rHhqh3O8FDl6D4ZH2M+6qppXY6ZoBIAfkP/fmImIMH0VYYOvtkwMYHYn1PHMIJpiB+ LvKGOjteZN3cRezYNM0+qmZc7Eiwv8qKlTdPWqneOuWy5PbYUGgghuYYKdWvzJZDme qtbDn1o8Mb/54sdfwJC0NiNNel0cK+vemUvu43PQZoil0bZhZLRgJ7RQPW6/KNBKRg Wwe5As8UEx30E+dy/nHV9YidrAx+e5JA5DqfOuKChbzqQEDRf081qyTkzCUucSCiKC Mkj6rFmOnUzgw== Received: by mail-lj1-f182.google.com with SMTP id z20so4529495ljq.3 for ; Fri, 09 Sep 2022 23:59:36 -0700 (PDT) X-Gm-Message-State: ACgBeo3BdRRnqLTNosUWtv5CXyFFbYM/MHobhrTDQPfU79qpnoLtfULO L65NHzDyOUhbFl07GfdMnva1wnluvvjHUbDGN6A= X-Google-Smtp-Source: AA6agR6RiUnzEwEoqQXeN/oZpCCV97bb921abDW5TB/mzXKNTBYVW4BhsDZYOMqU38DpzQsqvfjmwvAjooFrtUQefI0= X-Received: by 2002:a2e:9988:0:b0:26a:d12c:3739 with SMTP id w8-20020a2e9988000000b0026ad12c3739mr4556169lji.291.1662793174643; Fri, 09 Sep 2022 23:59:34 -0700 (PDT) MIME-Version: 1.0 References: <20220821141637.2531871-1-ardb@kernel.org> <23dfba3f-62d0-f443-9de0-4e1339280528@bsdio.com> <2d307167-a475-8f75-e897-bd0a99f43c29@quicinc.com> In-Reply-To: From: "Ard Biesheuvel" Date: Sat, 10 Sep 2022 07:59:23 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references To: "Feng, Bob C" Cc: Leif Lindholm , Rebecca Cran , "devel@edk2.groups.io" , "Gao, Liming" , "Kinney, Michael D" Content-Type: text/plain; charset="UTF-8" On Wed, 7 Sept 2022 at 14:04, Feng, Bob C wrote: > > Acked-by: Bob Feng > Merged as #3322 Thanks all, > -----Original Message----- > From: Leif Lindholm > Sent: Tuesday, September 6, 2022 7:54 PM > To: Ard Biesheuvel ; Rebecca Cran > Cc: devel@edk2.groups.io; Feng, Bob C ; Gao, Liming ; Kinney, Michael D > Subject: Re: [edk2-devel] [PATCH] BaseTools/GenFw AARCH64: Convert more types of explicit GOT references > > On 2022-09-06 11:30, Ard Biesheuvel wrote: > > Bob, Liming, Leif: any thoughts? > > I'm happy with this. > (Spotted one typo - "ofset".) > > Acked-by: Leif Lindholm > > > On Sun, 21 Aug 2022 at 16:33, Rebecca Cran wrote: > >> > >> Tested-by: Rebecca Cran > >> > >> > >> On 8/21/22 08:16, Ard Biesheuvel wrote: > >>> Rebecca reports that builds of AArch64 DSCs that involve PIE linking > >>> when using ELF based toolchains are failing in some cases, resulting > >>> in an error message like > >>> > >>> bad definition for symbol '_GLOBAL_OFFSET_TABLE_'@0x72d8 or > >>> unsupported symbol type. For example, absolute and undefined symbols > >>> are not supported. > >>> > >>> The reason turns out to be that, while GenFw does carry some logic > >>> to convert GOT based symbol references into direct ones (which is > >>> always possible given that our ELF to PE/COFF conversion only > >>> supports fully linked executables), it does not support all possible > >>> combinations of relocations that the linker may emit to load symbol > >>> addresses from the GOT. > >>> > >>> In particular, when performing a non-LTO link on object code built > >>> with GCC using -fpie, we may end up with GOT based references such > >>> as the one below, where the address of the GOT itself is taken, and > >>> the ofset of the symbol in the GOT is reflected in the immediate > >>> offset of the subsequent LDR instruction. > >>> > >>> 838: adrp x0, 16000 > >>> 838: R_AARCH64_ADR_PREL_PG_HI21 _GLOBAL_OFFSET_TABLE_ > >>> 83c: ldr x0, [x0, #2536] > >>> 83c: R_AARCH64_LD64_GOTPAGE_LO15 _gPcd_BinaryPatch_PcdFdBaseAddress > >>> > >>> The reason that we omit GOT based symbol references when performing > >>> ELF to PE/COFF conversion is that the GOT is not described by static > >>> ELF relocations, which means that the ELF file lacks the metadata to > >>> generate the PE/COFF relocations covering the GOT table in the > >>> PE/COFF executable. Given that none of the usual motivations for > >>> using a GOT (copy on write footprint, shared libraries) apply to EFI > >>> executables in the first place, the easiest way around this is to > >>> convert all GOT based symbol address loads to PC relative ADR/ADRP instructions. > >>> > >>> So implement this handling for R_AARCH64_LD64_GOTPAGE_LO15 and > >>> R_AARCH64_LD64_GOTOFF_LO15 relocations as well, and turn the LDR > >>> instructions in question into ADR instructions that generate the > >>> address immediately. > >>> > >>> This leaves the reference to _GLOBAL_OFFSET_TABLE_ itself, which is > >>> what generated the error to begin with. Considering that this symbol > >>> is never referenced (i.e., it doesn't appear anywhere in the code) > >>> and is only meaningful in combination with R_*_GOT_* based > >>> relocations that follow it, we can just disregard any references to > >>> it entirely, given that we convert all of those followup relocations into direct references. > >>> > >>> Cc: Rebecca Cran > >>> Cc: Bob Feng > >>> Cc: Liming Gao > >>> Cc: "Kinney, Michael D" > >>> Cc: Leif Lindholm > >>> Signed-off-by: Ard Biesheuvel > >>> --- > >>> > >>> This patch can be tested using the following method: > >>> > >>> - add the following lines to > >>> ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf > >>> > >>> [BuildOptions] > >>> GCC:*_*_AARCH64_CC_FLAGS = -fpie > >>> GCC:*_*_AARCH64_DLINK_FLAGS = -Wl,-Bsymbolic,-pie > >>> > >>> - build ArmVirtPkg/ArmVirtQemuKernel.dsc in DEBUG mode using GCC49 > >>> (other combos might work as well) and observe the build failure > >>> > >>> - apply this patch and observe that the build failure is gone > >>> > >>> - boot the resulting image in QEMU using the -kernel ../QEMU_EFI.fd > >>> command line option and observe that the image boots as usual > >>> > >>> BaseTools/Source/C/GenFw/Elf64Convert.c | 35 ++++++++++++++++++++ > >>> 1 file changed, 35 insertions(+) > >>> > >>> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c > >>> b/BaseTools/Source/C/GenFw/Elf64Convert.c > >>> index 35e96dd05bc2..3173ca9280f4 100644 > >>> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c > >>> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c > >>> @@ -1305,6 +1305,22 @@ WriteSections64 ( > >>> Elf_Shdr *SymShdr; > >>> UINT8 *Targ; > >>> > >>> + // > >>> + // The _GLOBAL_OFFSET_TABLE_ symbol is not actually an absolute symbol, > >>> + // but carries the SHN_ABS section index for historical reasons. > >>> + // It must be accompanied by a R_*_GOT_* type relocation on a > >>> + // subsequent instruction, which we handle below, specifically to avoid > >>> + // the GOT indirection, and to refer to the symbol directly. This means > >>> + // we can simply disregard direct references to the GOT symbol itself, > >>> + // as the resulting value will never be used. > >>> + // > >>> + if (Sym->st_shndx == SHN_ABS) { > >>> + const UINT8 *SymName = GetSymName (Sym); > >>> + if (strcmp ((CHAR8 *)SymName, "_GLOBAL_OFFSET_TABLE_") == 0) { > >>> + continue; > >>> + } > >>> + } > >>> + > >>> // > >>> // Check section header index found in symbol table and get the section > >>> // header location. > >>> @@ -1448,6 +1464,23 @@ WriteSections64 ( > >>> switch (ELF_R_TYPE(Rel->r_info)) { > >>> INT64 Offset; > >>> > >>> + case R_AARCH64_LD64_GOTOFF_LO15: > >>> + case R_AARCH64_LD64_GOTPAGE_LO15: > >>> + // > >>> + // Convert into an ADR instruction that refers to the symbol directly. > >>> + // > >>> + Offset = Sym->st_value - Rel->r_offset; > >>> + > >>> + *(UINT32 *)Targ &= 0x1000001f; > >>> + *(UINT32 *)Targ |= ((Offset & 0x1ffffc) << (5 - 2)) | > >>> + ((Offset & 0x3) << 29); > >>> + > >>> + if (Offset < -0x100000 || Offset > 0xfffff) { > >>> + Error (NULL, 0, 3000, "Invalid", "WriteSections64(): %s failed to relax GOT based symbol reference - image is too big (>1 MiB).", > >>> + mInImageName); > >>> + break; > >>> + } > >>> + break; > >>> + > >>> case R_AARCH64_LD64_GOT_LO12_NC: > >>> // > >>> // Convert into an ADD instruction - see R_AARCH64_ADR_GOT_PAGE below. > >>> @@ -1686,6 +1719,8 @@ WriteRelocations64 ( > >>> case R_AARCH64_LDST128_ABS_LO12_NC: > >>> case R_AARCH64_ADR_GOT_PAGE: > >>> case R_AARCH64_LD64_GOT_LO12_NC: > >>> + case R_AARCH64_LD64_GOTOFF_LO15: > >>> + case R_AARCH64_LD64_GOTPAGE_LO15: > >>> // > >>> // No fixups are required for relative relocations, provided that > >>> // the relative offsets between sections have been > >>> preserved in >