Laszlo: I will take BZ#671 and create the formal patch for review.
Thanks Liming > -----Original Message----- > From: Laszlo Ersek [mailto:[email protected]] > Sent: Tuesday, August 22, 2017 7:59 PM > To: Shi, Steven <[email protected]>; Ard Biesheuvel > <[email protected]> > Cc: edk2-devel-01 <[email protected]>; Alex Williamson > <[email protected]>; Justen, Jordan L > <[email protected]>; Gao, Liming <[email protected]>; Kinney, > Michael D <[email protected]>; Paolo Bonzini > <[email protected]> > Subject: Re: [edk2] [PATCH 1/1] BaseTools/tools_def.template: revert to large > code model for X64/GCC5/LTO > > 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 [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
