From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22c.google.com (mail-wm0-x22c.google.com [IPv6:2a00:1450:400c:c09::22c]) (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 BD0BA1A1DFA for ; Wed, 3 Aug 2016 13:47:14 -0700 (PDT) Received: by mail-wm0-x22c.google.com with SMTP id o80so353306827wme.1 for ; Wed, 03 Aug 2016 13:47:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=references:mime-version:in-reply-to:content-transfer-encoding :message-id:cc:from:subject:date:to; bh=VY2hPcG+KqDXbELMHmYu6p3MR7u4PNYyINp5usHfLlM=; b=SG27BkkCr1JJ+DyjqoiOKzFP0eJQ1RFsGt2E30ok3nYUlUxDdsJsEO7wuehvztJS8N VMLvbHK8Xp6EbWaC6bVzB2yOd9IuG1LY5lQUNyEdrxxLadYHdxMurib1k6MnK9SV5DFX SaFmjxKPhMKYOSG1Kr7W8XcwM2HAIcBdOkJZc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:references:mime-version:in-reply-to :content-transfer-encoding:message-id:cc:from:subject:date:to; bh=VY2hPcG+KqDXbELMHmYu6p3MR7u4PNYyINp5usHfLlM=; b=DkeOHoEAGVN1A5nc7fs3twUuPgonZ86EZm3ofsq6khonzPN5pReg01r1gu+ALs8Hes u37VLNCraFahmIlyumWl8YadJxZhjKkHMfprjjWP+RkGE2isVA9xS4D78bZxdJXDhbtr Wa1SC5HouKs01dPXHPXbYqZL4WQ6X83gvaERB321x5OMOJZPpstD4uetpZM5nybt1dgN j6T6BOW7E5yWmtDbmVUoWXbvtCw65KnmMhJn2UUiSitZL3dadWCdCgWa92bsbaBv+rmA lAMfDCQjTgJCuo84blHcHmH4+BSxXZvVoS9kdL/68/hmq8R8nWKcilY8Yjy5l7zkkiz6 6pfw== X-Gm-Message-State: AEkoouuZlWYpVMlk/gvQuKb2CXJupwb9OpOTPx6ZjprWvo/a4goCz+7lcnkfw/oU5VquRvEa X-Received: by 10.28.29.215 with SMTP id d206mr27498293wmd.75.1470257233023; Wed, 03 Aug 2016 13:47:13 -0700 (PDT) Received: from [192.168.1.34] (3.red-81-34-118.dynamicip.rima-tde.net. [81.34.118.3]) by smtp.gmail.com with ESMTPSA id va3sm9425758wjb.18.2016.08.03.13.47.11 (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 03 Aug 2016 13:47:11 -0700 (PDT) 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> Mime-Version: 1.0 (1.0) In-Reply-To: <147025523821.2704.13575318169553931780@jljusten-ivb> Message-Id: <61A48C30-DDE7-40EF-B266-B06014C35991@linaro.org> Cc: "Shi, Steven" , "Kinney, Michael D" , edk2-devel-01 , "afish@apple.com" , "Gao, Liming" , mischief@offblast.org X-Mailer: iPhone Mail (13G34) From: Ard Biesheuvel Date: Wed, 3 Aug 2016 22:47:09 +0200 To: Jordan Justen 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:47:15 -0000 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable > On 3 aug. 2016, at 22:13, Jordan Justen wrote:= >=20 > Where does this patch stand? I think Ard was asking for it to be > split, but also wondering if it was really required... >=20 > Some devs are still reporting unsupported relocation errors during the > build. (mischief on irc, for example.) >=20 The patch is not correct in its current form. PLT relocations (0x4) can be h= andled ok, so we can split that off and merge it. The GOT based relocation h= andling needs non-trivial rework, and may currently create corrupt binaries.= Which unhandled reloc types are being reported? >=20 >> On 2016-08-02 05:00:43, Ard Biesheuvel wrote: >> On 2 August 2016 at 13:40, Shi, Steven wrote: >>>>=20 >>>> CoffAddFixup() must be used for absolute symbol references only. These >>>> instructions contain relative symbol references, which are >>>> recalculated in WriteSections64(). >>>>=20 >>>> 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., >>>>=20 >>>> + CoffAddFixup( >>>> + (UINT32)(UINTN)((UINT64) >>>> mCoffSectionsOffset[RelShdr->sh_info] + GoTPcRelPtrOffset), >>>> + EFI_IMAGE_REL_BASED_DIR64); >>>>=20 >>>> 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'. >>>>=20 >>>> 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 I= sDuplicatedCoffFixup() to check whether a converting fixup offset is duplica= ted before we use CoffAddFixup() to really add it? If it is new, we add it, o= therwise just skip it. >>=20 >> 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) >>=20 >> 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. >>=20 >> So it would be better to simply emit the relocations, but introduce a >> sorting pass that merges all duplicates as well. >>=20 >> Thanks, >> Ard. >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel