Re: sparse attribute packed on structures

[Jacob Keller <jacob.e.keller@intel.com>]

On 12/15/2020 12:56 PM, Luc Van Oostenryck wrote:
> On Tue, Dec 15, 2020 at 10:15:35AM -0800, Jacob Keller wrote:
>> Hi,
>>
>> I'm looking into an issue with sparse not calculating the size of a
>> packed structure correctly, causing some static assertions to fail due
>> to an incorrect size.
>>
>> With a structure like this:
>>
>> struct a {
>> 	uint32_t a;
>> 	uint8_t b;
>> 	uint8_t c;
>> } __attribute__ ((packed));
>>
>> The packed attribute doesn't seem to get applied to the whole structure.
>> Thus, the sparse sizeof evaluation for this results in 8 bytes (64
>> bits), when GCC would produce a structure of size 6 bytes (48 bits).
>>
>> If I use something instead like this:
>>
>> struct a {
>> 	uint32_t a __attribute__ ((packed));
>> 	uint8_t b __attribute__ ((packed));
>> 	uint8_t c __attribute__ ((packed));
>> } __attribute__ ((packed));
>>
>> Then the size is calculated correctly.
>>
>> I saw that there is support in parse.c for parsing attribute packed, but
>> it doesn't seem to have a way to propagate from a structure down to its
>> members.
>>
>> I thought it would be relatively straight forward to implement by adding
>> a MOD_PACKED, but that doesn't seem to actually get assigned to the
>> struct symbol, so when I tried that it didn't work.
>>
>> I would very much like to help get structure size packing to work properly.
>>
>> The following diff is what I tried initially, but it doesn't actually
>> work as expected. I'm not sure what is wrong, or what is the best method
>> to actually get the packed modifier to save into the structure symbol so
>> that it can be checked when determining the structure size.
>>
>> Help would be appreciated.
> 
> Hi,
> 
> There is at least 3 issues with the packed attribute:
> 1) at parsing time, types attributes are not applied to the
>    corresponding symbol,

Yea, this is what I couldn't figure out.

> 2) the size calculation must take the attribute in account,

This is what I got working IFF I hacked the symbol to say MOD_PACKED.
But I couldn't figure out (1) so I got stuck.

> 3) the linearization of memory access must be adapted to be able>    to access unaligned members otherwise the check access complain
>    loudly.
> 

Yea I figured something would break here, I was basically mostly
interested in handling the size.

> Sorry, I don't have much time for this now but at first sight your patch
> seems on the right track. I can look at it more closely this WE but
> meanwhile I've pushed a branch 'packed' on
> 	git://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git
> 
> This branch contains an unfinished patches but it should more or less
> handle the points 1) & 2) and circumvent point 3) by disabling access
> checking for bitfields.
> 

Ok thanks! I'll take a look at the packed branch, that should help me.

Perhaps I can take a stab at (3) based on whats in that branch.

> I hope this will help you,
> -- Luc
> 

Great, Thanks!
- Jake