[PATCH 0/8] Fix various warnings from clang 3.8.0

[PATCH 0/8] Fix various warnings from clang 3.8.0

[John Baldwin]

clang triggers a few warnings on binutils and gdb, but I've tried
to only submit patches for ones that I think are actual bugs (though
in some cases I believe the bugs are harmless in practice).

There might be better ways to address some of these warnings.  Two
that I can think of are:
- gdb/score-dep.c might choose to use a 'pc < start' check in the
  caller of the patched function and throw an error for that case
  (if it can occurr).
- gdb/sh64-tdep.c could explicitly set 'uses_fp' in the cases that
  it stores a value in the FP register rather than using a check
  at the end of the function.

John Baldwin (8):
  Use correct enum type for do_elf_stt_common.
  Change the size field of MSP430_Opcode_Decoded to a plain integer.
  Initialize 'ra' to zero to avoid uninitialized use.
  Remove check for negative size.
  Detect a frameless frame by comparing the FP register to -1.
  Pass a NULL pointer as the last argument to find_pc_partial_function.
  Remove unneeded checks on type lengths.
  Use unsigned integer constant with left shifts.

 binutils/ChangeLog             |  4 ++++
 binutils/objcopy.c             |  2 +-
 gdb/ChangeLog                  | 27 +++++++++++++++++++++++++++
 gdb/ada-lang.c                 |  4 ++--
 gdb/rs6000-tdep.c              |  1 +
 gdb/score-tdep.c               |  8 +-------
 gdb/sh64-tdep.c                |  2 +-
 gdb/tui/tui-stack.c            |  2 +-
 gdb/v850-tdep.c                |  2 +-
 include/ChangeLog              |  5 +++++
 include/opcode/msp430-decode.h |  9 +--------
 11 files changed, 45 insertions(+), 21 deletions(-)

-- 
2.7.0

[PATCH 1/8] Use correct enum type for do_elf_stt_common.

[John Baldwin]

binutils/ChangeLog:

	* objcopy.c (do_elf_stt_common): Use correct type.
---
 binutils/ChangeLog | 4 ++++
 binutils/objcopy.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index f8e7c76..7a7edb9 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,7 @@
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
+	* objcopy.c (do_elf_stt_common): Use correct type.
+
 2016-06-02  Nick Clifton  <nickc@redhat.com>
 
 	PR 20089
diff --git a/binutils/objcopy.c b/binutils/objcopy.c
index 06fcea3..76170cb 100644
--- a/binutils/objcopy.c
+++ b/binutils/objcopy.c
@@ -221,7 +221,7 @@ static enum
 } do_debug_sections = nothing;
 
 /* Whether to generate ELF common symbols with the STT_COMMON type.  */
-static enum bfd_link_discard do_elf_stt_common = unchanged;
+static enum bfd_link_elf_stt_common do_elf_stt_common = unchanged;
 
 /* Whether to change the leading character in symbol names.  */
 static bfd_boolean change_leading_char = FALSE;
-- 
2.7.0

Re: [PATCH 1/8] Use correct enum type for do_elf_stt_common.

[Alan Modra]

On Sat, Jun 11, 2016 at 01:48:28PM -0700, John Baldwin wrote:
> 	* objcopy.c (do_elf_stt_common): Use correct type.

OK (obvious).

-- 
Alan Modra
Australia Development Lab, IBM

[PATCH 2/8] Change the size field of MSP430_Opcode_Decoded to a plain integer.

[John Baldwin]

The size field was defined as an instance of an enum, but existing code
treats the size field as a plain integer containing a bit count.

include/ChangeLog:

	* opcode/msp430-decode.h (MSP430_Size): Remove.
	(Msp430_Opcode_Decoded): Change type of size to int.
---
 include/ChangeLog              | 5 +++++
 include/opcode/msp430-decode.h | 9 +--------
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/include/ChangeLog b/include/ChangeLog
index 722b8a5..97d506e 100644
--- a/include/ChangeLog
+++ b/include/ChangeLog
@@ -1,3 +1,8 @@
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
+	* opcode/msp430-decode.h (MSP430_Size): Remove.
+	(Msp430_Opcode_Decoded): Change type of size to int.
+
 2016-06-11  Alan Modra  <amodra@gmail.com>
 
 	* coff/sparc.h (COFF_ADJUST_SYM_OUT_POST): Define.
diff --git a/include/opcode/msp430-decode.h b/include/opcode/msp430-decode.h
index b19a51a..f9e1b4c 100644
--- a/include/opcode/msp430-decode.h
+++ b/include/opcode/msp430-decode.h
@@ -87,13 +87,6 @@ typedef struct
   unsigned char	       condition : 3;
 } MSP430_Opcode_Operand;
 
-typedef enum
-{
-  MSP430_Byte = 0,
-  MSP430_Word,
-  MSP430_Addr
-} MSP430_Size;
-
 /* These numerically match the bit encoding.  */
 typedef enum
 {
@@ -125,7 +118,7 @@ typedef struct
   unsigned		repeats:5;	/* Contains COUNT-1, or register number.  */
   int			n_bytes;	/* Opcode size in BYTES.  */
   char *		syntax;
-  MSP430_Size		size;		/* Operand size in BITS.  */
+  int			size;		/* Operand size in BITS.  */
   MSP430_Condition	cond;
   /* By convention, these are [0]destination, [1]source.  */
   MSP430_Opcode_Operand	op[2];
-- 
2.7.0

Re: [PATCH 2/8] Change the size field of MSP430_Opcode_Decoded to a plain integer.

[Alan Modra]

On Sat, Jun 11, 2016 at 01:48:29PM -0700, John Baldwin wrote:
> 	* opcode/msp430-decode.h (MSP430_Size): Remove.
> 	(Msp430_Opcode_Decoded): Change type of size to int.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

[PATCH 7/8] Remove unneeded checks on type lengths.

[John Baldwin]

Type lengths are unsigned, so are always greater than zero.  A check
against the length of 'tgt_type' is retained to prevent dividing by zero.
---
 gdb/ChangeLog   | 4 ++++
 gdb/v850-tdep.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d188af9..4c4389d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-06-11  John Baldwin  <jhb@FreeBSD.org>
 
+	* v850-tdep.c (v850_use_struct_convention): Trim type length checks.
+
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
 	* tui/tui-stack.c (tui_show_frame_info): Fix type mismatch.
 
 2016-06-11  John Baldwin  <jhb@FreeBSD.org>
diff --git a/gdb/v850-tdep.c b/gdb/v850-tdep.c
index 612eec3..0b50580 100644
--- a/gdb/v850-tdep.c
+++ b/gdb/v850-tdep.c
@@ -559,7 +559,7 @@ v850_use_struct_convention (struct gdbarch *gdbarch, struct type *type)
 	  if (TYPE_CODE (fld_type) == TYPE_CODE_ARRAY)
 	    {
 	      tgt_type = TYPE_TARGET_TYPE (fld_type);
-	      if (TYPE_LENGTH (fld_type) >= 0 && TYPE_LENGTH (tgt_type) >= 0
+	      if (TYPE_LENGTH (tgt_type) > 0
 		  && TYPE_LENGTH (fld_type) / TYPE_LENGTH (tgt_type) > 2)
 		return 1;
 	    }
-- 
2.7.0

Re: [PATCH 7/8] Remove unneeded checks on type lengths.

[Yao Qi]

On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
> Type lengths are unsigned, so are always greater than zero.  A check

The type length can be zero.

> against the length of 'tgt_type' is retained to prevent dividing by zero.

Your patch is good to me.

-- 
Yao (齐尧)

[PATCH 3/8] Initialize 'ra' to zero to avoid uninitialized use.

[John Baldwin]

If the instruction in this case does not include an RA field, then 'ra'
is used uninitialized.  Use the same idiom used elsewhere in this file of
initializing ra to zero before check for an RA field.

gdb/ChangeLog:

	* rs6000-tdep.c (ppc_process_record_op31): Initialize ra.
---
 gdb/ChangeLog     | 4 ++++
 gdb/rs6000-tdep.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 335476b..38fcb30 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
+	* rs6000-tdep.c (ppc_process_record_op31): Initialize ra.
+
 2016-06-10  Tom Tromey  <tom@tromey.com>
 
 	* gdbtypes.c (arch_type, arch_integer_type, arch_character_type)
diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c
index 645fe92..1ed1354 100644
--- a/gdb/rs6000-tdep.c
+++ b/gdb/rs6000-tdep.c
@@ -4631,6 +4631,7 @@ ppc_process_record_op31 (struct gdbarch *gdbarch, struct regcache *regcache,
 	  || at_dcsz == 0)
 	at_dcsz = 128; /* Assume 128-byte cache line size (POWER8)  */
 
+      ra = 0;
       if (PPC_RA (insn) != 0)
 	regcache_raw_read_unsigned (regcache,
 				    tdep->ppc_gp0_regnum + PPC_RA (insn), &ra);
-- 
2.7.0

Re: [PATCH 3/8] Initialize 'ra' to zero to avoid uninitialized use.

[Yao Qi]

On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
> If the instruction in this case does not include an RA field, then 'ra'
> is used uninitialized.  Use the same idiom used elsewhere in this file of
> initializing ra to zero before check for an RA field.
>
> gdb/ChangeLog:
>
>         * rs6000-tdep.c (ppc_process_record_op31): Initialize ra.

Patch is good to me.

-- 
Yao (齐尧)

[PATCH 5/8] Detect a frameless frame by comparing the FP register to -1.

[John Baldwin]

Since CORE_ADDR is unsigned, the saved FP register is always greater than
or equal to zero.  Comparing to -1 results in a direct comparison with the
value used to initialize the saved FP register to an uninitialized state.
---
 gdb/ChangeLog   | 5 +++++
 gdb/sh64-tdep.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 73f42e2..243b717 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-06-11  John Baldwin  <jhb@FreeBSD.org>
 
+	* sh64-tdep.c (sh64_analyze_prologue): Compare FP register against
+	-1 to detect missing FP.
+
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
 	* score-tdep.c (score7_malloc_and_get_memblock): Remove check for
 	negative size.
 
diff --git a/gdb/sh64-tdep.c b/gdb/sh64-tdep.c
index e6b1e27..f5a6504 100644
--- a/gdb/sh64-tdep.c
+++ b/gdb/sh64-tdep.c
@@ -971,7 +971,7 @@ sh64_analyze_prologue (struct gdbarch *gdbarch,
 	}
     }
 
-  if (cache->saved_regs[MEDIA_FP_REGNUM] >= 0)
+  if (cache->saved_regs[MEDIA_FP_REGNUM] == -1)
     cache->uses_fp = 1;
 }
 
-- 
2.7.0

Re: [PATCH 5/8] Detect a frameless frame by comparing the FP register to -1.

[Yao Qi]

On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
>
> -  if (cache->saved_regs[MEDIA_FP_REGNUM] >= 0)
> +  if (cache->saved_regs[MEDIA_FP_REGNUM] == -1)
>      cache->uses_fp = 1;

I suspect it should be " != -1".  saved_regs[MEDIA_FP_REGNUM] is initialized
to -1, so if it is not the initialized value (-1), FP should be set in
the prologue, and mark the flag uses_fp.

-- 
Yao (齐尧)

Re: [PATCH 5/8] Detect a frameless frame by comparing the FP register to -1.

[John Baldwin]

On Monday, June 13, 2016 12:22:41 PM Yao Qi wrote:
> On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
> >
> > -  if (cache->saved_regs[MEDIA_FP_REGNUM] >= 0)
> > +  if (cache->saved_regs[MEDIA_FP_REGNUM] == -1)
> >      cache->uses_fp = 1;
> 
> I suspect it should be " != -1".  saved_regs[MEDIA_FP_REGNUM] is initialized
> to -1, so if it is not the initialized value (-1), FP should be set in
> the prologue, and mark the flag uses_fp.

Yes, you are correct.  Alternatively, we could change this code to explicitly
set 'cache->uses_fp' in the cases earlier in this function where it sets
MEDIA_FP_REGNUM to a value.  sh-tdep.c follows this model, but it only needs
to set uses_fp in a few places.

-- 
John Baldwin

Re: [PATCH 5/8] Detect a frameless frame by comparing the FP register to -1.

[Yao Qi]

On Tue, Jun 14, 2016 at 7:46 PM, John Baldwin <jhb@freebsd.org> wrote:
> On Monday, June 13, 2016 12:22:41 PM Yao Qi wrote:
>> On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
>> >
>> > -  if (cache->saved_regs[MEDIA_FP_REGNUM] >= 0)
>> > +  if (cache->saved_regs[MEDIA_FP_REGNUM] == -1)
>> >      cache->uses_fp = 1;
>>
>> I suspect it should be " != -1".  saved_regs[MEDIA_FP_REGNUM] is initialized
>> to -1, so if it is not the initialized value (-1), FP should be set in
>> the prologue, and mark the flag uses_fp.
>
> Yes, you are correct.  Alternatively, we could change this code to explicitly
> set 'cache->uses_fp' in the cases earlier in this function where it sets
> MEDIA_FP_REGNUM to a value.  sh-tdep.c follows this model, but it only needs
> to set uses_fp in a few places.
>

That is fine by me.

-- 
Yao (齐尧)

[PATCH 8/8] Use unsigned integer constant with left shifts.

[John Baldwin]

This avoids undefined behavior.

gdb/ChangeLog:

	* ada-lang.c (ada_unpack_from_contents): Use unsigned constants with
	left shifts.
---
 gdb/ChangeLog  | 5 +++++
 gdb/ada-lang.c | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4c4389d..1077e2f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-06-11  John Baldwin  <jhb@FreeBSD.org>
 
+	* ada-lang.c (ada_unpack_from_contents): Use unsigned constants with
+	left shifts.
+
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
 	* v850-tdep.c (v850_use_struct_convention): Trim type length checks.
 
 2016-06-11  John Baldwin  <jhb@FreeBSD.org>
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 412aa97..0e951a4 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -2486,7 +2486,7 @@ ada_unpack_from_contents (const gdb_byte *src, int bit_offset, int bit_size,
       accumSize += HOST_CHAR_BIT - unusedLS;
       if (accumSize >= HOST_CHAR_BIT)
         {
-          unpacked[unpacked_idx] = accum & ~(~0L << HOST_CHAR_BIT);
+          unpacked[unpacked_idx] = accum & ~(~0UL << HOST_CHAR_BIT);
           accumSize -= HOST_CHAR_BIT;
           accum >>= HOST_CHAR_BIT;
           unpacked_bytes_left -= 1;
@@ -2500,7 +2500,7 @@ ada_unpack_from_contents (const gdb_byte *src, int bit_offset, int bit_size,
   while (unpacked_bytes_left > 0)
     {
       accum |= sign << accumSize;
-      unpacked[unpacked_idx] = accum & ~(~0L << HOST_CHAR_BIT);
+      unpacked[unpacked_idx] = accum & ~(~0UL << HOST_CHAR_BIT);
       accumSize -= HOST_CHAR_BIT;
       if (accumSize < 0)
 	accumSize = 0;
-- 
2.7.0

[PATCH 6/8] Pass a NULL pointer as the last argument to find_pc_partial_function.

[John Baldwin]

gdb/ChangeLog:

	* tui/tui-stack.c (tui_show_frame_info): Fix type mismatch.
---
 gdb/ChangeLog       | 4 ++++
 gdb/tui/tui-stack.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 243b717..d188af9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2016-06-11  John Baldwin  <jhb@FreeBSD.org>
 
+	* tui/tui-stack.c (tui_show_frame_info): Fix type mismatch.
+
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
 	* sh64-tdep.c (sh64_analyze_prologue): Compare FP register against
 	-1 to detect missing FP.
 
diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
index da6c7b1..b6cf144 100644
--- a/gdb/tui/tui-stack.c
+++ b/gdb/tui/tui-stack.c
@@ -416,7 +416,7 @@ tui_show_frame_info (struct frame_info *fi)
 	    {
 	      if (find_pc_partial_function (get_frame_pc (fi),
 					    (const char **) NULL,
-					    &low, (CORE_ADDR) 0) == 0)
+					    &low, NULL) == 0)
 		{
 		  /* There is no symbol available for current PC.  There is no
 		     safe way how to "disassemble backwards".  */
-- 
2.7.0

Re: [PATCH 6/8] Pass a NULL pointer as the last argument to find_pc_partial_function.

[Yao Qi]

On Sat, Jun 11, 2016 at 9:48 PM, John Baldwin <jhb@freebsd.org> wrote:
> gdb/ChangeLog:
>
>         * tui/tui-stack.c (tui_show_frame_info): Fix type mismatch.

Patch is good to me.

-- 
Yao (齐尧)

[PATCH 4/8] Remove check for negative size.

[John Baldwin]

Since CORE_ADDR is unsigned, this value can never be negative.

gdb/ChangeLog:

	* score-tdep.c (score7_malloc_and_get_memblock): Remove check for
	negative size.
---
 gdb/ChangeLog    | 5 +++++
 gdb/score-tdep.c | 8 +-------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 38fcb30..73f42e2 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2016-06-11  John Baldwin  <jhb@FreeBSD.org>
 
+	* score-tdep.c (score7_malloc_and_get_memblock): Remove check for
+	negative size.
+
+2016-06-11  John Baldwin  <jhb@FreeBSD.org>
+
 	* rs6000-tdep.c (ppc_process_record_op31): Initialize ra.
 
 2016-06-10  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/score-tdep.c b/gdb/score-tdep.c
index 0d817f8..8e08d05 100644
--- a/gdb/score-tdep.c
+++ b/gdb/score-tdep.c
@@ -813,13 +813,7 @@ score7_malloc_and_get_memblock (CORE_ADDR addr, CORE_ADDR size)
   int ret;
   gdb_byte *memblock = NULL;
 
-  if (size < 0)
-    {
-      error (_("Error: malloc size < 0 in file:%s, line:%d!"),
-             __FILE__, __LINE__);
-      return NULL;
-    }
-  else if (size == 0)
+  if (size == 0)
     return NULL;
 
   memblock = (gdb_byte *) xmalloc (size);
-- 
2.7.0