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

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

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