Re: [PATCH 2/2] BaseTools/tools_def: AARCH64: disable LTO type mismatch warnings

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 21 June 2017 at 02:11, Gao, Liming <liming.gao@intel.com> wrote:
> Reviewed-by: Liming Gao <liming.gao@intel.com>
>

Thanks, pushed as 9ba8baae15ca


>>-----Original Message-----
>>From: Leif Lindholm [mailto:leif.lindholm@linaro.org]
>>Sent: Wednesday, June 21, 2017 4:00 AM
>>To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Zhu,
>>Yonghong <yonghong.zhu@intel.com>; lersek@redhat.com
>>Subject: Re: [PATCH 2/2] BaseTools/tools_def: AARCH64: disable LTO type
>>mismatch warnings
>>
>>On Tue, Jun 20, 2017 at 08:43:54PM +0200, Ard Biesheuvel wrote:
>>> On AARCH64, any code that may execute with the MMU off needs to be
>>built
>>> with -mstrict-align, given that unaligned accesses are not allowed unless
>>> the MMU is enabled. This does not only affect SEC and PEI modules, but
>>> also static libraries of the BASE type, which may be linked into such
>>> modules, as well as into modules of other types. As it turns out, the
>>> presence of -mstrict-align is reflected in the internal representations
>>> of the types defined in those libraries.
>>>
>>> When -fstrict-aliasing is passed to GCC, it assumes that pointers to
>>> objects of different types cannot refer to the same memory location, and
>>> attempts to exploit this fact when optimizing the code. Since such
>>> assumptions are only valid under very strict conditions which are not
>>> guaranteed to be met in EDK2, we disable this optimization by passing
>>> -fno-strict-aliasing by default.
>>
>>Just a comment - you don't necessarily have to update this excellent
>>commit mesage, but:
>>-fstrict-aliasing is GCC default, because it is a restriction in the C
>>language. Because it's a bit non-obvious, things can go hilariously
>>wrong in very non-obvious ways, and the potential optimization gains
>>are ulikely to be generally relevant, -fno-strict-aliasing is a
>>sensible thing to always have set (like we do).
>>
>>> When LTO is in effect, this applies equally to the code generation that
>>> may occur at link time, which is why the linker warns about unexpected
>>> differences in type definitions between the intermediate representations
>>> that are present in the object files being linked. This may result in
>>> warnings such as the one below, even if -fno-strict-aliasing is used:
>>>
>>>   MdePkg/Include/Library/BaseLib.h:1712:1:
>>>   warning: type of 'StrToGuid' does not match original declaration
>>>   [-Wlto-type-mismatch]
>>>    StrToGuid (
>>>    ^
>>>   MdePkg/Library/BaseLib/SafeString.c:1506:1:
>>>   note: 'StrToGuid' was previously declared here
>>>    StrToGuid (
>>>    ^
>>>   MdePkg/Library/BaseLib/SafeString.c:1506:1:
>>>   note: code may be misoptimized unless -fno-strict-aliasing is used
>>>
>>> This warning is inadvertently triggered when linking BASE libraries built
>>> with -mstrict-align into modules of types other than SEC or PEI, since the
>>> types are subtly different, even though the use of code that maintains
>>> strict alignment in a module that does not care about this is unlikely to
>>> cause problems. And even if it did, it would still only affect code built
>>> with -fstrict-aliasing enabled, which we disable unconditionally. So let's
>>> just silence the warning by passing -Wno-lto-type-mismatch.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>>Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>>
>>> ---
>>>  BaseTools/Conf/tools_def.template | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/BaseTools/Conf/tools_def.template
>>b/BaseTools/Conf/tools_def.template
>>> index 7a58ce365ed2..9a3ba9defb12 100755
>>> --- a/BaseTools/Conf/tools_def.template
>>> +++ b/BaseTools/Conf/tools_def.template
>>> @@ -5407,7 +5407,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS     =
>>DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS
>>>    DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
>>>
>>>  RELEASE_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -
>>flto -Wno-unused-but-set-variable -mcmodel=tiny -fomit-frame-pointer
>>> -RELEASE_GCC5_AARCH64_DLINK_FLAGS =
>>DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -
>>L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-
>>through=-llto-aarch64
>>> +RELEASE_GCC5_AARCH64_DLINK_FLAGS =
>>DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os -
>>L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 -Wl,-plugin-opt=-pass-
>>through=-llto-aarch64 -Wno-lto-type-mismatch
>>>
>>>    NOOPT_GCC5_AARCH64_CC_FLAGS    = DEF(GCC5_AARCH64_CC_FLAGS) -
>>O0 -mcmodel=small
>>>    NOOPT_GCC5_AARCH64_DLINK_FLAGS =
>>DEF(GCC5_AARCH64_DLINK_FLAGS) -z common-page-size=0x1000 -O0
>>> --
>>> 2.7.4
>>>