* [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64
@ 2023-03-06 8:37 Patrick Rudolph
2023-03-06 8:37 ` [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64 Patrick Rudolph
` (4 more replies)
0 siblings, 5 replies; 20+ messages in thread
From: Patrick Rudolph @ 2023-03-06 8:37 UTC (permalink / raw)
Cc: devel, guo.dong, ray.ni, sean, james.lu, gua.guo
The clang toolchain might default to fPIE/fPIC, which prevents
lld from linking the objects into a binary.
Specify -fno-pie -fno-pic as done on GCC to fix linking.
Test:
Building the Universal Payload using the command
'python UefiPayloadPkg/UniversalPayloadBuild.py -a IA32' actually
works.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=4356
---
BaseTools/Conf/tools_def.template | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 471eb67c0c..9b59bd75c3 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -2888,7 +2888,7 @@ DEFINE CLANGDWARF_X64_DLINK2_FLAGS = -Wl,--defsym=PECOFF_HEADER_SIZE=0x22
*_CLANGDWARF_IA32_RC_PATH = DEF(CLANGDWARF_IA32_PREFIX)llvm-rc
*_CLANGDWARF_IA32_ASLCC_FLAGS = DEF(GCC_ASLCC_FLAGS) -m32 -fno-lto DEF(CLANG38_IA32_TARGET)
-*_CLANGDWARF_IA32_ASLDLINK_FLAGS = DEF(CLANGDWARF_IA32_X64_ASLDLINK_FLAGS) -Wl,-m,elf_i386 -fuse-ld=lld
+*_CLANGDWARF_IA32_ASLDLINK_FLAGS = DEF(CLANGDWARF_IA32_X64_ASLDLINK_FLAGS) -Wl,-m,elf_i386 -fuse-ld=lld -no-pie
*_CLANGDWARF_IA32_ASM_FLAGS = DEF(GCC5_ASM_FLAGS) -m32 -march=i386 DEF(CLANG38_IA32_TARGET)
*_CLANGDWARF_IA32_RC_FLAGS = DEF(GCC_IA32_RC_FLAGS)
*_CLANGDWARF_IA32_OBJCOPY_FLAGS =
@@ -2897,17 +2897,17 @@ DEFINE CLANGDWARF_X64_DLINK2_FLAGS = -Wl,--defsym=PECOFF_HEADER_SIZE=0x22
*_CLANGDWARF_IA32_ASLPP_FLAGS = DEF(GCC_ASLPP_FLAGS) DEF(CLANG38_IA32_TARGET)
*_CLANGDWARF_IA32_VFRPP_FLAGS = DEF(GCC_VFRPP_FLAGS) DEF(CLANG38_IA32_TARGET)
-DEBUG_CLANGDWARF_IA32_CC_FLAGS = DEF(CLANG38_ALL_CC_FLAGS) -m32 -Oz -flto -march=i586 DEF(CLANG38_IA32_TARGET) -g -malign-double
+DEBUG_CLANGDWARF_IA32_CC_FLAGS = DEF(CLANG38_ALL_CC_FLAGS) -fno-pic -fno-pie -m32 -Oz -flto -march=i586 DEF(CLANG38_IA32_TARGET) -g -malign-double
DEBUG_CLANGDWARF_IA32_DLINK_FLAGS = DEF(CLANGDWARF_IA32_X64_DLINK_FLAGS) -flto -Wl,-O3 -Wl,-melf_i386 -Wl,--oformat,elf32-i386
-DEBUG_CLANGDWARF_IA32_DLINK2_FLAGS = DEF(CLANGDWARF_IA32_DLINK2_FLAGS) -O3 -fuse-ld=lld
+DEBUG_CLANGDWARF_IA32_DLINK2_FLAGS = DEF(CLANGDWARF_IA32_DLINK2_FLAGS) -O3 -fuse-ld=lld -no-pie
-RELEASE_CLANGDWARF_IA32_CC_FLAGS = DEF(CLANG38_ALL_CC_FLAGS) -m32 -Oz -flto -march=i586 DEF(CLANG38_IA32_TARGET) -malign-double
+RELEASE_CLANGDWARF_IA32_CC_FLAGS = DEF(CLANG38_ALL_CC_FLAGS) -fno-pic -fno-pie -m32 -Oz -flto -march=i586 DEF(CLANG38_IA32_TARGET) -malign-double
RELEASE_CLANGDWARF_IA32_DLINK_FLAGS = DEF(CLANGDWARF_IA32_X64_DLINK_FLAGS) -flto -Wl,-O3 -Wl,-melf_i386 -Wl,--oformat,elf32-i386
-RELEASE_CLANGDWARF_IA32_DLINK2_FLAGS = DEF(CLANGDWARF_IA32_DLINK2_FLAGS) -O3 -fuse-ld=lld
+RELEASE_CLANGDWARF_IA32_DLINK2_FLAGS = DEF(CLANGDWARF_IA32_DLINK2_FLAGS) -O3 -fuse-ld=lld -no-pie
-NOOPT_CLANGDWARF_IA32_CC_FLAGS = DEF(CLANG38_ALL_CC_FLAGS) -m32 -O0 -march=i586 DEF(CLANG38_IA32_TARGET) -g -malign-double
+NOOPT_CLANGDWARF_IA32_CC_FLAGS = DEF(CLANG38_ALL_CC_FLAGS) -fno-pic -fno-pie -m32 -O0 -march=i586 DEF(CLANG38_IA32_TARGET) -g -malign-double
NOOPT_CLANGDWARF_IA32_DLINK_FLAGS = DEF(CLANGDWARF_IA32_X64_DLINK_FLAGS) -Wl,-O0 -Wl,-melf_i386 -Wl,--oformat,elf32-i386
-NOOPT_CLANGDWARF_IA32_DLINK2_FLAGS = DEF(CLANGDWARF_IA32_DLINK2_FLAGS) -O0 -fuse-ld=lld
+NOOPT_CLANGDWARF_IA32_DLINK2_FLAGS = DEF(CLANGDWARF_IA32_DLINK2_FLAGS) -O0 -fuse-ld=lld -no-pie
##########################
# CLANGDWARF X64 definitions
--
2.39.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
2023-03-06 8:37 [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64 Patrick Rudolph
@ 2023-03-06 8:37 ` Patrick Rudolph
2023-03-06 8:39 ` Sean Rhodes
2023-03-06 8:37 ` [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX Patrick Rudolph
` (3 subsequent siblings)
4 siblings, 1 reply; 20+ messages in thread
From: Patrick Rudolph @ 2023-03-06 8:37 UTC (permalink / raw)
Cc: devel, guo.dong, ray.ni, sean, james.lu, gua.guo
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
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX
2023-03-06 8:37 [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64 Patrick Rudolph
2023-03-06 8:37 ` [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64 Patrick Rudolph
@ 2023-03-06 8:37 ` Patrick Rudolph
2023-03-06 8:39 ` Sean Rhodes
2023-03-15 16:54 ` [edk2-devel] " Sheng Lean Tan
2023-03-15 16:45 ` [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64 Sheng Lean Tan
` (2 subsequent siblings)
4 siblings, 2 replies; 20+ messages in thread
From: Patrick Rudolph @ 2023-03-06 8:37 UTC (permalink / raw)
Cc: devel, guo.dong, ray.ni, sean, james.lu, gua.guo
Add protocol gEfiHiiPackageListProtocolGuid to DEPEX
to make sure it's present before using it.
Fixes ASSERTION seen on DEBUG build.
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
---
ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf b/ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
index b0c8e8f84b..d0d849a1eb 100644
--- a/ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
@@ -57,4 +57,4 @@
gEfiShellDynamicCommandProtocolGuid ## PRODUCES
[DEPEX]
- TRUE
+ gEfiHiiPackageListProtocolGuid
--
2.39.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
2023-03-06 8:37 ` [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64 Patrick Rudolph
@ 2023-03-06 8:39 ` Sean Rhodes
2023-03-15 16:52 ` [edk2-devel] " Sheng Lean Tan
0 siblings, 1 reply; 20+ messages in thread
From: Sean Rhodes @ 2023-03-06 8:39 UTC (permalink / raw)
To: Patrick Rudolph; +Cc: devel, guo.dong, ray.ni, james.lu, gua.guo
[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]
Reviewed-by: Sean Rhodes <sean@starlabs.systems>
On Mon, 6 Mar 2023 at 08:38, Patrick Rudolph <patrick.rudolph@9elements.com>
wrote:
> 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: 2706 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX
2023-03-06 8:37 ` [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX Patrick Rudolph
@ 2023-03-06 8:39 ` Sean Rhodes
2023-03-15 16:54 ` [edk2-devel] " Sheng Lean Tan
1 sibling, 0 replies; 20+ messages in thread
From: Sean Rhodes @ 2023-03-06 8:39 UTC (permalink / raw)
To: Patrick Rudolph; +Cc: devel, guo.dong, ray.ni, james.lu, gua.guo
[-- Attachment #1: Type: text/plain, Size: 976 bytes --]
Reviewed-by: Sean Rhodes <sean@starlabs.systems>
On Mon, 6 Mar 2023 at 08:38, Patrick Rudolph <patrick.rudolph@9elements.com>
wrote:
> Add protocol gEfiHiiPackageListProtocolGuid to DEPEX
> to make sure it's present before using it.
>
> Fixes ASSERTION seen on DEBUG build.
>
> Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
> ---
> ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> index b0c8e8f84b..d0d849a1eb 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
> @@ -57,4 +57,4 @@
> gEfiShellDynamicCommandProtocolGuid ## PRODUCES
>
> [DEPEX]
> - TRUE
> + gEfiHiiPackageListProtocolGuid
> --
> 2.39.1
>
>
[-- Attachment #2: Type: text/html, Size: 1511 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64
2023-03-06 8:37 [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64 Patrick Rudolph
2023-03-06 8:37 ` [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64 Patrick Rudolph
2023-03-06 8:37 ` [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX Patrick Rudolph
@ 2023-03-15 16:45 ` Sheng Lean Tan
[not found] ` <174CA58C1DBF1462.24874@groups.io>
2023-03-15 22:16 ` Marvin Häuser
4 siblings, 0 replies; 20+ messages in thread
From: Sheng Lean Tan @ 2023-03-15 16:45 UTC (permalink / raw)
To: devel, patrick.rudolph; +Cc: guo.dong, ray.ni, sean, james.lu, gua.guo
[-- Attachment #1: Type: text/plain, Size: 56 bytes --]
Hi all,
Can someone please help to review this?
Thanks.
[-- Attachment #2: Type: text/html, Size: 108 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64
[not found] ` <174CA58C1DBF1462.24874@groups.io>
@ 2023-03-15 16:51 ` Sheng Lean Tan
0 siblings, 0 replies; 20+ messages in thread
From: Sheng Lean Tan @ 2023-03-15 16:51 UTC (permalink / raw)
To: devel, sheng.tan
Cc: patrick.rudolph, guo.dong, ray.ni, sean, james.lu, gua.guo,
Feng, Bob C, Gao, Liming, yuwei.chen
[-- Attachment #1: Type: text/plain, Size: 228 bytes --]
I think the right reviewer was not added.
Added them.
On Wed, 15 Mar 2023 at 17:46, Sheng Lean Tan via groups.io <sheng.tan=
9elements.com@groups.io> wrote:
> Hi all,
> Can someone please help to review this?
> Thanks.
>
>
>
[-- Attachment #2: Type: text/html, Size: 798 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
2023-03-06 8:39 ` Sean Rhodes
@ 2023-03-15 16:52 ` Sheng Lean Tan
0 siblings, 0 replies; 20+ messages in thread
From: Sheng Lean Tan @ 2023-03-15 16:52 UTC (permalink / raw)
To: devel, sean, Feng, Bob C, Gao, Liming, yuwei.chen
Cc: Patrick Rudolph, guo.dong, ray.ni, james.lu, gua.guo
[-- Attachment #1: Type: text/plain, Size: 28 bytes --]
adding the right reviewers.
[-- Attachment #2: Type: text/html, Size: 49 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX
2023-03-06 8:37 ` [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX Patrick Rudolph
2023-03-06 8:39 ` Sean Rhodes
@ 2023-03-15 16:54 ` Sheng Lean Tan
2023-03-16 2:08 ` Gao, Zhichao
1 sibling, 1 reply; 20+ messages in thread
From: Sheng Lean Tan @ 2023-03-15 16:54 UTC (permalink / raw)
To: devel, patrick.rudolph, Zhichao Gao
Cc: guo.dong, ray.ni, sean, james.lu, gua.guo
[-- Attachment #1: Type: text/plain, Size: 15 bytes --]
Added Zhichao.
[-- Attachment #2: Type: text/html, Size: 47 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64
2023-03-06 8:37 [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64 Patrick Rudolph
` (3 preceding siblings ...)
[not found] ` <174CA58C1DBF1462.24874@groups.io>
@ 2023-03-15 22:16 ` Marvin Häuser
2023-03-15 22:50 ` Ard Biesheuvel
4 siblings, 1 reply; 20+ messages in thread
From: Marvin Häuser @ 2023-03-15 22:16 UTC (permalink / raw)
To: Patrick Rudolph, devel
[-- Attachment #1: Type: text/plain, Size: 143 bytes --]
Hi,
Why does the title mention X64? From what I can see, PIE is unaffected for X64 (and we really want it to be).
Best regards,
Marvin
[-- Attachment #2: Type: text/html, Size: 163 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64
2023-03-15 22:16 ` Marvin Häuser
@ 2023-03-15 22:50 ` Ard Biesheuvel
2023-03-15 22:56 ` Marvin Häuser
0 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2023-03-15 22:50 UTC (permalink / raw)
To: devel, mhaeuser; +Cc: Patrick Rudolph
On Wed, 15 Mar 2023 at 23:16, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
> Hi,
>
> Why does the title mention X64? From what I can see, PIE is unaffected for X64 (and we really want it to be).
>
Why?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64
2023-03-15 22:50 ` Ard Biesheuvel
@ 2023-03-15 22:56 ` Marvin Häuser
2023-03-16 0:05 ` Ard Biesheuvel
0 siblings, 1 reply; 20+ messages in thread
From: Marvin Häuser @ 2023-03-15 22:56 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: devel, Patrick Rudolph
> On 15. Mar 2023, at 23:51, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 15 Mar 2023 at 23:16, Marvin Häuser <mhaeuser@posteo.de> wrote:
>>
>> Hi,
>>
>> Why does the title mention X64? From what I can see, PIE is unaffected for X64 (and we really want it to be).
>>
>
> Why?
Why what? By “PIE is unaffected for X64” I meant by the patch (which only changes IA32 macros, no?). I can also see how the last part is a bit ambiguous, but I meant it literally - we really want X64 to be [unaffected] (which it is, by the patch, but the title implies otherwise, doesn’t it?).
Best regards,
Marvin
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64
2023-03-15 22:56 ` Marvin Häuser
@ 2023-03-16 0:05 ` Ard Biesheuvel
2023-03-17 14:07 ` Patrick Rudolph
0 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2023-03-16 0:05 UTC (permalink / raw)
To: Marvin Häuser; +Cc: devel, Patrick Rudolph
On Wed, 15 Mar 2023 at 23:57, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
>
> > On 15. Mar 2023, at 23:51, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 15 Mar 2023 at 23:16, Marvin Häuser <mhaeuser@posteo.de> wrote:
> >>
> >> Hi,
> >>
> >> Why does the title mention X64? From what I can see, PIE is unaffected for X64 (and we really want it to be).
> >>
> >
> > Why?
>
> Why what? By “PIE is unaffected for X64” I meant by the patch (which only changes IA32 macros, no?). I can also see how the last part is a bit ambiguous, but I meant it literally - we really want X64 to be [unaffected] (which it is, by the patch, but the title implies otherwise, doesn’t it?).
>
OK, i got confused. Thanks for clearing that up for me :-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX
2023-03-15 16:54 ` [edk2-devel] " Sheng Lean Tan
@ 2023-03-16 2:08 ` Gao, Zhichao
0 siblings, 0 replies; 20+ messages in thread
From: Gao, Zhichao @ 2023-03-16 2:08 UTC (permalink / raw)
To: devel@edk2.groups.io, Tan, Lean Sheng, Rudolph, Patrick
Cc: Dong, Guo, Ni, Ray, Rhodes, Sean, Lu, James, Guo, Gua
[-- Attachment #1: Type: text/plain, Size: 940 bytes --]
Did you make sure the change would really fix the issue?
Just add the DEPX would only make sure the protocol is installed but not installed to the image handle. The return value would be still error code.
And the issue is still there.
The HII image related protocol should not be the MUST have for the application. Using the DEBUG ASSERT is not appropriate. I would suggest to remove the ASSERT instead.
Thanks,
Zhichao
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sheng Lean Tan
Sent: Thursday, March 16, 2023 12:54 AM
To: devel@edk2.groups.io; Rudolph, Patrick <patrick.rudolph@9elements.com>; Gao, Zhichao <zhichao.gao@intel.com>
Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Rhodes, Sean <sean@starlabs.systems>; Lu, James <james.lu@intel.com>; Guo, Gua <gua.guo@intel.com>
Subject: Re: [edk2-devel] [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX
Added Zhichao.
[-- Attachment #2: Type: text/html, Size: 3500 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64
2023-03-16 0:05 ` Ard Biesheuvel
@ 2023-03-17 14:07 ` Patrick Rudolph
0 siblings, 0 replies; 20+ messages in thread
From: Patrick Rudolph @ 2023-03-17 14:07 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Marvin Häuser, devel
Fixed the title.
Sorry for the confusion.
On Thu, Mar 16, 2023 at 1:05 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 15 Mar 2023 at 23:57, Marvin Häuser <mhaeuser@posteo.de> wrote:
> >
> >
> > > On 15. Mar 2023, at 23:51, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Wed, 15 Mar 2023 at 23:16, Marvin Häuser <mhaeuser@posteo.de> wrote:
> > >>
> > >> Hi,
> > >>
> > >> Why does the title mention X64? From what I can see, PIE is unaffected for X64 (and we really want it to be).
> > >>
> > >
> > > Why?
> >
> > Why what? By “PIE is unaffected for X64” I meant by the patch (which only changes IA32 macros, no?). I can also see how the last part is a bit ambiguous, but I meant it literally - we really want X64 to be [unaffected] (which it is, by the patch, but the title implies otherwise, doesn’t it?).
> >
>
> OK, i got confused. Thanks for clearing that up for me :-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
2023-03-17 14:06 ` [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64 Patrick Rudolph
@ 2023-03-26 19:39 ` Sheng Lean Tan
[not found] ` <17500F66A352AD33.14179@groups.io>
1 sibling, 0 replies; 20+ messages in thread
From: Sheng Lean Tan @ 2023-03-26 19:39 UTC (permalink / raw)
To: devel, patrick.rudolph, Feng, Bob C, Gao, Liming
Cc: mhaeuser, ardb, yuwei.chen
[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]
Can someone also help to review this please?
Thanks.
On Fri, 17 Mar 2023 at 15:06, Patrick Rudolph <patrick.rudolph@9elements.com>
wrote:
> 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
>
>
>
> ------------
> 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/6757431
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [sheng.tan@9elements.com
> ]
> ------------
>
>
>
[-- Attachment #2: Type: text/html, Size: 3697 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
[not found] ` <17500F66A352AD33.14179@groups.io>
@ 2023-03-26 19:42 ` Sheng Lean Tan
0 siblings, 0 replies; 20+ messages in thread
From: Sheng Lean Tan @ 2023-03-26 19:42 UTC (permalink / raw)
To: devel, sheng.tan, rebecca
Cc: patrick.rudolph, Feng, Bob C, Gao, Liming, mhaeuser, ardb,
yuwei.chen
[-- Attachment #1: Type: text/plain, Size: 2623 bytes --]
Added @rebecca@bsdio.com <rebecca@bsdio.com> as well.
On Sun, 26 Mar 2023 at 21:39, Sheng Lean Tan via groups.io <sheng.tan=
9elements.com@groups.io> wrote:
> Can someone also help to review this please?
> Thanks.
>
>
> On Fri, 17 Mar 2023 at 15:06, Patrick Rudolph <
> patrick.rudolph@9elements.com> wrote:
>
>> 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
>>
>>
>>
>> ------------
>> 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/6757431
>> Group Owner: devel+owner@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [
>> sheng.tan@9elements.com]
>> ------------
>>
>>
>>
>
>
[-- Attachment #2: Type: text/html, Size: 4404 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
2023-03-28 5:38 回复: " gaoliming
@ 2023-03-28 11:25 ` Marvin Häuser
0 siblings, 0 replies; 20+ messages in thread
From: Marvin Häuser @ 2023-03-28 11:25 UTC (permalink / raw)
To: gaoliming
Cc: devel, patrick.rudolph, guo.dong, gua.guo, james.lu, ray.ni, ardb
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]
>> -=-=-=-=-=-=
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
2023-03-31 4:53 回复: " gaoliming
@ 2023-03-31 10:57 ` Marvin Häuser
2023-04-03 0:52 ` 回复: " gaoliming
0 siblings, 1 reply; 20+ messages in thread
From: Marvin Häuser @ 2023-03-31 10:57 UTC (permalink / raw)
To: gaoliming
Cc: devel, patrick.rudolph, guo.dong, gua.guo, james.lu, ray.ni, ardb
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]
>>>> -=-=-=-=-=-=
>>
>>
>>
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [edk2-devel] [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64
2023-04-03 0:52 ` 回复: " gaoliming
@ 2023-04-03 5:53 ` Patrick Rudolph
0 siblings, 0 replies; 20+ messages in thread
From: Patrick Rudolph @ 2023-04-03 5:53 UTC (permalink / raw)
To: devel, gaoliming; +Cc: mhaeuser, guo.dong, gua.guo, james.lu, ray.ni, ardb
Hi,
I cannot reproduce the issue on my setup. Please ignore it for now.
Regards,
Patrick Rudolph
On Mon, Apr 3, 2023 at 2:52 AM gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> Patrick:
> Can you give the reproduce step to generate ELF image that doesn't work with the option max-page-size?
>
> > -----邮件原件-----
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Marvin
> > H?user
> > 发送时间: 2023年3月31日 18:58
> > 收件人: 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
> >
> > 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]
> > >>>> -=-=-=-=-=-=
> > >>
> > >>
> > >>
> > >>
> > >>
> > >
> > >
> > >
> >
> >
> >
> >
> >
>
>
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2023-04-03 5:53 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-06 8:37 [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64 Patrick Rudolph
2023-03-06 8:37 ` [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64 Patrick Rudolph
2023-03-06 8:39 ` Sean Rhodes
2023-03-15 16:52 ` [edk2-devel] " Sheng Lean Tan
2023-03-06 8:37 ` [PATCH 3/3] ShellPkg/TftpDynamicCommand.inf: Add missing DEPEX Patrick Rudolph
2023-03-06 8:39 ` Sean Rhodes
2023-03-15 16:54 ` [edk2-devel] " Sheng Lean Tan
2023-03-16 2:08 ` Gao, Zhichao
2023-03-15 16:45 ` [edk2-devel] [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32_X64 Sheng Lean Tan
[not found] ` <174CA58C1DBF1462.24874@groups.io>
2023-03-15 16:51 ` Sheng Lean Tan
2023-03-15 22:16 ` Marvin Häuser
2023-03-15 22:50 ` Ard Biesheuvel
2023-03-15 22:56 ` Marvin Häuser
2023-03-16 0:05 ` Ard Biesheuvel
2023-03-17 14:07 ` Patrick Rudolph
-- strict thread matches above, loose matches on Subject: below --
2023-03-17 14:06 [PATCH 1/3] BaseTools/Conf/tools_def: Fix linking using CLANGDWARF_IA32 Patrick Rudolph
2023-03-17 14:06 ` [PATCH 2/3] BaseTools/Conf/tools_def: Fix CLANGDWARF_IA32_X64 Patrick Rudolph
2023-03-26 19:39 ` [edk2-devel] " Sheng Lean Tan
[not found] ` <17500F66A352AD33.14179@groups.io>
2023-03-26 19:42 ` Sheng Lean Tan
2023-03-28 5:38 回复: " gaoliming
2023-03-28 11:25 ` Marvin Häuser
2023-03-31 4:53 回复: " gaoliming
2023-03-31 10:57 ` Marvin Häuser
2023-04-03 0:52 ` 回复: " gaoliming
2023-04-03 5:53 ` Patrick Rudolph
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox