On 12/16/2016 09:46 AM, Jakub Jelinek wrote:
On Fri, Dec 16, 2016 at 09:36:25AM -0700, Martin Sebor wrote:
It does for me with an allmodconf. At -O2 I get three warnings, and at
-O3 I get two additional warnings. Now these additional ones happen way
too deep into the pipeline to be reliable. (For a reduced testcase see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78817#c8)

The warning looks quite justified in this case.  The first call
to sm_read_sector exits with the pointer being null and so the
second call is diagnosed.  That seems like a great example of
when the warning is useful.

No.  The first call to sm_read_sector just doesn't exit.  So it is warning
about dead code.

If the code is dead then GCC should eliminate it.  With it eliminated
the warning would be gone.  The warning isn't smart and it doesn't
try to be.  It only works with what GCC gives it.  In this case the
dump shows that GCC thinks the code is reachable.  If it isn't that
would seem to highlight a missed optimization opportunity, not a need
to make the warning smarter than the optimizer.

I don't claim it can't be improved but it seems pretty good as
it is already.  Among the 6 instances it's found in GCC three
look like real bugs.

None look like real bugs to me.

They do to me.  There are calls in gengtype.c to a function decorated
with attribute nonnull (lbasename) that pass to it a pointer that's
potentially null.  For example below.  get_input_file_name returns
null when inpf is null. There are other calls to get_input_file_name
in the file that check its return value (e.g., get_file_basename)
and so those that don't suggest bugs.  If there is no way for the
get_input_file_name function to return null then that suggests
the function should be declared returns_nonnull (and the return
NULL statement from it removed), and all those callers that check
for null simplified.  In any case, at a minimum the warning points
out an inconsistency that suggests a possible improvement.  That,
in my view, is one of the things that make warnings valuable even
if they don't identify the smoking gun.

--- a/gcc/gengtype.c
+++ b/gcc/gengtype.c
@@ -1747,7 +1747,10 @@ open_base_files (void)
 static const char *
 get_file_realbasename (const input_file *inpf)
 {
-  return lbasename (get_input_file_name (inpf));
+  if (const char *fname = get_input_file_name (inpf))
+    return lbasename (fname);
+
+  return NULL;
 }

Martin

Reply via email to