Re: [PATCH 0/4] Fix runtime issue in XenBusDxe when compiled with GCC5

[Laszlo Ersek <lersek@redhat.com>]

On 02/21/17 23:45, Jordan Justen wrote:
> On 2017-02-21 11:08:24, Rebecca Cran wrote:
>> On 2/21/2017 12:02 PM, Laszlo Ersek wrote:
>>
>>> But in this case, the full edk2 codebase has to be grepped for
>>> VA_LIST-taking functions, and all of them must be flipped to EFIAPI, if
>>> they currently aren't EFIAPI. Covering just XenStoreVSPrint() seems
>>> incomplete. (Note: CryptoPkg/Library/OpensslLib is an exception.)
>>
>> I think this was discussed previously but I can't remember: is there a 
>> reason for not just compiling everything with -mabi=ms ?
>>
> 
> Originally GCC didn't support -mabi=ms. Once it gained support, it
> then produced larger executables. Nowadays (and for quite some time),
> I think it generally results in smaller executables.
> 
> A benefit of not using -mabi=ms is that we are able to catch some
> cases of misused EFIAPI

I agree.

> with a compiler warning, or unfortunately in
> some cases with crashes or misbehaving code.
> 
> I think the benefit of helping keep EFIAPI clean means that we should
> continue to not use -mabi=ms for DEBUG builds.

I agree (also for NOOPT builds).

> But, I think it would
> be good to get the size advantages of -mabi=ms by enabling it for
> RELEASE builds.

That sounds useful too (even though it wouldn't make the current problem go away).

As one caveat, I believe -mabi=ms wouldn't be allowed for building OpensslLib even for RELEASE. See "-DNO_MSABI_VA_FUNCS" in "OpensslLib.inf":

commit b2dc04a87fab89307240dc0f30b9a23bb5726c81
Author: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Sun Jul 17 11:57:45 2016 +0200

    CryptoPkg: set new define to avoid MS ABI VA_LIST on GCC/X64
    
    Set the #define NO_MSABI_VA_FUNCS that will be introduced in a subsequent
    patch to avoid the use of the MS ABI in variadic functions. In EDK2, such
    functions normally require the EFIAPI modifier to be used, but for external
    libraries such as OpenSSL, which lack these annotations, it is easier to
    simply revert to the default SysV style VA_LIST ABI.
    
    Contributed-under: TianoCore Contribution Agreement 1.0
    Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
    Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
    Tested-by: Laszlo Ersek <lersek@redhat.com>
    Tested-By: Liming Gao <liming.gao@intel.com>
    Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks!
Laszlo