Re: 5.8: LTO exposes some new issues

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Roman Perepelitsa wrote on Tue, 28 Jul 2020 13:31 +0200:
> On Tue, Jul 28, 2020 at 1:20 PM Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > Roman Perepelitsa wrote on Tue, 28 Jul 2020 12:52 +0200:  
> > >
> > > How about this? The diff is a bit larger but the code is fairly
> > > straightforward. Only hashtable.c has access to internal fields, just
> > > like before the patch.
> > >
> > > In a nutshell, struct hashtable has only public data members. Within
> > > hashtable.c there is struct hashtableimpl, which has struct hashtable
> > > as the first data member. C allows casting a pointer to a struct to a
> > > pointer to its first data member and back without violating aliasing
> > > rules. Thus hashtable.c can cast struct hashtable* to struct
> > > hashtableimpl* in order to get access to internal fields.  
> >
> > Thanks, that addresses the previous point, but unfortunately it creates
> > another problem: people who read the .h file are liable to declare
> > local variables of type 'struct hashtable', or memcpy() them around,
> > and in either case, once such a variable gets to hashtable.c and the
> > private members are accessed, we'll get out-of-bounds reads.  
> 
> This problem exists in the current version of the code, too. The patch
> addresses one problem -- it removes undefined behavior due to ODR
> violation. If you want, I can extend the patch so that it also
> addresses the second problem you've identified although it might be
> betted done in a separate patch given that it's independent from the
> first.

Whatever you think best.  In general, if in doubt I'd err on the side
of splitting.  Thanks again for looking into this. ☺

Daniel