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

[Leif Lindholm <leif.lindholm@linaro.org>]

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
>