From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x231.google.com (mail-wm0-x231.google.com [IPv6:2a00:1450:400c:c09::231]) (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 3CD4A1A1DF8 for ; Wed, 3 Aug 2016 13:55:15 -0700 (PDT) Received: by mail-wm0-x231.google.com with SMTP id o80so353555717wme.1 for ; Wed, 03 Aug 2016 13:55:15 -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=nYYNRJG/QSjMenBoduEJfmM8eY6nBVJvYi5f/dGa1kw=; b=c8T3OTdDsib3i/WvbRhlyb4g71GCw+o/oHKQQs+Dc8bmiy4Qv1Xs0YmT1hP/H+5Yrr UjCdgz9Z8KHlq80Fjje+QUljQgJBM8vL/afdXhrAFakdsZkcGWn2ApYvaolkVB4gKITa uolAPfqDizmKY/oT65biBNdX9uLvQZOcq3EDI= 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=nYYNRJG/QSjMenBoduEJfmM8eY6nBVJvYi5f/dGa1kw=; b=mTif8vjXN66k9vBhiRVe9+OvXmA0158Ivyq85GLOLi4yqrq/xQaQ7jcRvaQqW0+BIy dj9qcetrrg88WZpugbcyW2N4kyHWwh1Libsvy1hAnqxwPoD6+WGwGzU7OtXAE+FWwgbu 1yF1tpwaqzs90xAgEofZM97cCIy6a+AdXAl7AVkcUWYmDqRbey1OKaRzk06HjJX6hffb io1A0L4BLpzy92pWg2335u88y7pkUKsiYAi6pX0eQr8MoZjFJ7TtaPRuN7R6RC2Oecmy xALtDuRBRiob/Do4fXYTjCbmJ+FY6IzpdL7uTkWuSMQzdzzOhFchSRDckK2SCvnTlVAN 4rlg== X-Gm-Message-State: AEkoouv2dBIuOmgvE7WCpCrkX0hcR1sJ9T+hbZmWCoH87CSt0TO4+ppH/KTUsAMgHW2WkaKt X-Received: by 10.194.16.162 with SMTP id h2mr64984028wjd.52.1470257713694; Wed, 03 Aug 2016 13:55: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 u72sm240316wmf.5.2016.08.03.13.55.12 (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 03 Aug 2016 13:55:13 -0700 (PDT) References: <1467967364-11556-1-git-send-email-steven.shi@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> <147025761018.14160.4581842911325157321@jljusten-ivb> Mime-Version: 1.0 (1.0) In-Reply-To: <147025761018.14160.4581842911325157321@jljusten-ivb> Message-Id: <27D7F3D8-CBBE-4C45-855D-5A48C03D1D41@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:55:11 +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:55:15 -0000 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable > On 3 aug. 2016, at 22:53, Jordan Justen wrote:= >=20 >> On 2016-08-03 13:47:09, Ard Biesheuvel wrote: >>=20 >>> On 3 aug. 2016, at 22:13, Jordan Justen wrot= e: >>>=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 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? >=20 > "unsupported ELF EM_X86_64 relocation 0x4" >=20 > You might join irc if you want to get some more info from mischief > about it. >=20 That's the plt one, which we can fix easily. I'll whip up a patch in the mor= ning (11 pm here now) >=20 >>=20 >>>=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. Thes= e >>>>>> 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 entr= y >>>>>> (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 entr= y >>>>>> containing the address of 'n', and the instructions perform a IP >>>>>> relative load of the contents of the GOT entry to retrieve the addres= s >>>>>> 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 multip= le fixups for the same GOT relocation entry. How about we introduce a simple= IsDuplicatedCoffFixup() to check whether a converting fixup offset is dupli= cated before we use CoffAddFixup() to really add it? If it is new, we add it= , otherwise 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