回复: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64

["gaoliming" <gaoliming@byosoft.com.cn>]

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 <devel@edk2.groups.io> 代表 Marvin
> H?user
> 发送时间: 2023年3月31日 18:58
> 收件人: gaoliming <gaoliming@byosoft.com.cn>
> 抄送: devel@edk2.groups.io; patrick.rudolph@9elements.com;
> guo.dong@intel.com; gua.guo@intel.com; james.lu@intel.com;
> ray.ni@intel.com; ardb@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 <gaoliming@byosoft.com.cn> 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 <devel@edk2.groups.io> 代表 Marvin
> >> H?user
> >> 发送时间: 2023年3月28日 19:26
> >> 收件人: gaoliming <gaoliming@byosoft.com.cn>
> >> 抄送: devel@edk2.groups.io; patrick.rudolph@9elements.com;
> >> guo.dong@intel.com; gua.guo@intel.com; james.lu@intel.com;
> >> ray.ni@intel.com; ardb@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 <gaoliming@byosoft.com.cn>
> 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 <devel@edk2.groups.io> 代表 Patrick
> >>>> Rudolph
> >>>> 发送时间: 2023年3月17日 22:06
> >>>> 抄送: devel@edk2.groups.io; guo.dong@intel.com;
> gua.guo@intel.com;
> >>>> james.lu@intel.com; ray.ni@intel.com; mhaeuser@posteo.de;
> >>>> ardb@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 <patrick.rudolph@9elements.com>
> >>>> 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+owner@edk2.groups.io
> >>>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> >>>> [gaoliming@byosoft.com.cn]
> >>>> -=-=-=-=-=-=
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> 
> 
> 
> 
>