From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-x241.google.com (mail-pa0-x241.google.com [IPv6:2607:f8b0:400e:c03::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D08591A1DF8 for ; Wed, 3 Aug 2016 13:55:06 -0700 (PDT) Received: by mail-pa0-x241.google.com with SMTP id cf3so14562758pad.2 for ; Wed, 03 Aug 2016 13:55:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=offblast-org.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=IuJNBzznYJGsNLGJw9TzTL6BDs8pHFekz4DvXv2JbQ0=; b=Vygf9ZzCRU1kY/Q+cQIecw9ZkdtUvgZKpEd9bNKjz41rVa+6gXx/Ujc85KJuUUl3Ze 5BiAUjclCmICbDgYT74cXvHiqG/syvfcAYunYIUdFobXXSJe1vMxkIQy24qwRP3KERWL J7fbfjLJAqxY53kIg1pIFlDkO2AhYcZpc4XluGw8jaQF5q0PpUWqvOBqH7OqSg3otn7c tosePkpqokWg9jOQEQFtdd0UakLuR/KbXSfBj2SETcQV/Hu0hFy/Sv+Y726SyhI5OSS1 hl5GhWXguqNjnho0FWWP620TzZf+IZpG8RS09ysTWBw4MXKnUYFX6z/7SGhNkyBWLEHS pjFw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=IuJNBzznYJGsNLGJw9TzTL6BDs8pHFekz4DvXv2JbQ0=; b=P+aO1dpoPpFlkFmefkn1rbS4oQU/v9nAuFqxdfFaM7ststINdNuvNo7CqyESv97/me /4RQYgnqRzsmeo0DltYYqlUMxg4s5wZqYmvCNdS8hjkOJo0LbQpVNC3teXq7+hXm9Bvy 6zL5n+fLpiGWACYyJNezK7E4XwpZtL6pAs593ZCN1m0C6nGkThWZV2kbdfh9msl+Pcrc UOXF/nNfyAUXplXNa+Fc+HOlOSeVK1SSsXCw2LvXB8q3Y6TIpkukrySFWa8gkbjK0eMQ q2ID2xNADWk/HGFFZZ/SKHn9uPdVatsxILTrjZllx0xBqAIHCtFpwKLyALptazGB4IrX QC3Q== X-Gm-Message-State: AEkoousxUBW7B3P6wx403K9dNSRkJBNfJtWqLCldfWa+OAL1i+saDCyT186X1Cty5yxQAw== X-Received: by 10.66.142.105 with SMTP id rv9mr587572pab.33.1470257706017; Wed, 03 Aug 2016 13:55:06 -0700 (PDT) Received: from [10.7.3.210] ([50.250.250.45]) by smtp.gmail.com with ESMTPSA id f84sm14720138pfd.87.2016.08.03.13.55.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Aug 2016 13:55:05 -0700 (PDT) To: Ard Biesheuvel , Jordan Justen References: <1467967364-11556-1-git-send-email-steven.shi@intel.com> <06C8AB66E78EE34A949939824ABE2B31033825EE@shsmsx102.ccr.corp.intel.com> <06C8AB66E78EE34A949939824ABE2B31033826FE@shsmsx102.ccr.corp.intel.com> <06C8AB66E78EE34A949939824ABE2B310338275F@shsmsx102.ccr.corp.intel.com> <06C8AB66E78EE34A949939824ABE2B3103383852@shsmsx102.ccr.corp.intel.com> <147025523821.2704.13575318169553931780@jljusten-ivb> <61A48C30-DDE7-40EF-B266-B06014C35991@linaro.org> Cc: "Shi, Steven" , "Kinney, Michael D" , edk2-devel-01 , "afish@apple.com" , "Gao, Liming" From: Nicolas Owens Message-ID: Date: Wed, 3 Aug 2016 13:55:02 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <61A48C30-DDE7-40EF-B266-B06014C35991@linaro.org> Subject: Re: [PATCH v2 2/7] BaseTools-GenFw:Add new x86_64 Elf relocation types for PIC/PIE code X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Aug 2016 20:55:07 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit On 08/03/2016 01:47 PM, Ard Biesheuvel wrote: > >> On 3 aug. 2016, at 22:13, Jordan Justen wrote: >> >> Where does this patch stand? I think Ard was asking for it to be >> split, but also wondering if it was really required... >> >> Some devs are still reporting unsupported relocation errors during the >> build. (mischief on irc, for example.) >> > > The patch is not correct in its current form. PLT relocations (0x4) can be handled ok, so we can split that off and merge it. The GOT based relocation handling needs non-trivial rework, and may currently create corrupt binaries. Which unhandled reloc types are being reported? currently, i see: "GenFw" -e UEFI_APPLICATION -o /home/mischief/src/efivim/edk2/Build/vim/DEBUG_GCC49/X64/vim/DEBUG/vim.efi /home/mischief/src/efivim/edk2/Build/vim/DEBUG_GCC49/X64/vim/DEBUG/vim.dll GenFw: ERROR 3000: Invalid make[1]: *** [/home/mischief/src/efivim/edk2/Build/vim/DEBUG_GCC49/X64/vim/DEBUG/vim.efi] Error 2 /home/mischief/src/efivim/edk2/Build/vim/DEBUG_GCC49/X64/vim/DEBUG/vim.dll unsupported ELF EM_X86_64 relocation 0x4. this is off edk2 master from about 2 days ago. > > >> >>> On 2016-08-02 05:00:43, Ard Biesheuvel wrote: >>> On 2 August 2016 at 13:40, Shi, Steven wrote: >>>>> >>>>> CoffAddFixup() must be used for absolute symbol references only. These >>>>> instructions contain relative symbol references, which are >>>>> recalculated in WriteSections64(). >>>>> >>>>> The only absolute symbol reference is the GOT entry for 'n', and your >>>>> code (in WriteRelocations64()) calculates the address of the GOT entry >>>>> (which is always in .text BTW) and adds a fixup for it, i.e., >>>>> >>>>> + CoffAddFixup( >>>>> + (UINT32)(UINTN)((UINT64) >>>>> mCoffSectionsOffset[RelShdr->sh_info] + GoTPcRelPtrOffset), >>>>> + EFI_IMAGE_REL_BASED_DIR64); >>>>> >>>>> This code adds a fixup to the PE/COFF .reloc section for the GOT entry >>>>> containing the address of 'n', and the instructions perform a IP >>>>> relative load of the contents of the GOT entry to retrieve the address >>>>> of 'n'. >>>>> >>>>> By adding two fixups, the PE/COFF loader will apply the load offset >>>>> twice, resulting in an incorrect value. >>>> OK, I get your point now. Yes, the current patch could generate multiple fixups for the same GOT relocation entry. How about we introduce a simple IsDuplicatedCoffFixup() to check whether a converting fixup offset is duplicated before we use CoffAddFixup() to really add it? If it is new, we add it, otherwise just skip it. >>> >>> That could work, but you have to be aware that fixups are best emitted >>> in the order they need to be applied in the binary, or it will become >>> very inefficient. (Please refer to the PE/COFF spec section that >>> explains the layout of the .reloc section) >>> >>> What it comes down to is that relocations are grouped by target page, >>> and for every place in the page that requires a relocation to be >>> applied, a 4 bit type is emitted followed by a 12-bit offset, which is >>> the offset into the current page. If you emit fixups for the current >>> instruction, followed by one for the GOT, it will basically take two >>> 'page switches' every time. >>> >>> So it would be better to simply emit the relocations, but introduce a >>> sorting pass that merges all duplicates as well. >>> >>> Thanks, >>> Ard. >>> _______________________________________________ >>> edk2-devel mailing list >>> edk2-devel@lists.01.org >>> https://lists.01.org/mailman/listinfo/edk2-devel