On Fri, Dec 16, 2016 at 01:01:13PM -0700, Jeff Law wrote:
> > Thanks. Reduced to something like:
> > int
> > foo (const char *name)
> > {
> > if (name)
> > return 6;
> > return __builtin_strlen (name);
> > }
> > This is warned about both with Martin's late warning and my after ccp2
> > warning version. We should include it in gcc testsuite.
> I'll note this is an example of a case where Andrew's work would likely help
> because it allows us to ask for a range of name_XX at the return statement
> and it'll give us back a range that is constrained by the path(s) which
> reach the return statement.
>
> Contrast to the current VRP work where each SSA_NAME has a range, but that
> range must be valid for every context in which that SSA_NAME appears.
Well, inside the current VRP it has separate ranges for the different paths
and actually replaces the name in the strlen argument with NULL during evrp,
so doesn't suffer from the current VRP limitations.
Anyway, let's consider the warning from the linux kernel with the closedir,
I guess it can be simplified to something along the lines of:
void baz (char *) __attribute__((nonnull));
char *bar (int);
int
foo (void)
{
char *p = bar (1);
int ret = 0;
if (p == 0)
{
bar (2);
bar (3);
bar (4);
ret = 1;
goto out;
}
bar (5);
bar (6);
bar (7);
bar (8);
out:
baz (p);
if (ret)
bar (10);
return ret;
}
Here we jump thread it and with Martin's warning position warn, with
my patch don't warn. But if the if (ret) bar (10); is removed, the
code has the same problem that on the error path p will be NULL, but it is
not going to be diagnosed. For -Wmaybe-nonnull we could e.g. look at if
the argument is a PHI that has NULL at any of the edges; but that doesn't
cover the above case, because p has just one def and so there will be no
PHIs.
And yet another testcase:
void foo (const char *) __attribute__((nonnull));
void
bar (int x)
{
foo (x ? (const char *) 0 : "");
}
Here we warn only with GCC 6 or with my patch, but not with current trunk,
so this case is a warning regression (if needed, I can split my patch into
smaller chunks, the part that addresses this is keeping the warning in the
FE and setting TREE_NO_WARNING if it warned, instead of not warning when
optimize in the FE at all). Handling COND_EXPR in the argument might be
more -Wmaybe-nonnull thing though. It isn't if you reach this source
location, there will be always UB, but if you reach this source location
along this and this path (and only careful analysis can discover if that
path is actually possible at runtime), then there will be UB.
Kind like we have -Wuninitialized and -Wmaybe-uninitialized.
Jakub
