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

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

[-- Attachment #1: Type: text/plain, Size: 4217 bytes --]


> On 31. Mar 2023, at 16:41, Ni, Ray <ray.ni@intel.com> wrote:
> 
> Why ELF header overflows into .text section?

That's a good question, isn't it? :)

From what I can see, these binaries don't pass post-processing like GenFw or such. GCC (and I think thus CLANGDWARF?) gets an extra objcopy step as part of linking [2], but the arguments are empty [3] and thus should be no-op (I hope?).

I suppose potential candidates are:

1) A bug in the LLD linker used by CLANGDWARF for IA32 and X64. That would be very surprising to me, especially as no other platform reported issues and LLD is well-established. But who knows, generally ELFs will have large alignment values compared to the 64 Bytes used by edk2.

2) A bug in llvm-objcopy used by UniversalPayloadBuild.py [1]. I'm honestly unfamiliar with objcopy variants and their quality/reliability.

3) A bug in the llvm-objcopy or CLANGDWARF tools_def commands on the edk2 side of things.

Some may disagree, but I would reduce 3) to either 1) or 2). I think even if the commands malformed and this causes the overflow, I believe LLD or objcopy should issue a warning regardless.

As I have no way to reproduce the issue, I cannot really help further, sorry.

Best regards,
Marvin

[1]
https://github.com/tianocore/edk2/blob/b08a19eae28e76fb5a296a604c27d06fab29b08a/UefiPayloadPkg/UniversalPayloadBuild.py#L163-L183

[2]
https://github.com/tianocore/edk2/blob/b08a19eae28e76fb5a296a604c27d06fab29b08a/BaseTools/Conf/build_rule.template#L298

[3]
https://github.com/tianocore/edk2/blob/b08a19eae28e76fb5a296a604c27d06fab29b08a/BaseTools/Conf/tools_def.template#L2895
https://github.com/tianocore/edk2/blob/b08a19eae28e76fb5a296a604c27d06fab29b08a/BaseTools/Conf/tools_def.template#L2931

> 
>> -----Original Message-----
>> From: Patrick Rudolph <patrick.rudolph@9elements.com>
>> Sent: Friday, March 17, 2023 10:06 PM
>> Cc: devel@edk2.groups.io; Dong, Guo <guo.dong@intel.com>; Guo, Gua
>> <gua.guo@intel.com>; Lu, James <james.lu@intel.com>; Ni, Ray
>> <ray.ni@intel.com>; mhaeuser@posteo.de; ardb@kernel.org
>> Subject: [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 <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
> 


[-- Attachment #2: Type: text/html, Size: 5560 bytes --]