[RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Tom Tromey]

I happened to notice an unused variable in the newly minted Rust code,
so I decided to take a look at adding some warnings to prevent this.

This series adds -Wunused-but-set-parameter and
-Wunused-but-set-variable to the build.  First come the patches to
remove all the warnings, and the final patch enables the compiler
flags.

I've split up the patches for easier review, and according to the type
of patch.  I think they're generally self-explanatory.

I built and regression tested this using --enable-targets=all on
x86-64 Fedora 23.  However, I could not update the various nat-*
files, so there are probably unfixed warnings lurking there.

Tom

[RFA 4/6] Remove some variables but call functions for side effects

[Tom Tromey]

This patch consolidates the (possibly-questionable) spots where we
remove a declaration but continue to call some function for side
effects.  In a couple of cases it wasn't entirely clear to me that
this mattered; and in some other cases it might be more aesthetically
pleasing to use ATTRIBUTE_UNUSED.  So, I broke this out into a
separate patch for simpler review.

2016-06-06  Tom Tromey  <tom@tromey.com>

	* arch-utils.c (default_skip_permanent_breakpoint): Remove
	"bp_insn".
	* disasm.c (do_assembly_only): Remove "num_displayed".
	* dwarf2read.c (read_abbrev_offset): Remove "length".
	(dwarf_decode_macro_bytes) <DW_MACINFO_vendor_ext>: Remove
	"constant".
	* m32c-tdep.c (make_regs): Remove "r2hl", "r3hl", and "intbhl".
	* microblaze-tdep.c (microblaze_frame_cache): Remove "func".
	* tracefile.c (trace_save): Remove "status".
---
 gdb/ChangeLog         | 12 ++++++++++++
 gdb/arch-utils.c      |  3 +--
 gdb/disasm.c          |  4 +---
 gdb/dwarf2read.c      |  9 +++++----
 gdb/m32c-tdep.c       |  9 +++------
 gdb/microblaze-tdep.c |  4 ++--
 gdb/tracefile.c       |  7 ++++---
 7 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9839c55..b164911 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,17 @@
 2016-06-06  Tom Tromey  <tom@tromey.com>
 
+	* arch-utils.c (default_skip_permanent_breakpoint): Remove
+	"bp_insn".
+	* disasm.c (do_assembly_only): Remove "num_displayed".
+	* dwarf2read.c (read_abbrev_offset): Remove "length".
+	(dwarf_decode_macro_bytes) <DW_MACINFO_vendor_ext>: Remove
+	"constant".
+	* m32c-tdep.c (make_regs): Remove "r2hl", "r3hl", and "intbhl".
+	* microblaze-tdep.c (microblaze_frame_cache): Remove "func".
+	* tracefile.c (trace_save): Remove "status".
+
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
 	* symfile.c (simple_overlay_update_1): Comment out initialization
 	of "size".
 	(simple_overlay_update): Likewise.
diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index 604042f..53121bc 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -857,10 +857,9 @@ default_skip_permanent_breakpoint (struct regcache *regcache)
 {
   struct gdbarch *gdbarch = get_regcache_arch (regcache);
   CORE_ADDR current_pc = regcache_read_pc (regcache);
-  const gdb_byte *bp_insn;
   int bp_len;
 
-  bp_insn = gdbarch_breakpoint_from_pc (gdbarch, &current_pc, &bp_len);
+  gdbarch_breakpoint_from_pc (gdbarch, &current_pc, &bp_len);
   current_pc += bp_len;
   regcache_write_pc (regcache, current_pc);
 }
diff --git a/gdb/disasm.c b/gdb/disasm.c
index f611721..bd1f760 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -737,13 +737,11 @@ do_assembly_only (struct gdbarch *gdbarch, struct ui_out *uiout,
 		  CORE_ADDR low, CORE_ADDR high,
 		  int how_many, int flags, struct ui_file *stb)
 {
-  int num_displayed = 0;
   struct cleanup *ui_out_chain;
 
   ui_out_chain = make_cleanup_ui_out_list_begin_end (uiout, "asm_insns");
 
-  num_displayed = dump_insns (gdbarch, uiout, di, low, high, how_many,
-                              flags, stb, NULL);
+  dump_insns (gdbarch, uiout, di, low, high, how_many, flags, stb, NULL);
 
   do_cleanups (ui_out_chain);
 }
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 6658a38..58b502b 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4497,12 +4497,12 @@ read_abbrev_offset (struct dwarf2_section_info *section,
 {
   bfd *abfd = get_section_bfd_owner (section);
   const gdb_byte *info_ptr;
-  unsigned int length, initial_length_size, offset_size;
+  unsigned int initial_length_size, offset_size;
   sect_offset abbrev_offset;
 
   dwarf2_read_section (dwarf2_per_objfile->objfile, section);
   info_ptr = section->buffer + offset.sect_off;
-  length = read_initial_length (abfd, info_ptr, &initial_length_size);
+  read_initial_length (abfd, info_ptr, &initial_length_size);
   offset_size = initial_length_size == 4 ? 4 : 8;
   info_ptr += initial_length_size + 2 /*version*/;
   abbrev_offset.sect_off = read_offset_1 (abfd, info_ptr, offset_size);
@@ -21578,9 +21578,10 @@ dwarf_decode_macro_bytes (bfd *abfd,
 	  if (!section_is_gnu)
 	    {
 	      unsigned int bytes_read;
-	      int constant;
 
-	      constant = read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
+	      /* This reads the constant, but since we don't recognize
+		 any vendor extensions, we ignore it.  */
+	      read_unsigned_leb128 (abfd, mac_ptr, &bytes_read);
 	      mac_ptr += bytes_read;
 	      read_direct_string (abfd, mac_ptr, &bytes_read);
 	      mac_ptr += bytes_read;
diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
index 59927a6..1e98f73 100644
--- a/gdb/m32c-tdep.c
+++ b/gdb/m32c-tdep.c
@@ -823,9 +823,6 @@ make_regs (struct gdbarch *arch)
   struct m32c_reg *sp;
   struct m32c_reg *r0hl;
   struct m32c_reg *r1hl;
-  struct m32c_reg *r2hl;
-  struct m32c_reg *r3hl;
-  struct m32c_reg *intbhl;
   struct m32c_reg *r2r0;
   struct m32c_reg *r3r1;
   struct m32c_reg *r3r1r2r0;
@@ -889,9 +886,9 @@ make_regs (struct gdbarch *arch)
 
   r0hl        = CHL (r0, tdep->int8);
   r1hl        = CHL (r1, tdep->int8);
-  r2hl        = CHL (r2, tdep->int8);
-  r3hl        = CHL (r3, tdep->int8);
-  intbhl      = CHL (intb, tdep->int16);
+  CHL (r2, tdep->int8);
+  CHL (r3, tdep->int8);
+  CHL (intb, tdep->int16);
 
   r2r0        = CCAT (r2,   r0,   tdep->int32);
   r3r1        = CCAT (r3,   r1,   tdep->int32);
diff --git a/gdb/microblaze-tdep.c b/gdb/microblaze-tdep.c
index 9cfe3fe..665ec0c 100644
--- a/gdb/microblaze-tdep.c
+++ b/gdb/microblaze-tdep.c
@@ -437,7 +437,6 @@ microblaze_frame_cache (struct frame_info *next_frame, void **this_cache)
 {
   struct microblaze_frame_cache *cache;
   struct gdbarch *gdbarch = get_frame_arch (next_frame);
-  CORE_ADDR func;
   int rn;
 
   if (*this_cache)
@@ -451,7 +450,8 @@ microblaze_frame_cache (struct frame_info *next_frame, void **this_cache)
   for (rn = 0; rn < gdbarch_num_regs (gdbarch); rn++)
     cache->register_offsets[rn] = -1;
 
-  func = get_frame_func (next_frame);
+  /* Call for side effects.  */
+  get_frame_func (next_frame);
 
   cache->pc = get_frame_address_in_block (next_frame);
 
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index c32bcf4..56fb0d2 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -57,7 +57,6 @@ trace_save (const char *filename, struct trace_file_writer *writer,
 	    int target_does_save)
 {
   struct trace_status *ts = current_trace_status ();
-  int status;
   struct uploaded_tp *uploaded_tps = NULL, *utp;
   struct uploaded_tsv *uploaded_tsvs = NULL, *utsv;
 
@@ -77,8 +76,10 @@ trace_save (const char *filename, struct trace_file_writer *writer,
     }
 
   /* Get the trace status first before opening the file, so if the
-     target is losing, we can get out without touching files.  */
-  status = target_get_trace_status (ts);
+     target is losing, we can get out without touching files.  Since
+     we're just calling this for side effects, we ignore the
+     result.  */
+  target_get_trace_status (ts);
 
   writer->ops->start (writer, filename);
 
-- 
2.5.5

Re: [RFA 4/6] Remove some variables but call functions for side effects

[Yao Qi]

On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
> This patch consolidates the (possibly-questionable) spots where we
> remove a declaration but continue to call some function for side
> effects.  In a couple of cases it wasn't entirely clear to me that
> this mattered; and in some other cases it might be more aesthetically
> pleasing to use ATTRIBUTE_UNUSED.  So, I broke this out into a
> separate patch for simpler review.
>
> 2016-06-06  Tom Tromey  <tom@tromey.com>
>
>         * arch-utils.c (default_skip_permanent_breakpoint): Remove
>         "bp_insn".
>         * disasm.c (do_assembly_only): Remove "num_displayed".
>         * dwarf2read.c (read_abbrev_offset): Remove "length".
>         (dwarf_decode_macro_bytes) <DW_MACINFO_vendor_ext>: Remove
>         "constant".
>         * m32c-tdep.c (make_regs): Remove "r2hl", "r3hl", and "intbhl".
>         * microblaze-tdep.c (microblaze_frame_cache): Remove "func".
>         * tracefile.c (trace_save): Remove "status".

Patch is good to me.

-- 
Yao (齐尧)

[RFA 3/6] Comment out some unused overlay code

[Tom Tromey]

This patch comments out some unneeded initializations in overlay code
in symfile.c.  Normally I would not comment out code like this, but
the uses of "size" in these functions are commented out.  If you'd
prefer I could just delete the comments.

2016-06-06  Tom Tromey  <tom@tromey.com>

	* symfile.c (simple_overlay_update_1): Comment out initialization
	of "size".
	(simple_overlay_update): Likewise.
---
 gdb/ChangeLog | 6 ++++++
 gdb/symfile.c | 8 ++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fbd7051..9839c55 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-06-06  Tom Tromey  <tom@tromey.com>
 
+	* symfile.c (simple_overlay_update_1): Comment out initialization
+	of "size".
+	(simple_overlay_update): Likewise.
+
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
 	* tui/tui-winsource.c (tui_show_source_line): Use
 	ATTRIBUTE_UNUSED.
 	* tui/tui-io.c (tui_puts): Use ATTRIBUTE_UNUSED.
diff --git a/gdb/symfile.c b/gdb/symfile.c
index b244332..5de2cbf 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3630,14 +3630,14 @@ simple_read_overlay_table (void)
 static int
 simple_overlay_update_1 (struct obj_section *osect)
 {
-  int i, size;
+  int i;
   bfd *obfd = osect->objfile->obfd;
   asection *bsect = osect->the_bfd_section;
   struct gdbarch *gdbarch = get_objfile_arch (osect->objfile);
   int word_size = gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
-  size = bfd_get_section_size (osect->the_bfd_section);
+  /* size = bfd_get_section_size (osect->the_bfd_section); */
   for (i = 0; i < cache_novlys; i++)
     if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
 	&& cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
@@ -3706,11 +3706,11 @@ simple_overlay_update (struct obj_section *osect)
   ALL_OBJSECTIONS (objfile, osect)
     if (section_is_overlay (osect))
     {
-      int i, size;
+      int i;
       bfd *obfd = osect->objfile->obfd;
       asection *bsect = osect->the_bfd_section;
 
-      size = bfd_get_section_size (bsect);
+      /* size = bfd_get_section_size (bsect); */
       for (i = 0; i < cache_novlys; i++)
 	if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
 	    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
-- 
2.5.5

Re: [RFA 3/6] Comment out some unused overlay code

[Yao Qi]

On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
> This patch comments out some unneeded initializations in overlay code
> in symfile.c.  Normally I would not comment out code like this, but
> the uses of "size" in these functions are commented out.  If you'd
> prefer I could just delete the comments.

I am inclined to remove these commented-out statements.

These statements were commented out in 1998, and nobody wanted to add
them back again since then,

Fri Jan 23 07:47:06 1998  Fred Fish  <fnf@cygnus.com>

        * config/d10v/tm-d10v.h (CALL_DUMMY): Define as "{ 0 }".
        (TARGET_READ_FP): Define to d10v_read_fp rather than d10v_read_sp.
        (TARGET_WRITE_FP): Define to d10v_write_fp rather than d10v_write_sp.
        (d10v_write_fp, d10v_read_fp): Add prototypes.
        * symtab.c (decode_line_1): Remove assignment of sals[0].pc field.
        * symfile.c (simple_overlay_update, simple_overlay_update_1):
        Ignore the size of overlay sections.  This check is redundant anyway.

-- 
Yao (齐尧)

Re: [RFA 3/6] Comment out some unused overlay code

[Tom Tromey]

>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> I am inclined to remove these commented-out statements.

Sounds good.  Here's the new patch.

Tom

commit 8c3dd5d738cbb40684d1210392ae540799906a84
Author: Tom Tromey <tom@tromey.com>
Date:   Mon Jun 6 13:45:59 2016 -0600

    Remove some unused overlay code
    
    This patch removes some unneeded initializations in overlay code in
    symfile.c.  It also deletes some old commented-out code.
    
    2016-06-06  Tom Tromey  <tom@tromey.com>
    
    	* symfile.c (simple_overlay_update_1): Remove initialization
    	of "size", and commented-out code.
    	(simple_overlay_update): Likewise.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index eacc3fa..ab545c8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
+	* symfile.c (simple_overlay_update_1): Remove initialization
+	of "size", and commented-out code.
+	(simple_overlay_update): Likewise.
+
 2016-06-28  Tom Tromey  <tom@tromey.com>
 
 	* tui/tui-winsource.c (tui_show_source_line): Use getcurx.
diff --git a/gdb/symfile.c b/gdb/symfile.c
index d29e96c..7d7843e 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3584,25 +3584,22 @@ simple_read_overlay_table (void)
 static int
 simple_overlay_update_1 (struct obj_section *osect)
 {
-  int i, size;
+  int i;
   bfd *obfd = osect->objfile->obfd;
   asection *bsect = osect->the_bfd_section;
   struct gdbarch *gdbarch = get_objfile_arch (osect->objfile);
   int word_size = gdbarch_long_bit (gdbarch) / TARGET_CHAR_BIT;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
-  size = bfd_get_section_size (osect->the_bfd_section);
   for (i = 0; i < cache_novlys; i++)
     if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
-	&& cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
-	/* && cache_ovly_table[i][OSIZE] == size */ )
+	&& cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect))
       {
 	read_target_long_array (cache_ovly_table_base + i * word_size,
 				(unsigned int *) cache_ovly_table[i],
 				4, word_size, byte_order);
 	if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
-	    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
-	    /* && cache_ovly_table[i][OSIZE] == size */ )
+	    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect))
 	  {
 	    osect->ovly_mapped = cache_ovly_table[i][MAPPED];
 	    return 1;
@@ -3660,15 +3657,13 @@ simple_overlay_update (struct obj_section *osect)
   ALL_OBJSECTIONS (objfile, osect)
     if (section_is_overlay (osect))
     {
-      int i, size;
+      int i;
       bfd *obfd = osect->objfile->obfd;
       asection *bsect = osect->the_bfd_section;
 
-      size = bfd_get_section_size (bsect);
       for (i = 0; i < cache_novlys; i++)
 	if (cache_ovly_table[i][VMA] == bfd_section_vma (obfd, bsect)
-	    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect)
-	    /* && cache_ovly_table[i][OSIZE] == size */ )
+	    && cache_ovly_table[i][LMA] == bfd_section_lma (obfd, bsect))
 	  { /* obj_section matches i'th entry in ovly_table.  */
 	    osect->ovly_mapped = cache_ovly_table[i][MAPPED];
 	    break;		/* finished with inner for loop: break out.  */

Re: [RFA 3/6] Comment out some unused overlay code

[Yao Qi]

On Tue, Jun 28, 2016 at 9:55 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
> Yao> I am inclined to remove these commented-out statements.
>
> Sounds good.  Here's the new patch.
>

Patch is good to me.

-- 
Yao (齐尧)

[RFA 1/6] Change reopen_exec_file to check result of stat

[Tom Tromey]

This seems to be a real bug found by -Wunused-but-set-variable.  If
"stat" fails for some reason, gdb would use the uninitialized "st".

2016-06-06  Tom Tromey  <tom@tromey.com>

	* corefile.c (reopen_exec_file): Only examine st.st_mtime if stat
	succeeded.
---
 gdb/ChangeLog  | 5 +++++
 gdb/corefile.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index af4ddcc..649bb0b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
+	* corefile.c (reopen_exec_file): Only examine st.st_mtime if stat
+	succeeded.
+
 2016-06-02  Jon Turney  <jon.turney@dronecode.org.uk>
 
 	* windows-nat.c (handle_output_debug_string): Return type of
diff --git a/gdb/corefile.c b/gdb/corefile.c
index 6cc2afc..64de931 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -144,7 +144,7 @@ reopen_exec_file (void)
   cleanups = make_cleanup (xfree, filename);
   res = stat (filename, &st);
 
-  if (exec_bfd_mtime && exec_bfd_mtime != st.st_mtime)
+  if (res == 0 && exec_bfd_mtime && exec_bfd_mtime != st.st_mtime)
     exec_file_attach (filename, 0);
   else
     /* If we accessed the file since last opening it, close it now;
-- 
2.5.5

Re: [RFA 1/6] Change reopen_exec_file to check result of stat

[Yao Qi]

On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
> This seems to be a real bug found by -Wunused-but-set-variable.  If
> "stat" fails for some reason, gdb would use the uninitialized "st".
>
> 2016-06-06  Tom Tromey  <tom@tromey.com>
>
>         * corefile.c (reopen_exec_file): Only examine st.st_mtime if stat
>         succeeded.

Patch is good to me.

-- 
Yao (齐尧)

[RFA 2/6] Use ATTRIBUTE_UNUSED in some places

[Tom Tromey]

A few spots needed ATTRIBUTE_UNUSED to cope with the new warnings.

In the TUI this happens due to the odd way that "getyx" works -- it
essentially uses references, so a variable must be passed in, even if
the code doesn't need to use it.

The case in inflow.c is just a mass of ifdefs; and while the only use
of "result" is guarded by "#if 0", I thought it simplest to leave it
all in place.

2016-06-06  Tom Tromey  <tom@tromey.com>

	* tui/tui-winsource.c (tui_show_source_line): Use
	ATTRIBUTE_UNUSED.
	* tui/tui-io.c (tui_puts): Use ATTRIBUTE_UNUSED.
	(tui_redisplay_readline): Likewise.
	* inflow.c (child_terminal_ours_1): Use ATTRIBUTE_UNUSED.
---
 gdb/ChangeLog           | 8 ++++++++
 gdb/inflow.c            | 2 +-
 gdb/tui/tui-io.c        | 6 ++++--
 gdb/tui/tui-winsource.c | 3 ++-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 649bb0b..fbd7051 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,13 @@
 2016-06-06  Tom Tromey  <tom@tromey.com>
 
+	* tui/tui-winsource.c (tui_show_source_line): Use
+	ATTRIBUTE_UNUSED.
+	* tui/tui-io.c (tui_puts): Use ATTRIBUTE_UNUSED.
+	(tui_redisplay_readline): Likewise.
+	* inflow.c (child_terminal_ours_1): Use ATTRIBUTE_UNUSED.
+
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
 	* corefile.c (reopen_exec_file): Only examine st.st_mtime if stat
 	succeeded.
 
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 4c80dbd..9b94b29 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -419,7 +419,7 @@ child_terminal_ours_1 (int output_only)
          pgrp.  */
       sighandler_t osigttou = NULL;
 #endif
-      int result;
+      int result ATTRIBUTE_UNUSED;
 
 #ifdef SIGTTOU
       if (job_control)
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 3fa32db..35025ce 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -172,7 +172,8 @@ tui_puts (const char *string)
 	  /* Expand TABs, since ncurses on MS-Windows doesn't.  */
 	  if (c == '\t')
 	    {
-	      int line, col;
+	      int line ATTRIBUTE_UNUSED;
+	      int col;
 
 	      getyx (w, line, col);
 	      do
@@ -198,7 +199,8 @@ tui_redisplay_readline (void)
 {
   int prev_col;
   int height;
-  int col, line;
+  int col;
+  int line ATTRIBUTE_UNUSED;
   int c_pos;
   int c_line;
   int in;
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 48975b6..48bfbe4 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -273,7 +273,8 @@ static void
 tui_show_source_line (struct tui_win_info *win_info, int lineno)
 {
   struct tui_win_element *line;
-  int x, y;
+  int x;
+  int y ATTRIBUTE_UNUSED;
 
   line = win_info->generic.content[lineno - 1];
   if (line->which_element.source.is_exec_point)
-- 
2.5.5

Re: [RFA 2/6] Use ATTRIBUTE_UNUSED in some places

[Pedro Alves]

On 06/06/2016 10:33 PM, Tom Tromey wrote:
> A few spots needed ATTRIBUTE_UNUSED to cope with the new warnings.
> 
> In the TUI this happens due to the odd way that "getyx" works -- it
> essentially uses references, so a variable must be passed in, even if
> the code doesn't need to use it.

I think we should use getcurx/getcury instead.

> 
> The case in inflow.c is just a mass of ifdefs; and while the only use
> of "result" is guarded by "#if 0", I thought it simplest to leave it
> all in place.
> 

Agreed.

Thanks,
Pedro Alves

Re: [RFA 2/6] Use ATTRIBUTE_UNUSED in some places

[Tom Tromey]

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

Pedro> I think we should use getcurx/getcury instead.

I've split the patch into two.

The inflow bits remain the same as what was posted.  I can repost this
if you want.

This new patch replaces the uses of getyx with getcurx.

Tom

commit 2caa6df91fe83630faa615011a724748feee7a30
Author: Tom Tromey <tom@tromey.com>
Date:   Tue Jun 28 14:25:49 2016 -0600

    Use getcurx in curses code
    
    As suggested by Pedro, this changes a few spots to use getcurx, rather
    than getyx.  This avoids some unused variable warnings.
    
    2016-06-28  Tom Tromey  <tom@tromey.com>
    
    	* tui/tui-winsource.c (tui_show_source_line): Use getcurx.
    	* tui/tui-io.c (tui_puts): Use getcurx.
    	(tui_redisplay_readline): Likewise.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 57631e8..eacc3fa 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2016-06-28  Tom Tromey  <tom@tromey.com>
+
+	* tui/tui-winsource.c (tui_show_source_line): Use getcurx.
+	* tui/tui-io.c (tui_puts): Use getcurx.
+	(tui_redisplay_readline): Likewise.
+
 2016-06-06  Tom Tromey  <tom@tromey.com>
 
 	* inflow.c (child_terminal_ours_1): Use ATTRIBUTE_UNUSED.
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index ed79b44..93bed88 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -172,9 +172,9 @@ tui_puts (const char *string)
 	  /* Expand TABs, since ncurses on MS-Windows doesn't.  */
 	  if (c == '\t')
 	    {
-	      int line, col;
+	      int col;
 
-	      getyx (w, line, col);
+	      col = getcurx (w);
 	      do
 		{
 		  waddch (w, ' ');
@@ -198,7 +198,7 @@ tui_redisplay_readline (void)
 {
   int prev_col;
   int height;
-  int col, line;
+  int col;
   int c_pos;
   int c_line;
   int in;
@@ -230,7 +230,7 @@ tui_redisplay_readline (void)
   for (in = 0; prompt && prompt[in]; in++)
     {
       waddch (w, prompt[in]);
-      getyx (w, line, col);
+      col = getcurx (w);
       if (col <= prev_col)
         height++;
       prev_col = col;
@@ -256,7 +256,7 @@ tui_redisplay_readline (void)
       else if (c == '\t')
 	{
 	  /* Expand TABs, since ncurses on MS-Windows doesn't.  */
-	  getyx (w, line, col);
+	  col = getcurx (w);
 	  do
 	    {
 	      waddch (w, ' ');
@@ -269,7 +269,7 @@ tui_redisplay_readline (void)
 	}
       if (c == '\n')
 	TUI_CMD_WIN->detail.command_info.start_line = getcury (w);
-      getyx (w, line, col);
+      col = getcurx (w);
       if (col < prev_col)
         height++;
       prev_col = col;
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 48975b6..09080b8 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -273,7 +273,7 @@ static void
 tui_show_source_line (struct tui_win_info *win_info, int lineno)
 {
   struct tui_win_element *line;
-  int x, y;
+  int x;
 
   line = win_info->generic.content[lineno - 1];
   if (line->which_element.source.is_exec_point)
@@ -285,11 +285,11 @@ tui_show_source_line (struct tui_win_info *win_info, int lineno)
     wattroff (win_info->generic.handle, A_STANDOUT);
 
   /* Clear to end of line but stop before the border.  */
-  getyx (win_info->generic.handle, y, x);
+  x = getcurx (win_info->generic.handle);
   while (x + 1 < win_info->generic.width)
     {
       waddch (win_info->generic.handle, ' ');
-      getyx (win_info->generic.handle, y, x);
+      x = getcurx (win_info->generic.handle);
     }
 }
 

Re: [RFA 2/6] Use ATTRIBUTE_UNUSED in some places

[Pedro Alves]

On 06/28/2016 09:58 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I think we should use getcurx/getcury instead.
> 
> I've split the patch into two.
> 
> The inflow bits remain the same as what was posted.  I can repost this
> if you want.

No need.

> 
> This new patch replaces the uses of getyx with getcurx.
> 
> Tom
> 
> commit 2caa6df91fe83630faa615011a724748feee7a30
> Author: Tom Tromey <tom@tromey.com>
> Date:   Tue Jun 28 14:25:49 2016 -0600
> 
>     Use getcurx in curses code
>     
>     As suggested by Pedro, this changes a few spots to use getcurx, rather
>     than getyx.  This avoids some unused variable warnings.
>     
>     2016-06-28  Tom Tromey  <tom@tromey.com>
>     
>     	* tui/tui-winsource.c (tui_show_source_line): Use getcurx.
>     	* tui/tui-io.c (tui_puts): Use getcurx.
>     	(tui_redisplay_readline): Likewise.
> 

OK.

Thanks,
Pedro Alves

[RFA 6/6] Add -Wunused-but-set-* to build

[Tom Tromey]

This adds -Wunused-but-set-variable and -Wunused-but-set-parameter to
configure.

2016-06-06  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
	* warning.m4 (AM_GDB_WARNINGS) <build_warnings>: Add
	-Wunused-but-set-parameter, -Wunused-but-set-variable.

2016-06-06  Tom Tromey  <tom@tromey.com>

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

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index eec280d..9c0ff17 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2016-06-06  Tom Tromey  <tom@tromey.com>
 
+	* configure: Rebuild.
+	* warning.m4 (AM_GDB_WARNINGS) <build_warnings>: Add
+	-Wunused-but-set-parameter, -Wunused-but-set-variable.
+
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
 	* mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
 	(mips_o32_push_dummy_call): Remove "stack_used_p".
 	* aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
diff --git a/gdb/configure b/gdb/configure
index 60ea884..957d0e4 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -14242,7 +14242,7 @@ fi
 build_warnings="-Wall -Wpointer-arith \
 -Wno-unused -Wunused-value -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
--Wempty-body"
+-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable"
 
 # Now add in C and C++ specific options, depending on mode.
 if test "$enable_build_with_cxx" = "yes"; then
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 079507a..8bef0bd 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,7 @@
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
+	* configure: Rebuild.
+
 2016-06-02  Jon Turney  <jon.turney@dronecode.org.uk>
 
 	* win32-low.c (win32_create_inferior): Add pointer casts for C++.
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 746218e..2926deb 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -6291,7 +6291,7 @@ fi
 build_warnings="-Wall -Wpointer-arith \
 -Wno-unused -Wunused-value -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
--Wempty-body"
+-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable"
 
 # Now add in C and C++ specific options, depending on mode.
 if test "$enable_build_with_cxx" = "yes"; then
diff --git a/gdb/warning.m4 b/gdb/warning.m4
index 55f1eb3..8d7ce68 100644
--- a/gdb/warning.m4
+++ b/gdb/warning.m4
@@ -39,7 +39,7 @@ fi
 build_warnings="-Wall -Wpointer-arith \
 -Wno-unused -Wunused-value -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
--Wempty-body"
+-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable"
 
 # Now add in C and C++ specific options, depending on mode.
 if test "$enable_build_with_cxx" = "yes"; then
-- 
2.5.5

Re: [RFA 6/6] Add -Wunused-but-set-* to build

[Trevor Saunders]

On Mon, Jun 06, 2016 at 03:33:32PM -0600, Tom Tromey wrote:
> This adds -Wunused-but-set-variable and -Wunused-but-set-parameter to
> configure.

 great, I started on that a little while ago but didn't get to pushing
 these bits upstream.

> +++ b/gdb/warning.m4
> @@ -39,7 +39,7 @@ fi
>  build_warnings="-Wall -Wpointer-arith \
>  -Wno-unused -Wunused-value -Wunused-function \
>  -Wno-switch -Wno-char-subscripts \
> --Wempty-body"
> +-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable"

isn't everything in -Wunused enabled now? can we just delete -Wno-unused
and use -Wall to get us -Wunused?

Trev

Re: [RFA 6/6] Add -Wunused-but-set-* to build

[Tom Tromey]

>>>>> "Trevor" == Trevor Saunders <tbsaunde@tbsaunde.org> writes:

Trevor> isn't everything in -Wunused enabled now? can we just delete
Trevor> -Wno-unused and use -Wall to get us -Wunused?

I didn't think of that -- thanks.  From the gcc docs I see
-Wunused-label, -Wunused-local-typedefs, and -Wunused-parameter.  That
final one seems difficult for gdb given the many functions that are
called via function pointers but which do not use all their arguments.

Once the switch to C++ is complete, such parameters could be nameless.
To me that seems better than sticking ATTRIBUTE_UNUSED in many places.

Tom

Re: [RFA 6/6] Add -Wunused-but-set-* to build

[Trevor Saunders]

On Tue, Jun 07, 2016 at 08:46:35PM -0600, Tom Tromey wrote:
> >>>>> "Trevor" == Trevor Saunders <tbsaunde@tbsaunde.org> writes:
> 
> Trevor> isn't everything in -Wunused enabled now? can we just delete
> Trevor> -Wno-unused and use -Wall to get us -Wunused?
> 
> I didn't think of that -- thanks.  From the gcc docs I see
> -Wunused-label, -Wunused-local-typedefs, and -Wunused-parameter.  That
> final one seems difficult for gdb given the many functions that are
> called via function pointers but which do not use all their arguments.

the docs for -Wunused-parameter are... tricky, but I believe what they
say is -Wall -Wunused does not enable -Wunused-parameter, to enable
-Wunused-parameter you either need to pass it explicitly, or pass
-Wextra -Wunused.

> Once the switch to C++ is complete, such parameters could be nameless.
> To me that seems better than sticking ATTRIBUTE_UNUSED in many places.

I'd agree, and at that point it might make sense to enable
-Wunused-parameter.

Trev

> 
> Tom

Re: [RFA 6/6] Add -Wunused-but-set-* to build

[Tom Tromey]

Trevor> the docs for -Wunused-parameter are... tricky, but I believe what they
Trevor> say is -Wall -Wunused does not enable -Wunused-parameter, to enable
Trevor> -Wunused-parameter you either need to pass it explicitly, or pass
Trevor> -Wextra -Wunused.

Yes, I see.  I think I misread them.  They say:

    '-Wunused'
         All the above '-Wunused' options combined.

         In order to get a warning about an unused function parameter, you
         must either specify '-Wextra -Wunused' (note that '-Wall' implies
         '-Wunused'), or separately specify '-Wunused-parameter'.

The first line implies that -Wunused-parameter is part of this, but then
subsequent text says not.

I'll give it a try and see what happens.

Tom

Re: [RFA 6/6] Add -Wunused-but-set-* to build

[Tom Tromey]

Tom> I'll give it a try and see what happens.

It turns out that -Wunused-variable causes a number of errors (104 in my
build).  So, I think that while this change would be worthwhile, it's a
bit too much for me at present.

Tom

Re: [RFA 6/6] Add -Wunused-but-set-* to build

[Trevor Saunders]

On Tue, Jun 07, 2016 at 10:15:59PM -0600, Tom Tromey wrote:
> Tom> I'll give it a try and see what happens.
> 
> It turns out that -Wunused-variable causes a number of errors (104 in my
> build).  So, I think that while this change would be worthwhile, it's a
> bit too much for me at present.

Sure, I'll probably get to it in a couple weeks if you don't get there
first (I eventually want to merge bfd/warnings.m4 and gdb/warnings.m4)

Trev

> 
> Tom

[RFA 5/6] Remove unused variables

[Tom Tromey]

This patch removes set-but-unused variables.  This holds all the
removals I consider to be simple and relatively uncontroversial.

2016-06-06  Tom Tromey  <tom@tromey.com>

	* mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
	(mips_o32_push_dummy_call): Remove "stack_used_p".
	* aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
	"insn_bit28".
	* rust-lang.c (rust_print_type): Remove "len".
	* rust-exp.y (super_name): Remove "current_len".
	* python/py-framefilter.c (py_print_type): Remove "type".
	* mdebugread.c (parse_partial_symbols): Remove
	"past_first_source_file".
	<N_SO>: Remove "valu", "first_so_symnum", "prev_textlow_not_set".
	* m2-valprint.c (m2_print_unbounded_array): Remove
	"content_type".
	(m2_val_print): Remove "i".
	* linespec.c (unexpected_linespec_error): Remove "cleanup".
	* f-valprint.c (f_val_print): Remove "i".
	* elfread.c (elf_symtab_read): Remove "offset".
	* dwarf2-frame.c (dwarf2_fetch_cfa_info): Remove "addr_size".
---
 gdb/ChangeLog               | 20 ++++++++++++++++++++
 gdb/aarch64-tdep.c          |  3 +--
 gdb/dwarf2-frame.c          |  2 --
 gdb/elfread.c               |  3 ---
 gdb/f-valprint.c            |  5 ++---
 gdb/linespec.c              |  3 +--
 gdb/m2-valprint.c           |  4 ----
 gdb/mdebugread.c            | 18 +-----------------
 gdb/mips-tdep.c             | 13 +------------
 gdb/python/py-framefilter.c |  2 --
 gdb/rust-exp.y              |  4 +---
 gdb/rust-lang.c             |  3 +--
 12 files changed, 28 insertions(+), 52 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b164911..eec280d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,25 @@
 2016-06-06  Tom Tromey  <tom@tromey.com>
 
+	* mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
+	(mips_o32_push_dummy_call): Remove "stack_used_p".
+	* aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
+	"insn_bit28".
+	* rust-lang.c (rust_print_type): Remove "len".
+	* rust-exp.y (super_name): Remove "current_len".
+	* python/py-framefilter.c (py_print_type): Remove "type".
+	* mdebugread.c (parse_partial_symbols): Remove
+	"past_first_source_file".
+	<N_SO>: Remove "valu", "first_so_symnum", "prev_textlow_not_set".
+	* m2-valprint.c (m2_print_unbounded_array): Remove
+	"content_type".
+	(m2_val_print): Remove "i".
+	* linespec.c (unexpected_linespec_error): Remove "cleanup".
+	* f-valprint.c (f_val_print): Remove "i".
+	* elfread.c (elf_symtab_read): Remove "offset".
+	* dwarf2-frame.c (dwarf2_fetch_cfa_info): Remove "addr_size".
+
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
 	* arch-utils.c (default_skip_permanent_breakpoint): Remove
 	"bp_insn".
 	* disasm.c (do_assembly_only): Remove "num_displayed".
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 88fcf4b..92b57e5 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2981,11 +2981,10 @@ aarch64_record_data_proc_reg (insn_decode_record *aarch64_insn_r)
 static unsigned int
 aarch64_record_data_proc_imm (insn_decode_record *aarch64_insn_r)
 {
-  uint8_t reg_rd, insn_bit28, insn_bit23, insn_bits24_27, setflags;
+  uint8_t reg_rd, insn_bit23, insn_bits24_27, setflags;
   uint32_t record_buf[4];
 
   reg_rd = bits (aarch64_insn_r->aarch64_insn, 0, 4);
-  insn_bit28 = bit (aarch64_insn_r->aarch64_insn, 28);
   insn_bit23 = bit (aarch64_insn_r->aarch64_insn, 23);
   insn_bits24_27 = bits (aarch64_insn_r->aarch64_insn, 24, 27);
 
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 2f6355a..11258ea 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -908,7 +908,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
   struct dwarf2_fde *fde;
   CORE_ADDR text_offset;
   struct dwarf2_frame_state fs;
-  int addr_size;
 
   memset (&fs, 0, sizeof (struct dwarf2_frame_state));
 
@@ -923,7 +922,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
   fs.data_align = fde->cie->data_alignment_factor;
   fs.code_align = fde->cie->code_alignment_factor;
   fs.retaddr_column = fde->cie->return_address_register;
-  addr_size = fde->cie->addr_size;
 
   /* Check for "quirks" - known bugs in producers.  */
   dwarf2_frame_find_quirks (&fs, fde);
diff --git a/gdb/elfread.c b/gdb/elfread.c
index d4400bb..e90466b 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -233,7 +233,6 @@ elf_symtab_read (struct objfile *objfile, int type,
   asymbol *sym;
   long i;
   CORE_ADDR symaddr;
-  CORE_ADDR offset;
   enum minimal_symbol_type ms_type;
   /* Name of the last file symbol.  This is either a constant string or is
      saved on the objfile's filename cache.  */
@@ -263,8 +262,6 @@ elf_symtab_read (struct objfile *objfile, int type,
 	  continue;
 	}
 
-      offset = ANOFFSET (objfile->section_offsets,
-			 gdb_bfd_section_index (objfile->obfd, sym->section));
       if (type == ST_DYNAMIC
 	  && sym->section == bfd_und_section_ptr
 	  && (sym->flags & BSF_FUNCTION))
diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
index 1264737..f31b4c7 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -218,7 +218,6 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 {
   struct gdbarch *gdbarch = get_type_arch (type);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  unsigned int i = 0;	/* Number of characters printed.  */
   struct type *elttype;
   CORE_ADDR addr;
   int index;
@@ -292,8 +291,8 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	    {
 	      if (want_space)
 		fputs_filtered (" ", stream);
-	      i = val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
-				    stream, options);
+	      val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
+				stream, options);
 	    }
 	  return;
 	}
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 7162163..ccedec8 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1519,10 +1519,9 @@ unexpected_linespec_error (linespec_parser *parser)
       || token.type == LSTOKEN_KEYWORD)
     {
       char *string;
-      struct cleanup *cleanup;
 
       string = copy_token_string (token);
-      cleanup = make_cleanup (xfree, string);
+      make_cleanup (xfree, string);
       throw_error (GENERIC_ERROR,
 		   _("malformed linespec error: unexpected %s, \"%s\""),
 		   token_type_strings[token.type], string);
diff --git a/gdb/m2-valprint.c b/gdb/m2-valprint.c
index 2d3a32f..a53aa84 100644
--- a/gdb/m2-valprint.c
+++ b/gdb/m2-valprint.c
@@ -162,13 +162,11 @@ m2_print_unbounded_array (struct type *type, const gdb_byte *valaddr,
 			  struct ui_file *stream, int recurse,
 			  const struct value_print_options *options)
 {
-  struct type *content_type;
   CORE_ADDR addr;
   LONGEST len;
   struct value *val;
 
   type = check_typedef (type);
-  content_type = TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (type, 0));
 
   addr = unpack_pointer (TYPE_FIELD_TYPE (type, 0),
 			 (TYPE_FIELD_BITPOS (type, 0) / 8) +
@@ -316,7 +314,6 @@ m2_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	      const struct value_print_options *options)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  unsigned int i = 0;	/* Number of characters printed.  */
   unsigned len;
   struct type *elttype;
   CORE_ADDR addr;
@@ -355,7 +352,6 @@ m2_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	      LA_PRINT_STRING (stream, TYPE_TARGET_TYPE (type),
 			       valaddr + embedded_offset, len, NULL,
 			       0, options);
-	      i = len;
 	    }
 	  else
 	    {
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index c46e880..a6a2efe 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -2368,7 +2368,6 @@ parse_partial_symbols (struct objfile *objfile)
   SYMR sh;
   struct partial_symtab *pst;
   int textlow_not_set = 1;
-  int past_first_source_file = 0;
 
   /* List of current psymtab's include files.  */
   const char **psymtab_include_list;
@@ -2957,16 +2956,8 @@ parse_partial_symbols (struct objfile *objfile)
 
 		  case N_SO:
 		    {
-		      CORE_ADDR valu;
 		      static int prev_so_symnum = -10;
-		      static int first_so_symnum;
 		      const char *p;
-		      int prev_textlow_not_set;
-
-		      valu = sh.value + ANOFFSET (objfile->section_offsets,
-						  SECT_OFF_TEXT (objfile));
-
-		      prev_textlow_not_set = textlow_not_set;
 
 		      /* A zero value is probably an indication for the
 			 SunPRO 3.0 compiler.  dbx_end_psymtab explicitly tests
@@ -2974,19 +2965,12 @@ parse_partial_symbols (struct objfile *objfile)
 
 		      if (sh.value == 0
 			  && gdbarch_sofun_address_maybe_missing (gdbarch))
-			{
-			  textlow_not_set = 1;
-			  valu = 0;
-			}
+			textlow_not_set = 1;
 		      else
 			textlow_not_set = 0;
 
-		      past_first_source_file = 1;
-
 		      if (prev_so_symnum != symnum - 1)
 			{		/* Here if prev stab wasn't N_SO.  */
-			  first_so_symnum = symnum;
-
 			  if (pst)
 			    {
 			      pst = (struct partial_symtab *) 0;
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 6098f71..4e4d79e 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -2939,7 +2939,6 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
   int non_prologue_insns = 0;
   long frame_offset = 0;	/* Size of stack frame.  */
   long frame_adjust = 0;	/* Offset of FP from SP.  */
-  CORE_ADDR frame_addr = 0;	/* Value of $30, used as frame pointer.  */
   int prev_delay_slot = 0;
   int in_delay_slot;
   CORE_ADDR prev_pc;
@@ -3068,7 +3067,6 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
 	      else if (sreg == MIPS_SP_REGNUM && dreg == 30)
 				/* (D)ADDIU $fp, $sp, imm */
 		{
-		  frame_addr = sp + offset;
 		  frame_adjust = offset;
 		  frame_reg = 30;
 		}
@@ -3144,10 +3142,7 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
 	      dreg = b5s5_reg (insn);
 	      if (sreg == MIPS_SP_REGNUM && dreg == 30)
 				/* MOVE  $fp, $sp */
-		{
-		  frame_addr = sp;
-		  frame_reg = 30;
-		}
+		frame_reg = 30;
 	      else if ((sreg & 0x1c) != 0x4)
 				/* MOVE  reg, $a0-$a3 */
 		this_non_prologue_insn = 1;
@@ -5502,8 +5497,6 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	    }
 	  while (len > 0)
 	    {
-	      /* Remember if the argument was written to the stack.  */
-	      int stack_used_p = 0;
 	      int partial_len = (len < MIPS32_REGSIZE ? len : MIPS32_REGSIZE);
 
 	      if (mips_debug)
@@ -5518,7 +5511,6 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		     promoted to int before being stored?  */
 		  int longword_offset = 0;
 		  CORE_ADDR addr;
-		  stack_used_p = 1;
 
 		  if (mips_debug)
 		    {
@@ -5960,8 +5952,6 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 				  && len % MIPS64_REGSIZE != 0);
 	  while (len > 0)
 	    {
-	      /* Remember if the argument was written to the stack.  */
-	      int stack_used_p = 0;
 	      int partial_len = (len < MIPS64_REGSIZE ? len : MIPS64_REGSIZE);
 
 	      if (mips_debug)
@@ -5976,7 +5966,6 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		     promoted to int before being stored?  */
 		  int longword_offset = 0;
 		  CORE_ADDR addr;
-		  stack_used_p = 1;
 		  if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
 		    {
 		      if ((typecode == TYPE_CODE_INT
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index aa25911..b663f50 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -211,13 +211,11 @@ py_print_type (struct ui_out *out, struct value *val)
 
   TRY
     {
-      struct type *type;
       struct ui_file *stb;
       struct cleanup *cleanup;
 
       stb = mem_fileopen ();
       cleanup = make_cleanup_ui_file_delete (stb);
-      type = check_typedef (value_type (val));
       type_print (value_type (val), "", stb, -1);
       ui_out_field_stream (out, "type", stb);
       do_cleanups (cleanup);
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index c1a863c..05e59db 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -968,17 +968,15 @@ super_name (const struct rust_op *ident, unsigned int n_supers)
       int i;
       int len;
       VEC (int) *offsets = NULL;
-      unsigned int current_len, previous_len;
+      unsigned int current_len;
       struct cleanup *cleanup;
 
       cleanup = make_cleanup (VEC_cleanup (int), &offsets);
       current_len = cp_find_first_component (scope);
-      previous_len = 0;
       while (scope[current_len] != '\0')
 	{
 	  VEC_safe_push (int, offsets, current_len);
 	  gdb_assert (scope[current_len] == ':');
-	  previous_len = current_len;
 	  /* The "::".  */
 	  current_len += 2;
 	  current_len += cp_find_first_component (scope
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 5df99ce..a7b4aae 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -840,14 +840,13 @@ rust_print_type (struct type *type, const char *varstring,
     case TYPE_CODE_UNION:
       {
 	/* ADT enums */
-	int i, len = 0;
+	int i;
 
 	fputs_filtered ("enum ", stream);
 	if (TYPE_TAG_NAME (type) != NULL)
 	  {
 	    fputs_filtered (TYPE_TAG_NAME (type), stream);
 	    fputs_filtered (" ", stream);
-	    len = strlen (TYPE_TAG_NAME (type));
 	  }
 	fputs_filtered ("{\n", stream);      
 
-- 
2.5.5

Re: [RFA 5/6] Remove unused variables

[Yao Qi]

On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
> diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
> index aa25911..b663f50 100644
> --- a/gdb/python/py-framefilter.c
> +++ b/gdb/python/py-framefilter.c
> @@ -211,13 +211,11 @@ py_print_type (struct ui_out *out, struct value *val)
>
>    TRY
>      {
> -      struct type *type;
>        struct ui_file *stb;
>        struct cleanup *cleanup;
>
>        stb = mem_fileopen ();
>        cleanup = make_cleanup_ui_file_delete (stb);
> -      type = check_typedef (value_type (val));
>        type_print (value_type (val), "", stb, -1);
>        ui_out_field_stream (out, "type", stb);
>        do_cleanups (cleanup);

check_typedef has side effects, IIUC, so we can't remove the call to it.

-- 
Yao (齐尧)

Re: [RFA 5/6] Remove unused variables

[Tom Tromey]

>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> check_typedef has side effects, IIUC, so we can't remove the call to it.

Here's a new version with this restored.

Tom

commit 0c28bdeb632c8fffb6e1fbe2567198b9080f5847
Author: Tom Tromey <tom@tromey.com>
Date:   Mon Jun 6 14:18:30 2016 -0600

    Remove unused variables
    
    This patch removes set-but-unused variables.  This holds all the
    removals I consider to be simple and relatively uncontroversial.
    
    2016-06-06  Tom Tromey  <tom@tromey.com>
    
    	* mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
    	(mips_o32_push_dummy_call): Remove "stack_used_p".
    	* aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
    	"insn_bit28".
    	* rust-lang.c (rust_print_type): Remove "len".
    	* rust-exp.y (super_name): Remove "current_len".
    	* python/py-framefilter.c (py_print_type): Remove "type".
    	* mdebugread.c (parse_partial_symbols): Remove
    	"past_first_source_file".
    	<N_SO>: Remove "valu", "first_so_symnum", "prev_textlow_not_set".
    	* m2-valprint.c (m2_print_unbounded_array): Remove
    	"content_type".
    	(m2_val_print): Remove "i".
    	* linespec.c (unexpected_linespec_error): Remove "cleanup".
    	* f-valprint.c (f_val_print): Remove "i".
    	* elfread.c (elf_symtab_read): Remove "offset".
    	* dwarf2-frame.c (dwarf2_fetch_cfa_info): Remove "addr_size".

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index db76f1c..288aa3b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,25 @@
 2016-06-06  Tom Tromey  <tom@tromey.com>
 
+	* mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
+	(mips_o32_push_dummy_call): Remove "stack_used_p".
+	* aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
+	"insn_bit28".
+	* rust-lang.c (rust_print_type): Remove "len".
+	* rust-exp.y (super_name): Remove "current_len".
+	* python/py-framefilter.c (py_print_type): Remove "type".
+	* mdebugread.c (parse_partial_symbols): Remove
+	"past_first_source_file".
+	<N_SO>: Remove "valu", "first_so_symnum", "prev_textlow_not_set".
+	* m2-valprint.c (m2_print_unbounded_array): Remove
+	"content_type".
+	(m2_val_print): Remove "i".
+	* linespec.c (unexpected_linespec_error): Remove "cleanup".
+	* f-valprint.c (f_val_print): Remove "i".
+	* elfread.c (elf_symtab_read): Remove "offset".
+	* dwarf2-frame.c (dwarf2_fetch_cfa_info): Remove "addr_size".
+
+2016-06-06  Tom Tromey  <tom@tromey.com>
+
 	* arch-utils.c (default_skip_permanent_breakpoint): Remove
 	"bp_insn".
 	* disasm.c (do_assembly_only): Remove "num_displayed".
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index e5ce13e..e97e2f4 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2981,11 +2981,10 @@ aarch64_record_data_proc_reg (insn_decode_record *aarch64_insn_r)
 static unsigned int
 aarch64_record_data_proc_imm (insn_decode_record *aarch64_insn_r)
 {
-  uint8_t reg_rd, insn_bit28, insn_bit23, insn_bits24_27, setflags;
+  uint8_t reg_rd, insn_bit23, insn_bits24_27, setflags;
   uint32_t record_buf[4];
 
   reg_rd = bits (aarch64_insn_r->aarch64_insn, 0, 4);
-  insn_bit28 = bit (aarch64_insn_r->aarch64_insn, 28);
   insn_bit23 = bit (aarch64_insn_r->aarch64_insn, 23);
   insn_bits24_27 = bits (aarch64_insn_r->aarch64_insn, 24, 27);
 
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 2f6355a..11258ea 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -908,7 +908,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
   struct dwarf2_fde *fde;
   CORE_ADDR text_offset;
   struct dwarf2_frame_state fs;
-  int addr_size;
 
   memset (&fs, 0, sizeof (struct dwarf2_frame_state));
 
@@ -923,7 +922,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
   fs.data_align = fde->cie->data_alignment_factor;
   fs.code_align = fde->cie->code_alignment_factor;
   fs.retaddr_column = fde->cie->return_address_register;
-  addr_size = fde->cie->addr_size;
 
   /* Check for "quirks" - known bugs in producers.  */
   dwarf2_frame_find_quirks (&fs, fde);
diff --git a/gdb/elfread.c b/gdb/elfread.c
index d4400bb..e90466b 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -233,7 +233,6 @@ elf_symtab_read (struct objfile *objfile, int type,
   asymbol *sym;
   long i;
   CORE_ADDR symaddr;
-  CORE_ADDR offset;
   enum minimal_symbol_type ms_type;
   /* Name of the last file symbol.  This is either a constant string or is
      saved on the objfile's filename cache.  */
@@ -263,8 +262,6 @@ elf_symtab_read (struct objfile *objfile, int type,
 	  continue;
 	}
 
-      offset = ANOFFSET (objfile->section_offsets,
-			 gdb_bfd_section_index (objfile->obfd, sym->section));
       if (type == ST_DYNAMIC
 	  && sym->section == bfd_und_section_ptr
 	  && (sym->flags & BSF_FUNCTION))
diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
index 08215e2..e1a677e 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -218,7 +218,6 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 {
   struct gdbarch *gdbarch = get_type_arch (type);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  unsigned int i = 0;	/* Number of characters printed.  */
   int printed_field = 0; /* Number of fields printed.  */
   struct type *elttype;
   CORE_ADDR addr;
@@ -293,8 +292,8 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	    {
 	      if (want_space)
 		fputs_filtered (" ", stream);
-	      i = val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
-				    stream, options);
+	      val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
+				stream, options);
 	    }
 	  return;
 	}
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 7162163..ccedec8 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1519,10 +1519,9 @@ unexpected_linespec_error (linespec_parser *parser)
       || token.type == LSTOKEN_KEYWORD)
     {
       char *string;
-      struct cleanup *cleanup;
 
       string = copy_token_string (token);
-      cleanup = make_cleanup (xfree, string);
+      make_cleanup (xfree, string);
       throw_error (GENERIC_ERROR,
 		   _("malformed linespec error: unexpected %s, \"%s\""),
 		   token_type_strings[token.type], string);
diff --git a/gdb/m2-valprint.c b/gdb/m2-valprint.c
index 2d3a32f..a53aa84 100644
--- a/gdb/m2-valprint.c
+++ b/gdb/m2-valprint.c
@@ -162,13 +162,11 @@ m2_print_unbounded_array (struct type *type, const gdb_byte *valaddr,
 			  struct ui_file *stream, int recurse,
 			  const struct value_print_options *options)
 {
-  struct type *content_type;
   CORE_ADDR addr;
   LONGEST len;
   struct value *val;
 
   type = check_typedef (type);
-  content_type = TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (type, 0));
 
   addr = unpack_pointer (TYPE_FIELD_TYPE (type, 0),
 			 (TYPE_FIELD_BITPOS (type, 0) / 8) +
@@ -316,7 +314,6 @@ m2_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	      const struct value_print_options *options)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  unsigned int i = 0;	/* Number of characters printed.  */
   unsigned len;
   struct type *elttype;
   CORE_ADDR addr;
@@ -355,7 +352,6 @@ m2_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	      LA_PRINT_STRING (stream, TYPE_TARGET_TYPE (type),
 			       valaddr + embedded_offset, len, NULL,
 			       0, options);
-	      i = len;
 	    }
 	  else
 	    {
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index c46e880..a6a2efe 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -2368,7 +2368,6 @@ parse_partial_symbols (struct objfile *objfile)
   SYMR sh;
   struct partial_symtab *pst;
   int textlow_not_set = 1;
-  int past_first_source_file = 0;
 
   /* List of current psymtab's include files.  */
   const char **psymtab_include_list;
@@ -2957,16 +2956,8 @@ parse_partial_symbols (struct objfile *objfile)
 
 		  case N_SO:
 		    {
-		      CORE_ADDR valu;
 		      static int prev_so_symnum = -10;
-		      static int first_so_symnum;
 		      const char *p;
-		      int prev_textlow_not_set;
-
-		      valu = sh.value + ANOFFSET (objfile->section_offsets,
-						  SECT_OFF_TEXT (objfile));
-
-		      prev_textlow_not_set = textlow_not_set;
 
 		      /* A zero value is probably an indication for the
 			 SunPRO 3.0 compiler.  dbx_end_psymtab explicitly tests
@@ -2974,19 +2965,12 @@ parse_partial_symbols (struct objfile *objfile)
 
 		      if (sh.value == 0
 			  && gdbarch_sofun_address_maybe_missing (gdbarch))
-			{
-			  textlow_not_set = 1;
-			  valu = 0;
-			}
+			textlow_not_set = 1;
 		      else
 			textlow_not_set = 0;
 
-		      past_first_source_file = 1;
-
 		      if (prev_so_symnum != symnum - 1)
 			{		/* Here if prev stab wasn't N_SO.  */
-			  first_so_symnum = symnum;
-
 			  if (pst)
 			    {
 			      pst = (struct partial_symtab *) 0;
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 6098f71..4e4d79e 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -2939,7 +2939,6 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
   int non_prologue_insns = 0;
   long frame_offset = 0;	/* Size of stack frame.  */
   long frame_adjust = 0;	/* Offset of FP from SP.  */
-  CORE_ADDR frame_addr = 0;	/* Value of $30, used as frame pointer.  */
   int prev_delay_slot = 0;
   int in_delay_slot;
   CORE_ADDR prev_pc;
@@ -3068,7 +3067,6 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
 	      else if (sreg == MIPS_SP_REGNUM && dreg == 30)
 				/* (D)ADDIU $fp, $sp, imm */
 		{
-		  frame_addr = sp + offset;
 		  frame_adjust = offset;
 		  frame_reg = 30;
 		}
@@ -3144,10 +3142,7 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
 	      dreg = b5s5_reg (insn);
 	      if (sreg == MIPS_SP_REGNUM && dreg == 30)
 				/* MOVE  $fp, $sp */
-		{
-		  frame_addr = sp;
-		  frame_reg = 30;
-		}
+		frame_reg = 30;
 	      else if ((sreg & 0x1c) != 0x4)
 				/* MOVE  reg, $a0-$a3 */
 		this_non_prologue_insn = 1;
@@ -5502,8 +5497,6 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	    }
 	  while (len > 0)
 	    {
-	      /* Remember if the argument was written to the stack.  */
-	      int stack_used_p = 0;
 	      int partial_len = (len < MIPS32_REGSIZE ? len : MIPS32_REGSIZE);
 
 	      if (mips_debug)
@@ -5518,7 +5511,6 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		     promoted to int before being stored?  */
 		  int longword_offset = 0;
 		  CORE_ADDR addr;
-		  stack_used_p = 1;
 
 		  if (mips_debug)
 		    {
@@ -5960,8 +5952,6 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 				  && len % MIPS64_REGSIZE != 0);
 	  while (len > 0)
 	    {
-	      /* Remember if the argument was written to the stack.  */
-	      int stack_used_p = 0;
 	      int partial_len = (len < MIPS64_REGSIZE ? len : MIPS64_REGSIZE);
 
 	      if (mips_debug)
@@ -5976,7 +5966,6 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		     promoted to int before being stored?  */
 		  int longword_offset = 0;
 		  CORE_ADDR addr;
-		  stack_used_p = 1;
 		  if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
 		    {
 		      if ((typecode == TYPE_CODE_INT
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index aa25911..6692ac5 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -211,13 +211,12 @@ py_print_type (struct ui_out *out, struct value *val)
 
   TRY
     {
-      struct type *type;
       struct ui_file *stb;
       struct cleanup *cleanup;
 
       stb = mem_fileopen ();
       cleanup = make_cleanup_ui_file_delete (stb);
-      type = check_typedef (value_type (val));
+      check_typedef (value_type (val));
       type_print (value_type (val), "", stb, -1);
       ui_out_field_stream (out, "type", stb);
       do_cleanups (cleanup);
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index aeb6058..456ffe5 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -968,17 +968,15 @@ super_name (const struct rust_op *ident, unsigned int n_supers)
       int i;
       int len;
       VEC (int) *offsets = NULL;
-      unsigned int current_len, previous_len;
+      unsigned int current_len;
       struct cleanup *cleanup;
 
       cleanup = make_cleanup (VEC_cleanup (int), &offsets);
       current_len = cp_find_first_component (scope);
-      previous_len = 0;
       while (scope[current_len] != '\0')
 	{
 	  VEC_safe_push (int, offsets, current_len);
 	  gdb_assert (scope[current_len] == ':');
-	  previous_len = current_len;
 	  /* The "::".  */
 	  current_len += 2;
 	  current_len += cp_find_first_component (scope
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 23ddd5a..0642cfb 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -886,7 +886,6 @@ rust_print_type (struct type *type, const char *varstring,
 	  {
 	    fputs_filtered (TYPE_TAG_NAME (type), stream);
 	    fputs_filtered (" ", stream);
-	    len = strlen (TYPE_TAG_NAME (type));
 	  }
 	fputs_filtered ("{\n", stream);
 

Re: [RFA 5/6] Remove unused variables

[Yao Qi]

On Tue, Jun 28, 2016 at 9:56 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
> Yao> check_typedef has side effects, IIUC, so we can't remove the call to it.
>
> Here's a new version with this restored.
>

It is good to me.

-- 
Yao (齐尧)

Re: [RFA 5/6] Remove unused variables

[Tom Tromey]

>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> check_typedef has side effects, IIUC, so we can't remove the call to it.

Tom> Here's a new version with this restored.

Yao> It is good to me.

After rebasing, this patch needed a small update -- jit.c now has some
newly unused variables.

Please review, thanks.

Tom

commit c097ec493b76209e6cf830ffca9b36fe2c2643fc
Author: Tom Tromey <tom@tromey.com>
Date:   Mon Jun 6 14:18:30 2016 -0600

    Remove unused variables
    
    This patch removes set-but-unused variables.  This holds all the
    removals I consider to be simple and relatively uncontroversial.
    
    2016-07-13  Tom Tromey  <tom@tromey.com>
    
    	* mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
    	(mips_o32_push_dummy_call): Remove "stack_used_p".
    	* aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
    	"insn_bit28".
    	* rust-lang.c (rust_print_type): Remove "len".
    	* rust-exp.y (super_name): Remove "current_len".
    	* python/py-framefilter.c (py_print_type): Remove "type".
    	* mdebugread.c (parse_partial_symbols): Remove
    	"past_first_source_file".
    	<N_SO>: Remove "valu", "first_so_symnum", "prev_textlow_not_set".
    	* m2-valprint.c (m2_print_unbounded_array): Remove
    	"content_type".
    	(m2_val_print): Remove "i".
    	* linespec.c (unexpected_linespec_error): Remove "cleanup".
    	* f-valprint.c (f_val_print): Remove "i".
    	* elfread.c (elf_symtab_read): Remove "offset".
    	* dwarf2-frame.c (dwarf2_fetch_cfa_info): Remove "addr_size".
    	* jit.c (jit_dealloc_cache): Remove "i" and "frame_arch".

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e92203b..5ae3141 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,26 @@
 2016-07-13  Tom Tromey  <tom@tromey.com>
 
+	* mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
+	(mips_o32_push_dummy_call): Remove "stack_used_p".
+	* aarch64-tdep.c (aarch64_record_data_proc_imm): Remove
+	"insn_bit28".
+	* rust-lang.c (rust_print_type): Remove "len".
+	* rust-exp.y (super_name): Remove "current_len".
+	* python/py-framefilter.c (py_print_type): Remove "type".
+	* mdebugread.c (parse_partial_symbols): Remove
+	"past_first_source_file".
+	<N_SO>: Remove "valu", "first_so_symnum", "prev_textlow_not_set".
+	* m2-valprint.c (m2_print_unbounded_array): Remove
+	"content_type".
+	(m2_val_print): Remove "i".
+	* linespec.c (unexpected_linespec_error): Remove "cleanup".
+	* f-valprint.c (f_val_print): Remove "i".
+	* elfread.c (elf_symtab_read): Remove "offset".
+	* dwarf2-frame.c (dwarf2_fetch_cfa_info): Remove "addr_size".
+	* jit.c (jit_dealloc_cache): Remove "i" and "frame_arch".
+
+2016-07-13  Tom Tromey  <tom@tromey.com>
+
 	* arch-utils.c (default_skip_permanent_breakpoint): Remove
 	"bp_insn".
 	* disasm.c (do_assembly_only): Remove "num_displayed".
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index e5ce13e..e97e2f4 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -2981,11 +2981,10 @@ aarch64_record_data_proc_reg (insn_decode_record *aarch64_insn_r)
 static unsigned int
 aarch64_record_data_proc_imm (insn_decode_record *aarch64_insn_r)
 {
-  uint8_t reg_rd, insn_bit28, insn_bit23, insn_bits24_27, setflags;
+  uint8_t reg_rd, insn_bit23, insn_bits24_27, setflags;
   uint32_t record_buf[4];
 
   reg_rd = bits (aarch64_insn_r->aarch64_insn, 0, 4);
-  insn_bit28 = bit (aarch64_insn_r->aarch64_insn, 28);
   insn_bit23 = bit (aarch64_insn_r->aarch64_insn, 23);
   insn_bits24_27 = bits (aarch64_insn_r->aarch64_insn, 24, 27);
 
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 2f6355a..11258ea 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -908,7 +908,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
   struct dwarf2_fde *fde;
   CORE_ADDR text_offset;
   struct dwarf2_frame_state fs;
-  int addr_size;
 
   memset (&fs, 0, sizeof (struct dwarf2_frame_state));
 
@@ -923,7 +922,6 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
   fs.data_align = fde->cie->data_alignment_factor;
   fs.code_align = fde->cie->code_alignment_factor;
   fs.retaddr_column = fde->cie->return_address_register;
-  addr_size = fde->cie->addr_size;
 
   /* Check for "quirks" - known bugs in producers.  */
   dwarf2_frame_find_quirks (&fs, fde);
diff --git a/gdb/elfread.c b/gdb/elfread.c
index d4400bb..e90466b 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -233,7 +233,6 @@ elf_symtab_read (struct objfile *objfile, int type,
   asymbol *sym;
   long i;
   CORE_ADDR symaddr;
-  CORE_ADDR offset;
   enum minimal_symbol_type ms_type;
   /* Name of the last file symbol.  This is either a constant string or is
      saved on the objfile's filename cache.  */
@@ -263,8 +262,6 @@ elf_symtab_read (struct objfile *objfile, int type,
 	  continue;
 	}
 
-      offset = ANOFFSET (objfile->section_offsets,
-			 gdb_bfd_section_index (objfile->obfd, sym->section));
       if (type == ST_DYNAMIC
 	  && sym->section == bfd_und_section_ptr
 	  && (sym->flags & BSF_FUNCTION))
diff --git a/gdb/f-valprint.c b/gdb/f-valprint.c
index 08215e2..e1a677e 100644
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -218,7 +218,6 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 {
   struct gdbarch *gdbarch = get_type_arch (type);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  unsigned int i = 0;	/* Number of characters printed.  */
   int printed_field = 0; /* Number of fields printed.  */
   struct type *elttype;
   CORE_ADDR addr;
@@ -293,8 +292,8 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	    {
 	      if (want_space)
 		fputs_filtered (" ", stream);
-	      i = val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
-				    stream, options);
+	      val_print_string (TYPE_TARGET_TYPE (type), NULL, addr, -1,
+				stream, options);
 	    }
 	  return;
 	}
diff --git a/gdb/jit.c b/gdb/jit.c
index 2b6cf77..0a9806e 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -1165,12 +1165,8 @@ static void
 jit_dealloc_cache (struct frame_info *this_frame, void *cache)
 {
   struct jit_unwind_private *priv_data = (struct jit_unwind_private *) cache;
-  struct gdbarch *frame_arch;
-  int i;
 
   gdb_assert (priv_data->regcache != NULL);
-  frame_arch = get_frame_arch (priv_data->this_frame);
-
   regcache_xfree (priv_data->regcache);
   xfree (priv_data);
 }
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 7162163..ccedec8 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1519,10 +1519,9 @@ unexpected_linespec_error (linespec_parser *parser)
       || token.type == LSTOKEN_KEYWORD)
     {
       char *string;
-      struct cleanup *cleanup;
 
       string = copy_token_string (token);
-      cleanup = make_cleanup (xfree, string);
+      make_cleanup (xfree, string);
       throw_error (GENERIC_ERROR,
 		   _("malformed linespec error: unexpected %s, \"%s\""),
 		   token_type_strings[token.type], string);
diff --git a/gdb/m2-valprint.c b/gdb/m2-valprint.c
index 2d3a32f..a53aa84 100644
--- a/gdb/m2-valprint.c
+++ b/gdb/m2-valprint.c
@@ -162,13 +162,11 @@ m2_print_unbounded_array (struct type *type, const gdb_byte *valaddr,
 			  struct ui_file *stream, int recurse,
 			  const struct value_print_options *options)
 {
-  struct type *content_type;
   CORE_ADDR addr;
   LONGEST len;
   struct value *val;
 
   type = check_typedef (type);
-  content_type = TYPE_TARGET_TYPE (TYPE_FIELD_TYPE (type, 0));
 
   addr = unpack_pointer (TYPE_FIELD_TYPE (type, 0),
 			 (TYPE_FIELD_BITPOS (type, 0) / 8) +
@@ -316,7 +314,6 @@ m2_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	      const struct value_print_options *options)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
-  unsigned int i = 0;	/* Number of characters printed.  */
   unsigned len;
   struct type *elttype;
   CORE_ADDR addr;
@@ -355,7 +352,6 @@ m2_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	      LA_PRINT_STRING (stream, TYPE_TARGET_TYPE (type),
 			       valaddr + embedded_offset, len, NULL,
 			       0, options);
-	      i = len;
 	    }
 	  else
 	    {
diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
index c46e880..a6a2efe 100644
--- a/gdb/mdebugread.c
+++ b/gdb/mdebugread.c
@@ -2368,7 +2368,6 @@ parse_partial_symbols (struct objfile *objfile)
   SYMR sh;
   struct partial_symtab *pst;
   int textlow_not_set = 1;
-  int past_first_source_file = 0;
 
   /* List of current psymtab's include files.  */
   const char **psymtab_include_list;
@@ -2957,16 +2956,8 @@ parse_partial_symbols (struct objfile *objfile)
 
 		  case N_SO:
 		    {
-		      CORE_ADDR valu;
 		      static int prev_so_symnum = -10;
-		      static int first_so_symnum;
 		      const char *p;
-		      int prev_textlow_not_set;
-
-		      valu = sh.value + ANOFFSET (objfile->section_offsets,
-						  SECT_OFF_TEXT (objfile));
-
-		      prev_textlow_not_set = textlow_not_set;
 
 		      /* A zero value is probably an indication for the
 			 SunPRO 3.0 compiler.  dbx_end_psymtab explicitly tests
@@ -2974,19 +2965,12 @@ parse_partial_symbols (struct objfile *objfile)
 
 		      if (sh.value == 0
 			  && gdbarch_sofun_address_maybe_missing (gdbarch))
-			{
-			  textlow_not_set = 1;
-			  valu = 0;
-			}
+			textlow_not_set = 1;
 		      else
 			textlow_not_set = 0;
 
-		      past_first_source_file = 1;
-
 		      if (prev_so_symnum != symnum - 1)
 			{		/* Here if prev stab wasn't N_SO.  */
-			  first_so_symnum = symnum;
-
 			  if (pst)
 			    {
 			      pst = (struct partial_symtab *) 0;
diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 6098f71..4e4d79e 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -2939,7 +2939,6 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
   int non_prologue_insns = 0;
   long frame_offset = 0;	/* Size of stack frame.  */
   long frame_adjust = 0;	/* Offset of FP from SP.  */
-  CORE_ADDR frame_addr = 0;	/* Value of $30, used as frame pointer.  */
   int prev_delay_slot = 0;
   int in_delay_slot;
   CORE_ADDR prev_pc;
@@ -3068,7 +3067,6 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
 	      else if (sreg == MIPS_SP_REGNUM && dreg == 30)
 				/* (D)ADDIU $fp, $sp, imm */
 		{
-		  frame_addr = sp + offset;
 		  frame_adjust = offset;
 		  frame_reg = 30;
 		}
@@ -3144,10 +3142,7 @@ micromips_scan_prologue (struct gdbarch *gdbarch,
 	      dreg = b5s5_reg (insn);
 	      if (sreg == MIPS_SP_REGNUM && dreg == 30)
 				/* MOVE  $fp, $sp */
-		{
-		  frame_addr = sp;
-		  frame_reg = 30;
-		}
+		frame_reg = 30;
 	      else if ((sreg & 0x1c) != 0x4)
 				/* MOVE  reg, $a0-$a3 */
 		this_non_prologue_insn = 1;
@@ -5502,8 +5497,6 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	    }
 	  while (len > 0)
 	    {
-	      /* Remember if the argument was written to the stack.  */
-	      int stack_used_p = 0;
 	      int partial_len = (len < MIPS32_REGSIZE ? len : MIPS32_REGSIZE);
 
 	      if (mips_debug)
@@ -5518,7 +5511,6 @@ mips_o32_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		     promoted to int before being stored?  */
 		  int longword_offset = 0;
 		  CORE_ADDR addr;
-		  stack_used_p = 1;
 
 		  if (mips_debug)
 		    {
@@ -5960,8 +5952,6 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 				  && len % MIPS64_REGSIZE != 0);
 	  while (len > 0)
 	    {
-	      /* Remember if the argument was written to the stack.  */
-	      int stack_used_p = 0;
 	      int partial_len = (len < MIPS64_REGSIZE ? len : MIPS64_REGSIZE);
 
 	      if (mips_debug)
@@ -5976,7 +5966,6 @@ mips_o64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 		     promoted to int before being stored?  */
 		  int longword_offset = 0;
 		  CORE_ADDR addr;
-		  stack_used_p = 1;
 		  if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG)
 		    {
 		      if ((typecode == TYPE_CODE_INT
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index aa25911..6692ac5 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -211,13 +211,12 @@ py_print_type (struct ui_out *out, struct value *val)
 
   TRY
     {
-      struct type *type;
       struct ui_file *stb;
       struct cleanup *cleanup;
 
       stb = mem_fileopen ();
       cleanup = make_cleanup_ui_file_delete (stb);
-      type = check_typedef (value_type (val));
+      check_typedef (value_type (val));
       type_print (value_type (val), "", stb, -1);
       ui_out_field_stream (out, "type", stb);
       do_cleanups (cleanup);
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index aeb6058..456ffe5 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -968,17 +968,15 @@ super_name (const struct rust_op *ident, unsigned int n_supers)
       int i;
       int len;
       VEC (int) *offsets = NULL;
-      unsigned int current_len, previous_len;
+      unsigned int current_len;
       struct cleanup *cleanup;
 
       cleanup = make_cleanup (VEC_cleanup (int), &offsets);
       current_len = cp_find_first_component (scope);
-      previous_len = 0;
       while (scope[current_len] != '\0')
 	{
 	  VEC_safe_push (int, offsets, current_len);
 	  gdb_assert (scope[current_len] == ':');
-	  previous_len = current_len;
 	  /* The "::".  */
 	  current_len += 2;
 	  current_len += cp_find_first_component (scope
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 17b20c6..3deb525 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -888,7 +888,6 @@ rust_print_type (struct type *type, const char *varstring,
 	  {
 	    fputs_filtered (TYPE_TAG_NAME (type), stream);
 	    fputs_filtered (" ", stream);
-	    len = strlen (TYPE_TAG_NAME (type));
 	  }
 	fputs_filtered ("{\n", stream);
 

Re: [RFA 5/6] Remove unused variables

[Yao Qi]

On Wed, Jul 13, 2016 at 9:42 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
> Yao> check_typedef has side effects, IIUC, so we can't remove the call to it.
>
> Tom> Here's a new version with this restored.
>
> Yao> It is good to me.
>
> After rebasing, this patch needed a small update -- jit.c now has some
> newly unused variables.
>
> Please review, thanks.
>

It is good to me.

-- 
Yao (齐尧)

Re: [RFA 5/6] Remove unused variables

[Maciej W. Rozycki]

On Wed, 13 Jul 2016, Tom Tromey wrote:

> commit c097ec493b76209e6cf830ffca9b36fe2c2643fc
> Author: Tom Tromey <tom@tromey.com>
> Date:   Mon Jun 6 14:18:30 2016 -0600
> 
>     Remove unused variables
>     
>     This patch removes set-but-unused variables.  This holds all the
>     removals I consider to be simple and relatively uncontroversial.

 I see you got to committing it actually now, before I got to the MIPS 
part (sorry about it), already proposed by Trevor Saunders previously BTW.

>     2016-07-13  Tom Tromey  <tom@tromey.com>
>     
>     	* mips-tdep.c (micromips_scan_prologue): Remove "frame_addr".
>     	(mips_o32_push_dummy_call): Remove "stack_used_p".

 TBH I disagree the changes to `micromips_scan_prologue' are 
uncontroversial -- it looks to me like the presence of `frame_addr', 
clearly copied over from `mips32_scan_prologue', is a sign of broken 
virtual frame pointer tracking code, which has not been completed for 
frames produced by microMIPS code.  Obviously they need to follow the same 
rules WRT `alloca' calls as frames made with regular MIPS code, as there's 
nothing special defined in the ABI for microMIPS code.

 Of course one could argue that keeping broken code (though in a manner 
harmless to irrelevant cases) has little value, but at least it serves as 
a reminder to do something about it sometime.  I'll see if I can add the 
missing piece regardless sometime, though regrettably I can't give it a 
high priority.

 Overall with recent and less so improvements to GCC's and other 
compilers' optimizers I think these heuristic unwinders have hardly any 
value nowadays, they seem unable to get through function prologues 
containing arbitrary instructions thrown there by the scheduler.  This is 
very annoying in a common case where you interrupt a debuggee in the 
middle of a sleeping syscall, with no way to backtrace through stripped 
system shared libraries.

 What I think could work is combining PDR records with reduced heuristic 
unwinders, which in that case do not actually need to search for frame 
offsets as they're already provided in PDR records.  So for non-leaf 
frames there's really nothing to do beyond interpreting these records, 
because by the time a nested function call has been made, the caller's 
frame has already been fully populated.

 For leaf frames it's a tad more complicated, because the records do not 
carry information as to where in the prologue individual slots are 
initialised (i.e. registers stored).  But for that you do not have to 
teach the unwinder of the whole instruction set, it just needs to know 
these actually used for register saves, which there are a small number 
only.  And the frame offset does not have to be decoded from the 
instruction stream either, as it's already in the PDR record.

 One could argue DWARF records are the modern and therefore natural choice 
for this, but typically they're gone with stripping unless special care is 
taken and records are removed selectively.  I doubt such care is taken in 
the field on a regular basis -- i.e. people just run `strip' or maybe even 
`gcc -s', or use the `install-strip' Makefile target, which again just 
boils down to `strip' typically -- though I'd be happy to be proved wrong.

 PDR records are always kept OTOH and they are really tiny, so they don't 
consume much disk space either.  Unlike DWARF records they're also usually 
present even in handcoded assembly, as they're set up with the `.frame', 
`.mask' and `.fmask' pseudo-ops, the former of which is actually required 
for MIPS PIC code to assemble at all, and PDR generation is on by default 
in GAS.  The only drawback is some toolchain configurations may disable 
them explicitly with the `-mno-pdr' GAS option in the GCC driver.

 NB, on the contrary the changes to `mips_o32_push_dummy_call' look good 
to me as they stand, without reservation.

 FWIW,

  Maciej

Re: [RFA 5/6] Remove unused variables

[Paul_Koning]

> On Jul 20, 2016, at 2:36 PM, Maciej W. Rozycki <macro@imgtec.com> wrote:
> 
> ...
> Overall with recent and less so improvements to GCC's and other 
> compilers' optimizers I think these heuristic unwinders have hardly any 
> value nowadays, they seem unable to get through function prologues 
> containing arbitrary instructions thrown there by the scheduler.  This is 
> very annoying in a common case where you interrupt a debuggee in the 
> middle of a sleeping syscall, with no way to backtrace through stripped 
> system shared libraries.

My experience is that the heuristic unwinders can be made to handle a lot of what's thrown at them now, but it takes quite a lot of extra heuristics to do so.  I have much of this on an internal version.  Should I look into making them available?

One thing I've done that may not be generally interesting: make the unwinders work in the kernel (NetBSD) and able to unwind across exception frames so you can use kernel debugging and see the stack all the way into the calling process.  I haven't found this all that interesting in online debugging, but it has sometimes been useful in analyzing kernel crash dumps.

	paul

Re: [RFA 5/6] Remove unused variables

[Maciej W. Rozycki]

On Wed, 20 Jul 2016, Paul_Koning@Dell.com wrote:

> > Overall with recent and less so improvements to GCC's and other 
> > compilers' optimizers I think these heuristic unwinders have hardly any 
> > value nowadays, they seem unable to get through function prologues 
> > containing arbitrary instructions thrown there by the scheduler.  This is 
> > very annoying in a common case where you interrupt a debuggee in the 
> > middle of a sleeping syscall, with no way to backtrace through stripped 
> > system shared libraries.
> 
> My experience is that the heuristic unwinders can be made to handle a 
> lot of what's thrown at them now, but it takes quite a lot of extra 
> heuristics to do so.  I have much of this on an internal version.  
> Should I look into making them available?

 Absolutely!

> One thing I've done that may not be generally interesting: make the 
> unwinders work in the kernel (NetBSD) and able to unwind across 
> exception frames so you can use kernel debugging and see the stack all 
> the way into the calling process.  I haven't found this all that 
> interesting in online debugging, but it has sometimes been useful in 
> analyzing kernel crash dumps.

 I think it's a separate matter -- and for post-mortem debugging (and even 
live debugging e.g. with QEMU's integrated debug stub or if you're a lucky 
one who has JTAG probe hardware to hand) you can actually implement an 
exception frame sniffer/unwinder so that GDB can examine it automagically.  
As an example see mips-sde-tdep.c, handling exception frames from the old 
Algorithmics/MTI SDE toolkit/board support package.  If you'd like to add 
a similar handler for the NetBSD kernel, then I'll gladly accept it.

 In a typical user app debug scenario you have cases where you want to 
interrupt the program and see where it is, examine its local state, often 
in the function that made the C library call which ended up in the syscall 
just interrupted.  In the absence of either debug information all the way 
through the syscall's entry point or special support it does not work 
however with ABIs such as these used with the MIPS processor, where the 
structure of the stack frame is variable and you cannot backtrace by just 
taking the value from the frame pointer register and using it recursively 
to fetch previous frame pointers from the stack.

 Unlike say with x86 or Power, where you may not be able to get complete 
information about the innermost or some intermediate frames, but at least 
you can backtrace far enough to reach a frame associated with a function 
from your actual program being debugged and be able to fully examine the 
state there as well as within any previous frames.

 Based on my personal experience with debugging software I think it is a 
serious shortcoming of the MIPS backend, so if you have a way to improve 
the current situation, then by any means please share it and make people's 
life easier.  It'll be a huge step forward even if sometime in the future 
we may get ourselves other means, such as what I have outlined in my 
previous e-mail or maybe yet something else.

  Maciej

Re: [RFA 5/6] Remove unused variables

[Tom Tromey]

Maciej>  Of course one could argue that keeping broken code (though in a manner 
Maciej> harmless to irrelevant cases) has little value, but at least it serves as 
Maciej> a reminder to do something about it sometime.

I think on the whole it's preferable to file a bug for such cases; or to
comment out the offending code and put in an explanation.  The warnings
catch real bugs; but they are not as useful if they are noisy.

Tom

Re: [RFA 5/6] Remove unused variables

[Maciej W. Rozycki]

On Wed, 20 Jul 2016, Tom Tromey wrote:

> Maciej>  Of course one could argue that keeping broken code (though in a manner 
> Maciej> harmless to irrelevant cases) has little value, but at least it serves as 
> Maciej> a reminder to do something about it sometime.
> 
> I think on the whole it's preferable to file a bug for such cases; or to
> comment out the offending code and put in an explanation.  The warnings
> catch real bugs; but they are not as useful if they are noisy.

 Good point, I have filed PR tdep/20406 to track this issue.

  Maciej

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Yao Qi]

On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
> I built and regression tested this using --enable-targets=all on
> x86-64 Fedora 23.  However, I could not update the various nat-*
> files, so there are probably unfixed warnings lurking there.

Can you use cross compiler to cross build native gdb to catch these warnings?
otherwise, once this patch series go in, gdb build on host other than linux may
be broken.

-- 
Yao (齐尧)

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Tom Tromey]

>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
>> I built and regression tested this using --enable-targets=all on
>> x86-64 Fedora 23.  However, I could not update the various nat-*
>> files, so there are probably unfixed warnings lurking there.

Yao> Can you use cross compiler to cross build native gdb to catch these
Yao> warnings?

It seems I won't get around to this.  How about I check in everything
except the patch to enable the warning?  That way at least some progress
has been made.

Tom

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Yao Qi]

On Tue, Jul 12, 2016 at 6:06 PM, Tom Tromey <tom@tromey.com> wrote:
>>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:
>
> Yao> On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
>>> I built and regression tested this using --enable-targets=all on
>>> x86-64 Fedora 23.  However, I could not update the various nat-*
>>> files, so there are probably unfixed warnings lurking there.
>
> Yao> Can you use cross compiler to cross build native gdb to catch these
> Yao> warnings?
>
> It seems I won't get around to this.  How about I check in everything
> except the patch to enable the warning?  That way at least some progress
> has been made.
>

Yes, that is fine by me.

-- 
Yao (齐尧)

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Tom Tromey]

Yao> Can you use cross compiler to cross build native gdb to catch these
Yao> warnings?

Tom> It seems I won't get around to this.  How about I check in everything
Tom> except the patch to enable the warning?  That way at least some progress
Tom> has been made.

Yao> Yes, that is fine by me.

Thanks.  I've pushed the series except for the final patch.

I filed a bug for the last step:

https://sourceware.org/bugzilla/show_bug.cgi?id=20371


I tried adding a couple of other warnings recently, namely
-Wmisleading-indentation (gdb is clean here) and -Wduplicated-cond
(this exposed one real bug, filed; and a couple of minor non-bugs in the
python directory).

However, since I can't really do builds on other platforms, someone else
will have to submit those patches.

Tom

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Pedro Alves]

On 06/28/2016 04:02 PM, Yao Qi wrote:
> On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
>> I built and regression tested this using --enable-targets=all on
>> x86-64 Fedora 23.  However, I could not update the various nat-*
>> files, so there are probably unfixed warnings lurking there.
> 
> Can you use cross compiler to cross build native gdb to catch these warnings?
> otherwise, once this patch series go in, gdb build on host other than linux may
> be broken.

The worse that can happen is a warning becomes a build error
due to -Werror.  Since people can always use --disable-werror
to work around it, and that is enabled by default in releases, I
don't think a temporary -Werror build break on master is a major
problem.

What would you say would be a sufficient set of hosts to test before
enabling a warning?

Testing all supported hosts and all architectures would be unfeasible, 
naturally.

I'd think it'd be acceptable to just build on a couple of the
more common hosts, in the name of forward progress.  

Cross testing for mingw should be easy (Fedora has a cross mingw toolchain
in the main repo).  Then there's the GCC compile farm; that could be used
to cover AIX and some BSD.  

Any commonly-used host we miss, the buildbot should detect a problem promptly.
Hosts that don't have build slaves set up naturally have to rely on someone
interested in them building gdb regularly.  If there's no one doing
that and the build goes broken for long before someone notices, that
just indicates that not many people actually care about the port, and so
shouldn't we.

Thanks,
Pedro Alves

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Yao Qi]

On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <palves@redhat.com> wrote:
> On 06/28/2016 04:02 PM, Yao Qi wrote:
>> On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <tom@tromey.com> wrote:
>>> I built and regression tested this using --enable-targets=all on
>>> x86-64 Fedora 23.  However, I could not update the various nat-*
>>> files, so there are probably unfixed warnings lurking there.
>>
>> Can you use cross compiler to cross build native gdb to catch these warnings?
>> otherwise, once this patch series go in, gdb build on host other than linux may
>> be broken.
>
> The worse that can happen is a warning becomes a build error
> due to -Werror.  Since people can always use --disable-werror
> to work around it, and that is enabled by default in releases, I
> don't think a temporary -Werror build break on master is a major
> problem.
>
> What would you say would be a sufficient set of hosts to test before
> enabling a warning?
>
> Testing all supported hosts and all architectures would be unfeasible,
> naturally.

That isn't what I meant.  We need to cover the hosts we are able to, like
mingw, and unix machine we can access in gcc compile farm.

>
> I'd think it'd be acceptable to just build on a couple of the
> more common hosts, in the name of forward progress.
>

Yes, I agree.

-- 
Yao (齐尧)

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Pedro Alves]

On 07/21/2016 12:10 PM, Yao Qi wrote:
> On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <palves@redhat.com> wrote:

>>
>> I'd think it'd be acceptable to just build on a couple of the
>> more common hosts, in the name of forward progress.
>>
> 
> Yes, I agree.
> 

I had a mingw-w64 build tree already handy, so I gave it a try.  It
needs a couple patches.  I'll send them as reply to this email.

Thanks,
Pedro Alves

[PATCH 1/2] Remove unused variable in gdb/varobj.c when built without Python support

[Pedro Alves]

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* varobj.c (varobj_value_get_print_value): Move "gdbarch" to block
	scope that uses it.
---
 gdb/varobj.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/varobj.c b/gdb/varobj.c
index 6f56cba..fb1349a 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2453,7 +2453,6 @@ varobj_value_get_print_value (struct value *value,
   struct type *type = NULL;
   long len = 0;
   char *encoding = NULL;
-  struct gdbarch *gdbarch = NULL;
   /* Initialize it just to avoid a GCC false warning.  */
   CORE_ADDR str_addr = 0;
   int string_print = 0;
@@ -2464,7 +2463,6 @@ varobj_value_get_print_value (struct value *value,
   stb = mem_fileopen ();
   old_chain = make_cleanup_ui_file_delete (stb);
 
-  gdbarch = get_type_arch (value_type (value));
 #if HAVE_PYTHON
   if (gdb_python_initialized)
     {
@@ -2518,6 +2516,7 @@ varobj_value_get_print_value (struct value *value,
 
 		      if (s)
 			{
+			  struct gdbarch *gdbarch;
 			  char *hint;
 
 			  hint = gdbpy_get_display_hint (value_formatter);
@@ -2530,6 +2529,7 @@ varobj_value_get_print_value (struct value *value,
 
 			  len = strlen (s);
 			  thevalue = (char *) xmemdup (s, len + 1, len + 1);
+			  gdbarch = get_type_arch (value_type (value));
 			  type = builtin_type (gdbarch)->builtin_char;
 			  xfree (s);
 
-- 
2.5.5

Re: [PATCH 1/2] Remove unused variable in gdb/varobj.c when built without Python support

[Yao Qi]

On Thu, Jul 21, 2016 at 12:37 PM, Pedro Alves <palves@redhat.com> wrote:
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
>         * varobj.c (varobj_value_get_print_value): Move "gdbarch" to block
>         scope that uses it.

It is good to me.

-- 
Yao (齐尧)

Re: [PATCH 1/2] Remove unused variable in gdb/varobj.c when built without Python support

[Pedro Alves]

On 07/21/2016 03:01 PM, Yao Qi wrote:
> On Thu, Jul 21, 2016 at 12:37 PM, Pedro Alves <palves@redhat.com> wrote:
>> gdb/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>
>>         * varobj.c (varobj_value_get_print_value): Move "gdbarch" to block
>>         scope that uses it.
> 
> It is good to me.
> 

Pushed.

Thanks,
Pedro Alves

[PATCH 2/2] Remove unused variable in windows-nat.c

[Pedro Alves]

Leave the call for side effects.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* windows-nat.c (handle_exception): Remove "th".
---
 gdb/windows-nat.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 149403a..c95dc9a 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1014,13 +1014,12 @@ display_selectors (char * args, int from_tty)
 static int
 handle_exception (struct target_waitstatus *ourstatus)
 {
-  windows_thread_info *th;
   DWORD code = current_event.u.Exception.ExceptionRecord.ExceptionCode;
 
   ourstatus->kind = TARGET_WAITKIND_STOPPED;
 
   /* Record the context of the current thread.  */
-  th = thread_rec (current_event.dwThreadId, -1);
+  thread_rec (current_event.dwThreadId, -1);
 
   switch (code)
     {
-- 
2.5.5

Re: [PATCH 2/2] Remove unused variable in windows-nat.c

[Yao Qi]

On Thu, Jul 21, 2016 at 12:37 PM, Pedro Alves <palves@redhat.com> wrote:
> Leave the call for side effects.
>
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>
>         * windows-nat.c (handle_exception): Remove "th".

It is good to me.

-- 
Yao (齐尧)

Re: [PATCH 2/2] Remove unused variable in windows-nat.c

[Pedro Alves]

On 07/21/2016 03:03 PM, Yao Qi wrote:
> On Thu, Jul 21, 2016 at 12:37 PM, Pedro Alves <palves@redhat.com> wrote:
>> Leave the call for side effects.
>>
>> gdb/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>
>>         * windows-nat.c (handle_exception): Remove "th".
> 
> It is good to me.
> 

Pushed.

Thanks,
Pedro Alves

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Pedro Alves]

On 07/21/2016 12:35 PM, Pedro Alves wrote:
> On 07/21/2016 12:10 PM, Yao Qi wrote:
>> On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <palves@redhat.com> wrote:
> 
>>>
>>> I'd think it'd be acceptable to just build on a couple of the
>>> more common hosts, in the name of forward progress.
>>>
>>
>> Yes, I agree.
>>
> 
> I had a mingw-w64 build tree already handy, so I gave it a try.  It
> needs a couple patches.  I'll send them as reply to this email.

I tried a cross to macOS/OS X/darwin/whatever-it's-called-nowadays
now, with -Wunused-but-set-parameter -Wunused-but-set-variable + -O2,
and no new warnings appeared.

Thanks,
Pedro Alves

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Pedro Alves]

On 07/21/2016 12:56 PM, Pedro Alves wrote:
> On 07/21/2016 12:35 PM, Pedro Alves wrote:
>> On 07/21/2016 12:10 PM, Yao Qi wrote:
>>> On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <palves@redhat.com> wrote:
>>
>>>>
>>>> I'd think it'd be acceptable to just build on a couple of the
>>>> more common hosts, in the name of forward progress.
>>>>
>>>
>>> Yes, I agree.
>>>
>>
>> I had a mingw-w64 build tree already handy, so I gave it a try.  It
>> needs a couple patches.  I'll send them as reply to this email.
> 
> I tried a cross to macOS/OS X/darwin/whatever-it's-called-nowadays
> now, with -Wunused-but-set-parameter -Wunused-but-set-variable + -O2,
> and no new warnings appeared.

djgpp [1] has a few pre-existing build errors due to C++, but after 
fixing those, -Wunused-but-set-parameter -Wunused-but-set-variable
introduce no new problems.

[1] - Installed gcc 6.1 from here ftp://ftp.delorie.com/pub/djgpp/rpms/

Thanks,
Pedro Alves

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Pedro Alves]

On 07/21/2016 01:15 PM, Pedro Alves wrote:
> On 07/21/2016 12:56 PM, Pedro Alves wrote:
>> On 07/21/2016 12:35 PM, Pedro Alves wrote:
>>> On 07/21/2016 12:10 PM, Yao Qi wrote:
>>>> On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <palves@redhat.com> wrote:
>>>
>>>>>
>>>>> I'd think it'd be acceptable to just build on a couple of the
>>>>> more common hosts, in the name of forward progress.
>>>>>
>>>>
>>>> Yes, I agree.
>>>>
>>>
>>> I had a mingw-w64 build tree already handy, so I gave it a try.  It
>>> needs a couple patches.  I'll send them as reply to this email.
>>
>> I tried a cross to macOS/OS X/darwin/whatever-it's-called-nowadays
>> now, with -Wunused-but-set-parameter -Wunused-but-set-variable + -O2,
>> and no new warnings appeared.
> 
> djgpp [1] has a few pre-existing build errors due to C++, but after 
> fixing those, -Wunused-but-set-parameter -Wunused-but-set-variable
> introduce no new problems.

AIX (gcc111 on the compile farm) compiles fine.  It fails to link
due to the stabs overflow problem, but that's a preexisting problem.

Thanks,
Pedro Alves

> 
> [1] - Installed gcc 6.1 from here ftp://ftp.delorie.com/pub/djgpp/rpms/
> 
> Thanks,
> Pedro Alves
> 

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Pedro Alves]

On 07/21/2016 03:47 PM, Pedro Alves wrote:
> On 07/21/2016 01:15 PM, Pedro Alves wrote:
>> On 07/21/2016 12:56 PM, Pedro Alves wrote:
>>> On 07/21/2016 12:35 PM, Pedro Alves wrote:
>>>> On 07/21/2016 12:10 PM, Yao Qi wrote:
>>>>> On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <palves@redhat.com> wrote:
>>>>
>>>>>>
>>>>>> I'd think it'd be acceptable to just build on a couple of the
>>>>>> more common hosts, in the name of forward progress.
>>>>>>
>>>>>
>>>>> Yes, I agree.
>>>>>
>>>>
>>>> I had a mingw-w64 build tree already handy, so I gave it a try.  It
>>>> needs a couple patches.  I'll send them as reply to this email.
>>>
>>> I tried a cross to macOS/OS X/darwin/whatever-it's-called-nowadays
>>> now, with -Wunused-but-set-parameter -Wunused-but-set-variable + -O2,
>>> and no new warnings appeared.
>>
>> djgpp [1] has a few pre-existing build errors due to C++, but after 
>> fixing those, -Wunused-but-set-parameter -Wunused-but-set-variable
>> introduce no new problems.
> 
> AIX (gcc111 on the compile farm) compiles fine.  It fails to link
> due to the stabs overflow problem, but that's a preexisting problem.

No build regressions on

 PPC64 GNU/Linux
 S390 GNU/Linux 
 x64-64 NetBSD

Any host missing?  I think we're good to go.

Thanks,
Pedro Alves

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Yao Qi]

On Thu, Jul 21, 2016 at 4:18 PM, Pedro Alves <palves@redhat.com> wrote:
> No build regressions on
>
>  PPC64 GNU/Linux
>  S390 GNU/Linux
>  x64-64 NetBSD
>
> Any host missing?  I think we're good to go.
>

I don't think of any.  Yes, please :)

-- 
Yao (齐尧)

Re: [RFA 0/6] Add -Wunused-but-set-parameter and -Wunused-but-set-variable

[Tom Tromey]

>>>>> "Yao" == Yao Qi <qiyaoltc@gmail.com> writes:

Yao> On Thu, Jul 21, 2016 at 4:18 PM, Pedro Alves <palves@redhat.com> wrote:
>> No build regressions on
>> 
>> PPC64 GNU/Linux
>> S390 GNU/Linux
>> x64-64 NetBSD
>> 
>> Any host missing?  I think we're good to go.
>> 

Yao> I don't think of any.  Yes, please :)

I'll push the patch in momentarily.  Feel free to ping me on any build
problems.  I can't test them but I am happy to write a patch for someone
else to try.


There are a few more warnings that would be good to enable.

-Wmisleading-indentation (for me gdb is already clean here)

-Wduplicated-cond
-Wtautological-compare

I have a patch for a couple of problems pointed out by this, but there
is a larger one, https://sourceware.org/bugzilla/show_bug.cgi?id=20362
- a pretty big bug in arm_record_vfp_data_proc_insn.

It would be handy to have a "try" server on the buildbot to test patches
like this.

Tom