I will prepare a new patch with the suggested changes:
(a) more efficient overflow check like
if (num > SIZE_MAX / elem_size)
(b) replacing union of VLA with pointers to arrays like
void *data = malloc (...);
T32 (*a32)[n] = data;
T64 (*a64)[n] = data;
Please feel free to suggest better solutions.
Thanks.
On Mon, Oct 5, 2015 at 11:45 AM, Mark Wielaard <[email protected]> wrote:
> On Fri, Oct 02, 2015 at 12:10:47AM +0300, Alexander Cherepanov wrote:
> > On 2015-09-16 18:25, Mark Wielaard wrote:
> > >On Fri, 2015-09-11 at 12:22 -0700, Roland McGrath wrote:
> > >>It looks fine to me from a quick skim, but Mark should review and test
> it too.
> > >
> > >I am not super enthusiastic about this change, it seems to just take
> > >away type/size information that the compiler/bounds checking tools can
> > >use.
> >
> > I'm not sure I fully understand the problem. As I understand it unions of
> > VLAs are not Ok while VLAs are Ok and even desirable due to bounds
> checking,
> > right?
>
> There are two issues. First with GCC VLA types are OK and desired compared
> to types not using bounds. But if such a VLAs might be unbounded then they
> should not be stack allocated (this is enforced by building with
> -Wstack-usage except for a couple of files that haven't been converted).
> Secondly there are people who want to use clang to build elfutils. clang
> doesn't support various GNU extensions used in the code like VLAs. So
> for those people any VLA type seems problematic.
>
> > Why not just use VLAs of unions? Cold memory?
>
> I am not sure clang supports such VLA types. If it does, then I would say
> VLAs of unions are preferred above a type that doesn't include bounds.
>
> > Given that the current approach (before the patch) already required to
> write
> > superfluous "->" perhaps an approach requiring a superfluous "*" will
> fit?
> > Like this:
> >
> > void *data = malloc (...);
> > T32 (*a32)[n] = data;
> > T64 (*a64)[n] = data;
> >
> > Then the use looks like "(*a32)[i].member". Clang seems to be happy and
> its
> > UBSAN works fine.
>
> If that works that would probably be preferred since then ubsan can see
> the array bounds and help catch issues. You can build and run elfutils
> and the tests with configure --enable-sanitize-undefined to use ubsan
> checking.
>
> Cheers,
>
> Mark
>
