From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x22e.google.com (mail-io0-x22e.google.com [IPv6:2607:f8b0:4001:c06::22e]) (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 C89F61A1E12 for ; Tue, 2 Aug 2016 05:00:44 -0700 (PDT) Received: by mail-io0-x22e.google.com with SMTP id q83so210705296iod.1 for ; Tue, 02 Aug 2016 05:00:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=lwlCl4Ua47FAbnD/GJWh/Oj/kF3Hpesw6juLiPPqat8=; b=J3pv9h+xuYdghEEFgPUATrLtdmXNjzNf1t7yeUo63ibMUF7IlPUD7jQX8qakZHP/t6 RoeaA2N6iAqZCJH4TOpuyjbjmOb5XDKULolmnRpXTRNpB6xonGYMg6b4AuWOeFBh1QQE ++cqbExCm8O42Zc4aI6OT9EEUEhvsTfDd9vzI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=lwlCl4Ua47FAbnD/GJWh/Oj/kF3Hpesw6juLiPPqat8=; b=YzJE0m9fVClFVoUJbOfXfb38veVwSf9cVRcR98eTZ68BCd77sjbMaPMjqyblXh9vqc V09kCKXepYfMUq3ZvC10YbhkDK49MU+ykp98f7FRZWc4u6xYa0u1+xv0SLeaMEV9+LWR tfoByTC8OFXUqvL0rEm+6/GINlIiFAJXgeBb8W8kqaE1hxvQWrv8p+k6lTGheWbAyT/w vAnWsmr6GFjO40b9pkPQYS328+Pg7a1UhJEEMgLQyBmoDfXGlFUYMFSldRxXSX4M6Xx4 qtsUjD5oxGf4esGIAvyLEY2CM8JCvsBN+eklbNOQg/eVFVXJ/TLJeSq69DGsL00Pb84Q o3vw== X-Gm-Message-State: AEkoouuXedbssVLNDfvRKdMXDdPJ+3REzVvP4Dofrkcmfspb5P36kTZG5fOQwlYO7ezVjR6EI6QwxCvou9Ylr8gA X-Received: by 10.107.40.133 with SMTP id o127mr58858697ioo.183.1470139244030; Tue, 02 Aug 2016 05:00:44 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Tue, 2 Aug 2016 05:00:43 -0700 (PDT) In-Reply-To: <06C8AB66E78EE34A949939824ABE2B3103383852@shsmsx102.ccr.corp.intel.com> References: <1467967364-11556-1-git-send-email-steven.shi@intel.com> <1467967364-11556-3-git-send-email-steven.shi@intel.com> <06C8AB66E78EE34A949939824ABE2B3103381245@shsmsx102.ccr.corp.intel.com> <06C8AB66E78EE34A949939824ABE2B31033813C8@shsmsx102.ccr.corp.intel.com> <06C8AB66E78EE34A949939824ABE2B3103381AD8@shsmsx102.ccr.corp.intel.com> <06C8AB66E78EE34A949939824ABE2B3103381BF3@shsmsx102.ccr.corp.intel.com> <06C8AB66E78EE34A949939824ABE2B3103382483@shsmsx102.ccr.corp.intel.com> <06C8AB66E78EE34A949939824ABE2B31033824FA@shsmsx102.ccr.corp.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> From: Ard Biesheuvel Date: Tue, 2 Aug 2016 14:00:43 +0200 Message-ID: To: "Shi, Steven" Cc: edk2-devel-01 , "Gao, Liming" , "afish@apple.com" , "Justen, Jordan L" , "Kinney, Michael D" 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: Tue, 02 Aug 2016 12:00:45 -0000 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 I= sDuplicatedCoffFixup() to check whether a converting fixup offset is duplic= ated 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.