[U-Boot] [PATCH v8 14/30] efi: Don't build sandbox with __attribute__((ms_abi))

[Simon Glass]

Hi Alex,

On 25 June 2018 at 05:23, Alexander Graf <agraf at suse.de> wrote:
> On 06/23/2018 04:16 PM, Simon Glass wrote:
>>
>> Hi Alex,
>>
>> On 23 June 2018 at 01:28, Alexander Graf <agraf at suse.de> wrote:
>>>
>>>
>>> On 22.06.18 21:28, Simon Glass wrote:
>>>>
>>>> Hi Alex,
>>>>
>>>>
>>>> On 22 June 2018 at 06:11, Alexander Graf <agraf at suse.de> wrote:
>>>>>
>>>>> On 06/21/2018 09:45 PM, Simon Glass wrote:
>>>>>>
>>>>>> Hi Alex,
>>>>>>
>>>>>> On 21 June 2018 at 03:59, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>
>>>>>>> On 06/21/2018 04:02 AM, Simon Glass wrote:
>>>>>>>>
>>>>>>>> Hi Alex,
>>>>>>>>
>>>>>>>> On 20 June 2018 at 02:56, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>>>
>>>>>>>>> On 06/20/2018 12:02 AM, Simon Glass wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Alex,
>>>>>>>>>>
>>>>>>>>>> On 18 June 2018 at 08:46, Alexander Graf <agraf at suse.de> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 06/18/2018 04:08 PM, Simon Glass wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> There appears to be a bug [1] in gcc when using varargs with
>>>>>>>>>>>> this
>>>>>>>>>>>> attribute. Disable it for sandbox so that functions which use
>>>>>>>>>>>> that
>>>>>>>>>>>> can
>>>>>>>>>>>> work correctly, such as install_multiple_protocol_interfaces().
>>>>>>>>>>>>
>>>>>>>>>>>> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> See my patch instead please.
>>>>>>>>>>
>>>>>>>>>> OK I see it now. Do you know what gcc fixes this problem?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The bug you found was really just a gcc bug that hit early gcc6
>>>>>>>>> versions.
>>>>>>>>> I
>>>>>>>>> doubt you're running into it :).
>>>>>>>>
>>>>>>>> OK, so in fact gcc does not support varargs problems with the
>>>>>>>> ms_abi?
>>>>>>>
>>>>>>>
>>>>>>> Gcc needs to know whether varargs are sysv varargs or ms varargs. And
>>>>>>> it
>>>>>>> differentiates between the two with different variable types for
>>>>>>> va_list.
>>>>>>>
>>>>>> Have you seen the builtin_va_list, etc.
>>>>>
>>>>>
>>>>> I think this sentence is missing content?
>>>>
>>>> I thought that builtin_va_list and friends would work regardless of
>>>> the calling standard being used. But it looks (from your patch) like
>>>> you have to explicitly use __builtin_ms_va_list. Is that right?
>>>
>>> I'm fairly sure builtin_va_list is just gcc's way of mapping the sysv
>>> va_list, but I'm not 100% sure. I can double check with our compiler
>>> people next week.
>>
>> OK looking forward to hearing. I'm not sure when the builtin came in,
>> but if it has been around for a while, and it supports both calling
>> standards, then it would be nice to use it.
>
>
> So according to our toolchain people the builtin is really just the internal
> gcc name for va_list, so it still adheres to the default calling ABI in its
> semantics.
>
> Apparently what one *can* do is to add -mabi=ms to the compiler flags which
> basically makes every function follow the ms abi. If that is set *maybe*
> va_list will also adhere to it.
>
> However, both him and me like the explicit approach (of this patch) better.
>
> He also quickly explained why the function abi can't directly have an effect
> on va_list. Basically at the parsing stage, gcc does not know if you want to
> use a va_list for yourself or to pass it into a function you call. And
> depending on that, you may want either a sysv abi va_list or an ms_abi
> va_list.

Eek that sounds complicated, but good to know. So it seems like your
solution is the best option.

Regards,
Simon