On Wed, Dec 16, 2020 at 04:43:24PM -0800, Jacob Keller wrote:
> On 12/16/2020 3:30 PM, Luc Van Oostenryck wrote:
> > On Tue, Dec 15, 2020 at 03:15:48PM -0800, Jacob Keller wrote:
> >>
> >> I did find one bug, in your step (3), you have a check against
> >> info->packed on symbol.c:162 in lay_out_struct, but nothing ever set the
> >> packed value. I think you just need to initialized info->packed from
> >> sym_packed at the top of examine_struct_union_type, i.e.
> >
> > A yes, I see, thank you. I think it was on purpose that it wasn't
> > yet enabled (things are it fuzzy because the code is ~2 year old)
> > and as I said it's unfinished.
> >
> > But, with your change, does it handles 'packed' more or less
> > correctly?
> >
> > -- Luc
> >
>
> Yes. Obviously we're limited in that we no longer check for
> out-of-bounds accesses on bitfields, but it at least produces the
> correct sizes for structures, and avoids the warnings that I was running
> into.
Nice, thanks for confirming this.
> Overall, I think the changes in that branch are solid, and look correct
> to me.
>
> I'm not sure what all the limitations are of having it produce incorrect
> load/store operations that don't work with the packed bitfields.. but at
> least for the code I was checking, it seems to be correct now.
Oh, it's not much, just the usual. Currently, sparse always access a
bitfield with its base type. So, if we have the following:
struct foo {
int a;
int b:24;
} __packed;
struct foo s;
Then accessing s.b will first load the full word and then mask the
upper bits, but:
1) this will access bytes 5,6,7 & 8 but the structure is only 7 bytes
long and so the access checking should fail.
2) accessing byte-by-byte and then assemble the result is endian
specific and until now sparse had succeed o ignore this kind
of 'details'.
-- Luc
