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