On 08/22/17 10:00, Shi, Steven wrote:
> It is a link flag misuse in our GCC build toolchains, not
> compiler/linker's problem. Below patch can fix the wrong assembly
> function relocation type in PIE binary. With below patch, all the
> GCC5, GCC49 and GCC48 can build correctly images of OvmfPkgIa32X64 and
> OvmfPkgX64 platforms without my previous simple work around in my
> side. Please try it in your side.
>
> Since we are using GCC as linker command, we MUST pass -pie to ld with
> "-Wl,-pie", not just "--pie" or "-fpie".
>
> So, this means we never correctly build small+PIE 64bits code with GCC
> toolchains before (CLANG38 is correct). If you failed to enable
> PIE/LTO build before, it is worthy to revisit those failures with
> "-Wl,-pie". FYI.
The first question of Paolo (CC'd) was, when he saw this issue in my
last status report, whether we added "-fpie" to the link command line as
well. And, I confirmed, we did. This is how I responded to him:
> - in "BaseTools/Conf/build_rule.template", there's
>
> > [Static-Library-File]
> > <InputFile>
> > *.lib
> >
> > <ExtraDependency>
> > $(MAKE_FILE)
> >
> > <OutputFile>
> > $(DEBUG_DIR)(+)$(MODULE_NAME).dll
> >
> > [...]
> >
> > <Command.GCC>
> > "$(DLINK)" -o ${dst} $(DLINK_FLAGS)
> > -Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS)
> > $(DLINK2_FLAGS)
> > "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
>
> (see "$(CC_FLAGS)")
>
> - and in the build log, I see
>
> > "gcc" \
> > -o
> > Build/OvmfX64/DEBUG_GCC5/X64/UefiCpuPkg/CpuMpPei/CpuMpPei/DEBUG/CpuMpPei.dll
> > \
> > -nostdlib \
> > -Wl,-n,-q,--gc-sections \
> > -z common-page-size=0x40 \
> > -Wl,--entry,_ModuleEntryPoint \
> > -u _ModuleEntryPoint \
> >
> > -Wl,-Map,Build/OvmfX64/DEBUG_GCC5/X64/UefiCpuPkg/CpuMpPei/CpuMpPei/DEBUG/CpuMpPei.map
> > \
> > -Wl,-melf_x86_64,--oformat=elf64-x86-64 \
> > -flto \
> > -Os \
> >
> > -Wl,--start-group,@Build/OvmfX64/DEBUG_GCC5/X64/UefiCpuPkg/CpuMpPei/CpuMpPei/OUTPUT/static_library_files.lst,--end-group
> > \
> > -g \
> > -fshort-wchar \
> > -fno-builtin \
> > -fno-strict-aliasing \
> > -Wall \
> > -Werror \
> > -Wno-array-bounds \
> > -ffunction-sections \
> > -fdata-sections \
> > -include AutoGen.h \
> > -fno-common \
> > -DSTRING_ARRAY_NAME=CpuMpPeiStrings \
> > -m64 \
> > -fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" \
> > -maccumulate-outgoing-args \
> > -mno-red-zone \
> > -Wno-address \
> > -mcmodel=small \
> > -fpie \
> > -fno-asynchronous-unwind-tables \
> > -Wno-address \
> > -flto \
> > -DUSING_LTO \
> > -Os \
> > -mno-mmx \
> > -mno-sse \
> > -D DISABLE_NEW_DEPRECATED_INTERFACES \
> > -Wl,--defsym=PECOFF_HEADER_SIZE=0x228 \
> > -Wl,--script=BaseTools/Scripts/GccBase.lds \
> > -Wno-error
I don't understand why we need "-Wl,-pie" separately. The "gcc" binary
should pass it through to "ld" as necessary, IMO. This is what the gcc
documentation says:
> 3.13 Options for Linking
> ========================
> '-pie'
> Produce a position independent executable on targets that support
> it. For predictable results, you must also specify the same set
> of options used for compilation ('-fpie', '-fPIE', or model
> suboptions) when you specify this linker option.
>
> 3.18 Options for Code Generation Conventions
> ============================================
> '-fpie'
> '-fPIE'
> These options are similar to '-fpic' and '-fPIC', but generated
> position independent code can be only linked into executables.
> Usually these options are used when '-pie' GCC option is used
> during linking.
>
> '-fpie' and '-fPIE' both define the macros '__pie__' and
> '__PIE__'. The macros have the value 1 for '-fpie' and 2 for
> '-fPIE'.
This seems to suggest that "-pie" is the *master* switch (used only when
linking), and "-fpie" is a *prerequisite* for it (to be used both when
linking and compiling). Is this right?
If so, then I think this is a gcc usability bug. We don't generally
start our thinking from the linker side. The above implies that the
simple (hosted) command line:
$ gcc -o example -fpie source1.c source2.c
could also result in miscompilation, because "-pie" is not given, only
"-fpie".
On 08/22/17 10:00, Shi, Steven wrote:
> diff --git a/BaseTools/Conf/tools_def.template
> b/BaseTools/Conf/tools_def.template
> index 1fa3ca3..9e46d65 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -4375,7 +4375,7 @@ DEFINE GCC44_IA32_X64_DLINK_COMMON = -nostdlib
> -Wl,-n,-q,--gc-sections -z comm
> DEFINE GCC44_IA32_X64_ASLDLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON)
> -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable
> DEFINE GCC44_IA32_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_COMMON)
> -Wl,--entry,$(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT)
> -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map
> DEFINE GCC44_IA32_DLINK2_FLAGS = -Wl,--defsym=PECOFF_HEADER_SIZE=0x220
> DEF(GCC_DLINK2_FLAGS_COMMON)
> -DEFINE GCC44_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_FLAGS)
> -Wl,-melf_x86_64,--oformat=elf64-x86-64
> +DEFINE GCC44_X64_DLINK_FLAGS = DEF(GCC44_IA32_X64_DLINK_FLAGS)
> -Wl,-melf_x86_64,--oformat=elf64-x86-64 -Wl,-pie
> DEFINE GCC44_X64_DLINK2_FLAGS = -Wl,--defsym=PECOFF_HEADER_SIZE=0x228
> DEF(GCC_DLINK2_FLAGS_COMMON)
> DEFINE GCC44_ASM_FLAGS = DEF(GCC_ASM_FLAGS)
>
> @@ -4455,7 +4455,7 @@ DEFINE GCC49_IA32_X64_DLINK_COMMON = -nostdlib
> -Wl,-n,-q,--gc-sections -z comm
> DEFINE GCC49_IA32_X64_ASLDLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_COMMON)
> -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable
> DEFINE GCC49_IA32_X64_DLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_COMMON)
> -Wl,--entry,$(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT)
> -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map
> DEFINE GCC49_IA32_DLINK2_FLAGS = DEF(GCC48_IA32_DLINK2_FLAGS)
> -DEFINE GCC49_X64_DLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_FLAGS)
> -Wl,-melf_x86_64,--oformat=elf64-x86-64
> +DEFINE GCC49_X64_DLINK_FLAGS = DEF(GCC49_IA32_X64_DLINK_FLAGS)
> -Wl,-melf_x86_64,--oformat=elf64-x86-64 -Wl,-pie
> DEFINE GCC49_X64_DLINK2_FLAGS = DEF(GCC48_X64_DLINK2_FLAGS)
> DEFINE GCC49_ASM_FLAGS = DEF(GCC48_ASM_FLAGS)
> DEFINE GCC49_ARM_ASM_FLAGS = DEF(GCC48_ARM_ASM_FLAGS)
Do we think that this was an omission in commit a1b8baccc30b ("BaseTools
GCC: use 'gcc' as the linker command for GCC44 and later", 2016-07-23)?
Honestly, I'm quite a bit annoyed by this parameter forwarding mess
between "gcc" and "ld". If someone can submit a formal patch to fix
this, please do so (I also suggest to reassign BZ#671 to BaseTools
then). If there's an actual patch I can fetch or apply with git, I'll be
happy to test it.
Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
