[RFA 4/9] Add a fall-through comment to stabsread.c

[RFA 4/9] Add a fall-through comment to stabsread.c

[Tom Tromey]

This adds a fall-through comment to stabsread.c.  I skimmed the stabs
manual a bit and it seems that 'p' and 'P' are similar enough that
this makes sense.  Also, stabs is mostly deprecated, and the code has
been this way for a long time, so it seemed safest to keep the status
quo.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* stabsread.c (define_symbol) <case 'p'>: Add fall-through
	comment.
---
 gdb/ChangeLog   | 5 +++++
 gdb/stabsread.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 8d39290642..a56702aa1d 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -1100,6 +1100,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
 	    }
 	  break;
 	}
+      /* Fall through.  */
 
     case 'P':
       /* acc seems to use P to declare the prototypes of functions that
-- 
2.13.6

[RFA 0/9] Enable -Wimplicit-fallthrough

[Tom Tromey]

This series enables -Wimplicit-fallthrough for gdb.  I think this is
valuable because it can prevent the occasional bug.

A couple of the patches are straightforward.  For example, patch #1
changes existing "fall-through" comments to the correct form that will
be recognized by GCC.

In some cases I was not sure which change to make.  I've split all
these out in this series for easier review.  Please examine them
carefully.  This applies to patches 4, 5, 6, and 8.

I chose to use comments rather than the new fall-through attribute
because the comments are more portable: they can be ignored by
compilers that do not understand them, and they do not require
additional configury.

Tested by the buildbot.

Tom

[RFA 2/9] Add missing ATTRIBUTE_NORETURNs

[Tom Tromey]

This patch adds a missing ATTRIBUTE_NORETURN.  This lets
-Wimplicit-fallthrough recognize that a given case does not fall
through.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* dwarf2loc.c (unimplemented): Add ATTRIBUTE_NORETURN.
---
 gdb/ChangeLog   | 4 ++++
 gdb/dwarf2loc.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 6c84e4ad7e..f7eff7d6a5 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -2865,7 +2865,7 @@ dwarf2_loc_desc_get_symbol_read_needs (const gdb_byte *data, size_t size,
 /* A helper function that throws an unimplemented error mentioning a
    given DWARF operator.  */
 
-static void
+static void ATTRIBUTE_NORETURN
 unimplemented (unsigned int op)
 {
   const char *name = get_DW_OP_name (op);
-- 
2.13.6

[RFA 9/9] Add -Wimplicit-fallthrough

[Tom Tromey]

This adds -Wimplicit-fallthrough to the set of default warnings.

../ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
	* warning.m4 (AM_GDB_WARNINGS): Add -Wimplicit-fallthrough.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
---
 gdb/ChangeLog           |  5 +++++
 gdb/configure           | 26 +++++++++++++-------------
 gdb/gdbserver/ChangeLog |  4 ++++
 gdb/gdbserver/configure | 31 +++++++++++++++++++------------
 gdb/warning.m4          |  2 +-
 5 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/gdb/configure b/gdb/configure
index b313152018..648f08f2e8 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -4463,9 +4463,9 @@ else
 fi
 
   if test "$plugins" = "yes"; then
-    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing dlopen" >&5
-$as_echo_n "checking for library containing dlopen... " >&6; }
-if test "${ac_cv_search_dlopen+set}" = set; then :
+    { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing dlsym" >&5
+$as_echo_n "checking for library containing dlsym... " >&6; }
+if test "${ac_cv_search_dlsym+set}" = set; then :
   $as_echo_n "(cached) " >&6
 else
   ac_func_search_save_LIBS=$LIBS
@@ -4478,11 +4478,11 @@ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 #ifdef __cplusplus
 extern "C"
 #endif
-char dlopen ();
+char dlsym ();
 int
 main ()
 {
-return dlopen ();
+return dlsym ();
   ;
   return 0;
 }
@@ -4495,25 +4495,25 @@ for ac_lib in '' dl; do
     LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
   fi
   if ac_fn_c_try_link "$LINENO"; then :
-  ac_cv_search_dlopen=$ac_res
+  ac_cv_search_dlsym=$ac_res
 fi
 rm -f core conftest.err conftest.$ac_objext \
     conftest$ac_exeext
-  if test "${ac_cv_search_dlopen+set}" = set; then :
+  if test "${ac_cv_search_dlsym+set}" = set; then :
   break
 fi
 done
-if test "${ac_cv_search_dlopen+set}" = set; then :
+if test "${ac_cv_search_dlsym+set}" = set; then :
 
 else
-  ac_cv_search_dlopen=no
+  ac_cv_search_dlsym=no
 fi
 rm conftest.$ac_ext
 LIBS=$ac_func_search_save_LIBS
 fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_dlopen" >&5
-$as_echo "$ac_cv_search_dlopen" >&6; }
-ac_res=$ac_cv_search_dlopen
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_dlsym" >&5
+$as_echo "$ac_cv_search_dlsym" >&6; }
+ac_res=$ac_cv_search_dlsym
 if test "$ac_res" != no; then :
   test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
 
@@ -15364,7 +15364,7 @@ build_warnings="-Wall -Wpointer-arith \
 -Wno-switch -Wno-char-subscripts \
 -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable \
 -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized \
--Wno-mismatched-tags \
+-Wno-mismatched-tags -Wimplicit-fallthrough \
 -Wno-error=deprecated-register"
 
 case "${host}" in
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index ab09261946..3e3d6c50b3 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -4658,7 +4658,7 @@ program_transform_name=`$as_echo "$program_transform_name" | sed "$ac_script"`
 # We require a C++11 compiler.  Check if one is available, and if
 # necessary, set CXX_DIALECT to some -std=xxx switch.
 
-      ax_cxx_compile_cxx11_required=true
+  ax_cxx_compile_alternatives="11 0x"    ax_cxx_compile_cxx11_required=true
   ac_ext=cpp
 ac_cpp='$CXXCPP $CPPFLAGS'
 ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
@@ -4974,7 +4974,8 @@ $as_echo "$ax_cv_cxx_compile_cxx11" >&6; }
   fi
 
     if test x$ac_success = xno; then
-    for switch in -std=gnu++11 -std=gnu++0x; do
+    for alternative in ${ax_cxx_compile_alternatives}; do
+      switch="-std=gnu++${alternative}"
       cachevar=`$as_echo "ax_cv_cxx_compile_cxx11_$switch" | $as_tr_sh`
       { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CXX supports C++11 features with $switch" >&5
 $as_echo_n "checking whether $CXX supports C++11 features with $switch... " >&6; }
@@ -5292,16 +5293,17 @@ $as_echo "$ac_res" >&6; }
   fi
 
     if test x$ac_success = xno; then
-                for switch in -std=c++11 -std=c++0x +std=c++11 "-h std=c++11"; do
-      cachevar=`$as_echo "ax_cv_cxx_compile_cxx11_$switch" | $as_tr_sh`
-      { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CXX supports C++11 features with $switch" >&5
+                for alternative in ${ax_cxx_compile_alternatives}; do
+      for switch in -std=c++${alternative} +std=c++${alternative} "-h std=c++${alternative}"; do
+        cachevar=`$as_echo "ax_cv_cxx_compile_cxx11_$switch" | $as_tr_sh`
+        { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CXX supports C++11 features with $switch" >&5
 $as_echo_n "checking whether $CXX supports C++11 features with $switch... " >&6; }
 if { as_var=$cachevar; eval "test \"\${$as_var+set}\" = set"; }; then :
   $as_echo_n "(cached) " >&6
 else
   ac_save_CXX="$CXX"
-         CXX="$CXX $switch"
-         cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+           CXX="$CXX $switch"
+           cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 
@@ -5596,14 +5598,18 @@ else
   eval $cachevar=no
 fi
 rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
-         CXX="$ac_save_CXX"
+           CXX="$ac_save_CXX"
 fi
 eval ac_res=\$$cachevar
 	       { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
 $as_echo "$ac_res" >&6; }
-      if eval test x\$$cachevar = xyes; then
-        CXX_DIALECT="$switch"
-        ac_success=yes
+        if eval test x\$$cachevar = xyes; then
+          CXX_DIALECT="$switch"
+          ac_success=yes
+          break
+        fi
+      done
+      if test x$ac_success = xyes; then
         break
       fi
     done
@@ -7165,7 +7171,8 @@ build_warnings="-Wall -Wpointer-arith \
 -Wno-switch -Wno-char-subscripts \
 -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable \
 -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized \
--Wno-mismatched-tags"
+-Wno-mismatched-tags -Wimplicit-fallthrough \
+-Wno-error=deprecated-register"
 
 case "${host}" in
   *-*-mingw32*)
diff --git a/gdb/warning.m4 b/gdb/warning.m4
index 3cfae65e78..791465110f 100644
--- a/gdb/warning.m4
+++ b/gdb/warning.m4
@@ -41,7 +41,7 @@ build_warnings="-Wall -Wpointer-arith \
 -Wno-switch -Wno-char-subscripts \
 -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable \
 -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized \
--Wno-mismatched-tags \
+-Wno-mismatched-tags -Wimplicit-fallthrough \
 -Wno-error=deprecated-register"
 
 case "${host}" in
-- 
2.13.6

[RFA 3/9] Fix "obvious" fall-through warnings

[Tom Tromey]

This patch fixes the subset of -Wimplicit-fallthrough warnings that I
considered obvious.  In most cases it was obvious from context that
falling through was desired; here I added the appropriate comment.  In
a couple of cases it seemed clear that a "break" was missing.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* riscv-tdep.c (riscv_isa_xlen): Add fall-through comment.
	* utils.c (can_dump_core) <LIMIT_CUR>: Add fall-through comment.
	* eval.c (fetch_subexp_value) <MEMORY_ERROR>: Add fall-through
	comment.
	* d-valprint.c (d_val_print) <TYPE_CODE_STRUCT>: Add fall-through
	comment.
	* coffread.c (coff_symtab_read) <C_LABEL>: Add fall-through
	comment.
---
 gdb/ChangeLog    | 11 +++++++++++
 gdb/coffread.c   |  1 +
 gdb/d-valprint.c |  1 +
 gdb/eval.c       |  1 +
 gdb/riscv-tdep.c |  1 +
 gdb/utils.c      |  1 +
 6 files changed, 16 insertions(+)

diff --git a/gdb/coffread.c b/gdb/coffread.c
index 192d38c331..f24ec0713a 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -928,6 +928,7 @@ coff_symtab_read (minimal_symbol_reader &reader,
 	     backtraces, so filter them out (from phdm@macqel.be).  */
 	  if (within_function)
 	    break;
+	  /* Fall through.  */
 	case C_STAT:
 	case C_THUMBLABEL:
 	case C_THUMBSTAT:
diff --git a/gdb/d-valprint.c b/gdb/d-valprint.c
index e2d8431362..579d3c8c91 100644
--- a/gdb/d-valprint.c
+++ b/gdb/d-valprint.c
@@ -88,6 +88,7 @@ d_val_print (struct type *type, int embedded_offset,
 				  stream, recurse, val, options);
 	if (ret == 0)
 	  break;
+	/* Fall through.  */
       default:
 	c_val_print (type, embedded_offset, address, stream,
 		     recurse, val, options);
diff --git a/gdb/eval.c b/gdb/eval.c
index b6fbfcf6c9..4c908c9538 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -215,6 +215,7 @@ fetch_subexp_value (struct expression *exp, int *pc, struct value **valp,
 	case MEMORY_ERROR:
 	  if (!preserve_errors)
 	    break;
+	  /* Fall through.  */
 	default:
 	  throw_exception (ex);
 	  break;
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 149e5e3cec..9fa458b79b 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -369,6 +369,7 @@ riscv_isa_xlen (struct gdbarch *gdbarch)
     {
     default:
       warning (_("unknown xlen size, assuming 4 bytes"));
+      /* Fall through.  */
     case 1:
       return 4;
     case 2:
diff --git a/gdb/utils.c b/gdb/utils.c
index b957b0dc5c..a084046c18 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -275,6 +275,7 @@ can_dump_core (enum resource_limit_kind limit_kind)
     case LIMIT_CUR:
       if (rlim.rlim_cur == 0)
 	return 0;
+      /* Fall through.  */
 
     case LIMIT_MAX:
       if (rlim.rlim_max == 0)
-- 
2.13.6

[RFA 1/9] Fix "fall through" comments

[Tom Tromey]

This patch updates existing "fall through" comments so that they can
be recognized by gcc's -Wimplicit-fallthrough comment-parsing
heuristic.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* s390-tdep.c (s390_process_record): Fix fall-through comments.
	* xcoffread.c (scan_xcoff_symtab): Move comment later.
	* symfile.c (section_is_mapped): Fix fall-through comment.
	* stabsread.c (define_symbol, read_member_functions): Fix
	fall-through comment.
	* s390-linux-tdep.c (s390_process_record): Fix fall-through
	comment.
	* remote.c (remote_wait_as): Fix fall-through comment.
	* p-exp.y (yylex): Fix fall-through comment.
	* nat/x86-dregs.c (x86_length_and_rw_bits): Fix fall-through
	comment.
	* msp430-tdep.c (msp430_gdbarch_init): Fix fall-through comment.
	* mdebugread.c (parse_partial_symbols): Fix fall-through comment.
	* jv-exp.y (yylex): Fix fall-through comment.
	* go-exp.y (lex_one_token): Fix fall-through comment.
	* gdbtypes.c (get_discrete_bounds, rank_one_type): Fix
	fall-through comment.
	* f-exp.y (yylex): Fix fall-through comment.
	* dwarf2read.c (process_die): Fix fall-through comments.
	* dbxread.c (process_one_symbol): Fix fall-through comment.
	* d-exp.y (lex_one_token): Fix fall-through comment.
	* cp-name-parser.y (yylex): Fix fall-through comment.
	* coffread.c (coff_symtab_read): Fix fall-through comment.
	* c-exp.y (lex_one_token): Fix fall-through comment.
	* arm-tdep.c (arm_decode_miscellaneous): Fix fall-through
	comment.
	* arch/arm.c (arm_instruction_changes_pc): Fix fall-through
	comment.
---
 gdb/ChangeLog        | 31 +++++++++++++++++++++++++++++++
 gdb/arch/arm.c       |  3 ++-
 gdb/arm-tdep.c       |  1 +
 gdb/c-exp.y          |  2 +-
 gdb/coffread.c       |  3 ++-
 gdb/cp-name-parser.y |  2 +-
 gdb/d-exp.y          |  2 +-
 gdb/dbxread.c        |  4 ++--
 gdb/dwarf2read.c     |  3 ++-
 gdb/f-exp.y          |  2 +-
 gdb/gdbtypes.c       |  6 +++---
 gdb/go-exp.y         |  2 +-
 gdb/mdebugread.c     |  3 ++-
 gdb/msp430-tdep.c    |  2 +-
 gdb/nat/x86-dregs.c  |  2 +-
 gdb/p-exp.y          |  2 +-
 gdb/remote.c         |  2 +-
 gdb/s390-tdep.c      |  6 ++++--
 gdb/stabsread.c      |  5 +++--
 gdb/symfile.c        |  2 +-
 gdb/xcoffread.c      |  2 +-
 21 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/gdb/arch/arm.c b/gdb/arch/arm.c
index 560b8cc075..41c91ba2a8 100644
--- a/gdb/arch/arm.c
+++ b/gdb/arch/arm.c
@@ -153,7 +153,8 @@ arm_instruction_changes_pc (uint32_t this_instr)
 	       modify PC.  */
 	    return 0;
 	  }
-	/* Data processing instruction.  Fall through.  */
+	/* Data processing instruction.  */
+	/* Fall through.  */
 
       case 0x1:
 	if (bits (this_instr, 12, 15) == 15)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index f64df4c574..85aa176399 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -6568,6 +6568,7 @@ arm_decode_miscellaneous (struct gdbarch *gdbarch, uint32_t insn,
       else if (op == 0x3)
         /* Not really supported.  */
 	return arm_copy_unmodified (gdbarch, insn, "smc", dsc);
+      /* Fall through.  */
 
     default:
       return arm_copy_undef (gdbarch, insn, dsc);
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 8dc3c068a5..deb59c0ae8 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -2571,7 +2571,7 @@ lex_one_token (struct parser_state *par_state, bool *is_quoted_name)
 	  last_was_structop = true;
 	  goto symbol;		/* Nope, must be a symbol. */
 	}
-      /* FALL THRU into number case.  */
+      /* FALL THRU.  */
 
     case '0':
     case '1':
diff --git a/gdb/coffread.c b/gdb/coffread.c
index cad3e7e2f1..192d38c331 100644
--- a/gdb/coffread.c
+++ b/gdb/coffread.c
@@ -964,7 +964,8 @@ coff_symtab_read (minimal_symbol_reader &reader,
 	    /* At least on a 3b1, gcc generates swbeg and string labels
 	       that look like this.  Ignore them.  */
 	    break;
-	  /* Fall in for static symbols that don't start with '.'  */
+	  /* For static symbols that don't start with '.'...  */
+	  /* Fall through.  */
 	case C_THUMBEXT:
 	case C_THUMBEXTFUNC:
 	case C_EXT:
diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index f1077e3aa8..f522e46419 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -1668,7 +1668,7 @@ yylex (void)
 	  lexptr++;
 	  return '-';
 	}
-      /* FALL THRU into number case.  */
+      /* FALL THRU.  */
 
     try_number:
     case '0':
diff --git a/gdb/d-exp.y b/gdb/d-exp.y
index 03be93fbbc..508927bb00 100644
--- a/gdb/d-exp.y
+++ b/gdb/d-exp.y
@@ -1109,7 +1109,7 @@ lex_one_token (struct parser_state *par_state)
 	    last_was_structop = 1;
 	  goto symbol;		/* Nope, must be a symbol.  */
 	}
-      /* FALL THRU into number case.  */
+      /* FALL THRU.  */
 
     case '0':
     case '1':
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index bdf4fb9c79..d0926896e2 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -2883,9 +2883,9 @@ process_one_symbol (int type, int desc, CORE_ADDR valu, const char *name,
       unknown_symtype_complaint (hex_string (type));
       /* FALLTHROUGH */
 
-      /* The following symbol types don't need the address field
-         relocated, since it is either unused, or is absolute.  */
     define_a_symbol:
+      /* These symbol types don't need the address field relocated,
+         since it is either unused, or is absolute.  */
     case N_GSYM:		/* Global variable.  */
     case N_NSYMS:		/* Number of symbols (Ultrix).  */
     case N_NOMAP:		/* No map?  (Ultrix).  */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 4207e4c531..dd788ff3fd 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -10575,7 +10575,8 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
       cu->processing_has_namespace_info = 1;
       if (read_namespace_alias (die, cu))
 	break;
-      /* The declaration is not a global namespace alias: fall through.  */
+      /* The declaration is not a global namespace alias.  */
+      /* Fall through.  */
     case DW_TAG_imported_module:
       cu->processing_has_namespace_info = 1;
       if (die->child != NULL && (die->tag == DW_TAG_imported_declaration
diff --git a/gdb/f-exp.y b/gdb/f-exp.y
index ffd52cf3b1..e39f68417c 100644
--- a/gdb/f-exp.y
+++ b/gdb/f-exp.y
@@ -1008,7 +1008,7 @@ yylex (void)
       /* Might be a floating point number.  */
       if (lexptr[1] < '0' || lexptr[1] > '9')
 	goto symbol;		/* Nope, must be a symbol.  */
-      /* FALL THRU into number case.  */
+      /* FALL THRU.  */
       
     case '0':
     case '1':
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index b3a037971e..120fc08ba3 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1024,7 +1024,7 @@ get_discrete_bounds (struct type *type, LONGEST *lowp, LONGEST *highp)
 	  *highp = -*lowp - 1;
 	  return 0;
 	}
-      /* ... fall through for unsigned ints ...  */
+      /* fall through */
     case TYPE_CODE_CHAR:
       *lowp = 0;
       /* This round-about calculation is to avoid shifting by
@@ -4023,7 +4023,7 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
 	    return INTEGER_CONVERSION_BADNESS;
 	  else if (TYPE_LENGTH (arg) < TYPE_LENGTH (parm))
 	    return INTEGER_PROMOTION_BADNESS;
-	  /* >>> !! else fall through !! <<< */
+	  /* fall through */
 	case TYPE_CODE_CHAR:
 	  /* Deal with signed, unsigned, and plain chars for C++ and
 	     with int cases falling through from previous case.  */
@@ -4129,7 +4129,7 @@ rank_one_type (struct type *parm, struct type *arg, struct value *value)
 	  rank.subrank = distance_to_ancestor (parm, arg, 0);
 	  if (rank.subrank >= 0)
 	    return sum_ranks (BASE_CONVERSION_BADNESS, rank);
-	  /* else fall through */
+	  /* fall through */
 	default:
 	  return INCOMPATIBLE_TYPE_BADNESS;
 	}
diff --git a/gdb/go-exp.y b/gdb/go-exp.y
index a96e65534f..936d507c37 100644
--- a/gdb/go-exp.y
+++ b/gdb/go-exp.y
@@ -1088,7 +1088,7 @@ lex_one_token (struct parser_state *par_state)
 	    last_was_structop = 1;
 	  goto symbol;		/* Nope, must be a symbol. */
 	}
-      /* FALL THRU into number case.  */
+      /* FALL THRU.  */
 
     case '0':
     case '1':
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index c0bce55148..59c4c89839 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -3698,7 +3698,8 @@ parse_partial_symbols (minimal_symbol_reader &reader,
 		  break;
 		default:
 		  unknown_ext_complaint (debug_info->ssext + psh->iss);
-		  /* Fall through, pretend it's global.  */
+		  /* Pretend it's global.  */
+		  /* Fall through.  */
 		case stGlobal:
 		  /* Global common symbols are resolved by the runtime loader,
 		     ignore them.  */
diff --git a/gdb/msp430-tdep.c b/gdb/msp430-tdep.c
index 169b7e9505..b6e062a380 100644
--- a/gdb/msp430-tdep.c
+++ b/gdb/msp430-tdep.c
@@ -908,8 +908,8 @@ msp430_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 	      code_model = ca_tdep->code_model;
 	      break;
 	    }
-	  /* Otherwise, fall through...  */
 	}
+	/* Fall through.  */
       default:
 	error (_("Unknown msp430 isa"));
 	break;
diff --git a/gdb/nat/x86-dregs.c b/gdb/nat/x86-dregs.c
index f11a708e27..7604a36342 100644
--- a/gdb/nat/x86-dregs.c
+++ b/gdb/nat/x86-dregs.c
@@ -253,7 +253,7 @@ Invalid hardware breakpoint type %d in x86_length_and_rw_bits.\n"),
       case 8:
         if (TARGET_HAS_DR_LEN_8)
  	  return (DR_LEN_8 | rw);
-	/* ELSE FALL THROUGH */
+	/* FALL THROUGH */
       default:
 	internal_error (__FILE__, __LINE__, _("\
 Invalid hardware breakpoint length %d in x86_length_and_rw_bits.\n"), len);
diff --git a/gdb/p-exp.y b/gdb/p-exp.y
index 6b857e1a02..d527b776d9 100644
--- a/gdb/p-exp.y
+++ b/gdb/p-exp.y
@@ -1224,7 +1224,7 @@ yylex (void)
 	  goto symbol;		/* Nope, must be a symbol.  */
 	}
 
-      /* FALL THRU into number case.  */
+      /* FALL THRU.  */
 
     case '0':
     case '1':
diff --git a/gdb/remote.c b/gdb/remote.c
index 49013848d5..f6a97a3c48 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7388,7 +7388,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
 	  putpkt (buf);
 	  break;
 	}
-      /* else fallthrough */
+      /* fallthrough */
     default:
       warning (_("Invalid remote reply: %s"), buf);
       break;
diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c
index 408bb875d2..3d7ba02e7f 100644
--- a/gdb/s390-tdep.c
+++ b/gdb/s390-tdep.c
@@ -4318,7 +4318,8 @@ ex:
 		      return -1;
 		    break;
 		  }
-		/* For other instructions, fallthru.  */
+		/* For other instructions... */
+		/* Fall through.  */
 	      default:
 		fprintf_unfiltered (gdb_stdlog, "Warning: Unknown KM* function %02x at %s.\n",
 				    (int)tmp, paddress (gdbarch, addr));
@@ -4620,7 +4621,8 @@ ex:
 		      return -1;
 		    break;
 		  }
-		/* For KLMD, fallthru.  */
+		/* For KLMD...  */
+		/* Fall through.  */
 	      default:
 		fprintf_unfiltered (gdb_stdlog, "Warning: Unknown KMAC function %02x at %s.\n",
 				    (int)tmp, paddress (gdbarch, addr));
diff --git a/gdb/stabsread.c b/gdb/stabsread.c
index 0017f18c35..8d39290642 100644
--- a/gdb/stabsread.c
+++ b/gdb/stabsread.c
@@ -723,7 +723,7 @@ define_symbol (CORE_ADDR valu, const char *string, int desc, int type,
 	  /* SunPRO (3.0 at least) static variable encoding.  */
 	  if (gdbarch_static_transform_name_p (gdbarch))
 	    goto normal;
-	  /* ... fall through ...  */
+	  /* fall through */
 
 	default:
 	  complaint (&symfile_complaints, _("Unknown C++ symbol name `%s'"),
@@ -2532,7 +2532,8 @@ read_member_functions (struct field_info *fip, const char **pp,
 	      complaint (&symfile_complaints,
 			 _("member function type missing, got '%c'"),
 			 (*pp)[-1]);
-	      /* Fall through into normal member function.  */
+	      /* Normal member function.  */
+	      /* Fall through.  */
 
 	    case '.':
 	      /* normal member function.  */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 1e5297ee29..942e2e69fb 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2901,7 +2901,7 @@ section_is_mapped (struct obj_section *osect)
 	  if (osect->ovly_mapped == -1)
 	    gdbarch_overlay_update (gdbarch, osect);
 	}
-      /* fall thru to manual case */
+      /* fall thru */
     case ovly_on:		/* overlay debugging manual */
       return osect->ovly_mapped == 1;
     }
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index 8c707aa8fe..66e74439b1 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -2531,9 +2531,9 @@ scan_xcoff_symtab (minimal_symbol_reader &reader,
 	  }
 	  /* FALLTHROUGH */
 
+	case C_FCN:
 	  /* C_FCN is .bf and .ef symbols.  I think it is sufficient
 	     to handle only the C_FUN and C_EXT.  */
-	case C_FCN:
 
 	case C_BSTAT:
 	case C_ESTAT:
-- 
2.13.6

[RFA 5/9] Add fall-through comment to i386-tdep.c

[Tom Tromey]

This adds a fall-through comment in i386-tdep.c.  I am not sure if
this should be a fall-through or a break (it's possible even that it
does not matter), so I elected to preserve the status quo.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* i386-tdep.c (i386_process_record): Add fall-through comment.
---
 gdb/ChangeLog   | 4 ++++
 gdb/i386-tdep.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index bf4ca54303..8ab7b005b8 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -7130,6 +7130,7 @@ Do you want to stop the program?"),
 	      else if (ir.rm == 1)
 		break;
 	    }
+	  /* Fall through.  */
 	case 3:  /* lidt */
 	  if (ir.mod == 3)
 	    {
-- 
2.13.6

[RFA 8/9] Add a missing break in record_linux_system_call

[Tom Tromey]

This adds a "break" at the end of the RECORD_SYS_RECVFROM case in
record_linux_system_call.  This seemed correct to me.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* linux-record.c (record_linux_system_call) <case
	RECORD_SYS_RECVFROM>: Add "break".
---
 gdb/ChangeLog      | 5 +++++
 gdb/linux-record.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/gdb/linux-record.c b/gdb/linux-record.c
index 1e332e1dd9..1d0735e41a 100644
--- a/gdb/linux-record.c
+++ b/gdb/linux-record.c
@@ -881,6 +881,7 @@ Do you want to stop the program?"),
 	      if (record_linux_sockaddr (regcache, tdep, tmpulongest, len))
 		return -1;
 	    }
+	  break;
 	case RECORD_SYS_RECV:
 	  regcache_raw_read_unsigned (regcache, tdep->arg2,
 				      &tmpulongest);
-- 
2.13.6

[RFA 7/9] Add missing "breaks"

[Tom Tromey]

This adds a "break" to a couple of spots where it was erroneously
omitted.  I think these are the two (potential) real bugs caught by
this series.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (mi_cmd_trace_frame_collected) <REGISTERS_FORMAT>:
	Add missing "break".
	* mi/mi-cmd-stack.c (mi_cmd_stack_list_locals) <NO_FRAME_FILTERS>:
	Add missing "break".
---
 gdb/ChangeLog         | 7 +++++++
 gdb/mi/mi-cmd-stack.c | 1 +
 gdb/mi/mi-main.c      | 1 +
 3 files changed, 9 insertions(+)

diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index 443d1ed418..52660bdd49 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -245,6 +245,7 @@ mi_cmd_stack_list_locals (const char *command, char **argv, int argc)
 	    {
 	    case NO_FRAME_FILTERS:
 	      raw_arg = oind;
+	      break;
 	    case SKIP_UNAVAILABLE:
 	      skip_unavailable = 1;
 	      break;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index d7354606b5..3d9aa99b62 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2571,6 +2571,7 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 	  break;
 	case REGISTERS_FORMAT:
 	  registers_format = oarg[0];
+	  break;
 	case MEMORY_CONTENTS:
 	  memory_contents = 1;
 	  break;
-- 
2.13.6

[RFA 6/9] Add two fall-through comments in rs6000-tdep.c

[Tom Tromey]

This adds two fall-through comments in rs6000-tdep.c.  I looked at the
PPC instruction manual and convinced myself that this was correct.
And, this isn't a semantic change.  However, close review would still
be good.

ChangeLog
2018-04-21  Tom Tromey  <tom@tromey.com>

	* rs6000-tdep.c (ppc_process_record_op4)
	(ppc_process_record_op63): Add fall-through comment.
---
 gdb/ChangeLog     | 5 +++++
 gdb/rs6000-tdep.c | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 37cbcb672b..2f0012479b 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3969,6 +3969,7 @@ ppc_process_record_op4 (struct gdbarch *gdbarch, struct regcache *regcache,
 	  && vra != 7	/* Decimal Convert From National */
 	  && vra != 31)	/* Decimal Set Sign */
 	break;
+      /* Fall through.  */
 			/* 5.16 Decimal Integer Arithmetic Instructions */
     case 1:		/* Decimal Add Modulo */
     case 65:		/* Decimal Subtract Modulo */
@@ -5565,6 +5566,7 @@ ppc_process_record_op63 (struct gdbarch *gdbarch, struct regcache *regcache,
 	  case 22:	/* Move From FPSCR Control & set RN */
 	  case 23:	/* Move From FPSCR Control & set RN Immediate */
 	    record_full_arch_list_add_reg (regcache, tdep->ppc_fpscr_regnum);
+	    /* Fall through.  */
 	  case 0:	/* Move From FPSCR */
 	  case 24:	/* Move From FPSCR Lightweight */
 	    if (PPC_FIELD (insn, 11, 5) == 0 && PPC_RC (insn))
-- 
2.13.6

Re: [RFA 5/9] Add fall-through comment to i386-tdep.c

[John Baldwin]

On Saturday, April 21, 2018 12:30:52 PM Tom Tromey wrote:
> This adds a fall-through comment in i386-tdep.c.  I am not sure if
> this should be a fall-through or a break (it's possible even that it
> does not matter), so I elected to preserve the status quo.
> 
> ChangeLog
> 2018-04-21  Tom Tromey  <tom@tromey.com>
> 
> 	* i386-tdep.c (i386_process_record): Add fall-through comment.
> ---
>  gdb/ChangeLog   | 4 ++++
>  gdb/i386-tdep.c | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index bf4ca54303..8ab7b005b8 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -7130,6 +7130,7 @@ Do you want to stop the program?"),
>  	      else if (ir.rm == 1)
>  		break;
>  	    }
> +	  /* Fall through.  */
>  	case 3:  /* lidt */
>  	  if (ir.mod == 3)
>  	    {

I believe this is correct based on the diff that added the special cases
for xgetbv and xsetbv as previously ldgt and lidt were treated the same:

diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index b4dc646b37..b354462cb9 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -5172,6 +5172,19 @@ reswitch:
 	  break;
 	  /* lgdt */
 	case 2:
+	  if (ir.mod == 3)
+	    {
+	      /* xgetbv */
+	      if (ir.rm == 0)
+		{
+		  I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REAX_REGNUM);
+		  I386_RECORD_ARCH_LIST_ADD_REG (X86_RECORD_REDX_REGNUM);
+		  break;
+		}
+	      /* xsetbv */
+	      else if (ir.rm == 1)
+		break;
+	    }
 	  /* lidt */
 	case 3:
 	  if (ir.mod == 3)

-- 
John Baldwin

Re: [RFA 0/9] Enable -Wimplicit-fallthrough

[Pedro Alves]

On 04/21/2018 07:30 PM, Tom Tromey wrote:
> This series enables -Wimplicit-fallthrough for gdb.  I think this is
> valuable because it can prevent the occasional bug.

Agreed.

> 
> A couple of the patches are straightforward.  For example, patch #1
> changes existing "fall-through" comments to the correct form that will
> be recognized by GCC.
> 
> In some cases I was not sure which change to make.  I've split all
> these out in this series for easier review.  Please examine them
> carefully.  This applies to patches 4, 5, 6, and 8.
> 
> I chose to use comments rather than the new fall-through attribute
> because the comments are more portable: they can be ignored by
> compilers that do not understand them, and they do not require
> additional configury.

Yeah.  Maybe we'll be able to use [[fallthough]] at some point.

I sent a comment to patch 9/9.

Please update the commit log of patch 5/9 reflecting John's
finding, if it makes sense.

Otherwise looks good to me.  Please push.  You'll need to 
regenerate the configure scripts.

Thanks,
Pedro Alves

Re: [RFA 9/9] Add -Wimplicit-fallthrough

[Pedro Alves]

On 04/21/2018 07:30 PM, Tom Tromey wrote:
> This adds -Wimplicit-fallthrough to the set of default warnings.
> 
> ../ChangeLog
> 2018-04-21  Tom Tromey  <tom@tromey.com>
> 
> 	* configure: Rebuild.
> 	* warning.m4 (AM_GDB_WARNINGS): Add -Wimplicit-fallthrough.
> 
> ChangeLog
> 2018-04-21  Tom Tromey  <tom@tromey.com>
> 
> 	* configure: Rebuild.

Did you change your script to generate the ChangeLog's?
The paths in "../ChangeLog" and "ChangeLog" above look like
relative to gdb/gdbserver/, while I'd expect to see 
gdb/ChangeLog and gdb/gdbserver/ChangeLog instead.

(Same throughout the series, but this one stood out.)

Thanks,
Pedro Alves

Re: [RFA 9/9] Add -Wimplicit-fallthrough

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> On 04/21/2018 07:30 PM, Tom Tromey wrote:
>> This adds -Wimplicit-fallthrough to the set of default warnings.
>> 
>> ../ChangeLog
>> 2018-04-21  Tom Tromey  <tom@tromey.com>
>> 
>> * configure: Rebuild.
>> * warning.m4 (AM_GDB_WARNINGS): Add -Wimplicit-fallthrough.
>> 
>> ChangeLog
>> 2018-04-21  Tom Tromey  <tom@tromey.com>
>> 
>> * configure: Rebuild.

Pedro> Did you change your script to generate the ChangeLog's?
Pedro> The paths in "../ChangeLog" and "ChangeLog" above look like
Pedro> relative to gdb/gdbserver/, while I'd expect to see 
Pedro> gdb/ChangeLog and gdb/gdbserver/ChangeLog instead.

Pedro> (Same throughout the series, but this one stood out.)

I didn't change anything, so I guess there is some bug I never noticed
before.  I will try to find it.  Thanks.

Tom