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

Simon Glass sjg at chromium.org
Sat Jun 23 14:16:13 UTC 2018


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.

>
> Either way, I think this patch is good either way. For starters it's not
> gcc specific because it uses the normal va_args in the "normal" case.
> Also, it's not ambiguous. IMHO things are quite clear when reading the
> code if we explicitly differentiate between sysv and ms_abi va_args.

I'm OK with it since it gets us going.

Reviewed-by: Simon Glass <sjg at chromium.org>


More information about the U-Boot mailing list