While we are talking about what toolchain targets should do I don’t understand
this [1]? Why does OVMF need to turn on —keepexceptiontable? If these
toolchains require it why don’t they turn it on? I see Mike fixed it[2]. Is
this another case of a global GENFW_FLAG breaking things? Looks like the
CLANGPDB toolchain does what I would expect, and the other toolchains do not?
Is there some history here I’m missing?
[1] $ git grep "keepexceptiontable"
BaseTools/Conf/tools_def.template:2872:DEBUG_CLANGPDB_X64_GENFW_FLAGS =
--keepexceptiontable
BaseTools/Conf/tools_def.template:2882:NOOPT_CLANGPDB_X64_GENFW_FLAGS =
--keepexceptiontable
…
OvmfPkg/OvmfPkgIa32X64.dsc:80: MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfPkgIa32X64.dsc:81: GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfPkgIa32X64.dsc:82: INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:80: MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:81: GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfPkgX64.dsc:82: INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfXen.dsc:71: MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfXen.dsc:72: GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
OvmfPkg/OvmfXen.dsc:73: INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
[2] $ git show bbcafc442b2db
commit bbcafc442b2db91322dd3ba04e166236a41b111d
Author: mdkinney <mdkinney@6f19259b-4bc3-4df7-8a09-765794883524>
Date: Wed Oct 3 21:00:26 2012 +0000
The exception table information in X64 PE/COFF images is being stripped by
default in the OvmfPkg.
This patch preserves this information when SOURCE_DEBUG_ENABLE is set.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney <michael.d.kin...@intel.com>
Reviewed-by: Laszlo Ersek
git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13780
6f19259b-4bc3-4df7-8a09-765794883524
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 9b2ba8626ab3..40fd5e97b24e 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -41,7 +41,12 @@ [BuildOptions]
INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG
MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG
GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse
-
+!ifdef $(SOURCE_DEBUG_ENABLE)
+ MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable
+ GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
+ INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
+!endif
+
################################################################################
#
# SKU Identification section - list of all SKU IDs supported by this Platform.
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 9ff4a5de1005..c61d41dccbbe 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -41,6 +41,11 @@ [BuildOptions]
INTEL:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG
MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG
GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse
+!ifdef $(SOURCE_DEBUG_ENABLE)
+ MSFT:*_*_X64_GENFW_FLAGS = --keepexceptiontable
+ GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
+ INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
+!endif
################################################################################
#
Thanks,
Andrew Fish
> On Mar 24, 2021, at 8:38 PM, Andrew Fish <af...@apple.com> wrote:
>
> Liming,
>
> I understand how the tool works, but that is not how it is used. To make it
> work like other toolchains *_*_*_GENFW_FLAGS for ELF targets should pass
> —zero for builds that do not contain symbols. This is how it works today [1].
>
> The proposed fix is a global hammer ` RELEASE_*_*_GENFW_FLAGS = --zero` and
> it prevents some one from overriding the toolchain definition to turn on
> source level debugging for Release builds. In all the other places this is a
> tool chain choice. So my complaint is solving this globally vs. on a per
> toolchain basis. All the other toolchains don’t produce.
>
> If you look at the XCODE5 toolchian that we override the edk2 default
> DEBUG_XCODE5_X64_CC_FLAGS has -g and RELEASE_XCODE5_X64_CC_FLAGS does not. So
> the source level debugging or not is part of the compiler target choice.
> DEBUG_XCODE5_*_MTOC_FLAGS = -align 0x20 -d $(DEBUG_DIR)/$(MODULE_NAME).dll
> RELEASE_XCODE5_*_MTOC_FLAGS = -align 0x20
>
> For our override we pass -g to RELEASE builds CC_FLAGS and do
> `RELEASE_XCODE5_*_MTOC_FLAGS = -align 0x20 -d $(MODULE_GUID)` so the override
> to force --zero on every call to GenFw will break this.
>
> So my point is if the toolchain is generating a Debug Directory entry in a
> RELEASE target that does not support debugging then that target should
> setting `RELEASE_<target>_*_GENFW_FLAGS = —zero` to undo the unwanted work. I
> understand why the debug info gets added as it reduces the complexity of the
> ELF to PE/COFF conversion, so I’m not complaining about that just that the
> ELF targets don’t handle this themselves. So to me this fix is working the
> bug of the ELF targets not passing —zero on RELEASE builds to GenFw.
>
> [1] $ git grep _GENFW_FLAGS -- *.template
> BaseTools/Conf/tools_def.template:2872:DEBUG_CLANGPDB_X64_GENFW_FLAGS =
> --keepexceptiontable
> BaseTools/Conf/tools_def.template:2877:RELEASE_CLANGPDB_X64_GENFW_FLAGS =
> BaseTools/Conf/tools_def.template:2882:NOOPT_CLANGPDB_X64_GENFW_FLAGS =
> --keepexceptiontable
> BaseTools/Conf/tools_def.template:3124:*_*_*_GENFW_FLAGS =
>
> Thanks,
>
> Andrew Fish
>
>> On Mar 24, 2021, at 7:24 PM, gaoliming <gaolim...@byosoft.com.cn
>> <mailto:gaolim...@byosoft.com.cn>> wrote:
>>
>> Andrew:
>> GenFw generates debug entry, and zero debug entry. Its functionality is
>> same to the image without debug entry. So, new option is not introduced to
>> control whether GenFw generates debug entry when it converts ELF image to
>> EFI image.
>>
>> Thanks
>> Liming
>> 发件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> 代表 Andrew Fish via
>> groups.io <http://groups.io/>
>> 发送时间: 2021年3月25日 7:26
>> 收件人: edk2-devel-groups-io <devel@edk2.groups.io
>> <mailto:devel@edk2.groups.io>>; r...@burtonini.com
>> <mailto:r...@burtonini.com>
>> 抄送: ler...@redhat.com <mailto:ler...@redhat.com>
>> 主题: Re: [edk2-devel] [PATCH v2] OvmfPkg: strip build paths in release builds
>>
>> This breaks some usage we have have in our fork. We have symbols turned on
>> for Release builds, so this change would break that.
>>
>> It looks to me that the root cause of this issue might be that the GenFw is
>> blindly writing the debug directory entry into the PE/COFF? For native
>> PE/COFF I think this is controlled by linker flags? For Xcode/clang it is
>> controlled by the *_XCODE5_*_MTOC_FLAGS. So at this point it is kind of up
>> to each toolchain how they want to deal with symbols on release builds.
>>
>>
>> It seems kind of strange to insert a section and then zero it. Almost seems
>> like the intent of —zero was to post process compare images?
>>
>>
>> -z, --zero Zero the Debug Data Fields in the PE input image
>> file.
>> It also zeros the time stamp fields.
>> This option can be used to compare the binary efi
>> image.
>> It can't be combined with other action options
>> except for -o, -r option. It is a action option.
>> If it is combined with other action options, the
>> later
>> input action option will override the previous one.
>>
>>
>> And in case you are going to ask our fork uses relative paths from the Build
>> directory and/or a UUID string for the Debug Directory entry file name so it
>> is a constant value and does not impact build reproducibility.
>>
>>
>> From a feature stand point this change will break any hope of source level
>> debugging with RELEASE builds. I also think it changes the exception handler
>> code output in OVMF [1] for ELF toolchains. You are going to get the (No
>> PDB) vs. the file and path you were getting today. I assume if you had tools
>> that natively produce PE/COFF and did not have a Debug Directory entry the
>> same thing would happen prior to this change.
>>
>>
>> Status = PeCoffLoaderGetEntryPoint ((VOID *) Pe32Data, &EntryPoint);
>> if (EFI_ERROR (Status)) {
>> EntryPoint = NULL;
>> }
>> InternalPrintMessage ("!!!! Find image based on IP(0x%x) ", CurrentEip);
>> PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *) Pe32Data);
>> if (PdbPointer != NULL) {
>> InternalPrintMessage ("%a", PdbPointer);
>> } else {
>> InternalPrintMessage ("(No PDB) " );
>> }
>> InternalPrintMessage (
>> " (ImageBase=%016lp, EntryPoint=%016p) !!!!\n",
>> (VOID *) Pe32Data,
>> EntryPoint
>> );
>>
>>
>> Not saying we have to "stop the presses", but just trying to point out the
>> side effects of this change. It is not so much that this change is bad, but
>> that we have no way to turn off the Debug Directory Entry for ELF
>> conversion, so we seem to be working around that issue with a bigger hammer?
>>
>> [1]
>> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c#L117
>>
>> <https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c#L117>
>>
>> Thanks,
>>
>>
>> Andrew Fish
>>
>>
>>> On Mar 24, 2021, at 4:58 AM, Ross Burton <r...@burtonini.com
>>> <mailto:r...@burtonini.com>> wrote:
>>>
>>> GenFw will embed a NB10 section which contains the path to the input file,
>>> which means the output files have build paths embedded in them. To reduce
>>> information leakage and ensure reproducible builds, pass --zero in release
>>> builds to remove this information.
>>>
>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3256
>>> <https://bugzilla.tianocore.org/show_bug.cgi?id=3256>
>>> Signed-off-by: Ross Burton <ross.bur...@arm.com
>>> <mailto:ross.bur...@arm.com>>
>>> ---
>>> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 +
>>> OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
>>> OvmfPkg/OvmfPkgIa32.dsc | 1 +
>>> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>>> OvmfPkg/OvmfPkgX64.dsc | 1 +
>>> OvmfPkg/OvmfXen.dsc | 1 +
>>> 6 files changed, 6 insertions(+)
>>>
>>> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
>>> index 65c42284d9..69a05feea9 100644
>>> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
>>> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
>>> @@ -78,6 +78,7 @@
>>> GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> !endif
>>> + RELEASE_*_*_GENFW_FLAGS = --zero
>>>
>>> #
>>> # Disable deprecated APIs.
>>> diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
>>> index 4a1cdf5aca..132f55cf69 100644
>>> --- a/OvmfPkg/Bhyve/BhyveX64.dsc
>>> +++ b/OvmfPkg/Bhyve/BhyveX64.dsc
>>> @@ -76,6 +76,7 @@
>>> GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> !endif
>>> + RELEASE_*_*_GENFW_FLAGS = --zero
>>>
>>> #
>>> # Disable deprecated APIs.
>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>> index 1eaf3e99c6..93c209950c 100644
>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>> @@ -80,6 +80,7 @@
>>> !if $(TOOL_CHAIN_TAG) != "XCODE5" && $(TOOL_CHAIN_TAG) != "CLANGPDB"
>>> GCC:*_*_*_CC_FLAGS = -mno-mmx -mno-sse
>>> !endif
>>> + RELEASE_*_*_GENFW_FLAGS = --zero
>>>
>>> #
>>> # Disable deprecated APIs.
>>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>>> index 4a5a430147..97cc438250 100644
>>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>>> @@ -84,6 +84,7 @@
>>> GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> !endif
>>> + RELEASE_*_*_GENFW_FLAGS = --zero
>>>
>>> #
>>> # Disable deprecated APIs.
>>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>>> index d4d601b444..f544fb04bf 100644
>>> --- a/OvmfPkg/OvmfPkgX64.dsc
>>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>>> @@ -84,6 +84,7 @@
>>> GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> !endif
>>> + RELEASE_*_*_GENFW_FLAGS = --zero
>>>
>>> #
>>> # Disable deprecated APIs.
>>> diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>>> index 507029404f..fcaa35acf1 100644
>>> --- a/OvmfPkg/OvmfXen.dsc
>>> +++ b/OvmfPkg/OvmfXen.dsc
>>> @@ -74,6 +74,7 @@
>>> GCC:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
>>> !endif
>>> + RELEASE_*_*_GENFW_FLAGS = --zero
>>>
>>> #
>>> # Disable deprecated APIs.
>>> --
>>> 2.25.1
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73254): https://edk2.groups.io/g/devel/message/73254
Mute This Topic: https://groups.io/mt/81594755/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
