Re: [RFC][PATCH] PR tree-optimization/109071 - -Warray-bounds false positive warnings due to code duplication from jump threading

[Jeff Law <jeffreyalaw@gmail.com>]

On 5/13/24 1:48 PM, Qing Zhao wrote:
> -Warray-bounds is an important option to enable linux kernal to keep
> the array out-of-bound errors out of the source tree.
> 
> However, due to the false positive warnings reported in PR109071
> (-Warray-bounds false positive warnings due to code duplication from
> jump threading), -Warray-bounds=1 cannot be added on by default.
> 
> Although it's impossible to elinimate all the false positive warnings
> from -Warray-bounds=1 (See PR104355 Misleading -Warray-bounds
> documentation says "always out of bounds"), we should minimize the
> false positive warnings in -Warray-bounds=1.
> 
> The root reason for the false positive warnings reported in PR109071 is:
> 
> When the thread jump optimization tries to reduce the # of branches
> inside the routine, sometimes it needs to duplicate the code and
> split into two conditional pathes. for example:
> 
> The original code:
> 
> void sparx5_set (int * ptr, struct nums * sg, int index)
> {
>    if (index >= 4)
>      warn ();
>    *ptr = 0;
>    *val = sg->vals[index];
>    if (index >= 4)
>      warn ();
>    *ptr = *val;
> 
>    return;
> }
> 
> With the thread jump, the above becomes:
> 
> void sparx5_set (int * ptr, struct nums * sg, int index)
> {
>    if (index >= 4)
>      {
>        warn ();
>        *ptr = 0;		// Code duplications since "warn" does return;
>        *val = sg->vals[index];	// same this line.
> 				// In this path, since it's under the condition
> 				// "index >= 4", the compiler knows the value
> 				// of "index" is larger then 4, therefore the
> 				// out-of-bound warning.
>        warn ();
>      }
>    else
>      {
>        *ptr = 0;
>        *val = sg->vals[index];
>      }
>    *ptr = *val;
>    return;
> }
> 
> We can see, after the thread jump optimization, the # of branches inside
> the routine "sparx5_set" is reduced from 2 to 1, however,  due to the
> code duplication (which is needed for the correctness of the code), we
> got a false positive out-of-bound warning.
> 
> In order to eliminate such false positive out-of-bound warning,
> 
> A. Add one more flag for GIMPLE: is_splitted.
> B. During the thread jump optimization, when the basic blocks are
>     duplicated, mark all the STMTs inside the original and duplicated
>     basic blocks as "is_splitted";
> C. Inside the array bound checker, add the following new heuristic:
> 
> If
>     1. the stmt is duplicated and splitted into two conditional paths;
> +  2. the warning level < 2;
> +  3. the current block is not dominating the exit block
> Then not report the warning.
> 
> The false positive warnings are moved from -Warray-bounds=1 to
>   -Warray-bounds=2 now.
> 
> Bootstrapped and regression tested on both x86 and aarch64. adjusted
>   -Warray-bounds-61.c due to the false positive warnings.
> 
> Let me know if you have any comments and suggestions.
This sounds horribly wrong.   In the code above, the warning is correct.

Jeff