From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4334921D1E2FB for ; Fri, 25 Aug 2017 04:45:47 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 74498C058EA3; Fri, 25 Aug 2017 11:48:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 74498C058EA3 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-78.phx2.redhat.com [10.3.116.78]) by smtp.corp.redhat.com (Postfix) with ESMTP id 79E8160BE7; Fri, 25 Aug 2017 11:48:21 +0000 (UTC) To: Ard Biesheuvel Cc: "edk2-devel@lists.01.org" , Liming Gao References: <1503556125-7024-1-git-send-email-liming.gao@intel.com> From: Laszlo Ersek Message-ID: <2cf820ad-4a6d-648e-560b-d223bef9b45d@redhat.com> Date: Fri, 25 Aug 2017 13:48:20 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 25 Aug 2017 11:48:22 +0000 (UTC) Subject: Re: [Patch] BaseTools: Enable --whole-archive in GCC tool chain as the default option X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Aug 2017 11:45:47 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/25/17 12:30, Ard Biesheuvel wrote: > On 25 August 2017 at 11:28, Laszlo Ersek 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 >>> Cc: Yonghong Zhu >>> --- >>> 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 >> >> (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-f0ad53c2ed03@redhat.com 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