> On Jan 10, 2025, at 10:05, Richard Biener <richard.guent...@gmail.com> wrote: > > On Fri, Jan 10, 2025 at 3:27 PM Qing Zhao <qing.z...@oracle.com> wrote: >> >> >> >>> On Jan 10, 2025, at 03:00, Richard Biener <richard.guent...@gmail.com> >>> wrote: >>> >>> On Thu, Jan 9, 2025 at 9:39 PM Qing Zhao <qing.z...@oracle.com> wrote: >>>> >>>> >>>> >>>>> On Jan 9, 2025, at 14:10, Jeff Law <jeffreya...@gmail.com> wrote: >>>>> >>>>> >>>>> >>>>> On 1/9/25 10:48 AM, Qing Zhao wrote: >>>>> >>>>>>> >>>>>>> I think Jeff's patch is not reasonable since it boils down to not >>>>>>> diagnose >>>>>>> -Warray-bounds but instead remove those stmts. >>>>>> If these stmts are dead-code that are generated by compiler optimization >>>>>> (NOT from source code), >>>>>> removing them before diagnosis is correct. (To avoid false positive >>>>>> warnings). >>>>> But I don't think we generally know if the problematic statements came >>>>> from user code or were generated by the compiler. >>>> >>>> To help the compiler catches real problems in the source code and avoid >>>> false positive warnings introduced by the compiler transformation, we >>>> might need to add flags in the IR to distinguish this? >>> >>> Well, the issue is the problematic statements _are_ in user code, just >>> -Warray-bounds is too stupid to >>> look at SCEV for indices and instead relies on weaker value-ranges. >> >> A little confused here: are you saying that the testing case of PR92539 has >> __conditional__ UB in the source code level? >> If so, could you please clarify this a little bit more? (From my >> understanding of the source code, I didn’t see >> UB in the source code, do I miss anything obvious?) > > static bool eat(char const*& first, char const* last) > { > if (first != last && ischar(*first)) { > ++first; > return true; > } > return false; > } > > static bool eat_two(char const*& first, char const* last) > { > auto save = first; > if (eat(first, last) && eat(first, last)) > return true; > > The ++first is the conditional UB stmt for the 2nd eat(). It's > conditional on the first eat() returning true and first != last.
I think that the following check in the source code: if (first != last) Is the defensive code to avoid your mentioned potential UB. Isn’t it? From the user’s point of view, there should not be any out-of-bound accesses during runtime since the user already check the pointer against the last valid address to avoid the out-of-bound access Should the compiler recognize these bounds check in the source code and avoid issue warnings for them? > > The compiler now needs to prove that these conditions > are enough that UB never happens at runtime. Yes, constant propagation and then dead code elimination should be able to prove those conditions will not met and then eliminate those unreachable code after fully unrolling, as a result, the compiler can avoid issue warnings for them. Qing > uninit > analysis for example would diagnose if it cannot prove that > while -Warray-bounds simply always diagnoses regardless > of how the conditions are but it requires "obvious" UB. > > Richard. > >> >> thanks. >> >> Qing >>> >>> It's a problem we're never going to fully solve. Some of the >>> testcases show missed optimizations >>> which we can work on. Some show we diagnose IL we later are able to >>> optimize away, some >>> simply show that users are not always happy with how we decide on >>> suppressing a diagnostic. >>> >>> For the case at hand we should be able to optimize it fully. >>> >>> But optimizing based on UB is always going to be to interact with >>> diagnosing UB, so we have >>> to be careful. Our "late" diagnostics are most problematic here and >>> I'd argue moving those >>> earlier is the first thing we should try. >>> >>> Richard. >>> >>>> >>>> Qing >>>>> >>>>> Jeff >> >>