Re: sparse attribute packed on structures

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

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,
2) the size calculation must take the attribute in account,
3) the linearization of memory access must be adapted to be able
   to access unaligned members otherwise the check access complain
   loudly.

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.

I hope this will help you,
-- Luc