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

[Anthony PERARD <anthony.perard@citrix.com>]

On Thu, Feb 23, 2017 at 11:19:03AM +0100, Laszlo Ersek wrote:
> On 02/22/17 09:54, Gao, Liming wrote:
> > Laszlo:
> > In edk2, I find the several functions with VA_LIST have no EFIAPI.
> > They may use VA_ARG() or call other functions, but they don't use
> > VA_COPY(). In Base.h, VA_ARG() is defined as __builtin_va_arg(),
> > which is same to native one.
> 
> You are right; apparently __builtin_va_arg() works with __builtin_ms_va_list and __builtin_va_list alike.
> 
> However, as you say,
> 
> > VA_COPY() is defined as
> > __builtin_ms_va_copy(). So, I also think this is MS ABI request. That
> > means only if the function implementation uses VA_START(),VA_END() or
> > VA_COPY(), it must be declared with EFIAPI.
> 
> - __builtin_va_start / __builtin_ms_va_start,
> - __builtin_va_end / __builtin_ms_va_end,
> - __builtin_va_copy / __builtin_ms_va_copy
> 
> must be matched to __builtin_va_list vs. __builtin_ms_va_list.
> 
> > 
> > MdePkg\Library\BasePrintLib\PrintLibInternal.c BasePrintLibSPrintMarker()
> > ShellPkg\Library\UefiShellLib\UefiShellLib.c InternalShellPrintWorker()
> 
> Yes, you are right -- when looking for functions that should be made EFIAPI, we shouldn't search for VA_LIST, but VA_START|VA_END|VA_COPY.
> 
> Thanks for the correction.
> 
> The following command is a good start:
> 
>   git grep -E -n '\<(VA_START|VA_END|VA_COPY)\>|^[A-Za-z0-9_]' \
>   | grep -E -B 3 '\<(VA_START|VA_END|VA_COPY)\>'
> 
> I just went over the output (it was gruesome), and -- outside of EdkCompatibilityPkg -- I indeed found only a handful of affected functions:
> 
> - XenStoreVSPrint()         [OvmfPkg/XenBusDxe/XenStore.c]
> - VariableGetBestLanguage() [SecurityPkg/VariableAuthenticated/EsalVariableDxeSal/Variable.c]
> - SmmBootScriptWrite()      [Vlv2TbltDevicePkg/PlatformSmm/SmmScriptSave.c]
> 
> Anthony, can you please submit the patch for XenStoreVSPrint()?

Done.

Thanks,

-- 
Anthony PERARD