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


Marvin Häuser
 

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@...> 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@...>
抄送: devel@edk2.groups.io; patrick.rudolph@...;
guo.dong@...; gua.guo@...; james.lu@...;
ray.ni@...; ardb@...
主题: 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@...> 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@...; gua.guo@...;
james.lu@...; ray.ni@...; mhaeuser@...;
ardb@...
主题: [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@...>
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@...]
-=-=-=-=-=-=