[RFA 00/13] Add -Wunused-variable

[RFA 00/13] Add -Wunused-variable

[Tom Tromey]

An earlier thread pointed out the desirability of -Wunused-variable.
This series adds this to the build.

In a couple of spots I found it expedient to use ATTRIBUTE_UNUSED, but
mostly I avoided this.  I also examined the other existing uses of
ATTRIBUTE_UNUSED to make sure they seemed ok.  They did to me.

This series would not have been possible without Simon, who reduce the
needed configure test to a manageable size.

-Wunused-variable caught a couple real bugs.  See patch #8 and patch #9.

Tested by the buildbot.

Tom

[RFA 05/13] Make a few calls in *-tdep.c for effect

[Tom Tromey]

In a few cases, there were calls in *-tdep.c that I did not want to
remove because I was not certain that it was safe -- perhaps the side
effect of the function (generally throwing an error) was desired.

2018-07-12  Tom Tromey  <tom@tromey.com>

	* tic6x-tdep.c (tic6x_push_dummy_call): Call find_function_addr
	for effect.
	* sparc64-tdep.c (adi_examine_command): Call get_adi_info_proc for
	effect.
	* rs6000-lynx178-tdep.c (rs6000_lynx178_push_dummy_call): Call
	find_function_addr for effect.
	* nios2-tdep.c (nios2_push_dummy_call): Call find_function_addr
	for effect.
	* mep-tdep.c (mep_push_dummy_call): Call find_function_addr for
	effect.
---
 gdb/ChangeLog             | 13 +++++++++++++
 gdb/mep-tdep.c            |  4 +++-
 gdb/nios2-tdep.c          |  4 +++-
 gdb/rs6000-lynx178-tdep.c |  4 +++-
 gdb/sparc64-tdep.c        |  3 ++-
 gdb/tic6x-tdep.c          |  4 +++-
 6 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index c8a5ecfbe07..0a6fe7de663 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -2264,9 +2264,11 @@ mep_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   CORE_ADDR *copy = (CORE_ADDR *) alloca (argc * sizeof (copy[0]));
-  CORE_ADDR func_addr = find_function_addr (function, NULL);
   int i;
 
+  /* Call for side effects.  */
+  find_function_addr (function, NULL);
+
   /* The number of the next register available to hold an argument.  */
   int arg_reg;
 
diff --git a/gdb/nios2-tdep.c b/gdb/nios2-tdep.c
index 91b4381f0b7..230e39d6671 100644
--- a/gdb/nios2-tdep.c
+++ b/gdb/nios2-tdep.c
@@ -1820,9 +1820,11 @@ nios2_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int argnum;
   int len = 0;
   int stack_offset = 0;
-  CORE_ADDR func_addr = find_function_addr (function, NULL);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
+  /* Call for side effects.  */
+  find_function_addr (function, NULL);
+
   /* Set the return address register to point to the entry point of
      the program, where a breakpoint lies in wait.  */
   regcache_cooked_write_signed (regcache, NIOS2_RA_REGNUM, bp_addr);
diff --git a/gdb/rs6000-lynx178-tdep.c b/gdb/rs6000-lynx178-tdep.c
index 13eed3aeaea..cbd235791ea 100644
--- a/gdb/rs6000-lynx178-tdep.c
+++ b/gdb/rs6000-lynx178-tdep.c
@@ -44,7 +44,9 @@ rs6000_lynx178_push_dummy_call (struct gdbarch *gdbarch,
   gdb_byte tmp_buffer[50];
   int f_argno = 0;		/* current floating point argno */
   int wordsize = gdbarch_tdep (gdbarch)->wordsize;
-  CORE_ADDR func_addr = find_function_addr (function, NULL);
+
+  /* Call for side effects.  */
+  find_function_addr (function, NULL);
 
   struct value *arg = 0;
   struct type *type;
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index 8d96cfc0969..e8befaed6ec 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -462,7 +462,8 @@ adi_examine_command (const char *args, int from_tty)
     error (_("No ADI information"));
 
   pid_t pid = inferior_ptid.pid ();
-  sparc64_adi_info *proc = get_adi_info_proc (pid);
+  /* Call for side effects.  */
+  get_adi_info_proc (pid);
   int cnt = 1;
   const char *p = args;
   if (p && *p == '/')
diff --git a/gdb/tic6x-tdep.c b/gdb/tic6x-tdep.c
index efb8b0561b4..6a143422dca 100644
--- a/gdb/tic6x-tdep.c
+++ b/gdb/tic6x-tdep.c
@@ -882,13 +882,15 @@ tic6x_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
   int argnum;
   int stack_offset = 4;
   int references_offset = 4;
-  CORE_ADDR func_addr = find_function_addr (function, NULL);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   struct type *func_type = value_type (function);
   /* The first arg passed on stack.  Mostly the first 10 args are passed by
      registers.  */
   int first_arg_on_stack = 10;
 
+  /* Call for side effects.  */
+  find_function_addr (function, NULL);
+
   /* Set the return address register to point to the entry point of
      the program, where a breakpoint lies in wait.  */
   regcache_cooked_write_unsigned (regcache, TIC6X_RA_REGNUM, bp_addr);
-- 
2.13.6

Re: [RFA 05/13] Make a few calls in *-tdep.c for effect

[Simon Marchi]

On 2018-07-12 04:52 PM, Tom Tromey wrote:
> In a few cases, there were calls in *-tdep.c that I did not want to
> remove because I was not certain that it was safe -- perhaps the side
> effect of the function (generally throwing an error) was desired.

I am fairly confident that the find_function_addr calls are not necessary.
If you look at call_function_by_hand_dummy, it calls find_function_addr
with the same function argument before calling push_dummy_call:

  CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);

So if the function has to throw, it will be in this invocation.  The fact
that no widely tested architecture does that also leads me to think it's
unnecessary.

I don't think the get_adi_info_proc call is useful either.  All it does
is return the sparc64_adi_info belonging to inferior_ptid.  If it already
exists, the call does nothing.  If it does not already exist, it will be
created.  The call adi_available just before will have created it, if it
didn't already.

Simon

Re: [RFA 05/13] Make a few calls in *-tdep.c for effect

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> I am fairly confident that the find_function_addr calls are not necessary.
[...]

I'll just merge this into the first patch.

Tom

Re: [RFA 05/13] Make a few calls in *-tdep.c for effect

[Pedro Alves]

On 07/12/2018 10:58 PM, Simon Marchi wrote:
> On 2018-07-12 04:52 PM, Tom Tromey wrote:
>> In a few cases, there were calls in *-tdep.c that I did not want to
>> remove because I was not certain that it was safe -- perhaps the side
>> effect of the function (generally throwing an error) was desired.
> 
> I am fairly confident that the find_function_addr calls are not necessary.
> If you look at call_function_by_hand_dummy, it calls find_function_addr
> with the same function argument before calling push_dummy_call:
> 
>   CORE_ADDR funaddr = find_function_addr (function, &values_type, &ftype);
> 
> So if the function has to throw, it will be in this invocation.  The fact
> that no widely tested architecture does that also leads me to think it's
> unnecessary.
> 
> I don't think the get_adi_info_proc call is useful either.  All it does
> is return the sparc64_adi_info belonging to inferior_ptid.  If it already
> exists, the call does nothing.  If it does not already exist, it will be
> created.  The call adi_available just before will have created it, if it
> didn't already.

Agreed.

Thanks,
Pedro Alves

[RFA 10/13] Remove unused declaration from value.c

[Tom Tromey]

This removes an unused declaration from value_fetch_lazy_bitfield, but
leaves the call to check_typedef, because it may be called for effect.

gdb/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* value.c (value_fetch_lazy_bitfield): Call check_typedef for
	effect.
---
 gdb/ChangeLog | 5 +++++
 gdb/value.c   | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/value.c b/gdb/value.c
index 9f9e78ece2b..1747a62980f 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3726,7 +3726,7 @@ value_fetch_lazy_bitfield (struct value *val)
      per bitfield.  It would be even better to read only the containing
      word, but we have no way to record that just specific bits of a
      value have been fetched.  */
-  struct type *type = check_typedef (value_type (val));
+  check_typedef (value_type (val));
   struct value *parent = value_parent (val);
 
   if (value_lazy (parent))
-- 
2.13.6

Re: [RFA 10/13] Remove unused declaration from value.c

[Simon Marchi]

On 2018-07-12 04:52 PM, Tom Tromey wrote:
> This removes an unused declaration from value_fetch_lazy_bitfield, but
> leaves the call to check_typedef, because it may be called for effect.

Do you know for sure this is necessary (e.g. without this, some test fails),
or you are just being cautious?

Simon

Re: [RFA 10/13] Remove unused declaration from value.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 2018-07-12 04:52 PM, Tom Tromey wrote:
>> This removes an unused declaration from value_fetch_lazy_bitfield, but
>> leaves the call to check_typedef, because it may be called for effect.

Simon> Do you know for sure this is necessary (e.g. without this, some test fails),
Simon> or you are just being cautious?

Just being cautious.  It's entirely possible that this isn't needed.
Maybe removing it is correct in that nothing in the function appears to
need it, and if it causes a bug then that means that some other spot
ought to have called check_typedef.

Tom

Re: [RFA 10/13] Remove unused declaration from value.c

[Simon Marchi]

On 2018-07-13 16:51, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> On 2018-07-12 04:52 PM, Tom Tromey wrote:
>>> This removes an unused declaration from value_fetch_lazy_bitfield, 
>>> but
>>> leaves the call to check_typedef, because it may be called for 
>>> effect.
> 
> Simon> Do you know for sure this is necessary (e.g. without this, some
> test fails),
> Simon> or you are just being cautious?
> 
> Just being cautious.  It's entirely possible that this isn't needed.
> Maybe removing it is correct in that nothing in the function appears to
> need it, and if it causes a bug then that means that some other spot
> ought to have called check_typedef.
> 
> Tom

Then my opinion would be to check if removing it causes any test 
failure.  If not, I'd remove it (in its own commit such as this patch is 
good, so it's easy to bisect if needed).

Simon

Re: [RFA 10/13] Remove unused declaration from value.c

[Pedro Alves]

On 07/13/2018 10:49 PM, Simon Marchi wrote:

> Then my opinion would be to check if removing it causes any test failure.  If not, I'd remove it (in its own commit such as this patch is good, so it's easy to bisect if needed).

Agreed.  If/when we find out it was needed, we can add
a testcase then.

Thanks,
Pedro Alves

Re: [RFA 10/13] Remove unused declaration from value.c

[Tom Tromey]

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

Simon> Then my opinion would be to check if removing it causes any test
Simon> failure.  If not, I'd remove it (in its own commit such as this
Simon> patch is good, so it's easy to bisect if needed).

Pedro> Agreed.  If/when we find out it was needed, we can add
Pedro> a testcase then.

I've made this change.

Tom

[RFA 08/13] Fix ravenscar-thread.c to use arch_ops

[Tom Tromey]

The change to turn target ops into methods accidentally introduced a
bug in ravenscar-thread.c, changing some calls that were using
"arch_ops" to use the target beneath.

This patch changes ravenscar-thread.c to use these variables where
appropriate.

gdb/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* ravenscar-thread.c (ravenscar_thread_target::store_registers):
	Use arch_ops.
	(ravenscar_thread_target::prepare_to_store): Likewise.
---
 gdb/ChangeLog          | 6 ++++++
 gdb/ravenscar-thread.c | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 8bd31a5a725..e60fad87466 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -442,7 +442,7 @@ ravenscar_thread_target::store_registers (struct regcache *regcache,
       struct ravenscar_arch_ops *arch_ops
 	= gdbarch_ravenscar_ops (gdbarch);
 
-      beneath ()->store_registers (regcache, regnum);
+      arch_ops->to_store_registers (regcache, regnum);
     }
   else
     beneath ()->store_registers (regcache, regnum);
@@ -461,7 +461,7 @@ ravenscar_thread_target::prepare_to_store (struct regcache *regcache)
       struct ravenscar_arch_ops *arch_ops
 	= gdbarch_ravenscar_ops (gdbarch);
 
-      beneath ()->prepare_to_store (regcache);
+      arch_ops->to_prepare_to_store (regcache);
     }
   else
     beneath ()->prepare_to_store (regcache);
-- 
2.13.6

Re: [RFA 08/13] Fix ravenscar-thread.c to use arch_ops

[Simon Marchi]

On 2018-07-12 04:52 PM, Tom Tromey wrote:
> The change to turn target ops into methods accidentally introduced a
> bug in ravenscar-thread.c, changing some calls that were using
> "arch_ops" to use the target beneath.
> 
> This patch changes ravenscar-thread.c to use these variables where
> appropriate.
> 
> gdb/ChangeLog
> 2018-07-12  Tom Tromey  <tom@tromey.com>
> 
> 	* ravenscar-thread.c (ravenscar_thread_target::store_registers):
> 	Use arch_ops.
> 	(ravenscar_thread_target::prepare_to_store): Likewise.
> ---
>  gdb/ChangeLog          | 6 ++++++
>  gdb/ravenscar-thread.c | 4 ++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
> index 8bd31a5a725..e60fad87466 100644
> --- a/gdb/ravenscar-thread.c
> +++ b/gdb/ravenscar-thread.c
> @@ -442,7 +442,7 @@ ravenscar_thread_target::store_registers (struct regcache *regcache,
>        struct ravenscar_arch_ops *arch_ops
>  	= gdbarch_ravenscar_ops (gdbarch);
>  
> -      beneath ()->store_registers (regcache, regnum);
> +      arch_ops->to_store_registers (regcache, regnum);
>      }
>    else
>      beneath ()->store_registers (regcache, regnum);
> @@ -461,7 +461,7 @@ ravenscar_thread_target::prepare_to_store (struct regcache *regcache)
>        struct ravenscar_arch_ops *arch_ops
>  	= gdbarch_ravenscar_ops (gdbarch);
>  
> -      beneath ()->prepare_to_store (regcache);
> +      arch_ops->to_prepare_to_store (regcache);
>      }
>    else
>      beneath ()->prepare_to_store (regcache);
> 

It looks obvious to me, but let's ping Pedro just to be sure.

Simon

Re: [RFA 08/13] Fix ravenscar-thread.c to use arch_ops

[Pedro Alves]

On 07/14/2018 02:25 AM, Simon Marchi wrote:

> It looks obvious to me, but let's ping Pedro just to be sure.

Whoops.  Yes, this is correct.

Please also merge it to the 8.2 branch.

Thanks,
Pedro Alves

Re: [RFA 08/13] Fix ravenscar-thread.c to use arch_ops

[Tom Tromey]

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

Pedro> On 07/14/2018 02:25 AM, Simon Marchi wrote:
>> It looks obvious to me, but let's ping Pedro just to be sure.

Pedro> Whoops.  Yes, this is correct.

Pedro> Please also merge it to the 8.2 branch.

Thanks.
I also think the observer notification patch should go on the 8.2
branch.

Tom

Re: [RFA 08/13] Fix ravenscar-thread.c to use arch_ops

[Pedro Alves]

On 07/16/2018 04:36 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> On 07/14/2018 02:25 AM, Simon Marchi wrote:
>>> It looks obvious to me, but let's ping Pedro just to be sure.
> 
> Pedro> Whoops.  Yes, this is correct.
> 
> Pedro> Please also merge it to the 8.2 branch.
> 
> Thanks.
> I also think the observer notification patch should go on the 8.2
> branch.

I think so too.

Thanks,
Pedro Alves

[RFA 07/13] Remove unused declaration from py-prettyprint.c

[Tom Tromey]

This removes an unused declaration from py-prettyprint.c, but leaves
the call to value_contents_for_printing, as it may throw.

2018-07-12  Tom Tromey  <tom@tromey.com>

	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer): Call
	value_contents_for_printing for effect.
---
 gdb/ChangeLog               | 5 +++++
 gdb/python/py-prettyprint.c | 4 +++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index 21b1ce94879..bfcbc95178e 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -662,7 +662,9 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   struct gdbarch *gdbarch = get_type_arch (type);
   struct value *value;
   enum string_repr_result print_result;
-  const gdb_byte *valaddr = value_contents_for_printing (val);
+
+  /* Call for side effects.  */
+  value_contents_for_printing (val);
 
   /* No pretty-printer support for unavailable values.  */
   if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
-- 
2.13.6

Re: [RFA 07/13] Remove unused declaration from py-prettyprint.c

[Simon Marchi]

On 2018-07-12 04:52 PM, Tom Tromey wrote:
> This removes an unused declaration from py-prettyprint.c, but leaves
> the call to value_contents_for_printing, as it may throw.>
> 2018-07-12  Tom Tromey  <tom@tromey.com>
> 
> 	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer): Call
> 	value_contents_for_printing for effect.
> ---
>  gdb/ChangeLog               | 5 +++++
>  gdb/python/py-prettyprint.c | 4 +++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
> index 21b1ce94879..bfcbc95178e 100644
> --- a/gdb/python/py-prettyprint.c
> +++ b/gdb/python/py-prettyprint.c
> @@ -662,7 +662,9 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
>    struct gdbarch *gdbarch = get_type_arch (type);
>    struct value *value;
>    enum string_repr_result print_result;
> -  const gdb_byte *valaddr = value_contents_for_printing (val);
> +
> +  /* Call for side effects.  */
> +  value_contents_for_printing (val);
>  
>    /* No pretty-printer support for unavailable values.  */
>    if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
> 


Also LGTM.  However, since both extension languages call value_contents_for_printing
(from what I understand, to ensure the value has been fetched/is not lazy) and bail
if some bytes are unavailable, perhaps apply_ext_lang_val_pretty_printer should
do it instead.

Also, doing

  if (value->lazy)
    value_fetch_lazy (value);

would communicate better the intent than calling value_contents_for_printing for the
same side-effect.

Simon

Re: [RFA 07/13] Remove unused declaration from py-prettyprint.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> Also LGTM.  However, since both extension languages call value_contents_for_printing
Simon> (from what I understand, to ensure the value has been fetched/is not lazy) and bail
Simon> if some bytes are unavailable, perhaps apply_ext_lang_val_pretty_printer should
Simon> do it instead.

It seems possible that someday we'd want to expose unavailable bytes via
the Python Value API.

Simon> Also, doing

Simon>   if (value->lazy)
Simon>     value_fetch_lazy (value);

Simon> would communicate better the intent than calling value_contents_for_printing for the
Simon> same side-effect.

True, but then this is exposed to other possible changes in the value API.

Tom

Re: [RFA 07/13] Remove unused declaration from py-prettyprint.c

[Pedro Alves]

On 07/14/2018 01:39 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> Also LGTM.  However, since both extension languages call value_contents_for_printing
> Simon> (from what I understand, to ensure the value has been fetched/is not lazy) and bail
> Simon> if some bytes are unavailable, perhaps apply_ext_lang_val_pretty_printer should
> Simon> do it instead.
> 
> It seems possible that someday we'd want to expose unavailable bytes via
> the Python Value API.

Right, but that's not an issue if we morph the call to a direct
value_fetch_lazy call instead, AFAICS.

> 
> Simon> Also, doing
> 
> Simon>   if (value->lazy)
> Simon>     value_fetch_lazy (value);
> 
> Simon> would communicate better the intent than calling value_contents_for_printing for the
> Simon> same side-effect.

Right, what I was saying too in the guile patch.

> 
> True, but then this is exposed to other possible changes in the value API.

I don't see how using value_contents_for_printing instead helps with that?
 
Thanks,
Pedro Alves

[RFA 12/13] Add ATTRIBUTE_UNUSED to regdat.sh output

[Tom Tromey]

This changes regdat.sh to emit ATTRIBUTE_UNUSED for the
xmltarget_${name} variable.  This seemed simpler than trying to figure
out under exactly which circumstances each one was in fact used.

2018-07-12  Tom Tromey  <tom@tromey.com>

	* regformats/regdat.sh: Mark xmltarget_${name} with
	ATTRIBUTE_UNUSED.
---
 gdb/ChangeLog            | 5 +++++
 gdb/regformats/regdat.sh | 8 ++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
index 5a8564ac50c..7868e757582 100755
--- a/gdb/regformats/regdat.sh
+++ b/gdb/regformats/regdat.sh
@@ -164,12 +164,12 @@ done
 echo
 echo "static const char *expedite_regs_${name}[] = { \"`echo ${expedite} | sed 's/,/", "/g'`\", 0 };"
 if test "${feature}" != x; then
-  echo "static const char *xmltarget_${name} = 0;"
+  echo "static const char *xmltarget_${name} ATTRIBUTE_UNUSED = 0;"
 elif test "${xmltarget}" = x; then
   if test "${xmlarch}" = x && test "${xmlosabi}" = x; then
-    echo "static const char *xmltarget_${name} = 0;"
+    echo "static const char *xmltarget_${name} ATTRIBUTE_UNUSED = 0;"
   else
-    echo "static const char *xmltarget_${name} = \"@<target>\\"
+    echo "static const char *xmltarget_${name} ATTRIBUTE_UNUSED = \"@<target>\\"
     if test "${xmlarch}" != x; then
       echo "<architecture>${xmlarch}</architecture>\\"
     fi
@@ -179,7 +179,7 @@ elif test "${xmltarget}" = x; then
     echo "</target>\";"
   fi
 else
-  echo "static const char *xmltarget_${name} = \"${xmltarget}\";"
+  echo "static const char *xmltarget_${name} ATTRIBUTE_UNUSED = \"${xmltarget}\";"
 fi
 echo
 
-- 
2.13.6

Re: [RFA 12/13] Add ATTRIBUTE_UNUSED to regdat.sh output

[Pedro Alves]

On 07/12/2018 09:52 PM, Tom Tromey wrote:
> This changes regdat.sh to emit ATTRIBUTE_UNUSED for the
> xmltarget_${name} variable.  This seemed simpler than trying to figure
> out under exactly which circumstances each one was in fact used.

Can you give an example configuration where you got a warning?

Thanks,
Pedro Alves

> 
> 2018-07-12  Tom Tromey  <tom@tromey.com>
> 
> 	* regformats/regdat.sh: Mark xmltarget_${name} with
> 	ATTRIBUTE_UNUSED.
> ---
>  gdb/ChangeLog            | 5 +++++
>  gdb/regformats/regdat.sh | 8 ++++----
>  2 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/regformats/regdat.sh b/gdb/regformats/regdat.sh
> index 5a8564ac50c..7868e757582 100755
> --- a/gdb/regformats/regdat.sh
> +++ b/gdb/regformats/regdat.sh
> @@ -164,12 +164,12 @@ done
>  echo
>  echo "static const char *expedite_regs_${name}[] = { \"`echo ${expedite} | sed 's/,/", "/g'`\", 0 };"
>  if test "${feature}" != x; then
> -  echo "static const char *xmltarget_${name} = 0;"
> +  echo "static const char *xmltarget_${name} ATTRIBUTE_UNUSED = 0;"
>  elif test "${xmltarget}" = x; then
>    if test "${xmlarch}" = x && test "${xmlosabi}" = x; then
> -    echo "static const char *xmltarget_${name} = 0;"
> +    echo "static const char *xmltarget_${name} ATTRIBUTE_UNUSED = 0;"
>    else
> -    echo "static const char *xmltarget_${name} = \"@<target>\\"
> +    echo "static const char *xmltarget_${name} ATTRIBUTE_UNUSED = \"@<target>\\"
>      if test "${xmlarch}" != x; then
>        echo "<architecture>${xmlarch}</architecture>\\"
>      fi
> @@ -179,7 +179,7 @@ elif test "${xmltarget}" = x; then
>      echo "</target>\";"
>    fi
>  else
> -  echo "static const char *xmltarget_${name} = \"${xmltarget}\";"
> +  echo "static const char *xmltarget_${name} ATTRIBUTE_UNUSED = \"${xmltarget}\";"
>  fi
>  echo
>  
> 

Re: [RFA 12/13] Add ATTRIBUTE_UNUSED to regdat.sh output

[Tom Tromey]

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

Pedro> On 07/12/2018 09:52 PM, Tom Tromey wrote:
>> This changes regdat.sh to emit ATTRIBUTE_UNUSED for the
>> xmltarget_${name} variable.  This seemed simpler than trying to figure
>> out under exactly which circumstances each one was in fact used.

Pedro> Can you give an example configuration where you got a warning?

I only found this one via the buildbot:

https://gdb-build.sergiodj.net/builders/Fedora-s390x-m64/builds/9085


It fails like so:

s390-linux32-generated.c: In function ‘void init_registers_s390_linux32()’:
s390-linux32-generated.c:141:20: error: unused variable ‘xmltarget_s390_linux32’ [-Werror=unused-variable]
 static const char *xmltarget_s390_linux32 = "s390-linux32.xml";
                    ^~~~~~~~~~~~~~~~~~~~~~
s390-linux32v1-generated.c: In function ‘void init_registers_s390_linux32v1()’:
s390-linux32v1-generated.c:143:20: error: unused variable ‘xmltarget_s390_linux32v1’ [-Werror=unused-variable]
 static const char *xmltarget_s390_linux32v1 = "s390-linux32v1.xml";
                    ^~~~~~~~~~~~~~~~~~~~~~~~
s390-linux32-generated.c: At global scope:
s390-linux32-generated.c:141:20: error: ‘xmltarget_s390_linux32’ defined but not used [-Werror=unused-variable]
s390-linux32v1-generated.c: At global scope:
s390-linux32v1-generated.c:143:20: error: ‘xmltarget_s390_linux32v1’ defined but not used [-Werror=unused-variable]

Tom

Re: [RFA 12/13] Add ATTRIBUTE_UNUSED to regdat.sh output

[Pedro Alves]

On 07/16/2018 05:40 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> On 07/12/2018 09:52 PM, Tom Tromey wrote:
>>> This changes regdat.sh to emit ATTRIBUTE_UNUSED for the
>>> xmltarget_${name} variable.  This seemed simpler than trying to figure
>>> out under exactly which circumstances each one was in fact used.
> 
> Pedro> Can you give an example configuration where you got a warning?
> 
> I only found this one via the buildbot:
> 
> https://gdb-build.sergiodj.net/builders/Fedora-s390x-m64/builds/9085
> 
> 
> It fails like so:
> 
> s390-linux32-generated.c: In function ‘void init_registers_s390_linux32()’:
> s390-linux32-generated.c:141:20: error: unused variable ‘xmltarget_s390_linux32’ [-Werror=unused-variable]
>  static const char *xmltarget_s390_linux32 = "s390-linux32.xml";
>                     ^~~~~~~~~~~~~~~~~~~~~~
> s390-linux32v1-generated.c: In function ‘void init_registers_s390_linux32v1()’:
> s390-linux32v1-generated.c:143:20: error: unused variable ‘xmltarget_s390_linux32v1’ [-Werror=unused-variable]
>  static const char *xmltarget_s390_linux32v1 = "s390-linux32v1.xml";
>                     ^~~~~~~~~~~~~~~~~~~~~~~~
> s390-linux32-generated.c: At global scope:
> s390-linux32-generated.c:141:20: error: ‘xmltarget_s390_linux32’ defined but not used [-Werror=unused-variable]
> s390-linux32v1-generated.c: At global scope:
> s390-linux32v1-generated.c:143:20: error: ‘xmltarget_s390_linux32v1’ defined but not used [-Werror=unused-variable]

This is building an ipa object:

 s390-linux32-generated.c: In function ‘void init_registers_s390_linux32()’:
 s390-linux32-generated.c:141:20: error: unused variable ‘xmltarget_s390_linux32’ [-Werror=unused-variable]
  static const char *xmltarget_s390_linux32 = "s390-linux32.xml";
                     ^~~~~~~~~~~~~~~~~~~~~~
 s390-linux32v1-generated.c: In function ‘void init_registers_s390_linux32v1()’:
 s390-linux32v1-generated.c:143:20: error: unused variable ‘xmltarget_s390_linux32v1’ [-Werror=unused-variable]
  static const char *xmltarget_s390_linux32v1 = "s390-linux32v1.xml";
                     ^~~~~~~~~~~~~~~~~~~~~~~~
 s390-linux32-generated.c: At global scope:
 s390-linux32-generated.c:141:20: error: ‘xmltarget_s390_linux32’ defined but not used [-Werror=unused-variable]
 s390-linux32v1-generated.c: At global scope:
 s390-linux32v1-generated.c:143:20: error: ‘xmltarget_s390_linux32v1’ defined but not used [-Werror=unused-variable]
 cc1plus: all warnings being treated as errors
 make[4]: *** [Makefile:576: s390-linux32-ipa.o] Error 1

The generated files look something like this (here for x86-64):

 static const char *expedite_regs_amd64_linux[] = { "rbp", "rsp", "rip", 0 };
 static const char *xmltarget_amd64_linux = 0;
 
 #ifndef IN_PROCESS_AGENT
   result->xmltarget = xmltarget_amd64_linux;
 #endif
 
So the xmltarget_foo variable is defined unconditionally, but
used only when #ifndef IN_PROCESS_AGENT.  So if we wrap the variable
definition in !IN_PROCESS_AGENT too, the warning should go away.

I guess that getting warning or not is most likely dependent on
GCC version.

Thanks,
Pedro Alves

Re: [RFA 12/13] Add ATTRIBUTE_UNUSED to regdat.sh output

[Tom Tromey]

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

Pedro> So the xmltarget_foo variable is defined unconditionally, but
Pedro> used only when #ifndef IN_PROCESS_AGENT.  So if we wrap the variable
Pedro> definition in !IN_PROCESS_AGENT too, the warning should go away.

Thanks for looking into this.
I'm testing a series with this update.

Tom

Re: [RFA 12/13] Add ATTRIBUTE_UNUSED to regdat.sh output

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
Pedro> So the xmltarget_foo variable is defined unconditionally, but
Pedro> used only when #ifndef IN_PROCESS_AGENT.  So if we wrap the variable
Pedro> definition in !IN_PROCESS_AGENT too, the warning should go away.

Tom> Thanks for looking into this.
Tom> I'm testing a series with this update.

To my surprise this turned out amazingly badly.
I don't know why yet.

Tom

[RFA 09/13] Pass the correct argument to the observer in reread_symbols

[Tom Tromey]

This is actually a patch I found via another route.  Joel had asked me
to write a test, but I still have not found the time to do this.
Meanwhile, -Wunused-variable also found this error.

2018-07-12  Tom Tromey  <tom@tromey.com>

	* symfile.c (reread_symbols): Notify iter, not objfile.
---
 gdb/ChangeLog | 4 ++++
 gdb/symfile.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 48eca5cc0ea..29cd48572d0 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -2608,7 +2608,7 @@ reread_symbols (void)
 	 gdb::observers::new_objfile.notify (NULL) has been called by
 	 clear_symtab_users above.  Notify the new files now.  */
       for (auto iter : new_objfiles)
-	gdb::observers::new_objfile.notify (objfile);
+	gdb::observers::new_objfile.notify (iter);
 
       /* At least one objfile has changed, so we can consider that
          the executable we're debugging has changed too.  */
-- 
2.13.6

Re: [RFA 09/13] Pass the correct argument to the observer in reread_symbols

[Simon Marchi]

On 2018-07-12 04:52 PM, Tom Tromey wrote:
> This is actually a patch I found via another route.  Joel had asked me
> to write a test, but I still have not found the time to do this.
> Meanwhile, -Wunused-variable also found this error.

Heh, this is also a case of variables that should be declared in the
narrowest scope possible.

Simon

Re: [RFA 09/13] Pass the correct argument to the observer in reread_symbols

[Simon Marchi]

On 2018-07-12 10:49 PM, Simon Marchi wrote:
> On 2018-07-12 04:52 PM, Tom Tromey wrote:
>> This is actually a patch I found via another route.  Joel had asked me
>> to write a test, but I still have not found the time to do this.
>> Meanwhile, -Wunused-variable also found this error.
> 
> Heh, this is also a case of variables that should be declared in the
> narrowest scope possible.
> 
> Simon

It wasn't clear, but this LGTM.

Simon

[RFA 02/13] Unused variable fixes related to conditional compilation

[Tom Tromey]

This patch fixes various unused variable warnings that are related to
conditional compilation.  In these cases, either the variable is now
protected by the same #if as its uses, or the declaration is simply
lowered into the conditionally-compiled block.

gdb/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* windows-nat.c (saved_context): Conditionally define.
	* remote.c (remote_target::remote_btrace_maybe_reopen):
	Conditionally declare "warned".
	* inflow.c (sigquit_ours): Conditionally define.
	(new_tty): Move "tty" declaration inside #if.
	* guile/guile.c (guile_datadir): Conditionally define.
	* charset.c (set_be_le_names): Move some declarations inside #if.
	* btrace.c (parse_xml_btrace): Move "errcode" declaration inside
	#if.
	(parse_xml_btrace_conf): Likewise.
---
 gdb/ChangeLog     | 13 +++++++++++++
 gdb/btrace.c      |  6 ++----
 gdb/charset.c     |  6 +++---
 gdb/guile/guile.c |  2 ++
 gdb/inflow.c      |  6 ++++--
 gdb/remote.c      |  2 ++
 gdb/windows-nat.c |  2 ++
 7 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 35dc90e8e67..e25f047ce24 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -2202,10 +2202,9 @@ static const struct gdb_xml_element btrace_elements[] = {
 void
 parse_xml_btrace (struct btrace_data *btrace, const char *buffer)
 {
-  int errcode;
-
 #if defined (HAVE_LIBEXPAT)
 
+  int errcode;
   btrace_data result;
   result.format = BTRACE_FORMAT_NONE;
 
@@ -2303,10 +2302,9 @@ static const struct gdb_xml_element btrace_conf_elements[] = {
 void
 parse_xml_btrace_conf (struct btrace_config *conf, const char *xml)
 {
-  int errcode;
-
 #if defined (HAVE_LIBEXPAT)
 
+  int errcode;
   errcode = gdb_xml_parse_quick (_("btrace-conf"), "btrace-conf.dtd",
 				 btrace_conf_elements, xml, conf);
   if (errcode != 0)
diff --git a/gdb/charset.c b/gdb/charset.c
index fcb24a48823..8bb2b4d669f 100644
--- a/gdb/charset.c
+++ b/gdb/charset.c
@@ -295,9 +295,6 @@ static struct gdbarch *be_le_arch;
 static void
 set_be_le_names (struct gdbarch *gdbarch)
 {
-  int i, len;
-  const char *target_wide;
-
   if (be_le_arch == gdbarch)
     return;
   be_le_arch = gdbarch;
@@ -307,6 +304,9 @@ set_be_le_names (struct gdbarch *gdbarch)
   target_wide_charset_le_name = "UTF-32LE";
   target_wide_charset_be_name = "UTF-32BE";
 #else
+  int i, len;
+  const char *target_wide;
+
   target_wide_charset_le_name = NULL;
   target_wide_charset_be_name = NULL;
 
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 0bbbf6eac1f..efb59e3d2c3 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -46,8 +46,10 @@ int gdbscm_guile_major_version;
 int gdbscm_guile_minor_version;
 int gdbscm_guile_micro_version;
 
+#ifdef HAVE_GUILE
 /* The guile subdirectory within gdb's data-directory.  */
 static const char *guile_datadir;
+#endif
 
 /* Declared constants and enum for guile exception printing.  */
 const char gdbscm_print_excp_none[] = "none";
diff --git a/gdb/inflow.c b/gdb/inflow.c
index a3b5ba8abef..caff6462070 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -138,7 +138,9 @@ private:
    to SIG_IGN.  */
 
 static sighandler_t sigint_ours;
+#ifdef SIGQUIT
 static sighandler_t sigquit_ours;
+#endif
 
 /* The name of the tty (from the `tty' command) that we're giving to
    the inferior when starting it up.  This is only (and should only
@@ -825,11 +827,11 @@ check_syscall (const char *msg, int result)
 void
 new_tty (void)
 {
-  int tty;
-
   if (inferior_thisrun_terminal == 0)
     return;
 #if !defined(__GO32__) && !defined(_WIN32)
+  int tty;
+
 #ifdef TIOCNOTTY
   /* Disconnect the child process from our controlling terminal.  On some
      systems (SVR4 for example), this may cause a SIGTTOU, so temporarily
diff --git a/gdb/remote.c b/gdb/remote.c
index a81d67e5ede..a61680d242c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -13717,7 +13717,9 @@ remote_target::remote_btrace_maybe_reopen ()
   struct remote_state *rs = get_remote_state ();
   struct thread_info *tp;
   int btrace_target_pushed = 0;
+#if !defined (HAVE_LIBIPT)
   int warned = 0;
+#endif
 
   scoped_restore_current_thread restore_thread;
 
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 8f4646d1cca..f9870b8bd1e 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -146,8 +146,10 @@ static GetConsoleFontSize_ftype *GetConsoleFontSize;
 
 static int have_saved_context;	/* True if we've saved context from a
 				   cygwin signal.  */
+#ifdef __CYGWIN__
 static CONTEXT saved_context;	/* Containes the saved context from a
 				   cygwin signal.  */
+#endif
 
 /* If we're not using the old Cygwin header file set, define the
    following which never should have been in the generic Win32 API
-- 
2.13.6

Re: [RFA 02/13] Unused variable fixes related to conditional compilation

[Simon Marchi]

On 2018-07-12 04:51 PM, Tom Tromey wrote:
> This patch fixes various unused variable warnings that are related to
> conditional compilation.  In these cases, either the variable is now
> protected by the same #if as its uses, or the declaration is simply
> lowered into the conditionally-compiled block.

This LGTM.

Simon

Re: [RFA 02/13] Unused variable fixes related to conditional compilation

[Simon Marchi]

On 2018-07-13 09:16 PM, Simon Marchi wrote:
> On 2018-07-12 04:51 PM, Tom Tromey wrote:
>> This patch fixes various unused variable warnings that are related to
>> conditional compilation.  In these cases, either the variable is now
>> protected by the same #if as its uses, or the declaration is simply
>> lowered into the conditionally-compiled block.
> 
> This LGTM.
> 
> Simon
> 

I found a few more in a cross-compilation build that has !HAVE_ELF, for
some reason.

From e590cfda6ccb63a5270e36c636fb7daa567aa781 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Fri, 13 Jul 2018 21:44:52 -0400
Subject: [PATCH] conditional compilation

---
 gdb/arm-tdep.c    | 13 +++++++------
 gdb/rs6000-tdep.c |  5 ++---
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index b1a05bbeba2d..c3280ee211d1 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -8955,7 +8955,6 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 	  else if (ei_osabi == ELFOSABI_NONE || ei_osabi == ELFOSABI_GNU)
 	    {
 	      int eabi_ver = EF_ARM_EABI_VERSION (e_flags);
-	      int attr_arch, attr_profile;

 	      switch (eabi_ver)
 		{
@@ -9026,11 +9025,13 @@ arm_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
 		 executable file includes build attributes; GCC does
 		 copy them to the executable, but e.g. RealView does
 		 not.  */
-	      attr_arch = bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
-						    Tag_CPU_arch);
-	      attr_profile = bfd_elf_get_obj_attr_int (info.abfd,
-						       OBJ_ATTR_PROC,
-						       Tag_CPU_arch_profile);
+	      int attr_arch
+		= bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
+					    Tag_CPU_arch);
+	      int attr_profile
+		= bfd_elf_get_obj_attr_int (info.abfd, OBJ_ATTR_PROC,
+					    Tag_CPU_arch_profile);
+
 	      /* GCC specifies the profile for v6-M; RealView only
 		 specifies the profile for architectures starting with
 		 V7 (as opposed to architectures with a tag
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 45bf98267a99..e78de49b2e69 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -3558,7 +3558,6 @@ bfd_uses_spe_extensions (bfd *abfd)
   bfd_size_type size;
   gdb_byte *ptr;
   int success = 0;
-  int vector_abi;

   if (!abfd)
     return 0;
@@ -3567,8 +3566,8 @@ bfd_uses_spe_extensions (bfd *abfd)
   /* Using Tag_GNU_Power_ABI_Vector here is a bit of a hack, as the user
      could be using the SPE vector abi without actually using any spe
      bits whatsoever.  But it's close enough for now.  */
-  vector_abi = bfd_elf_get_obj_attr_int (abfd, OBJ_ATTR_GNU,
-					 Tag_GNU_Power_ABI_Vector);
+  int vector_abi = bfd_elf_get_obj_attr_int (abfd, OBJ_ATTR_GNU,
+					     Tag_GNU_Power_ABI_Vector);
   if (vector_abi == 3)
     return 1;
 #endif
-- 
2.18.0

Re: [RFA 02/13] Unused variable fixes related to conditional compilation

[Pedro Alves]

On 07/14/2018 10:50 PM, Simon Marchi wrote:

> I found a few more in a cross-compilation build that has !HAVE_ELF, for
> some reason.

LGTM.

Thanks,
Pedro Alves

Re: [RFA 02/13] Unused variable fixes related to conditional compilation

[Simon Marchi]

On 2018-07-16 09:45 AM, Pedro Alves wrote:
> On 07/14/2018 10:50 PM, Simon Marchi wrote:
> 
>> I found a few more in a cross-compilation build that has !HAVE_ELF, for
>> some reason.
> 
> LGTM.
> 
> Thanks,
> Pedro Alves
> 

I pushed this.

Simon

[RFA 01/13] Simple unused variable removals

[Tom Tromey]

This patch holds all the straightforward unused variable deletions.

gdb/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* ada-lang.c (ada_lookup_symbol_list_worker): Remove unused
	variable.
	* amd64-tdep.c (amd64_supply_xsave): Remove unused variable.
	* arm-tdep.c (arm_record_data_proc_misc_ld_str): Remove unused
	variable.
	* breakpoint.c (check_no_tracepoint_commands, update_watchpoint):
	Remove unused variable.
	* cli/cli-script.c (recurse_read_control_structure): Remove unused
	variable.
	* common/tdesc.c (print_xml_feature::visit): Remove unused
	variable.
	* compile/compile-object-load.c (store_regs): Remove unused
	variables.
	* complaints.c (clear_complaints): Remove unused variable.
	* corelow.c (core_target_open): Remove unused variable.
	* fbsd-tdep.c (fbsd_core_info_proc_status): Remove unused
	variable.
	* guile/scm-frame.c (gdbscm_frame_read_var): Remove unused
	variable.
	* guile/scm-symtab.c (stscm_print_sal_smob): Remove unused
	variable.
	* guile/scm-type.c (gdbscm_field_baseclass_p): Remove unused
	variable.
	* guile/scm-utils.c (gdbscm_parse_function_args): Remove unused
	variable.
	* hppa-tdep.c (hppa_stub_frame_unwind_cache): Remove unused
	variable.
	* ia64-tdep.c (examine_prologue): Remove unused variable.
	* infcall.c (run_inferior_call): Remove unused variable.
	* inferior.c (exit_inferior): Remove unused variable.
	* infrun.c (infrun_thread_ptid_changed): Remove unused variable.
	* linespec.c (decode_line_2): Remove unused variable.
	* linux-nat.c (super_close): Remove.
	* linux-tdep.c (linux_info_proc): Remove unused variable.
	* mi/mi-main.c (mi_execute_command): Remove unused variable.
	* microblaze-linux-tdep.c (microblaze_linux_sigtramp_cache):
	Remove unused variable.
	* parse.c (find_minsym_type_and_address): Remove unused variable.
	* printcmd.c (info_symbol_command, printf_floating): Remove unused
	variable.
	* python/py-breakpoint.c (bppy_set_commands): Remove unused
	variable.
	* python/py-unwind.c (unwind_infopy_dealloc): Remove unused
	variables.
	* record-btrace.c (record_btrace_target::store_registers): Remove
	unused variable.
	(cmd_show_record_btrace_cpu): Remove unused variable.
	* riscv-tdep.c (riscv_register_reggroup_p)
	(riscv_push_dummy_call, riscv_return_value): Remove unused
	variable.
	* rust-exp.y (literal): Remove unused variable.
	* rust-lang.c (rust_evaluate_subexp) <OP_RUST_ARARAY>: Remove
	unused variable.
	<STRUCTOP_ANONYMOUS>: Likewise.
	* s390-linux-tdep.c (s390_linux_init_abi_31)
	(s390_linux_init_abi_64): Remove unused variable.
	* ser-ming2.c (ser_windows_read_prim, pipe_select_thread)
	(file_select_thread, net_windows_open, _initialize_ser_windows):
	Remove unused variables.
	* spu-tdep.c (spu_get_overlay_table): Remove unused variable.
	* symtab.c (find_pc_sect_line): Remove unused variable.
	* target-memory.c (compute_garbled_blocks): Remove unused
	variable.
	(target_write_memory_blocks): Remove unused variable.
	* target.c (target_stack::unpush): Remove unused variables.
	* tracepoint.c (start_tracing, all_tracepoint_actions)
	(merge_uploaded_trace_state_variables)
	(print_one_static_tracepoint_marker): Remove unused variable.
	* unittests/basic_string_view/element_access/char/1.cc (test01):
	Remove unused variable.
	* windows-nat.c (windows_continue, windows_add_all_dlls)
	(do_initial_windows_stuff, windows_nat_target::create_inferior):
	Remove unused variables.
---
 gdb/ChangeLog                                      | 76 ++++++++++++++++++++++
 gdb/ada-lang.c                                     |  1 -
 gdb/amd64-tdep.c                                   |  1 -
 gdb/arm-tdep.c                                     |  2 +-
 gdb/breakpoint.c                                   |  4 +-
 gdb/cli/cli-script.c                               |  2 +-
 gdb/common/tdesc.c                                 |  1 -
 gdb/compile/compile-object-load.c                  |  2 -
 gdb/complaints.c                                   |  2 -
 gdb/corelow.c                                      |  1 -
 gdb/fbsd-tdep.c                                    |  1 -
 gdb/guile/scm-frame.c                              |  1 -
 gdb/guile/scm-symtab.c                             |  1 -
 gdb/guile/scm-type.c                               |  1 -
 gdb/guile/scm-utils.c                              |  1 -
 gdb/hppa-tdep.c                                    |  2 -
 gdb/ia64-tdep.c                                    |  7 +-
 gdb/infcall.c                                      |  1 -
 gdb/inferior.c                                     |  1 -
 gdb/infrun.c                                       |  2 -
 gdb/linespec.c                                     |  2 -
 gdb/linux-nat.c                                    |  4 --
 gdb/linux-tdep.c                                   |  1 -
 gdb/mi/mi-main.c                                   |  1 -
 gdb/microblaze-linux-tdep.c                        |  2 -
 gdb/parse.c                                        |  1 -
 gdb/printcmd.c                                     |  2 -
 gdb/python/py-breakpoint.c                         |  1 -
 gdb/python/py-unwind.c                             |  2 -
 gdb/record-btrace.c                                |  4 --
 gdb/riscv-tdep.c                                   |  7 --
 gdb/rust-exp.y                                     |  1 -
 gdb/rust-lang.c                                    |  4 +-
 gdb/s390-linux-tdep.c                              |  2 -
 gdb/ser-mingw.c                                    |  8 +--
 gdb/spu-tdep.c                                     |  2 -
 gdb/symtab.c                                       |  1 -
 gdb/target-memory.c                                |  2 -
 gdb/target.c                                       |  3 -
 gdb/tracepoint.c                                   |  4 --
 .../basic_string_view/element_access/char/1.cc     |  2 +-
 gdb/windows-nat.c                                  |  7 --
 42 files changed, 85 insertions(+), 88 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 89cbec35b03..d9854b74175 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -5786,7 +5786,6 @@ ada_lookup_symbol_list_worker (const lookup_name_info &lookup_name,
 {
   int syms_from_global_search;
   int ndefns;
-  int results_size;
   auto_obstack obstack;
 
   ada_add_all_symbols (&obstack, block, lookup_name,
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 190e086ebfe..088542d72b4 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -3373,7 +3373,6 @@ amd64_supply_xsave (struct regcache *regcache, int regnum,
       && gdbarch_bfd_arch_info (gdbarch)->bits_per_word == 64)
     {
       const gdb_byte *regs = (const gdb_byte *) xsave;
-      static const gdb_byte zero[I386_MAX_REGISTER_SIZE] = { 0 };
       ULONGEST clear_bv;
 
       clear_bv = i387_xsave_get_clear_bv (gdbarch, xsave);
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index fdfb360f5cc..b1a05bbeba2 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -10219,7 +10219,7 @@ arm_record_data_proc_misc_ld_str (insn_decode_record *arm_insn_r)
   uint32_t record_buf[8], record_buf_mem[8];
   ULONGEST u_regval[2] = {0};
 
-  uint32_t reg_src1 = 0, reg_dest = 0;
+  uint32_t reg_src1 = 0;
   uint32_t opcode1 = 0;
 
   arm_insn_r->opcode = bits (arm_insn_r->arm_insn, 21, 24);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b135db3db95..9e04c7ea83a 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1009,8 +1009,6 @@ check_no_tracepoint_commands (struct command_line *commands)
 
   for (c = commands; c; c = c->next)
     {
-      int i;
-
       if (c->control_type == while_stepping_control)
 	error (_("The 'while-stepping' command can "
 		 "only be used for tracepoints"));
@@ -1775,7 +1773,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
     {
       int pc = 0;
       std::vector<value_ref_ptr> val_chain;
-      struct value *v, *result, *next;
+      struct value *v, *result;
       struct program_space *frame_pspace;
 
       fetch_subexp_value (b->exp.get (), &pc, &v, &result, &val_chain, 0);
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 87ebf9fc00c..6f31a400197 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1081,7 +1081,7 @@ recurse_read_control_structure (gdb::function_view<const char * ()> read_next_li
 {
   enum misc_command_type val;
   enum command_control_type ret;
-  struct command_line **body_ptr, *child_tail, *next;
+  struct command_line *child_tail, *next;
   counted_command_line *current_body = &current_cmd->body_list_0;
 
   child_tail = NULL;
diff --git a/gdb/common/tdesc.c b/gdb/common/tdesc.c
index 68584a7f94f..43a066e1e64 100644
--- a/gdb/common/tdesc.c
+++ b/gdb/common/tdesc.c
@@ -314,7 +314,6 @@ void print_xml_feature::visit (const tdesc_type_vector *t)
 
 void print_xml_feature::visit (const tdesc_type_with_fields *t)
 {
-  struct tdesc_type_field *f;
   const static char *types[] = { "struct", "union", "flags", "enum" };
 
   gdb_assert (t->kind >= TDESC_TYPE_STRUCT && t->kind <= TDESC_TYPE_ENUM);
diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 9b3c51fe288..a83f95dda93 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -555,9 +555,7 @@ get_regs_type (struct symbol *func_sym, struct objfile *objfile)
 static void
 store_regs (struct type *regs_type, CORE_ADDR regs_base)
 {
-  thread_info *thread = inferior_thread ();
   struct gdbarch *gdbarch = target_gdbarch ();
-  struct regcache *regcache = get_thread_regcache (thread);
   int fieldno;
 
   for (fieldno = 0; fieldno < TYPE_NFIELDS (regs_type); fieldno++)
diff --git a/gdb/complaints.c b/gdb/complaints.c
index 1bf99d40f01..03102f13ab3 100644
--- a/gdb/complaints.c
+++ b/gdb/complaints.c
@@ -92,8 +92,6 @@ complaint_internal (const char *fmt, ...)
 void
 clear_complaints (int less_verbose)
 {
-  struct complain *p;
-
   counters.clear ();
 
   if (!less_verbose)
diff --git a/gdb/corelow.c b/gdb/corelow.c
index ecf966525ed..059ce2f6ebd 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -358,7 +358,6 @@ core_target_open (const char *arg, int from_tty)
 {
   const char *p;
   int siggy;
-  struct cleanup *old_chain;
   int scratch_chan;
   int flags;
 
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 9cea0098d6b..ac5b3bfa120 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -787,7 +787,6 @@ fbsd_core_info_proc_status (struct gdbarch *gdbarch)
 {
   const struct kinfo_proc_layout *kp;
   asection *section;
-  const char *state;
   unsigned char *descdata;
   int addr_bit, long_bit;
   size_t note_size;
diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
index 7b539677ffd..765bf1d07b8 100644
--- a/gdb/guile/scm-frame.c
+++ b/gdb/guile/scm-frame.c
@@ -878,7 +878,6 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
   else if (scm_is_string (symbol_scm))
     {
       const struct block *block = NULL;
-      struct cleanup *cleanup;
       struct gdb_exception except = exception_none;
 
       if (! SCM_UNBNDP (block_scm))
diff --git a/gdb/guile/scm-symtab.c b/gdb/guile/scm-symtab.c
index 7d94b817d0a..14d1f81836b 100644
--- a/gdb/guile/scm-symtab.c
+++ b/gdb/guile/scm-symtab.c
@@ -395,7 +395,6 @@ static int
 stscm_print_sal_smob (SCM self, SCM port, scm_print_state *pstate)
 {
   sal_smob *s_smob = (sal_smob *) SCM_SMOB_DATA (self);
-  symtab_smob *st_smob = (symtab_smob *) SCM_SMOB_DATA (s_smob->symtab_scm);
 
   gdbscm_printf (port, "#<%s ", symtab_smob_name);
   scm_write (s_smob->symtab_scm, port);
diff --git a/gdb/guile/scm-type.c b/gdb/guile/scm-type.c
index cc997563dab..12fc6c09dc3 100644
--- a/gdb/guile/scm-type.c
+++ b/gdb/guile/scm-type.c
@@ -1224,7 +1224,6 @@ gdbscm_field_baseclass_p (SCM self)
 {
   field_smob *f_smob
     = tyscm_get_field_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
-  struct field *field = tyscm_field_smob_to_field (f_smob);
   struct type *type = tyscm_field_smob_containing_type (f_smob);
 
   if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
diff --git a/gdb/guile/scm-utils.c b/gdb/guile/scm-utils.c
index 73b0dec73ea..c70bf57cfc7 100644
--- a/gdb/guile/scm-utils.c
+++ b/gdb/guile/scm-utils.c
@@ -387,7 +387,6 @@ gdbscm_parse_function_args (const char *func_name,
   SCM rest = SCM_EOL;
   /* Keep track of malloc'd strings.  We need to free them upon error.  */
   std::vector<char *> allocated_strings;
-  char *ptr;
 
   have_rest = validate_arg_format (format);
   num_keywords = count_keywords (keywords);
diff --git a/gdb/hppa-tdep.c b/gdb/hppa-tdep.c
index 1ea36704279..319096e056f 100644
--- a/gdb/hppa-tdep.c
+++ b/gdb/hppa-tdep.c
@@ -2435,9 +2435,7 @@ static struct hppa_stub_unwind_cache *
 hppa_stub_frame_unwind_cache (struct frame_info *this_frame,
 			      void **this_cache)
 {
-  struct gdbarch *gdbarch = get_frame_arch (this_frame);
   struct hppa_stub_unwind_cache *info;
-  struct unwind_table_entry *u;
 
   if (*this_cache)
     return (struct hppa_stub_unwind_cache *) *this_cache;
diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
index 0df62e26ab3..a512492dcf6 100644
--- a/gdb/ia64-tdep.c
+++ b/gdb/ia64-tdep.c
@@ -1525,11 +1525,8 @@ examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc,
 	         where the pc is.  If it's still early in the prologue
 		 this'll be wrong.  FIXME */
 	      if (this_frame)
-		{
-		  struct gdbarch *gdbarch = get_frame_arch (this_frame);
-		  saved_sp = get_frame_register_unsigned (this_frame,
-							  sp_regnum);
-		}
+		saved_sp = get_frame_register_unsigned (this_frame,
+							sp_regnum);
 	      spill_addr  = saved_sp
 	                  + (rM == 12 ? 0 : mem_stack_frame_size) 
 			  + imm;
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 172c26ac820..c995ab7c918 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -587,7 +587,6 @@ run_inferior_call (struct call_thread_fsm *sm,
   struct gdb_exception caught_error = exception_none;
   int saved_in_infcall = call_thread->control.in_infcall;
   ptid_t call_thread_ptid = call_thread->ptid;
-  inferior *call_thread_inf = call_thread->inf;
   enum prompt_state saved_prompt_state = current_ui->prompt_state;
   int was_running = call_thread->state == THREAD_RUNNING;
   int saved_ui_async = current_ui->async;
diff --git a/gdb/inferior.c b/gdb/inferior.c
index 16991d41b3b..76f5581b7db 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -232,7 +232,6 @@ exit_inferior_1 (struct inferior *inftoex, int silent)
 void
 exit_inferior (inferior *inf)
 {
-  int pid = inf->pid;
   exit_inferior_1 (inf, 0);
 }
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index dd7e69e718e..05523a27563 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2174,8 +2174,6 @@ start_step_over (void)
 static void
 infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
 {
-  struct displaced_step_inferior_state *displaced;
-
   if (inferior_ptid == old_ptid)
     inferior_ptid = new_ptid;
 }
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 2a4189278ef..790ddf47409 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1501,8 +1501,6 @@ decode_line_2 (struct linespec_state *self,
   for (i = 0; i < result->size (); ++i)
     {
       const struct linespec_canonical_name *canonical;
-      struct decode_line_2_item *item;
-
       std::string displayform;
 
       canonical = &self->canonical_names[i];
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 18d9a3872a5..86d3dfd2b89 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -191,10 +191,6 @@ struct linux_nat_target *linux_target;
 /* Does the current host support PTRACE_GETREGSET?  */
 enum tribool have_ptrace_getregset = TRIBOOL_UNKNOWN;
 
-/* The saved to_close method, inherited from inf-ptrace.c.
-   Called by our to_close.  */
-static void (*super_close) (struct target_ops *);
-
 static unsigned int debug_linux_nat;
 static void
 show_debug_linux_nat (struct ui_file *file, int from_tty,
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 5edf2ef2005..3cfa2a5aa48 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -726,7 +726,6 @@ linux_info_proc (struct gdbarch *gdbarch, const char *args,
   int status_f = (what == IP_STATUS || what == IP_ALL);
   int stat_f = (what == IP_STAT || what == IP_ALL);
   char filename[100];
-  char *data;
   int target_errno;
 
   if (args && isdigit (args[0]))
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 18ce4e675f3..2163d423028 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1999,7 +1999,6 @@ mi_execute_command (const char *cmd, int from_tty)
 	     again.  */
 	  && !command_notifies_uscc_observer (command.get ()))
 	{
-	  struct mi_interp *mi = (struct mi_interp *) top_level_interpreter ();
 	  int report_change = 0;
 
 	  if (command->thread == -1)
diff --git a/gdb/microblaze-linux-tdep.c b/gdb/microblaze-linux-tdep.c
index 5fc2cc8bab5..8bc153aec48 100644
--- a/gdb/microblaze-linux-tdep.c
+++ b/gdb/microblaze-linux-tdep.c
@@ -70,8 +70,6 @@ microblaze_linux_sigtramp_cache (struct frame_info *next_frame,
   CORE_ADDR base;
   CORE_ADDR gpregs;
   int regnum;
-  struct gdbarch *gdbarch = get_frame_arch (next_frame);
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
   base = frame_unwind_register_unsigned (next_frame, MICROBLAZE_SP_REGNUM);
   if (bias > 0 && get_frame_address_in_block (next_frame) != func)
diff --git a/gdb/parse.c b/gdb/parse.c
index acb08f24363..0cbdb48d23a 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -458,7 +458,6 @@ find_minsym_type_and_address (minimal_symbol *msymbol,
 			      CORE_ADDR *address_p)
 {
   bound_minimal_symbol bound_msym = {msymbol, objfile};
-  struct gdbarch *gdbarch = get_objfile_arch (objfile);
   struct obj_section *section = MSYMBOL_OBJ_SECTION (objfile, msymbol);
   enum minimal_symbol_type type = MSYMBOL_TYPE (msymbol);
 
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 696285ed8a4..2ecbd546532 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1297,7 +1297,6 @@ info_symbol_command (const char *arg, int from_tty)
       {
 	const char *obj_name, *mapped, *sec_name, *msym_name;
 	const char *loc_string;
-	struct cleanup *old_chain;
 
 	matches = 1;
 	offset = sect_addr - MSYMBOL_VALUE_ADDRESS (objfile, msymbol);
@@ -2286,7 +2285,6 @@ printf_floating (struct ui_file *stream, const char *format,
   /* Parameter data.  */
   struct type *param_type = value_type (value);
   struct gdbarch *gdbarch = get_type_arch (param_type);
-  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
 
   /* Determine target type corresponding to the format string.  */
   struct type *fmt_type;
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index a66e553cf83..e1db674647a 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -516,7 +516,6 @@ static int
 bppy_set_commands (PyObject *self, PyObject *newvalue, void *closure)
 {
   gdbpy_breakpoint_object *self_bp = (gdbpy_breakpoint_object *) self;
-  struct breakpoint *bp = self_bp->bp;
   struct gdb_exception except = exception_none;
 
   BPPY_SET_REQUIRE_VALID (self_bp);
diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
index 569d1f48d87..469ae489f43 100644
--- a/gdb/python/py-unwind.c
+++ b/gdb/python/py-unwind.c
@@ -329,8 +329,6 @@ static void
 unwind_infopy_dealloc (PyObject *self)
 {
   unwind_info_object *unwind_info = (unwind_info_object *) self;
-  int i;
-  saved_reg *reg;
 
   Py_XDECREF (unwind_info->pending_frame);
   delete unwind_info->saved_regs;
diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 09a3f6cc5c4..eafecd967d9 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -1569,8 +1569,6 @@ record_btrace_target::fetch_registers (struct regcache *regcache, int regno)
 void
 record_btrace_target::store_registers (struct regcache *regcache, int regno)
 {
-  struct target_ops *t;
-
   if (!record_btrace_generating_corefile
       && record_is_replaying (regcache->ptid ()))
     error (_("Cannot write registers while replaying."));
@@ -3085,8 +3083,6 @@ cmd_set_record_btrace_cpu (const char *args, int from_tty)
 static void
 cmd_show_record_btrace_cpu (const char *args, int from_tty)
 {
-  const char *cpu;
-
   if (args != nullptr && *args != 0)
     error (_("Trailing junk: '%s'."), args);
 
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 25a8fda29c4..0d23387e215 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -773,8 +773,6 @@ static int
 riscv_register_reggroup_p (struct gdbarch  *gdbarch, int regnum,
 			   struct reggroup *reggroup)
 {
-  int float_p;
-  int raw_p;
   unsigned int i;
 
   /* Used by 'info registers' and 'info registers <groupname>'.  */
@@ -2034,7 +2032,6 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
   int i;
   CORE_ADDR sp_args, sp_refs;
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
   struct riscv_arg_info *arg_info =
     (struct riscv_arg_info *) alloca (nargs * sizeof (struct riscv_arg_info));
@@ -2220,10 +2217,6 @@ riscv_return_value (struct gdbarch  *gdbarch,
 		    gdb_byte *readbuf,
 		    const gdb_byte *writebuf)
 {
-  enum type_code rv_type = TYPE_CODE (type);
-  unsigned int rv_size = TYPE_LENGTH (type);
-  int fp, regnum, flen;
-  ULONGEST tmp;
   struct riscv_call_info call_info (gdbarch);
   struct riscv_arg_info info;
   struct type *arg_type;
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index b60997681ae..98af8db606f 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -563,7 +563,6 @@ literal:
 		{ $$ = ast_dliteral ($1); }
 |	STRING
 		{
-		  const struct rust_op *str = ast_string ($1);
 		  struct set_field field;
 		  struct typed_val_int val;
 		  struct stoken token;
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 5f4aceb86ba..1871ec7df9b 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -1555,7 +1555,7 @@ rust_evaluate_subexp (struct type *expect_type, struct expression *exp,
 
     case OP_RUST_ARRAY:
       {
-	int pc = (*pos)++;
+	(*pos)++;
 	int copies;
 	struct value *elt;
 	struct value *ncopies;
@@ -1589,7 +1589,7 @@ rust_evaluate_subexp (struct type *expect_type, struct expression *exp,
         /* Anonymous field access, i.e. foo.1.  */
         struct value *lhs;
         int pc, field_number, nfields;
-        struct type *type, *variant_type;
+        struct type *type;
 
         pc = (*pos)++;
         field_number = longest_to_int (exp->elts[pc + 1].longconst);
diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
index d9ac40b625f..4561559a855 100644
--- a/gdb/s390-linux-tdep.c
+++ b/gdb/s390-linux-tdep.c
@@ -1150,7 +1150,6 @@ s390_linux_init_abi_any (struct gdbarch_info info, struct gdbarch *gdbarch)
 static void
 s390_linux_init_abi_31 (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
-  const struct target_desc *tdesc = info.target_desc;
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
   tdep->abi = ABI_LINUX_S390;
@@ -1167,7 +1166,6 @@ s390_linux_init_abi_31 (struct gdbarch_info info, struct gdbarch *gdbarch)
 static void
 s390_linux_init_abi_64 (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
-  const struct target_desc *tdesc = info.target_desc;
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
 
   tdep->abi = ABI_LINUX_ZSERIES;
diff --git a/gdb/ser-mingw.c b/gdb/ser-mingw.c
index 605a071770a..937b66dd57b 100644
--- a/gdb/ser-mingw.c
+++ b/gdb/ser-mingw.c
@@ -321,9 +321,8 @@ ser_windows_read_prim (struct serial *scb, size_t count)
 {
   struct ser_windows_state *state;
   OVERLAPPED ov;
-  DWORD bytes_read, bytes_read_tmp;
+  DWORD bytes_read;
   HANDLE h;
-  gdb_byte *p;
 
   state = (struct ser_windows_state *) scb->state;
   if (state->in_progress)
@@ -351,7 +350,6 @@ ser_windows_read_prim (struct serial *scb, size_t count)
 static int
 ser_windows_write_prim (struct serial *scb, const void *buf, size_t len)
 {
-  struct ser_windows_state *state;
   OVERLAPPED ov;
   DWORD bytes_written;
   HANDLE h;
@@ -634,7 +632,6 @@ pipe_select_thread (void *arg)
 {
   struct serial *scb = (struct serial *) arg;
   struct ser_console_state *state;
-  int event_index;
   HANDLE h;
 
   state = (struct ser_console_state *) scb->state;
@@ -677,7 +674,6 @@ file_select_thread (void *arg)
 {
   struct serial *scb = (struct serial *) arg;
   struct ser_console_state *state;
-  int event_index;
   HANDLE h;
 
   state = (struct ser_console_state *) scb->state;
@@ -1188,7 +1184,6 @@ net_windows_open (struct serial *scb, const char *name)
 {
   struct net_windows_state *state;
   int ret;
-  DWORD threadId;
 
   ret = net_open (scb, name);
   if (ret != 0)
@@ -1347,7 +1342,6 @@ void
 _initialize_ser_windows (void)
 {
   WSADATA wsa_data;
-  struct serial_ops *ops;
 
   HMODULE hm = NULL;
 
diff --git a/gdb/spu-tdep.c b/gdb/spu-tdep.c
index c85a7407871..ea915203256 100644
--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1820,8 +1820,6 @@ spu_get_overlay_table (struct objfile *objfile)
     {
       CORE_ADDR vma  = extract_unsigned_integer (ovly_table + 16*i + 0,
 						 4, byte_order);
-      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
-						 4, byte_order);
       CORE_ADDR pos  = extract_unsigned_integer (ovly_table + 16*i + 8,
 						 4, byte_order);
       CORE_ADDR buf  = extract_unsigned_integer (ovly_table + 16*i + 12,
diff --git a/gdb/symtab.c b/gdb/symtab.c
index d8a7a16e073..73c79833ce5 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3053,7 +3053,6 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent)
   struct symtab *iter_s;
   struct linetable *l;
   int len;
-  int i;
   struct linetable_entry *item;
   const struct blockvector *bv;
   struct bound_minimal_symbol msymbol;
diff --git a/gdb/target-memory.c b/gdb/target-memory.c
index 783e59dd551..670576de188 100644
--- a/gdb/target-memory.c
+++ b/gdb/target-memory.c
@@ -180,7 +180,6 @@ compute_garbled_blocks (const std::vector<memory_write_request> &erased_blocks,
 
   unsigned j;
   unsigned je = written_blocks.size ();
-  struct memory_write_request *erased_p;
 
   /* Look at each erased memory_write_request in turn, and
      see what part of it is subsequently written to.
@@ -265,7 +264,6 @@ target_write_memory_blocks (const std::vector<memory_write_request> &requests,
 			    void (*progress_cb) (ULONGEST, void *))
 {
   std::vector<memory_write_request> blocks = requests;
-  struct memory_write_request *r;
   std::vector<memory_write_request> regular;
   std::vector<memory_write_request> flash;
   std::vector<memory_write_request> erased, garbled;
diff --git a/gdb/target.c b/gdb/target.c
index 19d59ba603a..eba07cc9174 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -674,9 +674,6 @@ unpush_target (struct target_ops *t)
 bool
 target_stack::unpush (target_ops *t)
 {
-  struct target_ops **cur;
-  struct target_ops *tmp;
-
   if (t->to_stratum == dummy_stratum)
     internal_error (__FILE__, __LINE__,
 		    _("Attempt to unpush the dummy target"));
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index ab80b10ea1d..74d1183386c 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1543,7 +1543,6 @@ trace_reset_local_state (void)
 void
 start_tracing (const char *notes)
 {
-  struct trace_state_variable *tsv;
   int any_enabled = 0, num_to_download = 0;
   int ret;
 
@@ -2752,7 +2751,6 @@ all_tracepoint_actions (struct breakpoint *t)
      the fly, and don't cache it.  */
   if (*default_collect)
     {
-      struct command_line *default_collect_action;
       gdb::unique_xmalloc_ptr<char> default_collect_line
 	(xstrprintf ("collect %s", default_collect));
 
@@ -3179,7 +3177,6 @@ create_tsv_from_upload (struct uploaded_tsv *utsv)
 void
 merge_uploaded_trace_state_variables (struct uploaded_tsv **uploaded_tsvs)
 {
-  int ix;
   struct uploaded_tsv *utsv;
   struct trace_state_variable *tsv;
   int highest;
@@ -3682,7 +3679,6 @@ print_one_static_tracepoint_marker (int count,
   if (!tracepoints.empty ())
     {
       int ix;
-      struct breakpoint *b;
 
       {
 	ui_out_emit_tuple tuple_emitter (uiout, "tracepoints-at");
diff --git a/gdb/unittests/basic_string_view/element_access/char/1.cc b/gdb/unittests/basic_string_view/element_access/char/1.cc
index 7f8e79e6d00..bba1782b1e3 100644
--- a/gdb/unittests/basic_string_view/element_access/char/1.cc
+++ b/gdb/unittests/basic_string_view/element_access/char/1.cc
@@ -27,7 +27,7 @@ test01()
   typedef gdb::string_view::size_type csize_type;
   typedef gdb::string_view::const_reference cref;
   typedef gdb::string_view::reference ref;
-  csize_type csz01, csz02;
+  csize_type csz01;
 
   const gdb::string_view str01("tamarindo, costa rica");
   gdb::string_view str02("41st street beach, capitola, california");
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 6986dc355ac..8f4646d1cca 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -1298,7 +1298,6 @@ handle_exception (struct target_waitstatus *ourstatus)
 static BOOL
 windows_continue (DWORD continue_status, int id, int killed)
 {
-  int i;
   windows_thread_info *th;
   BOOL res;
 
@@ -1765,7 +1764,6 @@ windows_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
 static void
 windows_add_all_dlls (void)
 {
-  struct so_list *so;
   HMODULE dummy_hmodule;
   DWORD cb_needed;
   HMODULE *hmodules;
@@ -1815,7 +1813,6 @@ do_initial_windows_stuff (struct target_ops *ops, DWORD pid, int attaching)
 {
   int i;
   struct inferior *inf;
-  struct thread_info *tp;
 
   last_sig = GDB_SIGNAL_0;
   event_count = 0;
@@ -2516,16 +2513,12 @@ windows_nat_target::create_inferior (const char *exec_file,
   int tty;
   int ostdin, ostdout, ostderr;
 #else  /* !__CYGWIN__ */
-  char real_path[__PMAX];
   char shell[__PMAX]; /* Path to shell */
   const char *toexec;
   char *args, *allargs_copy;
   size_t args_len, allargs_len;
   int fd_inp = -1, fd_out = -1, fd_err = -1;
   HANDLE tty = INVALID_HANDLE_VALUE;
-  HANDLE inf_stdin = INVALID_HANDLE_VALUE;
-  HANDLE inf_stdout = INVALID_HANDLE_VALUE;
-  HANDLE inf_stderr = INVALID_HANDLE_VALUE;
   bool redirected = false;
   char *w32env;
   char *temp;
-- 
2.13.6

Re: [RFA 01/13] Simple unused variable removals

[Simon Marchi]

On 2018-07-12 04:51 PM, Tom Tromey wrote:
> This patch holds all the straightforward unused variable deletions.

I've looked at this patch and I didn't find anything that looked wrong
to my eyes.  In some cases, such as:

--- a/gdb/spu-tdep.c
+++ b/gdb/spu-tdep.c
@@ -1820,8 +1820,6 @@ spu_get_overlay_table (struct objfile *objfile)
     {
       CORE_ADDR vma  = extract_unsigned_integer (ovly_table + 16*i + 0,
 						 4, byte_order);
-      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
-						 4, byte_order);
       CORE_ADDR pos  = extract_unsigned_integer (ovly_table + 16*i + 8,
 						 4, byte_order);
       CORE_ADDR buf  = extract_unsigned_integer (ovly_table + 16*i + 12,


I wondered this was a bug (should that variable really be used), but I
wouldn't know without some quite extensive research...  so all-in-all, LGTM.

Simon

Re: [RFA 01/13] Simple unused variable removals

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> -      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
Simon> -						 4, byte_order);

Simon> I wondered this was a bug (should that variable really be used), but I
Simon> wouldn't know without some quite extensive research...  so all-in-all, LGTM.

Yes, this one was on the bubble and I almost put it in its own patch...
I can't recall offhand if there were others like this.

Tom

Re: [RFA 01/13] Simple unused variable removals

[Simon Marchi]

On 2018-07-14 08:40 AM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> -      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
> Simon> -						 4, byte_order);
> 
> Simon> I wondered this was a bug (should that variable really be used), but I
> Simon> wouldn't know without some quite extensive research...  so all-in-all, LGTM.
> 
> Yes, this one was on the bubble and I almost put it in its own patch...
> I can't recall offhand if there were others like this.
> 
> Tom

Here are a few more found by cross-compiling or trying to compile random
files.  I think we have to accept the fact that we'll find more after the
patch series is merged.

Simon


From 1f97f379df3e3eab48da3a4a6a6843add8a60673 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Fri, 13 Jul 2018 21:49:43 -0400
Subject: [PATCH] obvious

---
 gdb/aarch64-linux-nat.c            |  2 +-
 gdb/aix-thread.c                   |  5 -----
 gdb/arm-linux-nat.c                | 12 ++++--------
 gdb/gdbserver/linux-mips-low.c     |  2 --
 gdb/mips-linux-nat.c               |  1 -
 gdb/nat/aarch64-sve-linux-ptrace.c |  1 -
 gdb/ppc-linux-nat.c                | 10 ----------
 gdb/remote-sim.c                   |  3 ---
 gdb/sparc64-obsd-nat.c             |  1 -
 gdb/xtensa-linux-nat.c             |  2 --
 10 files changed, 5 insertions(+), 34 deletions(-)

diff --git a/gdb/aarch64-linux-nat.c b/gdb/aarch64-linux-nat.c
index 43af99d11d2d..678c89029d40 100644
--- a/gdb/aarch64-linux-nat.c
+++ b/gdb/aarch64-linux-nat.c
@@ -819,7 +819,7 @@ bool
 aarch64_linux_nat_target::stopped_data_address (CORE_ADDR *addr_p)
 {
   siginfo_t siginfo;
-  int i, tid;
+  int i;
   struct aarch64_debug_reg_state *state;

   if (!linux_nat_get_siginfo (inferior_ptid, &siginfo))
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 97592e5b1f70..6c8607af295d 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -671,8 +671,6 @@ giter_accum (struct thread_info *thread, void *bufp)
 static int
 ptid_cmp (ptid_t ptid1, ptid_t ptid2)
 {
-  int pid1, pid2;
-
   if (ptid1.pid () < ptid2.pid ())
     return -1;
   else if (ptid1.pid () > ptid2.pid ())
@@ -708,7 +706,6 @@ get_signaled_thread (void)
 {
   struct thrdsinfo64 thrinf;
   tid_t ktid = 0;
-  int result = 0;

   while (1)
   {
@@ -1502,7 +1499,6 @@ store_regs_user_thread (const struct regcache *regcache, pthdb_pthread_t pdtid)
   pthdb_context_t ctx;
   uint32_t int32;
   uint64_t int64;
-  double   dbl;

   if (debug_aix_thread)
     fprintf_unfiltered (gdb_stdlog,
@@ -1594,7 +1590,6 @@ store_regs_kernel_thread (const struct regcache *regcache, int regno,
   double fprs[ppc_num_fprs];
   struct ptxsprs sprs64;
   struct ptsprs  sprs32;
-  int i;

   if (debug_aix_thread)
     fprintf_unfiltered (gdb_stdlog,
diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
index 3c552ecde265..7d287c656800 100644
--- a/gdb/arm-linux-nat.c
+++ b/gdb/arm-linux-nat.c
@@ -205,7 +205,7 @@ store_fpregs (const struct regcache *regcache)
 static void
 fetch_regs (struct regcache *regcache)
 {
-  int ret, regno, tid;
+  int ret, tid;
   elf_gregset_t regs;

   /* Get the thread id for the ptrace call.  */
@@ -232,7 +232,7 @@ fetch_regs (struct regcache *regcache)
 static void
 store_regs (const struct regcache *regcache)
 {
-  int ret, regno, tid;
+  int ret, tid;
   elf_gregset_t regs;

   /* Get the thread id for the ptrace call.  */
@@ -339,7 +339,7 @@ static void
 fetch_vfp_regs (struct regcache *regcache)
 {
   gdb_byte regbuf[VFP_REGS_SIZE];
-  int ret, regno, tid;
+  int ret, tid;
   struct gdbarch *gdbarch = regcache->arch ();
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

@@ -368,7 +368,7 @@ static void
 store_vfp_regs (const struct regcache *regcache)
 {
   gdb_byte regbuf[VFP_REGS_SIZE];
-  int ret, regno, tid;
+  int ret, tid;
   struct gdbarch *gdbarch = regcache->arch ();
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

@@ -1064,7 +1064,6 @@ int
 arm_linux_nat_target::insert_hw_breakpoint (struct gdbarch *gdbarch,
 					    struct bp_target_info *bp_tgt)
 {
-  struct lwp_info *lp;
   struct arm_linux_hw_breakpoint p;

   if (arm_linux_get_hw_breakpoint_count () == 0)
@@ -1082,7 +1081,6 @@ int
 arm_linux_nat_target::remove_hw_breakpoint (struct gdbarch *gdbarch,
 					    struct bp_target_info *bp_tgt)
 {
-  struct lwp_info *lp;
   struct arm_linux_hw_breakpoint p;

   if (arm_linux_get_hw_breakpoint_count () == 0)
@@ -1134,7 +1132,6 @@ arm_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
 					 enum target_hw_bp_type rw,
 					 struct expression *cond)
 {
-  struct lwp_info *lp;
   struct arm_linux_hw_breakpoint p;

   if (arm_linux_get_hw_watchpoint_count () == 0)
@@ -1153,7 +1150,6 @@ arm_linux_nat_target::remove_watchpoint (CORE_ADDR addr,
 					 int len, enum target_hw_bp_type rw,
 					 struct expression *cond)
 {
-  struct lwp_info *lp;
   struct arm_linux_hw_breakpoint p;

   if (arm_linux_get_hw_watchpoint_count () == 0)
diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
index 2194af6c0a50..393488ac5ba9 100644
--- a/gdb/gdbserver/linux-mips-low.c
+++ b/gdb/gdbserver/linux-mips-low.c
@@ -852,7 +852,6 @@ static void
 mips_collect_ptrace_register (struct regcache *regcache,
 			      int regno, char *buf)
 {
-  const struct target_desc *tdesc = current_process ()->tdesc;
   int use_64bit = sizeof (PTRACE_XFER_TYPE) == 8;

   if (use_64bit && register_size (regcache->tdesc, regno) == 4)
@@ -872,7 +871,6 @@ static void
 mips_supply_ptrace_register (struct regcache *regcache,
 			     int regno, const char *buf)
 {
-  const struct target_desc *tdesc = current_process ()->tdesc;
   int use_64bit = sizeof (PTRACE_XFER_TYPE) == 8;

   if (use_64bit && register_size (regcache->tdesc, regno) == 4)
diff --git a/gdb/mips-linux-nat.c b/gdb/mips-linux-nat.c
index 21b1f583b922..f143367a13bc 100644
--- a/gdb/mips-linux-nat.c
+++ b/gdb/mips-linux-nat.c
@@ -677,7 +677,6 @@ mips_linux_nat_target::insert_watchpoint (CORE_ADDR addr, int len,
   struct mips_watchpoint *new_watch;
   struct mips_watchpoint **pw;

-  int i;
   int retval;

   if (!mips_linux_read_watch_registers (inferior_ptid.lwp (),
diff --git a/gdb/nat/aarch64-sve-linux-ptrace.c b/gdb/nat/aarch64-sve-linux-ptrace.c
index a2f9261ba6f8..7b4027195be9 100644
--- a/gdb/nat/aarch64-sve-linux-ptrace.c
+++ b/gdb/nat/aarch64-sve-linux-ptrace.c
@@ -67,7 +67,6 @@ std::unique_ptr<gdb_byte[]>
 aarch64_sve_get_sveregs (int tid)
 {
   struct iovec iovec;
-  struct user_sve_header header;
   uint64_t vq = aarch64_sve_get_vq (tid);

   if (vq == 0)
diff --git a/gdb/ppc-linux-nat.c b/gdb/ppc-linux-nat.c
index 9719a0e906de..0b7052d0f6de 100644
--- a/gdb/ppc-linux-nat.c
+++ b/gdb/ppc-linux-nat.c
@@ -532,11 +532,9 @@ static void
 fetch_register (struct regcache *regcache, int tid, int regno)
 {
   struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   /* This isn't really an address.  But ptrace thinks of it as one.  */
   CORE_ADDR regaddr = ppc_register_u_addr (gdbarch, regno);
   int bytes_transferred;
-  unsigned int offset;         /* Offset of registers within the u area.  */
   gdb_byte buf[PPC_MAX_REGISTER_SIZE];

   if (altivec_register_p (gdbarch, regno))
@@ -630,8 +628,6 @@ fetch_register (struct regcache *regcache, int tid, int regno)
 static int
 fetch_all_gp_regs (struct regcache *regcache, int tid)
 {
-  struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   gdb_gregset_t gregset;

   if (ptrace (PTRACE_GETREGS, tid, 0, (void *) &gregset) < 0)
@@ -728,7 +724,6 @@ fetch_fp_regs (struct regcache *regcache, int tid)
 static void
 fetch_ppc_registers (struct regcache *regcache, int tid)
 {
-  int i;
   struct gdbarch *gdbarch = regcache->arch ();
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

@@ -1004,8 +999,6 @@ store_register (const struct regcache *regcache, int tid, int regno)
 static int
 store_all_gp_regs (const struct regcache *regcache, int tid, int regno)
 {
-  struct gdbarch *gdbarch = regcache->arch ();
-  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   gdb_gregset_t gregset;

   if (ptrace (PTRACE_GETREGS, tid, 0, (void *) &gregset) < 0)
@@ -1122,7 +1115,6 @@ store_fp_regs (const struct regcache *regcache, int tid, int regno)
 static void
 store_ppc_registers (const struct regcache *regcache, int tid)
 {
-  int i;
   struct gdbarch *gdbarch = regcache->arch ();
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

@@ -1407,7 +1399,6 @@ hwdebug_insert_point (struct ppc_hw_breakpoint *b, int tid)
   gdb::unique_xmalloc_ptr<ppc_hw_breakpoint> p (XDUP (ppc_hw_breakpoint, b));
   struct hw_break_tuple *hw_breaks;
   struct thread_points *t;
-  struct hw_break_tuple *tuple;

   errno = 0;
   slot = ptrace (PPC_PTRACE_SETHWDEBUG, tid, 0, p.get ());
@@ -1627,7 +1618,6 @@ can_use_watchpoint_cond_accel (void)
   struct thread_points *p;
   int tid = inferior_ptid.lwp ();
   int cnt = hwdebug_info.num_condition_regs, i;
-  CORE_ADDR tmp_value;

   if (!have_ptrace_hwdebug_interface () || cnt == 0)
     return 0;
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index f76261dadf64..cc7a6f27fc99 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -377,9 +377,6 @@ end_callbacks (void)
 static int
 gdb_os_write_stdout (host_callback *p, const char *buf, int len)
 {
-  int i;
-  char b[2];
-
   ui_file_write (gdb_stdtarg, buf, len);
   return len;
 }
diff --git a/gdb/sparc64-obsd-nat.c b/gdb/sparc64-obsd-nat.c
index 593a7ac651b4..7f7bb78b656f 100644
--- a/gdb/sparc64-obsd-nat.c
+++ b/gdb/sparc64-obsd-nat.c
@@ -77,7 +77,6 @@ static int
 sparc64obsd_supply_pcb (struct regcache *regcache, struct pcb *pcb)
 {
   u_int64_t state;
-  int regnum;

   /* The following is true for OpenBSD 3.0:

diff --git a/gdb/xtensa-linux-nat.c b/gdb/xtensa-linux-nat.c
index 007a5c3f058f..ea113bdd7146 100644
--- a/gdb/xtensa-linux-nat.c
+++ b/gdb/xtensa-linux-nat.c
@@ -204,7 +204,6 @@ fetch_gregs (struct regcache *regcache, int regnum)
 {
   int tid = regcache->ptid ().lwp ();
   gdb_gregset_t regs;
-  int areg;

   if (ptrace (PTRACE_GETREGS, tid, 0, (long) &regs) < 0)
     {
@@ -223,7 +222,6 @@ store_gregs (struct regcache *regcache, int regnum)
 {
   int tid = regcache->ptid ().lwp ();
   gdb_gregset_t regs;
-  int areg;

   if (ptrace (PTRACE_GETREGS, tid, 0, (long) &regs) < 0)
     {
-- 
2.18.0

Re: [RFA 01/13] Simple unused variable removals

[Simon Marchi]

On 2018-07-14 08:40 AM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> -      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
> Simon> -						 4, byte_order);
> 
> Simon> I wondered this was a bug (should that variable really be used), but I
> Simon> wouldn't know without some quite extensive research...  so all-in-all, LGTM.
> 
> Yes, this one was on the bubble and I almost put it in its own patch...
> I can't recall offhand if there were others like this.
> 
> Tom
> 


And here's another one that I think is less obvious.  The call has side-effects
(it allocates the per-inferior data), but I don't think it's really needed.


From 4014eefe405a66c6e2ade325e14ed891e9f0f62a Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Fri, 13 Jul 2018 23:09:04 -0400
Subject: [PATCH] semi-obvious

---
 gdb/remote-sim.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index cc7a6f27fc99..d30b38ecb7ce 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -831,9 +831,6 @@ gdbsim_close_inferior (struct inferior *inf, void *arg)
 void
 gdbsim_target::close ()
 {
-  struct sim_inferior_data *sim_data
-    = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
-
   if (remote_debug)
     fprintf_unfiltered (gdb_stdlog, "gdbsim_close\n");

@@ -1184,9 +1181,6 @@ gdbsim_target::files_info ()
 void
 gdbsim_target::mourn_inferior ()
 {
-  struct sim_inferior_data *sim_data
-    = get_sim_inferior_data (current_inferior (), SIM_INSTANCE_NOT_NEEDED);
-
   if (remote_debug)
     fprintf_unfiltered (gdb_stdlog, "gdbsim_mourn_inferior:\n");

-- 
2.18.0

Re: [RFA 01/13] Simple unused variable removals

[Pedro Alves]

On 07/14/2018 10:55 PM, Simon Marchi wrote:

> And here's another one that I think is less obvious.  The call has side-effects
> (it allocates the per-inferior data), but I don't think it's really needed.

I'm pretty sure it's not needed.  Feel free to push in,
I don't think there's need to squash with Tromey's patch.

Thanks,
Pedro Alves

Re: [RFA 01/13] Simple unused variable removals

[Pedro Alves]

On 07/14/2018 01:40 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> -      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
> Simon> -						 4, byte_order);
> 
> Simon> I wondered this was a bug (should that variable really be used), but I
> Simon> wouldn't know without some quite extensive research...  so all-in-all, LGTM.
> 
> Yes, this one was on the bubble and I almost put it in its own patch...
> I can't recall offhand if there were others like this.

I skimmed the patch and it looks good to me to, and, that bit
also gave me pause.  I was wondering whether we should replace
it with a comment, saying that we're skipping 4 bytes which hold
the entry's size.  That might save someone some time in case the size
turns out to be needed.  (I have no idea where the structure being
extracted is documented, for example.)

Thanks,
Pedro Alves

Re: [RFA 01/13] Simple unused variable removals

[Tom Tromey]

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

Simon> -      CORE_ADDR size = extract_unsigned_integer (ovly_table + 16*i + 4,
Simon> -						 4, byte_order);
>> 
Simon> I wondered this was a bug (should that variable really be used), but I
Simon> wouldn't know without some quite extensive research...  so all-in-all, LGTM.
>> 
>> Yes, this one was on the bubble and I almost put it in its own patch...
>> I can't recall offhand if there were others like this.

Pedro> I skimmed the patch and it looks good to me to, and, that bit
Pedro> also gave me pause.  I was wondering whether we should replace
Pedro> it with a comment, saying that we're skipping 4 bytes which hold
Pedro> the entry's size.  That might save someone some time in case the size
Pedro> turns out to be needed.  (I have no idea where the structure being
Pedro> extracted is documented, for example.)

I've split this into a separate patch and added a comment.

Tom

[RFA 11/13] Remove unused variables from gdbserver

[Tom Tromey]

This removes a few unused variables from gdbserver.

The x86-tdesc.h change is a bit unusual for this series.  This file
was not defining the multiple-include guard symbol, so I've added that
here.  Also, it is hard to determine when i386_expedite_regs will be
needed, so this patch simply marks it ATTRIBUTE_UNUSED.

2018-07-12  Tom Tromey  <tom@tromey.com>

	* win32-low.c (win32_create_inferior): Remove unused variables.
	* gdbreplay.c (remote_open): Remove unused variable.
	* remote-utils.c (remote_prepare): Remove unused variable.
	* x86-tdesc.h (X86_TDESC_H): Define.
	(amd64_expedite_regs): Define conditionally.
	(i386_expedite_regs): Mark ATTRIBUTE_UNUSED.
	* linux-x86-tdesc.c (i386_tdescs): Move inside #if.
	* remote-utils.c (readchar): Remove unused variable.
---
 gdb/gdbserver/ChangeLog         | 11 +++++++++++
 gdb/gdbserver/gdbreplay.c       |  1 -
 gdb/gdbserver/linux-x86-tdesc.c |  4 ++--
 gdb/gdbserver/remote-utils.c    |  2 --
 gdb/gdbserver/win32-low.c       |  2 --
 gdb/gdbserver/x86-tdesc.h       |  6 +++++-
 6 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/gdb/gdbserver/gdbreplay.c b/gdb/gdbserver/gdbreplay.c
index 026bbfccaf0..b05209c4d14 100644
--- a/gdb/gdbserver/gdbreplay.c
+++ b/gdb/gdbserver/gdbreplay.c
@@ -157,7 +157,6 @@ remote_open (char *name)
 #ifdef USE_WIN32API
   static int winsock_initialized;
 #endif
-  char *port_str;
   int tmp;
   int tmp_desc;
   struct addrinfo hint;
diff --git a/gdb/gdbserver/linux-x86-tdesc.c b/gdb/gdbserver/linux-x86-tdesc.c
index c3aa20c982c..93122bffe83 100644
--- a/gdb/gdbserver/linux-x86-tdesc.c
+++ b/gdb/gdbserver/linux-x86-tdesc.c
@@ -69,10 +69,10 @@ xcr0_to_tdesc_idx (uint64_t xcr0, bool is_x32)
     return X86_TDESC_LAST;
 }
 
-static struct target_desc *i386_tdescs[X86_TDESC_LAST] = { };
-
 #if defined __i386__ || !defined IN_PROCESS_AGENT
 
+static struct target_desc *i386_tdescs[X86_TDESC_LAST] = { };
+
 /* Return the target description according to XCR0.  */
 
 const struct target_desc *
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index 1734c54e39a..38c90324288 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -231,7 +231,6 @@ void
 remote_prepare (const char *name)
 {
   client_state &cs = get_client_state ();
-  const char *port_str;
 #ifdef USE_WIN32API
   static int winsock_initialized;
 #endif
@@ -902,7 +901,6 @@ static unsigned char *readchar_bufp;
 static int
 readchar (void)
 {
-  client_state &cs = get_client_state ();
   int ch;
 
   if (readchar_bufcnt == 0)
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 8a20972bd4e..8276a2a3d55 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -634,8 +634,6 @@ win32_create_inferior (const char *program,
 #endif
   BOOL ret;
   DWORD flags;
-  int argslen;
-  int argc;
   PROCESS_INFORMATION pi;
   DWORD err;
   std::string str_program_args = stringify_argv (program_args);
diff --git a/gdb/gdbserver/x86-tdesc.h b/gdb/gdbserver/x86-tdesc.h
index c1641b2760f..48abd39b1ba 100644
--- a/gdb/gdbserver/x86-tdesc.h
+++ b/gdb/gdbserver/x86-tdesc.h
@@ -16,11 +16,15 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #ifndef X86_TDESC_H
+#define X86_TDESC_H
 
 /* The "expedite" registers for x86 targets.  */
-static const char *i386_expedite_regs[] = {"ebp", "esp", "eip", NULL};
+static const char *i386_expedite_regs[] ATTRIBUTE_UNUSED
+    = {"ebp", "esp", "eip", NULL};
 
+#ifdef __x86_64__
 /* The "expedite" registers for x86_64 targets.  */
 static const char *amd64_expedite_regs[] = {"rbp", "rsp", "rip", NULL};
+#endif
 
 #endif /* X86_TDESC_H */
-- 
2.13.6

Re: [RFA 11/13] Remove unused variables from gdbserver

[Simon Marchi]

On 2018-07-12 04:52 PM, Tom Tromey wrote:
> This removes a few unused variables from gdbserver.
> 
> The x86-tdesc.h change is a bit unusual for this series.  This file
> was not defining the multiple-include guard symbol, so I've added that
> here.  Also, it is hard to determine when i386_expedite_regs will be
> needed, so this patch simply marks it ATTRIBUTE_UNUSED.

That sounds reasonable to me, LGTM.

Simon

Re: [RFA 11/13] Remove unused variables from gdbserver

[Pedro Alves]

On 07/14/2018 03:47 AM, Simon Marchi wrote:
> On 2018-07-12 04:52 PM, Tom Tromey wrote:
>> This removes a few unused variables from gdbserver.
>>
>> The x86-tdesc.h change is a bit unusual for this series.  This file
>> was not defining the multiple-include guard symbol, so I've added that
>> here.  Also, it is hard to determine when i386_expedite_regs will be
>> needed, so this patch simply marks it ATTRIBUTE_UNUSED.
> 
> That sounds reasonable to me, LGTM.

To me too, but I'd suggest adding a comment, something around:

 /* The "expedite" registers for x86 targets.  Since whether the
    variable is used depends on host/configuration, we mark
    it ATTRIBUTE_UNUSED to keep it simple here.  */

Thanks,
Pedro Alves

[RFA 03/13] Use a previously unused variable in bfin-tdep.c

[Tom Tromey]

This changes bfin_push_dummy_call to use the result of check_typedef.
Calling check_typedef for effect was probably ok as well, but this
seemed a little nicer.

gdb/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* bfin-tdep.c (bfin_push_dummy_call): Use arg_type, not
	value_type.
---
 gdb/ChangeLog   | 5 +++++
 gdb/bfin-tdep.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/bfin-tdep.c b/gdb/bfin-tdep.c
index da62130231b..c84625c8948 100644
--- a/gdb/bfin-tdep.c
+++ b/gdb/bfin-tdep.c
@@ -526,7 +526,7 @@ bfin_push_dummy_call (struct gdbarch *gdbarch,
     {
       struct type *value_type = value_enclosing_type (args[i]);
       struct type *arg_type = check_typedef (value_type);
-      int container_len = (TYPE_LENGTH (value_type) + 3) & ~3;
+      int container_len = (TYPE_LENGTH (arg_type) + 3) & ~3;
 
       sp -= container_len;
       write_memory (sp, value_contents (args[i]), container_len);
-- 
2.13.6

Re: [RFA 03/13] Use a previously unused variable in bfin-tdep.c

[Simon Marchi]

On 2018-07-12 04:51 PM, Tom Tromey wrote:
> This changes bfin_push_dummy_call to use the result of check_typedef.
> Calling check_typedef for effect was probably ok as well, but this
> seemed a little nicer.

LGTM.

Simon

[RFA 13/13] Add -Wunused-variable to warnings.m4

[Tom Tromey]

This adds -Wunused-variable to the build.  This required a special
check in configure in order to work around a bug in GCC 4.9.  Simon
ound the correct test to use, so I've added him to the ChangeLog.

2018-07-12  Simon Marchi  <simon.marchi@polymtl.ca>
	    Tom Tromey  <tom@tromey.com>

	* warning.m4 (AM_GDB_WARNINGS): Add -Wunused-variable and special
	test for it.
	* configure: Rebuild.

gdb/gdbserver/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
---
 gdb/ChangeLog           |  7 +++++++
 gdb/configure           | 28 ++++++++++++++++++++++++++--
 gdb/gdbserver/ChangeLog |  4 ++++
 gdb/gdbserver/configure | 28 ++++++++++++++++++++++++++--
 gdb/warning.m4          | 17 +++++++++++++++--
 5 files changed, 78 insertions(+), 6 deletions(-)

diff --git a/gdb/configure b/gdb/configure
index 28756ed9826..1a1b3e9dafd 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -15467,7 +15467,7 @@ fi
 
 # The options we'll try to enable.
 build_warnings="-Wall -Wpointer-arith \
--Wno-unused -Wunused-value -Wunused-function \
+-Wno-unused -Wunused-value -Wunused-variable -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
 -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable \
 -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized \
@@ -15556,7 +15556,30 @@ $as_echo_n "checking compiler warning flags... " >&6; }
 	    CFLAGS="$CFLAGS -Werror $wtest"
 	    saved_CXXFLAGS="$CXXFLAGS"
 	    CXXFLAGS="$CXXFLAGS -Werror $wtest"
-	    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+	    if test "x$w" = "x-Wunused-variable"; then
+	      # Check for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=38958,
+	      # fixed in GCC 4.9.  This test is derived from the gdb
+	      # source code that triggered this bug in GCC.
+	      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+struct scoped_restore_base {};
+                 struct scoped_restore_tmpl : public scoped_restore_base {
+		   ~scoped_restore_tmpl() {}
+		 };
+int
+main ()
+{
+const scoped_restore_base &b = scoped_restore_tmpl();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  WARN_CFLAGS="${WARN_CFLAGS} $w"
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+	    else
+	      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
@@ -15571,6 +15594,7 @@ if ac_fn_cxx_try_compile "$LINENO"; then :
   WARN_CFLAGS="${WARN_CFLAGS} $w"
 fi
 rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+	    fi
 	    CFLAGS="$saved_CFLAGS"
 	    CXXFLAGS="$saved_CXXFLAGS"
 	esac
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 6064a1eb10f..043bc216e43 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -7237,7 +7237,7 @@ fi
 
 # The options we'll try to enable.
 build_warnings="-Wall -Wpointer-arith \
--Wno-unused -Wunused-value -Wunused-function \
+-Wno-unused -Wunused-value -Wunused-variable -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
 -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable \
 -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized \
@@ -7326,7 +7326,30 @@ $as_echo_n "checking compiler warning flags... " >&6; }
 	    CFLAGS="$CFLAGS -Werror $wtest"
 	    saved_CXXFLAGS="$CXXFLAGS"
 	    CXXFLAGS="$CXXFLAGS -Werror $wtest"
-	    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+	    if test "x$w" = "x-Wunused-variable"; then
+	      # Check for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=38958,
+	      # fixed in GCC 4.9.  This test is derived from the gdb
+	      # source code that triggered this bug in GCC.
+	      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+struct scoped_restore_base {};
+                 struct scoped_restore_tmpl : public scoped_restore_base {
+		   ~scoped_restore_tmpl() {}
+		 };
+int
+main ()
+{
+const scoped_restore_base &b = scoped_restore_tmpl();
+  ;
+  return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+  WARN_CFLAGS="${WARN_CFLAGS} $w"
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+	    else
+	      cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 
 int
@@ -7341,6 +7364,7 @@ if ac_fn_cxx_try_compile "$LINENO"; then :
   WARN_CFLAGS="${WARN_CFLAGS} $w"
 fi
 rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+	    fi
 	    CFLAGS="$saved_CFLAGS"
 	    CXXFLAGS="$saved_CXXFLAGS"
 	esac
diff --git a/gdb/warning.m4 b/gdb/warning.m4
index 632cc214ac0..00e7cd60508 100644
--- a/gdb/warning.m4
+++ b/gdb/warning.m4
@@ -37,7 +37,7 @@ fi
 
 # The options we'll try to enable.
 build_warnings="-Wall -Wpointer-arith \
--Wno-unused -Wunused-value -Wunused-function \
+-Wno-unused -Wunused-value -Wunused-variable -Wunused-function \
 -Wno-switch -Wno-char-subscripts \
 -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable \
 -Wno-sign-compare -Wno-narrowing -Wno-error=maybe-uninitialized \
@@ -118,7 +118,20 @@ then
 	    CFLAGS="$CFLAGS -Werror $wtest"
 	    saved_CXXFLAGS="$CXXFLAGS"
 	    CXXFLAGS="$CXXFLAGS -Werror $wtest"
-	    AC_TRY_COMPILE([],[],WARN_CFLAGS="${WARN_CFLAGS} $w",)
+	    if test "x$w" = "x-Wunused-variable"; then
+	      # Check for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=38958,
+	      # fixed in GCC 4.9.  This test is derived from the gdb
+	      # source code that triggered this bug in GCC.
+	      AC_TRY_COMPILE(
+	        [struct scoped_restore_base {};
+                 struct scoped_restore_tmpl : public scoped_restore_base {
+		   ~scoped_restore_tmpl() {}
+		 };],
+		[const scoped_restore_base &b = scoped_restore_tmpl();],
+		WARN_CFLAGS="${WARN_CFLAGS} $w",)
+	    else
+	      AC_TRY_COMPILE([],[],WARN_CFLAGS="${WARN_CFLAGS} $w",)
+	    fi
 	    CFLAGS="$saved_CFLAGS"
 	    CXXFLAGS="$saved_CXXFLAGS"
 	esac
-- 
2.13.6

[RFA 04/13] Call some functions in guile/ for effect

[Tom Tromey]

This changes a few spots in guile/ to remove a variable declaration
but to still call a function for effect.

gdb/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* guile/scm-pretty-print.c (gdbscm_apply_val_pretty_printer): Call
	value_contents_for_printing for effect.
	* guile/scm-cmd.c (gdbscm_dont_repeat): Call
	cmdscm_get_valid_command_smob_arg_unsafe for effect.
	* guile/scm-block.c (gdbscm_make_block_syms_iter): Call
	bkscm_get_valid_block_smob_arg_unsafe for effect.
---
 gdb/ChangeLog                | 9 +++++++++
 gdb/guile/scm-block.c        | 5 ++---
 gdb/guile/scm-cmd.c          | 6 +++---
 gdb/guile/scm-pretty-print.c | 4 +++-
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/gdb/guile/scm-block.c b/gdb/guile/scm-block.c
index d4566fc1ffe..9caff799dc1 100644
--- a/gdb/guile/scm-block.c
+++ b/gdb/guile/scm-block.c
@@ -613,9 +613,8 @@ bkscm_block_syms_progress_p (SCM scm)
 static SCM
 gdbscm_make_block_syms_iter (SCM self)
 {
-  block_smob *b_smob
-    = bkscm_get_valid_block_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
-  const struct block *block = b_smob->block;
+  /* Call for side effects.  */
+  bkscm_get_valid_block_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
   SCM progress, iter;
 
   progress = bkscm_make_block_syms_progress_smob ();
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 64243d1ba2e..9e5ff8f2636 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -267,9 +267,9 @@ gdbscm_command_valid_p (SCM self)
 static SCM
 gdbscm_dont_repeat (SCM self)
 {
-  /* We currently don't need anything from SELF, but still verify it.  */
-  command_smob *c_smob
-    = cmdscm_get_valid_command_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
+  /* We currently don't need anything from SELF, but still verify it.
+     Call for side effects.  */
+  cmdscm_get_valid_command_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
 
   dont_repeat ();
 
diff --git a/gdb/guile/scm-pretty-print.c b/gdb/guile/scm-pretty-print.c
index da1b7d2be15..8c96e7e3f8b 100644
--- a/gdb/guile/scm-pretty-print.c
+++ b/gdb/guile/scm-pretty-print.c
@@ -970,7 +970,9 @@ gdbscm_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   struct cleanup *cleanups;
   enum ext_lang_rc result = EXT_LANG_RC_NOP;
   enum string_repr_result print_result;
-  const gdb_byte *valaddr = value_contents_for_printing (val);
+
+  /* Call for side effects.  */
+  value_contents_for_printing (val);
 
   /* No pretty-printer support for unavailable values.  */
   if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
-- 
2.13.6

Re: [RFA 04/13] Call some functions in guile/ for effect

[Simon Marchi]

On 2018-07-12 04:51 PM, Tom Tromey wrote:
> This changes a few spots in guile/ to remove a variable declaration
> but to still call a function for effect.

LGTM, though it would be nice to mention in the comments what is the
desired side-effect (if known).

Simon

Re: [RFA 04/13] Call some functions in guile/ for effect

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> On 2018-07-12 04:51 PM, Tom Tromey wrote:
>> This changes a few spots in guile/ to remove a variable declaration
>> but to still call a function for effect.

Simon> LGTM, though it would be nice to mention in the comments what is the
Simon> desired side-effect (if known).

I really do not know, I just noticed that those functions can throw.

Tom

Re: [RFA 04/13] Call some functions in guile/ for effect

[Pedro Alves]

On 07/14/2018 01:39 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
> Simon> On 2018-07-12 04:51 PM, Tom Tromey wrote:
>>> This changes a few spots in guile/ to remove a variable declaration
>>> but to still call a function for effect.
> 
> Simon> LGTM, though it would be nice to mention in the comments what is the
> Simon> desired side-effect (if known).
> 
> I really do not know, I just noticed that those functions can throw.

No idea on the guile bits, but for this one:

> --- a/gdb/guile/scm-pretty-print.c
> +++ b/gdb/guile/scm-pretty-print.c
> @@ -970,7 +970,9 @@ gdbscm_apply_val_pretty_printer (const struct extension_language_defn *extlang,
>    struct cleanup *cleanups;
>    enum ext_lang_rc result = EXT_LANG_RC_NOP;
>    enum string_repr_result print_result;
> -  const gdb_byte *valaddr = value_contents_for_printing (val);
> +
> +  /* Call for side effects.  */
> +  value_contents_for_printing (val);
>  
>    /* No pretty-printer support for unavailable values.  */
>    if (!value_bytes_available (val, embedded_offset

... we can't call value_bytes_available on a lazy value (because that function must
be usable with values in the values history, that's why it works with a const value *),
so the desired side effect here is fetching a lazy value.  So I think we
should replace that with:

  if (value_lazy (val))
    value_fetch_lazy (val);

The Python seems to have the same issue, so I think it'd be
a little better to handle both this hunk and the equivalent bit
in Python in the same patch, and in its own patch.

Thanks,
Pedro Alves

Re: [RFA 04/13] Call some functions in guile/ for effect

[Tom Tromey]

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

Pedro> ... we can't call value_bytes_available on a lazy value (because that function must
Pedro> be usable with values in the values history, that's why it works with a const value *),
Pedro> so the desired side effect here is fetching a lazy value.  So I think we
Pedro> should replace that with:

Pedro>   if (value_lazy (val))
Pedro>     value_fetch_lazy (val);

Pedro> The Python seems to have the same issue, so I think it'd be
Pedro> a little better to handle both this hunk and the equivalent bit
Pedro> in Python in the same patch, and in its own patch.

I did this

Tom

[RFA 06/13] Remove dead code from m32c-tdep.c

[Tom Tromey]

This removes some dead code from m32c-tdep.c.  I broke this out into a
separate patch because the dead code seemed unusual, as if perhaps it
had not been completed; and so I thought it deserved extra scrutiny.

gdb/ChangeLog
2018-07-12  Tom Tromey  <tom@tromey.com>

	* m32c-tdep.c (mark_dma): Remove.
	(make_regs): Remove dead code.
---
 gdb/ChangeLog   |  5 +++++
 gdb/m32c-tdep.c | 23 -----------------------
 2 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
index f696568e3a7..6b1d1e83121 100644
--- a/gdb/m32c-tdep.c
+++ b/gdb/m32c-tdep.c
@@ -689,15 +689,6 @@ mark_general (struct m32c_reg *reg)
 }
 
 
-/* Mark REG as a DMA register, and return it.  */
-static struct m32c_reg *
-mark_dma (struct m32c_reg *reg)
-{
-  reg->dma_p = 1;
-  return reg;
-}
-
-
 /* Mark REG as a SYSTEM register, and return it.  */
 static struct m32c_reg *
 mark_system (struct m32c_reg *reg)
@@ -839,20 +830,6 @@ make_regs (struct gdbarch *arch)
   struct m32c_reg *pc          = G (RC (pc));
   struct m32c_reg *flg         = G (R16U (flg));
 
-  if (mach == bfd_mach_m32c)
-    {
-      struct m32c_reg *svf     = S (R16U (svf));
-      struct m32c_reg *svp     = S (RC (svp));
-      struct m32c_reg *vct     = S (RC (vct));
-
-      struct m32c_reg *dmd01   = DMA (RP (dmd, tdep->uint8));
-      struct m32c_reg *dct01   = DMA (RP (dct, tdep->uint16));
-      struct m32c_reg *drc01   = DMA (RP (drc, tdep->uint16));
-      struct m32c_reg *dma01   = DMA (RP (dma, tdep->data_addr_reg_type));
-      struct m32c_reg *dsa01   = DMA (RP (dsa, tdep->data_addr_reg_type));
-      struct m32c_reg *dra01   = DMA (RP (dra, tdep->data_addr_reg_type));
-    }
-
   num_raw_regs = tdep->num_regs;
 
   r0 	      = G (CB (r0, raw_r0_pair));
-- 
2.13.6

Re: [RFA 06/13] Remove dead code from m32c-tdep.c

[Simon Marchi]

On 2018-07-12 04:52 PM, Tom Tromey wrote:
> This removes some dead code from m32c-tdep.c.  I broke this out into a
> separate patch because the dead code seemed unusual, as if perhaps it
> had not been completed; and so I thought it deserved extra scrutiny.
> 
> gdb/ChangeLog
> 2018-07-12  Tom Tromey  <tom@tromey.com>
> 
> 	* m32c-tdep.c (mark_dma): Remove.
> 	(make_regs): Remove dead code.
> ---
>  gdb/ChangeLog   |  5 +++++
>  gdb/m32c-tdep.c | 23 -----------------------
>  2 files changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/gdb/m32c-tdep.c b/gdb/m32c-tdep.c
> index f696568e3a7..6b1d1e83121 100644
> --- a/gdb/m32c-tdep.c
> +++ b/gdb/m32c-tdep.c
> @@ -689,15 +689,6 @@ mark_general (struct m32c_reg *reg)
>  }
>  
>  
> -/* Mark REG as a DMA register, and return it.  */
> -static struct m32c_reg *
> -mark_dma (struct m32c_reg *reg)
> -{
> -  reg->dma_p = 1;
> -  return reg;
> -}
> -
> -
>  /* Mark REG as a SYSTEM register, and return it.  */
>  static struct m32c_reg *
>  mark_system (struct m32c_reg *reg)
> @@ -839,20 +830,6 @@ make_regs (struct gdbarch *arch)
>    struct m32c_reg *pc          = G (RC (pc));
>    struct m32c_reg *flg         = G (R16U (flg));
>  
> -  if (mach == bfd_mach_m32c)
> -    {
> -      struct m32c_reg *svf     = S (R16U (svf));
> -      struct m32c_reg *svp     = S (RC (svp));
> -      struct m32c_reg *vct     = S (RC (vct));
> -
> -      struct m32c_reg *dmd01   = DMA (RP (dmd, tdep->uint8));
> -      struct m32c_reg *dct01   = DMA (RP (dct, tdep->uint16));
> -      struct m32c_reg *drc01   = DMA (RP (drc, tdep->uint16));
> -      struct m32c_reg *dma01   = DMA (RP (dma, tdep->data_addr_reg_type));
> -      struct m32c_reg *dsa01   = DMA (RP (dsa, tdep->data_addr_reg_type));
> -      struct m32c_reg *dra01   = DMA (RP (dra, tdep->data_addr_reg_type));
> -    }
> -
>    num_raw_regs = tdep->num_regs;
>  
>    r0 	      = G (CB (r0, raw_r0_pair));
> 

I'm unsure about this one.  If you expand the macros, it looks something
like this:

mark_dma((add_reg (arch, "dmd" "0", tdep->uint8, m32c_sim_reg_dmd0, m32c_raw_read, m32c_raw_write, 0, 0, 0),
          add_reg (arch, "dmd" "1", tdep->uint8, m32c_sim_reg_dmd1, m32c_raw_read, m32c_raw_write, 0, 0, 0) - 1))

On one hand, the add_reg calls look important to me, because they add registers
to the gdbarch_tdep structure, so I would keep them.  But the mark_dma call looks
really fishy.  It only receives the first register, because this expression actually
ends up using the "comma" operator, returning the value on the right.  A little bit like
in this small program:

#include <stdio.h>

int foo(int a) {
  return a;
}

int main()
{
  int val = foo((1, 2));
  printf("%d\n", val);
}

$ gcc test.c -Wall
test.c: In function ‘main’:
test.c:9:19: warning: left-hand operand of comma expression has no effect [-Wunused-value]
   int val = foo((1, 2));
                   ^
$ ./a.out
2

And the expression on the right actually returns the first register (because of the -1).
So we end up marking only one of the two registers as DMA.  I have no idea if that's
intended or not.

So maybe the safe thing to do would be to make mark_dma return void, and get
rid of the local variables and assignments (but keep the DMA(...) calls).

Simon

Re: [RFA 06/13] Remove dead code from m32c-tdep.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:

Simon> I'm unsure about this one.

Boy, I really misread this code.
Good thing I broke this one out.

Simon> So maybe the safe thing to do would be to make mark_dma return void, and get
Simon> rid of the local variables and assignments (but keep the DMA(...) calls).

Yep.

Tom