On Mon, Jan 6, 2025 at 3:43 PM Qing Zhao <qing.z...@oracle.com> wrote:
>
>
>
> > On Jan 6, 2025, at 09:21, Jeff Law <jeffreya...@gmail.com> wrote:
> >
> >
> >
> > On 1/6/25 7:11 AM, Qing Zhao wrote:
> >>>
> >>> Given it doesn't cause user visible UB, we could insert the trap *before* 
> >>> the UB inducing statement.  That would then make the statement 
> >>> unreachable and it'd get removed avoiding the false positive diagnostic.
> >> Yes, that’s a good idea.
> >> However, in order to distinguish a user visible UB and a UB in the IL that 
> >> is introduced purely by compiler, we might need some new marking in the IR?
> > I don't think we've ever really tackled that question; the closest I can 
> > think of would be things like integer overflow which we try to avoid 
> > allowing the compiler to introduce.  If we take the integer overflow as the 
> > model, then that would say we should be tackling this during loop unrolling.
>
> UB that is introduced by compiler transformation is one important cause of 
> false positive warnings.
>
> There are two approaches to tackle this problem from my understanding:
>
> 1. Avoid generating such UB from the beginning. i.e, for every compiler 
> transformation that might introduce such UB, we should add check to avoid 
> generating it.
>
> 2. Marking the IR portion that were generated by compiler transformations, 
> then check whether the UB is compiler generated when issue static checker 
> warnings.
>
> Are there other approaches?

Note unrolling doesn't introduce UB - it makes conditional UB
"obvious".  Note -Warray-bounds wants to
diagnose UB, so doing path isolation and removing the UB would make
-Warray-bounds useless.

So unless the condition guarding the UB unrolling exposes is visibly
false to the compiler but we fail
to exploit that (missed optimization) there's not much that we can do.
I think "folding" away the UB
like what Jeff proposes trades false negatives for the false positive
diagnostics.

Note the unroller knows UB that effectively bounds the number of
iterations, even on conditional
paths and it uses this to limit the number of copies _and_ to prune
unreachable paths (exploiting
UB, avoiding diagnostics).  But one of the limitations is that it only
prunes paths in the last unrolled
copy which can be insufficient (ISTR some PR where I noticed this).

That said - I think for these unroller exposed cases of apparent false
positives we should improve
the path pruning in the unroller itself.  For the other cases the path
diagnostic might help clarify
that the UB happens on the 'n-th' iteration of the loop when some
additional condition is true/false.

So in the end Jeff - I think your patch isn't a good approach for the
issue at hand.

Richard.

> The above is very rough and initial idea at this moment.
>
> Qing
>
> >
> > jeff
> >
>

Reply via email to