On July 24, 2015 7:30:03 AM GMT+02:00, Jeff Law <l...@redhat.com> wrote: >On 07/23/2015 04:44 PM, Bernhard Reutner-Fischer wrote: >> On July 23, 2015 7:43:51 PM GMT+02:00, Jeff Law <l...@redhat.com> >wrote: >>> On 07/22/2015 09:29 AM, Manuel López-Ibáñez wrote: >>>> While looking at PR c/16351, I noticed that all tests proposed for >>>> -Wnull-attribute >>>> (https://gcc.gnu.org/ml/gcc-patches/2014-01/msg01715.html) could be >>>> warned from the FEs by simply extending the existing Wnonnull. >>>> >>>> Bootstrapped and regression tested on x86_64-linux-gnu. >>>> >>>> OK? >>>> >>>> >>>> gcc/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez <m...@gcc.gnu.org> >>>> >>>> PR c/16351 >>>> * doc/invoke.texi (Wnonnull): Document behavior for >>>> returns_nonnull. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez <m...@gcc.gnu.org> >>>> >>>> PR c/16351 >>>> * c-c++-common/wnonnull-1.c: New test. >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez <m...@gcc.gnu.org> >>>> >>>> PR c/16351 >>>> * typeck.c (check_return_expr): Call >maybe_warn_returns_nonnull. >>>> >>>> >>>> gcc/c-family/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez <m...@gcc.gnu.org> >>>> >>>> PR c/16351 >>>> * c-common.c (maybe_warn_returns_nonnull): New. >>>> * c-common.h (maybe_warn_returns_nonnull): Declare. >>>> >>>> gcc/c/ChangeLog: >>>> >>>> 2015-07-22 Manuel López-Ibáñez <m...@gcc.gnu.org> >>>> >>>> PR c/16351 >>>> * c-typeck.c (c_finish_return): Call >maybe_warn_returns_nonnull. >>> FWIW, we have the usual tension here between warning in the >front-end >>> vs >>> warning after optimization and exploiting dataflow analysis. >>> >>> Warning in the front-ends like this can generate false positives >(such >>> as a NULL return in an unreachable path and miss cases where the >NULL >>> has to be propagated into the return by later optimizations. >>> >>> However warning in the front-ends will tend to have more stable >>> diagnostics from release to release. >>> >>> Warning after optimization/analysis will tend to generate fewer >false >>> positives and can pick up cases where the didn't explicitly appear >in >>> the return statement, but had to be propagated in by the optimizers. >Of >>> >>> course, these warnings are less stable release-to-release and >require >>> the optimizers/analysis phases to be run. >>> >>> I've always preferred exploiting optimization and analysis to both >>> reduce false positives and expose the non-trivial which show up via >>> optimizations. But I also understand that's simply my preference >and >>> that others have a different preference. >>> >>> I'll tentatively approve for the trunk, but I think we still want >>> warnings after optimization/analysis. Which will likely lead to a >>> scheme like I proposed many years for uninitialized variables where >we >>> have multiple modes. One warns in the front-end like your >implemention >>> >>> does, the other defers the warning until after analysis & >optimization. >>> >>> So please keep 16351 open after committing. >> >> -W{no-,}{f,m}e IIRC was proposed before. Won't help >https://gcc.gnu.org/PR55035 though where struct stores just escape too >much -- AFAIU -- but still.. >> >Things like uninitialized variable analysis are inherently going to >cause false positives. It's just a fact of life. > >Looking at the reduced testcase in that PR, I'm pretty sure its a bogus > >reduction.
Yes, the original, smallish test case in comment #4 is different, AFAICS. > >We could have N == 0 when we encounter the head of the first loop. So >no members of temp[] will be initialized. We then have the call to >bar() which could change the value of N to 1. We then hit the second >loop and read temp[0] which is uninitialized. The warning for the >reduced testcase is correct. Agree. > >If the original source has the same overall structure (an unbound write > >between the key loops), then the warning is correct. If the writes can > >be proven to not clobber the loop bounds (such that the two key loops >iterate over the same number of elements), then we'd have a false >positive warning (and a failure of jump threading to discover the >unexecutable path). AFAIU the write in the testcase in comment #4 does not clobber loop bounds, so I think the warning is wrong. Thanks,
