On Fri, Dec 16, 2016 at 07:29:13PM +0100, Markus Trippelsdorf wrote:
> So, to be fair a gave Jakub's patch a try and it has exactly the same
> issues for the Linux kernel: sometimes the warning only triggers with
> -O3, e.g.:
>
> % cat sm_ftl.i
> int a;
> void mtd_read_oob(int);
> void sm_read_sector(int *buffer) {
> __builtin_memset(buffer, 0, 1);
> mtd_read_oob(a);
> }
> void sm_get_zone() { sm_read_sector(0); }
In this case I think the warning is right, if you ever call sm_get_zone,
it will always invoke UB.
> % gcc -c -Wnonnull -O2 sm_ftl.i
> % gcc -c -Wnonnull -O3 sm_ftl.i
> sm_ftl.i: In function ‘sm_get_zone’:
> sm_ftl.i:4:3: warning: argument 1 null where non-null expected [-Wnonnull]
> __builtin_memset(buffer, 0, 1);
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> sm_ftl.i:4:3: note: in a call to built-in function ‘__builtin_memset’
>
> Also, Jakub's patch doesn't catch the following case:
>
> (tools/perf/util/trace-event-info.c from Linux)
> //...
> dir = opendir(path);
> if (!dir) {
> err = -errno;
> pr_debug("can't read directory '%s'", path);
> goto out;
> }
> //... other stuff
> out:
> closedir(dir);
>
> Whereas Martin's does.
This is where you rely on jump threading to duplicate closedir,
otherwise you don't warn. If you don't jump thread it (say there are
a few more statements after closedir that make it not worth it), then you
don't warn even late. If the warning in calls.c is guarded with
-Wmaybe-nonnull instead of -Wnonnull, not included in -Wall, I have no
objections. Then users can select the amount of warnings and false
positives. Or -Wnonnull can have 3 levels, one warn in the FE only,
then after inlining in the second level and late in the 3rd level.
Jakub
