> On Mar 25, 2021, at 10:19 AM, Laszlo Ersek <ler...@redhat.com> wrote: > > On 03/25/21 00:25, Andrew Fish wrote: >> 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? > > Yes, that's my understanding, from TianoCore#3256. > >> 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. > > I'd like if we could satisfy both your use case and Ross's (Yocto's). > > Until we have a technical solution for that, is it important that we > revert the patch upstream? (If it's urgent, I'm going to ask someone > else to do that, because I'll be back in April.) >
Laszlo, Per my other email about —keepexceptiontable it looks like source level debug is intertwined with GenFw flags that are global, and this has been going on for a long time. So I think I’ll just file a BZ about this issue in general and not ask for changes in this patch. Thanks, Andrew Fish >> 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? > > I don't have suggestions alas, but am open to any solution that works > for you and Ross both. > > Thanks (and my apologies for breaking your process!), > Laszlo > >> >> [1] >> 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> 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 >>> Signed-off-by: Ross Burton <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 (#73291): https://edk2.groups.io/g/devel/message/73291 Mute This Topic: https://groups.io/mt/81574493/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-