Re: [edk2-devel] [Patch] BaseTools tools_def.template: Add back -fno-pie option in GCC49 tool chain

["Laszlo Ersek" <lersek@redhat.com>]

On 02/04/20 13:52, Gao, Liming wrote:
> Laszlo:
> 
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, February 4, 2020 8:02 PM
>> To: devel@edk2.groups.io; Gao, Liming <liming.gao@intel.com>
>> Cc: Feng, Bob C <bob.c.feng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: Re: [edk2-devel] [Patch] BaseTools tools_def.template: Add back -fno-pie option in GCC49 tool chain
>>
>> (+Ard)
>>
>> On 02/04/20 05:54, Liming Gao wrote:
>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2502
>>> This option is required to make GCC49 tool chain work with the high
>>> version GCC compiler.
>>>
>>> Cc: Bob Feng <bob.c.feng@intel.com>
>>> Signed-off-by: Liming Gao <liming.gao@intel.com>
>>> ---
>>>  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 feee2bbf16..d02424ae44 100755
>>> --- a/BaseTools/Conf/tools_def.template
>>> +++ b/BaseTools/Conf/tools_def.template
>>> @@ -1974,7 +1974,7 @@ DEFINE GCC48_ARM_ASLDLINK_FLAGS      = DEF(GCC_ARM_ASLDLINK_FLAGS) -Wl,--oformat
>>>  DEFINE GCC48_AARCH64_ASLDLINK_FLAGS  = DEF(GCC_AARCH64_ASLDLINK_FLAGS)
>>>  DEFINE GCC48_ASLCC_FLAGS             = DEF(GCC_ASLCC_FLAGS)
>>>
>>> -DEFINE GCC49_IA32_CC_FLAGS           = DEF(GCC48_IA32_CC_FLAGS)
>>> +DEFINE GCC49_IA32_CC_FLAGS           = DEF(GCC48_IA32_CC_FLAGS) -fno-pic -fno-pie
>>>  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,--defsym=PECOFF_HEADER_SIZE=0
>> DEF(GCC_DLINK2_FLAGS_COMMON) -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable
>>> @@ -1997,7 +1997,7 @@ DEFINE GCC49_ARM_ASLDLINK_FLAGS      = DEF(GCC48_ARM_ASLDLINK_FLAGS)
>>>  DEFINE GCC49_AARCH64_ASLDLINK_FLAGS  = DEF(GCC48_AARCH64_ASLDLINK_FLAGS)
>>>  DEFINE GCC49_ASLCC_FLAGS             = DEF(GCC48_ASLCC_FLAGS)
>>>
>>> -DEFINE GCC5_IA32_CC_FLAGS            = DEF(GCC49_IA32_CC_FLAGS) -fno-pic -fno-pie
>>> +DEFINE GCC5_IA32_CC_FLAGS            = DEF(GCC49_IA32_CC_FLAGS)
>>>  DEFINE GCC5_X64_CC_FLAGS             = DEF(GCC49_X64_CC_FLAGS)
>>>  DEFINE GCC5_IA32_X64_DLINK_COMMON    = DEF(GCC49_IA32_X64_DLINK_COMMON)
>>>  DEFINE GCC5_IA32_X64_ASLDLINK_FLAGS  = DEF(GCC49_IA32_X64_ASLDLINK_FLAGS)
>>>
>>
>> - What has changed relative to commit 11d0cd23dd1b ("BaseTools/tools_def
>> IA32: drop -no-pie linker option for GCC49", 2018-06-18)?
>>
>> - Also, if we are reverting one half of 11d0cd23dd1b (the compiler
>> flags), shouldn't we then revert the other half too (the linker flags)?
> 
> Yes. Half change is revert. CC_FLAGS is added back. DLINK flag is not,
> because GCC4.9 doesn't know the link option -no-pie. But, GCC 4.9 accepts the CC option -fno-pie.
> I verify this change. CC flags -fno-pie can resolve the build failure with GCC7.4. I also see -fno-pie option 
> Is in GCC ARM and AARCH64 arch. So, I think this change is enough. 
> 
>>
>> - The commit message says, "work with the high version GCC compiler".
>> What does that mean? If it is 4.9.x, with x>2, then I agree the patch is
>> justified (because commit 11d0cd23dd1b was apparently made for 4.9.2).
>> But if the phrase stands for gcc8 or so (just an example), then I don't
>> think the patch is a good idea; users of gcc8 can just specify the GCC5
>> toolchain.
>>
>> Ah, indeed, I need only look at TianoCore#2502:
>>
>> "GCC49 tool chain meets with the build failure when GCC7.4 compiler".
>>
>> So I think this approach is wrong. Unless there is a new gcc-4.9.x
>> release, i.e., after gcc-4.9.2, I think we still need commit
>> 11d0cd23dd1b in place. And, please use GCC5 for gcc-7.4 -- is there a
>> problem with that?
> 
> By design, GCC49 can work with the high version GCC compiler like GCC5. 
> GCC49 is the tool chain without LTO enable. GCC5 is the tool chain with LTO. 
> So, they are for two different GCC setting. They should both support 
> high version GCC compiler. GCC49 supported GCC compiler version is from GCC 4.9.
> GCC5 supported GCC compiler version is from GCC 5.0. I know GCC49 or GCC5 tool chain 
> name brings a little confuse. I will add more detail info in tools_def.txt for them. 

Ah right, thanks for reminding me of this!

OK, I no longer object to this patch.

Thanks!
Laszlo