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.)

Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to