Laszlo: I will update the patch with your comments. Ard: We collect the size impact in Ovmf platform. Its impact is small. So, my patch enable this option for all targets. Below is the data collected on OvmfIa32X64.dsc with GCC5 tool chain. Raw image is a little bigger. But, the compressed size is a little smaller.
PEIFV 178472 --> 179176 +704 (Bytes) DXEFV 4062512 --> 4075056 +12544 (Bytes) FVMAIN_COMPACT 1190896 --> 1184920 -5976 (Bytes) Thanks Liming >-----Original Message----- >From: edk2-devel [mailto:[email protected]] On Behalf Of >Laszlo Ersek >Sent: Friday, August 25, 2017 7:48 PM >To: Ard Biesheuvel <[email protected]> >Cc: [email protected]; Gao, Liming <[email protected]> >Subject: Re: [edk2] [Patch] BaseTools: Enable --whole-archive in GCC tool >chain as the default option > >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: > >http://mid.mail-archive.com/df25cbe5-a6a0-79d5-ccd7- >[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 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
