From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x229.google.com (mail-it0-x229.google.com [IPv6:2607:f8b0:4001:c0b::229]) (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 2694C1A1E05 for ; Sat, 30 Jul 2016 22:42:54 -0700 (PDT) Received: by mail-it0-x229.google.com with SMTP id f6so141519067ith.0 for ; Sat, 30 Jul 2016 22:42:54 -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=HCm9nfDuRZdWI9PeKnWq4PBhWMRdAHT3sm/oW/rKgBU=; b=HYanaVhFAuZUVsSfFNAbUf/hW7HvKdlknwgzvBBbeGqnwo9N1TaBEv8HKayYB6Dq/A AF9ORVhen30h0dZkP69oCcUZ9JbPmueXWbdkzEuDDWFp/FSdEVBxRngaW8wkzN01yOHs vav80sYaC0CSdFlSxHTx8IQH/Bdq+NCuf6cIU= 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=HCm9nfDuRZdWI9PeKnWq4PBhWMRdAHT3sm/oW/rKgBU=; b=F6ppFLi5Ihhx6Ralw0vC252sZSoXmK9a2Vs0Q9LmUziWGqaCTEQWgNDTpW2YCP+7ak RALhrE8jThDO/oc84RGrxErgcHtUIRW20qrYJOTTnUR+4HQRYJhtsreRxwdA+bTPeB05 S3OBI6vx8V7c+WqX7ZDHwk5qtLJSPSaFxo7vdr8jCkD+N0m3TLqX8ehcDWXeKLF29zPY XLys6htvO1oLZvcZrQAMj7K1AjQGYhV1eP6dE5k+mebmm+nxzp1OObWNkMuz9kjqusbI MledEpU3xd3QAmb+0Za4IoID1w7JfiMCIML7dji/Jz7S9CEqhWiNIEm3TfHaW9BUCvBE jISw== X-Gm-Message-State: AEkooutTJLwxPHKNNnSBGt4ZLmlf5wPkRW61aqVBbOA/gBBFr/W4nhKyBnSh4od/SQwiWZ+MKsvGQHoovM1YCD4M X-Received: by 10.36.86.134 with SMTP id o128mr55260380itb.5.1469943773285; Sat, 30 Jul 2016 22:42:53 -0700 (PDT) MIME-Version: 1.0 Received: by 10.36.204.195 with HTTP; Sat, 30 Jul 2016 22:42:52 -0700 (PDT) In-Reply-To: <06C8AB66E78EE34A949939824ABE2B31033813C8@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> From: Ard Biesheuvel Date: Sun, 31 Jul 2016 07:42:52 +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 05:42:54 -0000 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 need >> >> 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 e= mit the EFI_IMAGE_REL_BASED_DIR64 fixup reloc like below in the original Ge= nFw code. You can test it like this, remove the below CoffAddFixup() code a= nd build the OVMF with GCC5, you will see the Ovmf boot failure with cpu ex= ception. > > 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; }