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

classic Classic list List threaded Threaded
50 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Tom Tromey-2
>>>>> "Yao" == Yao Qi <[hidden email]> 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 <[hidden email]>
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  <[hidden email]>
   
    * 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  <[hidden email]>
 
+ * 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  <[hidden email]>
+
  * 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);
 
Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-2
In reply to this post by Pedro Alves-7
>>>>> "Pedro" == Pedro Alves <[hidden email]> 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 <[hidden email]>
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  <[hidden email]>
   
    * 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  <[hidden email]>
+
+ * 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  <[hidden email]>
 
  * 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);
     }
 }
 
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Yao Qi
In reply to this post by Tom Tromey-2
On Tue, Jun 28, 2016 at 9:56 PM, Tom Tromey <[hidden email]> wrote:
>>>>>> "Yao" == Yao Qi <[hidden email]> 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 (齐尧)
Reply | Threaded
Open this post in threaded view
|

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

Yao Qi
In reply to this post by Tom Tromey-2
On Tue, Jun 28, 2016 at 9:55 PM, Tom Tromey <[hidden email]> wrote:
>>>>>> "Yao" == Yao Qi <[hidden email]> writes:
>
> Yao> I am inclined to remove these commented-out statements.
>
> Sounds good.  Here's the new patch.
>

Patch is good to me.

--
Yao (齐尧)
Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
In reply to this post by Tom Tromey-2
On 06/28/2016 09:58 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <[hidden email]> 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 <[hidden email]>
> 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  <[hidden email]>
>    
>     * 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

Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-2
In reply to this post by Yao Qi
>>>>> "Yao" == Yao Qi <[hidden email]> writes:

Yao> On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]> wrote:

>>>>>> "Yao" == Yao Qi <[hidden email]> writes:
>
> Yao> On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> 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 (齐尧)
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Tom Tromey-2
In reply to this post by Yao Qi
>>>>> "Yao" == Yao Qi <[hidden email]> 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 <[hidden email]>
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  <[hidden email]>
   
    * 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  <[hidden email]>
 
+ * 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  <[hidden email]>
+
  * 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);
 
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Yao Qi
On Wed, Jul 13, 2016 at 9:42 PM, Tom Tromey <[hidden email]> wrote:

>>>>>> "Yao" == Yao Qi <[hidden email]> 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 (齐尧)
Reply | Threaded
Open this post in threaded view
|

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

Tom Tromey-2
In reply to this post by Yao Qi
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
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Maciej W. Rozycki-3
In reply to this post by Tom Tromey-2
On Wed, 13 Jul 2016, Tom Tromey wrote:

> commit c097ec493b76209e6cf830ffca9b36fe2c2643fc
> Author: Tom Tromey <[hidden email]>
> 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  <[hidden email]>
>    
>     * 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
Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Paul_Koning

> On Jul 20, 2016, at 2:36 PM, Maciej W. Rozycki <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

Re: [RFA 5/6] Remove unused variables

Tom Tromey-2
In reply to this post by Maciej W. Rozycki-3
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
Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
In reply to this post by Yao Qi
On 06/28/2016 04:02 PM, Yao Qi wrote:
> On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

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 <[hidden email]> wrote:

> On 06/28/2016 04:02 PM, Yao Qi wrote:
>> On Mon, Jun 6, 2016 at 10:33 PM, Tom Tromey <[hidden email]> 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 (齐尧)
Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
On 07/21/2016 12:10 PM, Yao Qi wrote:
> On Thu, Jul 21, 2016 at 11:38 AM, Pedro Alves <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
Leave the call for side effects.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
In reply to this post by Pedro Alves-7
gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <[hidden email]>

        * 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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
In reply to this post by Pedro Alves-7
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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

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

Pedro Alves-7
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 <[hidden email]> 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

123