Patchwork compile-loc2c: Fix uninitialized variable error

login
register
mail settings
Submitter Simon Marchi
Date June 10, 2017, 7:49 p.m.
Message ID <1497124148-11187-1-git-send-email-simon.marchi@ericsson.com>
Download mbox | patch
Permalink /patch/20904/
State New
Headers show

Comments

Simon Marchi - June 10, 2017, 7:49 p.m.
Compiling with clang gives this warning/error:

  /home/emaisin/src/binutils-gdb/gdb/compile/compile-loc2c.c:731:6: error: variable 'uoffset' is uninitialized when used here [-Werror,-Wuninitialized]
              uoffset += dwarf2_per_cu_text_offset (per_cu);
              ^~~~~~~
  /home/emaisin/src/binutils-gdb/gdb/compile/compile-loc2c.c:669:23: note: initialize the variable 'uoffset' to silence this warning
        uint64_t uoffset, reg;
                        ^
                         = 0

I am really not sure if what this patch does is good, but it is my best
guess.  DW_OP_addr means that there's an constant address provided by
the DWARF bytecode that should be pushed on the stack.  That address is
considered skipped by the "op_ptr += addr_size", but it is never read.
uoffset is indeed read just after, without having been assigned first.

So I think the intent is to read the address, it was just omitted.

gdb/ChangeLog:

	* compile/compile-loc2c.c (do_compile_dwarf_expr_to_c): Read
	address when op is DW_OP_addr.
---
 gdb/compile/compile-loc2c.c | 1 +
 1 file changed, 1 insertion(+)
Simon Marchi - June 25, 2017, 5:24 p.m.
On 2017-06-10 21:49, Simon Marchi wrote:
> Compiling with clang gives this warning/error:
> 
>   /home/emaisin/src/binutils-gdb/gdb/compile/compile-loc2c.c:731:6:
> error: variable 'uoffset' is uninitialized when used here
> [-Werror,-Wuninitialized]
>               uoffset += dwarf2_per_cu_text_offset (per_cu);
>               ^~~~~~~
>   /home/emaisin/src/binutils-gdb/gdb/compile/compile-loc2c.c:669:23:
> note: initialize the variable 'uoffset' to silence this warning
>         uint64_t uoffset, reg;
>                         ^
>                          = 0
> 
> I am really not sure if what this patch does is good, but it is my best
> guess.  DW_OP_addr means that there's an constant address provided by
> the DWARF bytecode that should be pushed on the stack.  That address is
> considered skipped by the "op_ptr += addr_size", but it is never read.
> uoffset is indeed read just after, without having been assigned first.
> 
> So I think the intent is to read the address, it was just omitted.
> 
> gdb/ChangeLog:
> 
> 	* compile/compile-loc2c.c (do_compile_dwarf_expr_to_c): Read
> 	address when op is DW_OP_addr.
> ---
>  gdb/compile/compile-loc2c.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
> index a53214f..ead1003 100644
> --- a/gdb/compile/compile-loc2c.c
> +++ b/gdb/compile/compile-loc2c.c
> @@ -722,6 +722,7 @@ do_compile_dwarf_expr_to_c (int indent, string_file 
> &stream,
>  	  break;
> 
>  	case DW_OP_addr:
> +	  uoffset = extract_unsigned_integer (op_ptr, addr_size, byte_order);
>  	  op_ptr += addr_size;
>  	  /* Some versions of GCC emit DW_OP_addr before
>  	     DW_OP_GNU_push_tls_address.  In this case the value is an

Hi Tom,

As you are the original author of that code, would it be possible for 
you to take a quick look, if you remember any of this :) ?

Thanks,

Simon

Patch

diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
index a53214f..ead1003 100644
--- a/gdb/compile/compile-loc2c.c
+++ b/gdb/compile/compile-loc2c.c
@@ -722,6 +722,7 @@  do_compile_dwarf_expr_to_c (int indent, string_file &stream,
 	  break;
 
 	case DW_OP_addr:
+	  uoffset = extract_unsigned_integer (op_ptr, addr_size, byte_order);
 	  op_ptr += addr_size;
 	  /* Some versions of GCC emit DW_OP_addr before
 	     DW_OP_GNU_push_tls_address.  In this case the value is an