From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by ml01.01.org (Postfix) with ESMTP id 634531A1DF8 for ; Wed, 3 Aug 2016 16:26:13 -0700 (PDT) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga103.fm.intel.com with ESMTP; 03 Aug 2016 16:26:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,468,1464678000"; d="scan'208";a="859311496" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga003.jf.intel.com with ESMTP; 03 Aug 2016 16:26:13 -0700 Received: from fmsmsx118.amr.corp.intel.com (10.18.116.18) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 3 Aug 2016 16:26:07 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx118.amr.corp.intel.com (10.18.116.18) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 3 Aug 2016 16:26:07 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.147]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.107]) with mapi id 14.03.0248.002; Thu, 4 Aug 2016 07:26:05 +0800 From: "Shi, Steven" To: Ard Biesheuvel , "Justen, Jordan L" CC: "Kinney, Michael D" , edk2-devel-01 , "afish@apple.com" , "Gao, Liming" , "mischief@offblast.org" Thread-Topic: [edk2] [PATCH v2 2/7] BaseTools-GenFw:Add new x86_64 Elf relocation types for PIC/PIE code Thread-Index: AQHR2PTIfEgqDmQOXkSZxkGjHzXOZaAwUHcAgADFQ0D//4q6gIABWuBw//+paACAAOGngIABCJcw//+seQAAEOGtQP//hWcA//93H4CAAJTIAP//eNrggACQyoD//3XEkAAShByA//9jwmD//0btAP/+Bl4Q//yMXgD/9vPNUP/uZZeA/9qvCQD/tVTNgP9qp9QA/tVPL4D9qfEcsA== Date: Wed, 3 Aug 2016 23:26:04 +0000 Message-ID: <06C8AB66E78EE34A949939824ABE2B3103384C71@shsmsx102.ccr.corp.intel.com> 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> <27D7F3D8-CBBE-4C45-855D-5A48C03D1D41@linaro.org> In-Reply-To: <27D7F3D8-CBBE-4C45-855D-5A48C03D1D41@linaro.org> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMTZmZmMwY2UtNDk4NC00MjdhLTg1MDQtNjdjNzIzNWUyYWZiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImlFSHZZOE9BZE1qbFY1MmZwVmo4N0Z5RjRkNVQ1OTBZekQxWkEzYXBjZ3c9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 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 23:26:13 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Ard, Pls go ahead to add R_X86_64_PLT32 firstly. Below is my original path. And = for the GOTPCREL type ones, I could finish enhance them after I back from v= acation next week. Thanks. https://github.com/shijunjing/edk2/commit/dd8b1c4625cbffc7e149571266e0ae058= 1bd207d Steven Shi Intel\SSG\STO\UEFI Firmware Tel: +86 021-61166522 iNet: 821-6522 > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] > Sent: Thursday, August 04, 2016 4:55 AM > To: Justen, Jordan L > Cc: Shi, Steven ; Kinney, Michael D > ; edk2-devel-01 ; > afish@apple.com; Gao, Liming ; > mischief@offblast.org > Subject: Re: [edk2] [PATCH v2 2/7] BaseTools-GenFw:Add new x86_64 Elf > relocation types for PIC/PIE code >=20 >=20 > > On 3 aug. 2016, at 22:53, Jordan Justen wro= te: > > > >> On 2016-08-03 13:47:09, 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 th= e > >>> 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? > > > > "unsupported ELF EM_X86_64 relocation 0x4" > > > > 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 = morning > (11 pm here now) >=20 >=20 > > > >> > >>> > >>>> 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 y= our > >>>>>> 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 add= ress > >>>>>> of 'n'. > >>>>>> > >>>>>> By adding two fixups, the PE/COFF loader will apply the load offse= t > >>>>>> 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, w= e add > it, otherwise just skip it. > >>>> > >>>> That could work, but you have to be aware that fixups are best emitt= ed > >>>> in the order they need to be applied in the binary, or it will becom= e > >>>> 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