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

["Marvin Häuser" <mhaeuser@posteo.de>]

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]
>>>> -=-=-=-=-=-=
>> 
>> 
>> 
>> 
>> 
> 
> 
>