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 EAF5B1A1DF1 for ; Sun, 31 Jul 2016 12:10:32 -0700 (PDT) Received: by mail-io0-x22e.google.com with SMTP id 38so170035658iol.0 for ; Sun, 31 Jul 2016 12:10:32 -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=p1x4yItee0DS59HtrE9LVvoxfgt4yqTEjurUG+zTphE=; b=Dorvef7Kt/e1z3s4ByqDA6yuzA2HwqXigyAzC5b+DjU3Y3/RQpVgHPIZnIB2/NonmH AsE8E2h1ShYYQ/weMZnNfzL2IoKBu5ZXsMPxSfw5WoTAlBlV6+G7gz5Ya1YIbgx7xYEf fUXX6rwLKzLPvAvfi6BhjiZzckcdXw/YoI73Q= 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=p1x4yItee0DS59HtrE9LVvoxfgt4yqTEjurUG+zTphE=; b=Uu9+T92sHcRC7HshAdzkASRWrJGH1MsRN0+osXHe0MwHXgwi/V9LpvVyUnUBONfIZc AWa2cD7PiUbL+ADIcilAVqtQ+NmpwkziieUqdx+GcpilKoTI+b6Tue1jpb8jhu5U4LW+ 2+tX1g6q4kHAVXvpjE0zcGmr85INJWtySYk6dr7hpNfQbk2zBpZaBEbN26CQpVY6qLuI tOlliZZ3Wjkavl1iD+P5Bwd6WJ6DqFPuG4qVb9FY0/uXW/a4Wx8zZ7gYU1PSluRf1eVz M1WYt4NeBsgAjO4foJgnDaDERxDkOmJ44GCvp6IQVMrsC2dPbFrFgXBiD5RSJtJDflWG JL7w== X-Gm-Message-State: AEkoouvxD1eLfw0uilSdKb1EgHxheYGdtUMx0TV27ozS90ufs9ud7UMFhC9TeqDEdjWx3i2aoIMAyYdli6uw3HnS X-Received: by 10.107.40.133 with SMTP id o127mr50946087ioo.183.1469992232259; Sun, 31 Jul 2016 12:10:32 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Sun, 31 Jul 2016 12:10:31 -0700 (PDT) In-Reply-To: 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> From: Ard Biesheuvel Date: Sun, 31 Jul 2016 21:10:31 +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: Sun, 31 Jul 2016 19:10:33 -0000 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 31 July 2016 at 07:42, Ard Biesheuvel wrote: > On 31 July 2016 at 05:08, Shi, Steven wrote: >>> OK. Could we do that in a separate patch? >> [Steven]: Yes. We could separate it in a new patch. >> >>> >> Is this necessary? I would expect the GOT entry itself to be already >>> >> covered by a R_X86_64_64 relocation, so I don't think there is a nee= d >>> >> to emit a EFI_IMAGE_REL_BASED_DIR64 PE/COFF reloc here. >>> > [Steven]: Do you ask whether it is still necessary to support the >>> GOTPCREL/GOTPCRELX/REX_GOTPCRELX new relocation types? I think they >>> are nice to have now, since new type support has no impact to old ones,= we >>> could just leave the code there for reference. >>> >>> No. The question is whether it is necessary to emit the >>> EFI_IMAGE_REL_BASED_DIR64 fixup reloc here. The relocation place is >>> the instruction, not the GOT entry, and I would expect the GOT entry >>> to be covered by a R_X86_64_64 relocation already >> [Steven]: Yes, it is necessary. Even the R_X86_64_64 relocation need to = emit the EFI_IMAGE_REL_BASED_DIR64 fixup reloc like below in the original G= enFw code. You can test it like this, remove the below CoffAddFixup() code = and build the OVMF with GCC5, you will see the Ovmf boot failure with cpu e= xception. >> >> case R_X86_64_64: >> VerboseMsg ("EFI_IMAGE_REL_BASED_DIR64 Offset: 0x%08X", >> mCoffSectionsOffset[RelShdr->sh_info] + (Rel->r_offset -= SecShdr->sh_addr)); >> CoffAddFixup( >> (UINT32) ((UINT64) mCoffSectionsOffset[RelShdr->sh_info] >> + (Rel->r_offset - SecShdr->sh_addr)), >> EFI_IMAGE_REL_BASED_DIR64); >> > > That was not my point. With your code, how many > EFI_IMAGE_REL_BASED_DIR64 fixups are added to the .reloc section for > the GOT entry of 'n'? > > int n; > int f () { return n; } > int g () { return n; } > int h () { return n; } I am also concerned about the GOTPCRELX/REX_GOTPCRELX relocations. Reading the x86_64 ABI docs, it appears that these may refer to instructions that have been modified by the linker. In that case, how do we deal with the relocation? Also, according to the doc, mov instructions may be emitted by the linker in some cases that are only valid in the lowest 2 GB of the address space. All in all, I think supporting GOT based relocations is a can of worms, and I would prefer to get rid of them completely if we can (i.e., using hidden visibility even for LTO, I have a fix for that I will sent out separately) Thanks, Ard.