This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] ubsan: remove bogus check for flexible array members


Marek Polacek <polacek@redhat.com>:

(Hi Marek and Jakub, I really appreciate your work on GCC and 
ubsan. I just want to add the minor enhancements necessary 
to make it really useful for me).

> > And this is broken code. I would argue that a user who uses the
> > ubsan *expects* this to be diagnosed. Atleast I was surprised
> > that it didn't catch more out-of-bounds accesses.
>  
> I wouldn't say broken, it's being used pretty often.  E.g.
> https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> "In ISO C90, you would have to give contents a length of 1"

Ok, let's add an option then. But that it is common doesn't
mean it is not broken (especially since there is a legal
way to that with GCC and C99) and I think there should be a 
way to diagnose it with ubsan.

> > If this is indeed intentional, we should:
> > 
> > - document these exceptions
> 
> See the link above + docs say for -fsanitize=bounds that "Flexible array
> members are not instrumented".

Flexible array members should not be intrumented. But
what is suprising to the user and should be documented
is that other arrays (e.g. which hjust happen to be the
last in a struct) are not instrumented.

> > - remove the misleading comment
> 
> Which one?
> The /* Detect flexible array members and suchlike.  */ IMHO says
> exactly what's meant to be done.

The reason it confused me was because flexible array members never
see this code because the are catched by the 'if' condition
directly before. 

> > - have test cases also for sizes > 0
> 
> Yes, we should have that.
> 
> > - fix it not to prevent instrumentation of all array accesses
> >   through pointers (i.e. when the array is not a member of a struct)
> 
> Yes, see the patch in my previous mail.

Thank you! I will test this and report back

> > - have a configuration option (e.g. -fsanitize=strict-bounds)
> 
> Yeah, something like that would be useful to have.  As Jakub
> mentioned, we discussed this in the past.  Probably GCC 6 stuff
> though.

I will look into this.

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]