Re: [PATCH 1/3] struct packed: Use gcc_struct on Windows

[Pedro Alves <pedro@palves.net>]

On 2022-07-21 7:28 p.m., Eli Zaretskii wrote:
>> Cc: gdb-patches@sourceware.org
>> From: Pedro Alves <pedro@palves.net>
>> Date: Thu, 21 Jul 2022 19:18:20 +0100
>>
>> On 2022-07-21 6:40 p.m., Eli Zaretskii wrote:
>>>> Cc: gdb-patches@sourceware.org
>>>> From: Pedro Alves <pedro@palves.net>
>>
>>>> Because without it the struct won't really be packed.
>>>
>>> Can you tell why is that necessary?
>>
>> Somehow I missed this question.  It is necessary because that's the whole
>> point of "struct packed".  To wrap some other type and pack it.  We use it
>> in some size-sensitive structures, to make them are as small as possible.
>> This is described in the gdbsupport/packed.h header.
> 
> You mean, this part:
> 
>   /* Each instantiation and full specialization of the packed template
>      defines a type that behaves like a given scalar type, but that has
>      byte alignment, and, may optionally have a smaller size than the
>      given scalar type.  This is typically used as alternative to
>      bit-fields (and ENUM_BITFIELD), when the fields must have separate
>      memory locations to avoid data races.  */
> 
> I don't think I see the rationale here, except "make them as small as
> possible".  Is that the reason?  

Well, we use bit fields in structs that need to be as small as possible.
But using bit fields is problematic in cases that lead to data races.
Instead of fixing the problem in every stop that might need it, we added
struct packed, to abstract it out.  Keeping the type as small is possible
is part of the point, otherwise we wouldn't have struct packed at all.

> If so, what would happen if the size
> is somewhat larger?  If the only result is less efficient GDB, I'd be
> happier if we sustained the efficiency hit, but stayed compatible to
> the ABI.
> 

I'm sorry, but that doesn't make sense.  This is used in GDB internals.
There's no ABI worry here.  Every compilation of every C++ file in
GDB sees the same struct packed definition.  It must, otherwise,
it would be a C++ ODR violation, meaning we'd have an invalid C++ program.

Please stop worrying about ABI compatibility here.  It's not relevant here.