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