diff mbox series

Fix -Wmaybe-uninitialized warning in opcodes/i386-dis.c

Message ID 20230421130802.2964785-1-tromey@adacore.com
State Unresolved
Headers
Series Fix -Wmaybe-uninitialized warning in opcodes/i386-dis.c |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

Tom Tromey April 21, 2023, 1:08 p.m. UTC 
  A recent change in opcodes/i386-dis.c caused a build failure on my
x86-64 Fedora 36 system:

../../binutils-gdb/opcodes/i386-dis.c: In function ‘OP_J’:
../../binutils-gdb/opcodes/i386-dis.c:12705:22: error: ‘val’ may be used uninitialized [-Werror=maybe-uninitialized]
12705 |           disp = val & 0x8000 ? val - 0x10000 : val;
      |                  ~~~~^~~~~~~~

This patch fixes the warning, but I don't know whether it's really
correct -- I just copied a pattern from eariler in this function.

opcodes/ChangeLog
2023-04-21  Tom Tromey  <tromey@adacore.com>

	* i386-dis.c (OP_J): Check result of get16.
---
 opcodes/ChangeLog  | 4 ++++
 opcodes/i386-dis.c | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Jan Beulich April 21, 2023, 1:24 p.m. UTC | #1 
On 21.04.2023 15:08, Tom Tromey wrote:
> A recent change in opcodes/i386-dis.c caused a build failure on my
> x86-64 Fedora 36 system:
> 
> ../../binutils-gdb/opcodes/i386-dis.c: In function ‘OP_J’:
> ../../binutils-gdb/opcodes/i386-dis.c:12705:22: error: ‘val’ may be used uninitialized [-Werror=maybe-uninitialized]
> 12705 |           disp = val & 0x8000 ? val - 0x10000 : val;
>       |                  ~~~~^~~~~~~~

Would be nice if you could (also) state the compiler variant and
version, as that's generally more meaningful than a distro one.

> This patch fixes the warning, but I don't know whether it's really
> correct -- I just copied a pattern from eariler in this function.

It is correct, and it was my bad to not check the function's return
value. Please commit.

Jan

> opcodes/ChangeLog
> 2023-04-21  Tom Tromey  <tromey@adacore.com>
> 
> 	* i386-dis.c (OP_J): Check result of get16.
> ---
>  opcodes/ChangeLog  | 4 ++++
>  opcodes/i386-dis.c | 3 ++-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
> index 46ba01fe093..f021bdaa3e7 100644
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -12701,7 +12701,8 @@ OP_J (instr_info *ins, int bytemode, int sizeflag)
>  	{
>  	  int val;
>  
> -	  get16 (ins, &val);
> +	  if (!get16 (ins, &val))
> +	    return false;
>  	  disp = val & 0x8000 ? val - 0x10000 : val;
>  	  /* In 16bit mode, address is wrapped around at 64k within
>  	     the same segment.  Otherwise, a data16 prefix on a jump
Tom Tromey April 21, 2023, 3:07 p.m. UTC | #2 
>>>>> "Jan" == Jan Beulich <jbeulich@suse.com> writes:

Jan> Would be nice if you could (also) state the compiler variant and
Jan> version, as that's generally more meaningful than a distro one.

I updated the commit message with this info.

>> This patch fixes the warning, but I don't know whether it's really
>> correct -- I just copied a pattern from eariler in this function.

Jan> It is correct, and it was my bad to not check the function's return
Jan> value. Please commit.

Thank you for the review.  I'm checking it in.

Tom
diff mbox series

Patch

diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index 46ba01fe093..f021bdaa3e7 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -12701,7 +12701,8 @@  OP_J (instr_info *ins, int bytemode, int sizeflag)
 	{
 	  int val;
 
-	  get16 (ins, &val);
+	  if (!get16 (ins, &val))
+	    return false;
 	  disp = val & 0x8000 ? val - 0x10000 : val;
 	  /* In 16bit mode, address is wrapped around at 64k within
 	     the same segment.  Otherwise, a data16 prefix on a jump