On 08/25/17 12:30, Ard Biesheuvel wrote: > On 25 August 2017 at 11:28, Laszlo Ersek <[email protected]> wrote: >> On 08/24/17 08:28, Liming Gao wrote: >>> https://bugzilla.tianocore.org/show_bug.cgi?id=581 >> >> (1) I suggest adding one sentence (before you push the patch): >> >> "The --whole-archive linker option helps us catch multiply defined symbols." >> >>> Contributed-under: TianoCore Contribution Agreement 1.1 >>> Signed-off-by: Liming Gao <[email protected]> >>> Cc: Yonghong Zhu <[email protected]> >>> --- >>> BaseTools/Conf/tools_def.template | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/BaseTools/Conf/tools_def.template >>> b/BaseTools/Conf/tools_def.template >>> index 24956e4..a85afd5 100755 >>> --- a/BaseTools/Conf/tools_def.template >>> +++ b/BaseTools/Conf/tools_def.template >>> @@ -4377,7 +4377,7 @@ DEFINE GCC44_IA32_CC_FLAGS = >>> DEF(GCC44_ALL_CC_FLAGS) -m32 -march=i586 >>> DEFINE GCC44_X64_CC_FLAGS = DEF(GCC44_ALL_CC_FLAGS) -m64 >>> -fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))" >>> -maccumulate-outgoing-args -mno-red-zone -Wno-address -mcmodel=small -fpie >>> -fno-asynchronous-unwind-tables >>> DEFINE GCC44_IA32_X64_DLINK_COMMON = -nostdlib -Wl,-n,-q,--gc-sections >>> -z common-page-size=0x20 >>> 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_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,--whole-archive >>> 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_DLINK2_FLAGS = >>> -Wl,--defsym=PECOFF_HEADER_SIZE=0x228 DEF(GCC_DLINK2_FLAGS_COMMON) >>> @@ -4457,7 +4457,7 @@ DEFINE GCC49_IA32_CC_FLAGS = >>> DEF(GCC48_IA32_CC_FLAGS) >>> DEFINE GCC49_X64_CC_FLAGS = DEF(GCC48_X64_CC_FLAGS) >>> DEFINE GCC49_IA32_X64_DLINK_COMMON = -nostdlib -Wl,-n,-q,--gc-sections >>> -z common-page-size=0x40 >>> 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_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,--whole-archive >>> 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_DLINK2_FLAGS = DEF(GCC48_X64_DLINK2_FLAGS) >>> >> >> This patch doesn't apply directly on current master, due to context >> differences introduced by 2f7f1e73c10f ("BaseTools: Add the missing -pie >> link option in GCC tool chain", 2017-08-23). >> >> However, it does apply to the parent of 2f7f1e73c10f, and from there it >> can be rebased without manual conflict resolution. >> >> I did just that, and tested the patch with GCC48, IA32, IA32X64, and X64 >> OVMF. It works for me: >> >> Tested-by: Laszlo Ersek <[email protected]> >> >> (2) If Ard agrees, can you please post a similar patch for ARM/AARCH64, >> GCC? (I think that should be a separate patch, so that I don't have to >> re-test the IA32/X64 change.) >> > > Didn't we decide this should be DEBUG/NOOPT only, due to code size increase?
I didn't remember off-hand, but I found this in my mailbox: [email protected]">http://mid.mail-archive.com/[email protected] On 05/26/17 11:05, Laszlo Ersek wrote: > - GCC toolchains: I think I'd like --whole-archive to become the > default (regardless of build target), since there don't seem to be > any relevant size costs to it. Up-thread, Mike wrote "Total used difference = 1816 bytes larger with -whole-archive". Nonetheless, if we want to be super cautious, I don't mind if RELEASE doesn't get --whole-archive. Thanks, Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

