[pushed] Mark END_CATCH as ATTRIBUTE_NORETURN (-Wmaybe-uninitialized warnings)

Message ID 20180530135709.2677-1-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves May 30, 2018, 1:57 p.m. UTC
  This commit fixes a set of -Wmaybe-uninitialized warnings in GDB and
GDBserver, seen with GCC 7.3.1 on F27 at -O2.  Specifically, all of
these:

 src/gdb/breakpoint.c:5040:4: warning: ‘e’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 src/gdb/cli/cli-cmds.c:277:71: warning: ‘tracker’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 src/gdb/cli/cli-cmds.c:302:22: warning: ‘word’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 src/gdb/gdbserver/server.c:1895:7: warning: ‘result’ may be used uninitialized in this function [-Wmaybe-uninitialized]
 src/gdb/gdbserver/server.c:1966:7: warning: ‘result’ may be used uninitialized in this function [-Wmaybe-uninitialized]

For example, looking at one of the gdbserver ones in more detail:

 ../../../src/gdb/gdbserver/server.c: In function ‘int handle_qxfer_btrace_conf(const char*, gdb_byte*, const gdb_byte*, ULONGEST, LONGEST)’:
 ../../../src/gdb/gdbserver/server.c:1966:7: warning: ‘result’ may be used uninitialized in this function [-Wmaybe-uninitialized]
	if (result != 0)
	^~

In this case (like the others), the 'result' variable is assigned in
both TRY and CATCH blocks:

      TRY
        {
          result = target_read_btrace_conf (thread->btrace, &cache);
          if (result != 0)
            memcpy (own_buf, cache.buffer, cache.used_size);
        }
      CATCH (exception, RETURN_MASK_ERROR)
        {
          sprintf (own_buf, "E.%s", exception.message);
          result = -1;
        }
      END_CATCH

      if (result != 0)
        return -3;

so it would seem like the warning is bogus.

However, END_CATCH is really a catch block in disguise, and that path
indeed does not initialize the variable:

#define END_CATCH				\
    catch (...)					\
      {						\
	exception_rethrow ();			\
      }						\
  }

exception_rethrow does not return normally (it rethrows the current
exception after running cleanups), but the compiler can not see that.
If it could return normally, then indeed 'result' could be used
uninitialized if the TRY block threw some non-gdb exception, which
would be caught by END_CATCH.

The fix it to let the compiler know that the exception_rethrow does
not return normally, using ATTRIBUTE_NORETURN.

gdb/ChangeLog:
2018-05-30  Pedro Alves  <palves@redhat.com>

	* common/common-exceptions.h (exception_rethrow): Use
	ATTRIBUTE_NORETURN.
---
 gdb/ChangeLog                  | 5 +++++
 gdb/common/common-exceptions.h | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0a730c88f9..c38ff5b14a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2018-05-30  Pedro Alves  <palves@redhat.com>
+
+	* common/common-exceptions.h (exception_rethrow): Use
+	ATTRIBUTE_NORETURN.
+
 2018-05-29  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	* breakpoint.c (print_solib_event, check_status_catch_solib):
diff --git a/gdb/common/common-exceptions.h b/gdb/common/common-exceptions.h
index 15c85e28ab..e873f9d061 100644
--- a/gdb/common/common-exceptions.h
+++ b/gdb/common/common-exceptions.h
@@ -146,7 +146,7 @@  extern int exceptions_state_mc_catch (struct gdb_exception *, int);
 #if GDB_XCPT != GDB_XCPT_SJMP
 extern void *exception_try_scope_entry (void);
 extern void exception_try_scope_exit (void *saved_state);
-extern void exception_rethrow (void);
+extern void exception_rethrow (void) ATTRIBUTE_NORETURN;
 #endif
 
 /* Macro to wrap up standard try/catch behavior.