回复: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
Patrick: Can you give the reproduce step to generate ELF image that doesn't work with the option max-page-size? > -邮件原件- > 发件人: devel@edk2.groups.io 代表 Marvin > H?user > 发送时间: 2023年3月31日 18:58 > 收件人: gaoliming > 抄送: devel@edk2.groups.io; patrick.rudo...@9elements.com; > guo.d...@intel.com; gua@intel.com; james...@intel.com; > ray...@intel.com; a...@kernel.org > 主题: Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix > CLANGDWARF_IA32_X64 > > Liming, > > Platform maintainers can decide whether or not they want to combat *binary > corruption*? Excuse me, but what the bloody hell? This needs a root cause > analysis for which part of the stack silently borks us and not an “oh, if > something fails, well, copy and paste this workaround… maybe”. If you give > me an efficient way to reproduce it, I’ll do it. > > Best regards, > Marvin > > > On 31. Mar 2023, at 06:54, gaoliming wrote: > > > > Marvin: > > Platform developer can decide how to configure this option in their DSC file > to resolve their problem. This is one option for the platform developer. > > > > Thanks > > Liming > >> -邮件原件- > >> 发件人: devel@edk2.groups.io 代表 Marvin > >> H?user > >> 发送时间: 2023年3月28日 19:26 > >> 收件人: gaoliming > >> 抄送: devel@edk2.groups.io; patrick.rudo...@9elements.com; > >> guo.d...@intel.com; gua@intel.com; james...@intel.com; > >> ray...@intel.com; a...@kernel.org > >> 主题: Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix > >> CLANGDWARF_IA32_X64 > >> > >> Hi all, > >> > On 28. Mar 2023, at 07:38, gaoliming > wrote: > >>> Patrick: > >>> I prefer to override this option in DSC instead of the change in > >>> tools_def.txt. > >> > >> A DSC override to fix *binary corruption* of an unknown cause? It is > ridiculous > >> this can even happen silently, even though it’s unclear which component > is at > >> fault. > >> > >>> Normal EFI image needs to set its page size for the smaller > >>> image size. > >>> > >>> You can see GCC DLINK option. It also sets page-size as 0x40. > >>> > >>> DEFINE GCC49_IA32_X64_DLINK_COMMON = -nostdlib > >> -Wl,-n,-q,--gc-sections -z > >>> common-page-size=0x40 > >> > >> Side note, the correct way to do this is setting max-page-size, not > >> common-page-size. > >> > >>> > >>> Thanks > >>> Liming > -邮件原件- > 发件人: devel@edk2.groups.io 代表 Patrick > Rudolph > 发送时间: 2023年3月17日 22:06 > 抄送: devel@edk2.groups.io; guo.d...@intel.com; > gua@intel.com; > james...@intel.com; ray...@intel.com; mhaeu...@posteo.de; > a...@kernel.org > 主题: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix > CLANGDWARF_IA32_X64 > > Drop the "-z max-page-size=0x40" option as it causes the ELF > header to overflow into the .text section, causing undefined > behaviour. > >> > >> That *definitely* is not a fix. Not only does this regress binary size for > >> platforms that have tight SPI space constraints, it also only masks the > >> issue. > In > >> the (frankly near-impossible) case the ELF header dramatically grows in > size, > >> who knows whether it can overflow again? > >> > >> Sorry, but the overall description is pretty vague. Which OS / compiler > version > >> are you using? Do you have any trivial way to detect the corruption? I > never > >> really touched UefiPayloadPkg and have nothing set up to boot it to > reproduce > >> the issue. > >> > >> Best regards, > >> Marvin > >> > > With high optimization level it corrupts essential code and > the binary would crash. It might work with low optimization > level though. As the default is to use Oz and LTO, it always > crashes. > > Test: > The ELF generated by > 'python UefiPayloadPkg/UniversalPayloadBuild.py -a IA32' boots. > > Signed-off-by: Patrick Rudolph > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4357 > --- > BaseTools/Conf/tools_def.template | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/BaseTools/Conf/tools_def.template > b/BaseTools/Conf/tools_def.template > index 9b59bd75c3..0c584ab390 100755 > --- a/BaseTools/Conf/tools_def.template > +++ b/BaseTools/Conf/tools_def.template > @@ -2866,7 +2866,7 @@ DEFINE CLANGDWARF_X64_PREFIX > = > ENV(CLANG_BIN) > > > # LLVM/CLANG doesn't support -n link option. So, it can't share the > same > IA32_X64_DLINK_COMMON flag. > > # LLVM/CLANG doesn't support common page size. So, it can't share the > same GccBase.lds script. > > -DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON = -nostdlib > -Wl,-q,--gc-sections -z max-page-size=0x40 > > +DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON = -nostdlib > -Wl,-q,--gc-sections > > DEFINE CLANGDWARF_DLINK2_FLAGS_COMMON = > -Wl,--script=$(EDK_TOOLS_PATH)/Scripts/ClangBase.lds > > DEFINE CLANGDWARF_IA32_X64_ASLDLINK_FLAGS = > DEF(CLANG
回复: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
Marvin: Platform developer can decide how to configure this option in their DSC file to resolve their problem. This is one option for the platform developer. Thanks Liming > -邮件原件- > 发件人: devel@edk2.groups.io 代表 Marvin > H?user > 发送时间: 2023年3月28日 19:26 > 收件人: gaoliming > 抄送: devel@edk2.groups.io; patrick.rudo...@9elements.com; > guo.d...@intel.com; gua@intel.com; james...@intel.com; > ray...@intel.com; a...@kernel.org > 主题: Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix > CLANGDWARF_IA32_X64 > > Hi all, > > > On 28. Mar 2023, at 07:38, gaoliming wrote: > > Patrick: > > I prefer to override this option in DSC instead of the change in > > tools_def.txt. > > A DSC override to fix *binary corruption* of an unknown cause? It is > ridiculous > this can even happen silently, even though it’s unclear which component is at > fault. > > > Normal EFI image needs to set its page size for the smaller > > image size. > > > > You can see GCC DLINK option. It also sets page-size as 0x40. > > > > DEFINE GCC49_IA32_X64_DLINK_COMMON = -nostdlib > -Wl,-n,-q,--gc-sections -z > > common-page-size=0x40 > > Side note, the correct way to do this is setting max-page-size, not > common-page-size. > > > > > Thanks > > Liming > >> -邮件原件- > >> 发件人: devel@edk2.groups.io 代表 Patrick > >> Rudolph > >> 发送时间: 2023年3月17日 22:06 > >> 抄送: devel@edk2.groups.io; guo.d...@intel.com; gua@intel.com; > >> james...@intel.com; ray...@intel.com; mhaeu...@posteo.de; > >> a...@kernel.org > >> 主题: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix > >> CLANGDWARF_IA32_X64 > >> > >> Drop the "-z max-page-size=0x40" option as it causes the ELF > >> header to overflow into the .text section, causing undefined > >> behaviour. > > That *definitely* is not a fix. Not only does this regress binary size for > platforms that have tight SPI space constraints, it also only masks the > issue. In > the (frankly near-impossible) case the ELF header dramatically grows in size, > who knows whether it can overflow again? > > Sorry, but the overall description is pretty vague. Which OS / compiler > version > are you using? Do you have any trivial way to detect the corruption? I never > really touched UefiPayloadPkg and have nothing set up to boot it to reproduce > the issue. > > Best regards, > Marvin > > >> > >> With high optimization level it corrupts essential code and > >> the binary would crash. It might work with low optimization > >> level though. As the default is to use Oz and LTO, it always > >> crashes. > >> > >> Test: > >> The ELF generated by > >> 'python UefiPayloadPkg/UniversalPayloadBuild.py -a IA32' boots. > >> > >> Signed-off-by: Patrick Rudolph > >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4357 > >> --- > >> BaseTools/Conf/tools_def.template | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/BaseTools/Conf/tools_def.template > >> b/BaseTools/Conf/tools_def.template > >> index 9b59bd75c3..0c584ab390 100755 > >> --- a/BaseTools/Conf/tools_def.template > >> +++ b/BaseTools/Conf/tools_def.template > >> @@ -2866,7 +2866,7 @@ DEFINE CLANGDWARF_X64_PREFIX= > >> ENV(CLANG_BIN) > >> > >> > >> # LLVM/CLANG doesn't support -n link option. So, it can't share the same > >> IA32_X64_DLINK_COMMON flag. > >> > >> # LLVM/CLANG doesn't support common page size. So, it can't share the > >> same GccBase.lds script. > >> > >> -DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON = -nostdlib > >> -Wl,-q,--gc-sections -z max-page-size=0x40 > >> > >> +DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON = -nostdlib > >> -Wl,-q,--gc-sections > >> > >> DEFINE CLANGDWARF_DLINK2_FLAGS_COMMON = > >> -Wl,--script=$(EDK_TOOLS_PATH)/Scripts/ClangBase.lds > >> > >> DEFINE CLANGDWARF_IA32_X64_ASLDLINK_FLAGS = > >> DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) > >> -Wl,--defsym=PECOFF_HEADER_SIZE=0 > >> DEF(CLANGDWARF_DLINK2_FLAGS_COMMON) > >> -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable > >> > >> DEFINE CLANGDWARF_IA32_X64_DLINK_FLAGS= > >> DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) > >> -Wl,--entry,$(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) > >> -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map,--whole-archive > >> > >> -- > >> 2.39.1 > >> > >> > >> > >> -=-=-=-=-=-= > >> Groups.io Links: You receive all messages sent to this group. > >> View/Reply Online (#101341): > >> https://edk2.groups.io/g/devel/message/101341 > >> Mute This Topic: https://groups.io/mt/97673649/4905953 > >> Group Owner: devel+ow...@edk2.groups.io > >> Unsubscribe: https://edk2.groups.io/g/devel/unsub > >> [gaolim...@byosoft.com.cn] > >> -=-=-=-=-=-= > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102234): https://edk2.groups.io/g/devel/message/102234 Mute This Topic: https://groups.io/mt/97967312/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
Patrick: I prefer to override this option in DSC instead of the change in tools_def.txt. Normal EFI image needs to set its page size for the smaller image size. You can see GCC DLINK option. It also sets page-size as 0x40. DEFINE GCC49_IA32_X64_DLINK_COMMON = -nostdlib -Wl,-n,-q,--gc-sections -z common-page-size=0x40 Thanks Liming > -邮件原件- > 发件人: devel@edk2.groups.io 代表 Patrick > Rudolph > 发送时间: 2023年3月17日 22:06 > 抄送: devel@edk2.groups.io; guo.d...@intel.com; gua@intel.com; > james...@intel.com; ray...@intel.com; mhaeu...@posteo.de; > a...@kernel.org > 主题: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix > CLANGDWARF_IA32_X64 > > Drop the "-z max-page-size=0x40" option as it causes the ELF > header to overflow into the .text section, causing undefined > behaviour. > > With high optimization level it corrupts essential code and > the binary would crash. It might work with low optimization > level though. As the default is to use Oz and LTO, it always > crashes. > > Test: > The ELF generated by > 'python UefiPayloadPkg/UniversalPayloadBuild.py -a IA32' boots. > > Signed-off-by: Patrick Rudolph > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4357 > --- > BaseTools/Conf/tools_def.template | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/BaseTools/Conf/tools_def.template > b/BaseTools/Conf/tools_def.template > index 9b59bd75c3..0c584ab390 100755 > --- a/BaseTools/Conf/tools_def.template > +++ b/BaseTools/Conf/tools_def.template > @@ -2866,7 +2866,7 @@ DEFINE CLANGDWARF_X64_PREFIX= > ENV(CLANG_BIN) > > > # LLVM/CLANG doesn't support -n link option. So, it can't share the same > IA32_X64_DLINK_COMMON flag. > > # LLVM/CLANG doesn't support common page size. So, it can't share the > same GccBase.lds script. > > -DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON = -nostdlib > -Wl,-q,--gc-sections -z max-page-size=0x40 > > +DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON = -nostdlib > -Wl,-q,--gc-sections > > DEFINE CLANGDWARF_DLINK2_FLAGS_COMMON = > -Wl,--script=$(EDK_TOOLS_PATH)/Scripts/ClangBase.lds > > DEFINE CLANGDWARF_IA32_X64_ASLDLINK_FLAGS = > DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) > -Wl,--defsym=PECOFF_HEADER_SIZE=0 > DEF(CLANGDWARF_DLINK2_FLAGS_COMMON) > -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable > > DEFINE CLANGDWARF_IA32_X64_DLINK_FLAGS= > DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) > -Wl,--entry,$(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) > -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map,--whole-archive > > -- > 2.39.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#101341): > https://edk2.groups.io/g/devel/message/101341 > Mute This Topic: https://groups.io/mt/97673649/4905953 > Group Owner: devel+ow...@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [gaolim...@byosoft.com.cn] > -=-=-=-=-=-= > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#102004): https://edk2.groups.io/g/devel/message/102004 Mute This Topic: https://groups.io/mt/97899693/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
