Re: [PATCH] ubsan: Honor -fstrict-flex-arrays= in -fsanitize=bounds [PR108894]

[Jakub Jelinek <jakub@redhat.com>]

On Tue, Feb 28, 2023 at 09:02:47AM +0000, Richard Biener wrote:
> > While this isn't really a regression, the -fstrict-flex-arrays*
> > option is new in GCC 13 and so I think we should make -fsanitize=bounds
> > play with it well from the beginning.
> > 
> > The current behavior is that -fsanitize=bounds considers all trailing
> > arrays as flexible member-like arrays and both -fsanitize=bounds and
> > -fsanitize=bounds-strict because of a bug don't even instrument
> > [0] arrays at all, not as trailing nor when followed by other members.
> > 
> > I think -fstrict-flex-arrays* options can be considered as language
> > mode changing options, by default flexible member-like arrays are
> > handled like flexible arrays, but that option can change the set of
> > the arrays which are treated like that.  So, -fsanitize=bounds should
> > change with that on what is considered acceptable and what isn't.
> > While -fsanitize=bounds-strict should reject them all always to
> > continue previous behavior.
> > 
> > The following patch implements that.  To support [0] array instrumentation,
> > I had to change the meaning of the bounds argument to .UBSAN_BOUNDS,
> > previously it was the TYPE_MAX_VALUE of the domain unless ignore_off_by_one
> > (used for taking address of the array element rather than accessing it;
> > in that case 1 is added to the bound argument) and the later lowered checks
> > were if (index > bound) report_failure ().
> > The problem with that is that for [0] arrays where (at least for C++)
> > the max value is all ones, for accesses that condition will be never true;
> > for addresses of elements it was working (in C++) correctly before.
> > This patch changes it to add 1 + ignore_off_by_one, so -1 becomes 0 or
> > 1 for &array_ref and changing the lowering to be if (index >= bound)
> > report_failure ().  Furthermore, as C represents the [0] arrays with
> > NULL TYPE_MAX_VALUE, I treated those like the C++ ones.
> 
> LGTM.  Btw, what does -fsanitize=bounds do for C++ code which lacks
> flexible arrays?  Does it treat all trailing arrays as fixed?

As -fstrict-flex-arrays* options and strict_flex_array attribute are
basically ignored right now for C++, I've kept the previous behavior
for C++ (except for fixing handling of [0] arrays), which is that it like
the rest of the compiler treats all trailing arrays as flexible member like
(ok, there is a loop looking for nested references, so
struct S { int a; struct T { int b; int c[1]; } d; int e; };
that the e member results in c not being treated flexible member-like).
If/when -fstrict-flex-arrays* support is added for C++, we'll need to
drop one of the !c_dialect_cxx () guards there even in c-ubsan.cc.

	Jakub