* [RFA 2/9] Add missing ATTRIBUTE_NORETURNs
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-04-21 18:31 ` [RFA 9/9] Add -Wimplicit-fallthrough Tom Tromey
` (8 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFA 9/9] Add -Wimplicit-fallthrough
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
2018-04-21 18:31 ` [RFA 2/9] Add missing ATTRIBUTE_NORETURNs Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-05-04 18:34 ` Pedro Alves
2018-04-21 18:31 ` [RFA 8/9] Add a missing break in record_linux_system_call Tom Tromey
` (7 subsequent siblings)
9 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFA 9/9] Add -Wimplicit-fallthrough
2018-04-21 18:31 ` [RFA 9/9] Add -Wimplicit-fallthrough Tom Tromey
@ 2018-05-04 18:34 ` Pedro Alves
2018-05-04 18:47 ` Tom Tromey
0 siblings, 1 reply; 14+ messages in thread
From: Pedro Alves @ 2018-05-04 18:34 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFA 9/9] Add -Wimplicit-fallthrough
2018-05-04 18:34 ` Pedro Alves
@ 2018-05-04 18:47 ` Tom Tromey
0 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-05-04 18:47 UTC (permalink / raw)
To: Pedro Alves; +Cc: Tom Tromey, gdb-patches
>>>>> "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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFA 8/9] Add a missing break in record_linux_system_call
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
2018-04-21 18:31 ` [RFA 2/9] Add missing ATTRIBUTE_NORETURNs Tom Tromey
2018-04-21 18:31 ` [RFA 9/9] Add -Wimplicit-fallthrough Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-04-21 18:31 ` [RFA 3/9] Fix "obvious" fall-through warnings Tom Tromey
` (6 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFA 3/9] Fix "obvious" fall-through warnings
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
` (2 preceding siblings ...)
2018-04-21 18:31 ` [RFA 8/9] Add a missing break in record_linux_system_call Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-04-21 18:31 ` [RFA 4/9] Add a fall-through comment to stabsread.c Tom Tromey
` (5 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFA 4/9] Add a fall-through comment to stabsread.c
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
` (3 preceding siblings ...)
2018-04-21 18:31 ` [RFA 3/9] Fix "obvious" fall-through warnings Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-04-21 18:31 ` [RFA 6/9] Add two fall-through comments in rs6000-tdep.c Tom Tromey
` (4 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFA 6/9] Add two fall-through comments in rs6000-tdep.c
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
` (4 preceding siblings ...)
2018-04-21 18:31 ` [RFA 4/9] Add a fall-through comment to stabsread.c Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-04-21 18:31 ` [RFA 5/9] Add fall-through comment to i386-tdep.c Tom Tromey
` (3 subsequent siblings)
9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFA 5/9] Add fall-through comment to i386-tdep.c
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
` (5 preceding siblings ...)
2018-04-21 18:31 ` [RFA 6/9] Add two fall-through comments in rs6000-tdep.c Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-04-24 20:37 ` John Baldwin
2018-04-21 18:31 ` [RFA 7/9] Add missing "breaks" Tom Tromey
` (2 subsequent siblings)
9 siblings, 1 reply; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFA 5/9] Add fall-through comment to i386-tdep.c
2018-04-21 18:31 ` [RFA 5/9] Add fall-through comment to i386-tdep.c Tom Tromey
@ 2018-04-24 20:37 ` John Baldwin
0 siblings, 0 replies; 14+ messages in thread
From: John Baldwin @ 2018-04-24 20:37 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [RFA 7/9] Add missing "breaks"
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
` (6 preceding siblings ...)
2018-04-21 18:31 ` [RFA 5/9] Add fall-through comment to i386-tdep.c Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-04-21 18:31 ` [RFA 1/9] Fix "fall through" comments Tom Tromey
2018-05-04 17:51 ` [RFA 0/9] Enable -Wimplicit-fallthrough Pedro Alves
9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* [RFA 1/9] Fix "fall through" comments
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
` (7 preceding siblings ...)
2018-04-21 18:31 ` [RFA 7/9] Add missing "breaks" Tom Tromey
@ 2018-04-21 18:31 ` Tom Tromey
2018-05-04 17:51 ` [RFA 0/9] Enable -Wimplicit-fallthrough Pedro Alves
9 siblings, 0 replies; 14+ messages in thread
From: Tom Tromey @ 2018-04-21 18:31 UTC (permalink / raw)
To: gdb-patches; +Cc: 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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [RFA 0/9] Enable -Wimplicit-fallthrough
2018-04-21 18:31 [RFA 0/9] Enable -Wimplicit-fallthrough Tom Tromey
` (8 preceding siblings ...)
2018-04-21 18:31 ` [RFA 1/9] Fix "fall through" comments Tom Tromey
@ 2018-05-04 17:51 ` Pedro Alves
9 siblings, 0 replies; 14+ messages in thread
From: Pedro Alves @ 2018-05-04 17:51 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
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
^ permalink raw reply [flat|nested] 14+ messages in thread