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

[Qing Zhao <qing.zhao@oracle.com>]

Hi, Jakub,

Thanks a lot for fixing this issue.

I have several questions in below:

> On Feb 28, 2023, at 3:26 AM, Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 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.


As my understanding, without your current patch, the current -fsanitize=bounds-strict behaves like -fstrict-flex-arrays=2, i.e:
it treats:
   [], [0] as flexible array members;
but
   [1], [4] as regular arrays

Then with your current patch, [0] will NOT be treated as flexible array members by default anymore, so, the -fsanitize=bounds-strict will
treats:
   [] as flexible array members;
but
   [0], [1], [4] as regular arrays
The same behavior as -fstrict-flex-arrays=3.

Therefore, -fsanitize=bounds-strict already implies -fstrict-flex-arrays=3. 

Is the above understanding correctly?

> 
> 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.

For [0] arrays, why C++ and C represent with different IR? 

thanks.

Qing
>