From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout01.posteo.de (mout01.posteo.de [185.67.36.65]) by mx.groups.io with SMTP id smtpd.web11.57165.1680274746967412101 for ; Fri, 31 Mar 2023 07:59:07 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=BSy8FbHw; spf=pass (domain: posteo.de, ip: 185.67.36.65, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 31EA4240080 for ; Fri, 31 Mar 2023 16:59:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1680274745; bh=rc/bfsYQKpYRQY7YU53haRPlGoulRWkU+udHNOmola4=; h=From:Subject:Date:Cc:To:From; b=BSy8FbHwLbc+TpwjSfhsUM1neXUshjMKo37pZba5GQonAiNcbfQrf3Eweo4S2byt9 edOx4foh+SY0MKO+oJwLm40Nzr/nkdxpk9HwygfYPUrUClUPiPeB9cf095H/35qStT U4zKNhX19j1TfACFu4mijRkqfCOUyGAWeka43756XDwGDxTpp4Sxe6CmH3P8q2INEc j7suXwHS/gf4OLlcmuu59A/HEiZPGYIE8btVtPY+D5JnF++JTn5b58nVWcEcJViMi7 Y8b1RqVQf/MnuL1cUVH7Sez0/t6bYNCeipBBLBqdNy3LXt2YfFU3a1bY/lPQk2TLVC WIojD3/RoyS1Q== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4Pp3Mq50T3z9rxD; Fri, 31 Mar 2023 16:59:03 +0200 (CEST) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-Id: <3FB3223E-3591-4B15-AE1F-757A88D86074@posteo.de> Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) Subject: Re: [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64 Date: Fri, 31 Mar 2023 14:58:53 +0000 In-Reply-To: Cc: "Rudolph, Patrick" , "devel@edk2.groups.io" , "Dong, Guo" , "Guo, Gua" , "Lu, James" , "ardb@kernel.org" To: "Ni, Ray" References: <20230317140627.1033739-1-patrick.rudolph@9elements.com> <20230317140627.1033739-2-patrick.rudolph@9elements.com> Content-Type: multipart/alternative; boundary="Apple-Mail=_FE23BF18-6060-4586-8A6A-DCA010D97B52" --Apple-Mail=_FE23BF18-6060-4586-8A6A-DCA010D97B52 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > On 31. Mar 2023, at 16:41, Ni, Ray wrote: >=20 > Why ELF header overflows into .text section? That's a good question, isn't it? :) =46rom 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/b08a19eae28e76fb5a296a604c27d06fab2= 9b08a/UefiPayloadPkg/UniversalPayloadBuild.py#L163-L183 [2] = https://github.com/tianocore/edk2/blob/b08a19eae28e76fb5a296a604c27d06fab2= 9b08a/BaseTools/Conf/build_rule.template#L298 [3] = https://github.com/tianocore/edk2/blob/b08a19eae28e76fb5a296a604c27d06fab2= 9b08a/BaseTools/Conf/tools_def.template#L2895 = https://github.com/tianocore/edk2/blob/b08a19eae28e76fb5a296a604c27d06fab2= 9b08a/BaseTools/Conf/tools_def.template#L2931 >=20 >> -----Original Message----- >> From: Patrick Rudolph >> Sent: Friday, March 17, 2023 10:06 PM >> Cc: devel@edk2.groups.io; Dong, Guo ; Guo, Gua >> ; Lu, James ; Ni, Ray >> ; mhaeuser@posteo.de; ardb@kernel.org >> Subject: [PATCH 2/3] BaseTools/Conf/tools_def: Fix >> CLANGDWARF_IA32_X64 >>=20 >> Drop the "-z max-page-size=3D0x40" option as it causes the ELF >> header to overflow into the .text section, causing undefined >> behaviour. >>=20 >> 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. >>=20 >> Test: >> The ELF generated by >> 'python UefiPayloadPkg/UniversalPayloadBuild.py -a IA32' boots. >>=20 >> Signed-off-by: Patrick Rudolph >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4357 >> --- >> BaseTools/Conf/tools_def.template | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >>=20 >> 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 =3D >> ENV(CLANG_BIN) >>=20 >>=20 >> # LLVM/CLANG doesn't support -n link option. So, it can't share the = same >> IA32_X64_DLINK_COMMON flag. >>=20 >> # LLVM/CLANG doesn't support common page size. So, it can't share the >> same GccBase.lds script. >>=20 >> -DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON =3D -nostdlib -Wl,-q,--gc- >> sections -z max-page-size=3D0x40 >>=20 >> +DEFINE CLANGDWARF_IA32_X64_DLINK_COMMON =3D -nostdlib -Wl,-q,-- >> gc-sections >>=20 >> DEFINE CLANGDWARF_DLINK2_FLAGS_COMMON =3D -Wl,-- >> script=3D$(EDK_TOOLS_PATH)/Scripts/ClangBase.lds >>=20 >> DEFINE CLANGDWARF_IA32_X64_ASLDLINK_FLAGS =3D >> DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) -Wl,-- >> defsym=3DPECOFF_HEADER_SIZE=3D0 >> DEF(CLANGDWARF_DLINK2_FLAGS_COMMON) -Wl,-- >> entry,ReferenceAcpiTable -u ReferenceAcpiTable >>=20 >> DEFINE CLANGDWARF_IA32_X64_DLINK_FLAGS =3D >> 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 >>=20 >> -- >> 2.39.1 >=20 --Apple-Mail=_FE23BF18-6060-4586-8A6A-DCA010D97B52 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii
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? :)

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

[2]

[3]


-----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=3D0x40" 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=3D4357
---
= 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 =        =3D
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   =3D = -nostdlib -Wl,-q,--gc-
sections -z max-page-size=3D0x40

+DEFINE = CLANGDWARF_IA32_X64_DLINK_COMMON   =3D -nostdlib = -Wl,-q,--
gc-sections

DEFINE CLANGDWARF_DLINK2_FLAGS_COMMON =     =3D = -Wl,--
script=3D$(EDK_TOOLS_PATH)/Scripts/ClangBase.lds

= DEFINE CLANGDWARF_IA32_X64_ASLDLINK_FLAGS = =3D
DEF(CLANGDWARF_IA32_X64_DLINK_COMMON) = -Wl,--
defsym=3DPECOFF_HEADER_SIZE=3D0
DEF(CLANGDWARF_DLINK2_FLAGS_C= OMMON) -Wl,--
entry,ReferenceAcpiTable -u ReferenceAcpiTable

= DEFINE CLANGDWARF_IA32_X64_DLINK_FLAGS =    =3D
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

--<= br>2.39.1


= --Apple-Mail=_FE23BF18-6060-4586-8A6A-DCA010D97B52--