[PATCH]: Tracking and reporting uninitialized variables

[PATCH]: Tracking and reporting uninitialized variables

[Caroline Tice]

[-- Attachment #1: Type: text/plain, Size: 2206 bytes --]


As part of some work I have been doing on improving debugging of  
optimized code, I
have created a GCC patch that tags variables it believes are  
uninitialized with a new
Dwarf op (an extension), DW_OP_GNU_uninit.   I have submitted that  
patch to the
GCC patches list and am waiting for approval to commit it.   I have  
also created the
following gdb patch to recognize the new op and inform the user when a  
variable
the user requests to see is uninitialized.

I have tested this patch on some small testcases and I have run the  
gdb dejagnu
testsuite with no regressions.  I am new to submitting things to this  
list, so if there
is anything else I ought to have done, please let me know (kindly!).

Is this patch okay to commit to gdb?

-- Caroline Tice
ctice@apple.com



2007-05-01  Caroline Tice  <ctice@apple.com>

         * c-valprint.c (c_value_print):  If the var_status field of the
         value struct is 0, print out "[uninitialized]" before the  
value.
         * dwarf2expr.c (add_piece): Make function non-static.
         (unsigned_address_type): Likewise.
         (signed_address_type): Likewise.
         (execute_stack_op): Initialize ctx->var_status field; allow
         DW_OP_GNU_uninit as legal op following a DW_OP_reg op or a
         DW_OP_regx op; add case for DW_OP_GNU_uninit and update
         ctx->var_status appropriately.
         * dwarf2expr.h (struct dwarf_expr_context): New field,  
var_status.
         (unsigned_address_type): Add extern declaration.
         (signed_address_type): Likewise.
         (add_piece): Likewise.
         * dwarf2loc.c (dwarf2_evaluate_loc_desc): Add call to
         set_var_status.
         * dwarf2read.c (dwarf_stack_op_name): Add case for  
DW_OP_GNU_uninit.
         (decode_locdesc): Add case for DW_OP_GNU_uninit.
         * value.c (struct value):  New field, var_status.
         (allocate_value): Initialize new field.
         (set_var_status): New function.
         (value_var_status): New function.
         * value.h (value_var_status): New extern declaration.
         (set_var_status): Likewise.
         * include/elf/dwarf2.h: (enum dwarf_location_atom): Add new  
DW_OP,
         DW_OP_GNU_uninit.



[-- Attachment #2: fsf-gdb-patch.txt --]
[-- Type: text/plain, Size: 9486 bytes --]

Index: gdb/c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.42
diff -c -3 -p -r1.42 c-valprint.c
*** gdb/c-valprint.c	26 Jan 2007 20:54:16 -0000	1.42
--- gdb/c-valprint.c	1 May 2007 22:10:43 -0000
*************** c_value_print (struct value *val, struct
*** 556,561 ****
--- 556,564 ----
  	}
      }
  
+   if (value_var_status (val) == 0)
+     fprintf_filtered (stream, " [uninitialized] ");
+ 
    if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
      {
        /* Attempt to determine real type of object */
Index: gdb/dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.19
diff -c -3 -p -r1.19 dwarf2expr.c
*** gdb/dwarf2expr.c	9 Jan 2007 17:58:50 -0000	1.19
--- gdb/dwarf2expr.c	1 May 2007 22:10:43 -0000
*************** dwarf_expr_fetch (struct dwarf_expr_cont
*** 106,112 ****
  }
  
  /* Add a new piece to CTX's piece list.  */
! static void
  add_piece (struct dwarf_expr_context *ctx,
             int in_reg, CORE_ADDR value, ULONGEST size)
  {
--- 106,112 ----
  }
  
  /* Add a new piece to CTX's piece list.  */
! void
  add_piece (struct dwarf_expr_context *ctx,
             int in_reg, CORE_ADDR value, ULONGEST size)
  {
*************** dwarf2_read_address (gdb_byte *buf, gdb_
*** 213,219 ****
  
  /* Return the type of an address, for unsigned arithmetic.  */
  
! static struct type *
  unsigned_address_type (void)
  {
    switch (TARGET_ADDR_BIT / TARGET_CHAR_BIT)
--- 213,219 ----
  
  /* Return the type of an address, for unsigned arithmetic.  */
  
! struct type *
  unsigned_address_type (void)
  {
    switch (TARGET_ADDR_BIT / TARGET_CHAR_BIT)
*************** unsigned_address_type (void)
*** 232,238 ****
  
  /* Return the type of an address, for signed arithmetic.  */
  
! static struct type *
  signed_address_type (void)
  {
    switch (TARGET_ADDR_BIT / TARGET_CHAR_BIT)
--- 232,238 ----
  
  /* Return the type of an address, for signed arithmetic.  */
  
! struct type *
  signed_address_type (void)
  {
    switch (TARGET_ADDR_BIT / TARGET_CHAR_BIT)
*************** execute_stack_op (struct dwarf_expr_cont
*** 257,262 ****
--- 257,263 ----
  		  gdb_byte *op_ptr, gdb_byte *op_end)
  {
    ctx->in_reg = 0;
+   ctx->var_status = 1;  /* Default is initialized.  */
  
    while (op_ptr < op_end)
      {
*************** execute_stack_op (struct dwarf_expr_cont
*** 383,389 ****
  	case DW_OP_reg29:
  	case DW_OP_reg30:
  	case DW_OP_reg31:
! 	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
  	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
  		   "used either alone or in conjuction with DW_OP_piece."));
  
--- 384,392 ----
  	case DW_OP_reg29:
  	case DW_OP_reg30:
  	case DW_OP_reg31:
! 	  if (op_ptr != op_end 
! 	      && *op_ptr != DW_OP_piece
! 	      && *op_ptr != DW_OP_GNU_uninit)
  	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
  		   "used either alone or in conjuction with DW_OP_piece."));
  
*************** execute_stack_op (struct dwarf_expr_cont
*** 394,400 ****
  
  	case DW_OP_regx:
  	  op_ptr = read_uleb128 (op_ptr, op_end, &reg);
! 	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
  	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
  		   "used either alone or in conjuction with DW_OP_piece."));
  
--- 397,405 ----
  
  	case DW_OP_regx:
  	  op_ptr = read_uleb128 (op_ptr, op_end, &reg);
! 	  if (op_ptr != op_end 
! 	      && *op_ptr != DW_OP_piece
! 	      && *op_ptr != DW_OP_GNU_uninit)
  	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
  		   "used either alone or in conjuction with DW_OP_piece."));
  
*************** execute_stack_op (struct dwarf_expr_cont
*** 704,709 ****
--- 709,718 ----
            }
            goto no_push;
  
+ 	case DW_OP_GNU_uninit:
+ 	  ctx->var_status = 0;
+ 	  goto no_push;
+ 
  	default:
  	  error (_("Unhandled dwarf expression opcode 0x%x"), op);
  	}
Index: gdb/dwarf2expr.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
retrieving revision 1.9
diff -c -3 -p -r1.9 dwarf2expr.h
*** gdb/dwarf2expr.h	9 Jan 2007 17:58:50 -0000	1.9
--- gdb/dwarf2expr.h	1 May 2007 22:10:43 -0000
*************** struct dwarf_expr_context
*** 76,81 ****
--- 76,84 ----
       will be on the expression stack.  */
    int in_reg;
  
+   /* Initialization status of variable.  */
+   int var_status;
+ 
    /* An array of pieces.  PIECES points to its first element;
       NUM_PIECES is its length.
  
*************** gdb_byte *read_sleb128 (gdb_byte *buf, g
*** 135,138 ****
--- 138,144 ----
  CORE_ADDR dwarf2_read_address (gdb_byte *buf, gdb_byte *buf_end,
  			       int *bytes_read);
  
+ extern struct type *unsigned_address_type (void);
+ extern struct type *signed_address_type (void);
+ extern void add_piece (struct dwarf_expr_context *, int, CORE_ADDR, ULONGEST);
  #endif /* dwarf2expr.h */
Index: gdb/dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.39
diff -c -3 -p -r1.39 dwarf2loc.c
*** gdb/dwarf2loc.c	24 Jan 2007 22:04:48 -0000	1.39
--- gdb/dwarf2loc.c	1 May 2007 22:10:43 -0000
*************** dwarf2_evaluate_loc_desc (struct symbol 
*** 256,261 ****
--- 256,263 ----
        VALUE_ADDRESS (retval) = address;
      }
  
+   set_var_status (retval, ctx->var_status);
+ 
    free_dwarf_expr_context (ctx);
  
    return retval;
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.218
diff -c -3 -p -r1.218 dwarf2read.c
*** gdb/dwarf2read.c	18 Apr 2007 13:25:04 -0000	1.218
--- gdb/dwarf2read.c	1 May 2007 22:10:44 -0000
*************** dwarf_stack_op_name (unsigned op)
*** 8629,8634 ****
--- 8629,8636 ----
        return "DW_OP_bit_piece";
      case DW_OP_GNU_push_tls_address:
        return "DW_OP_GNU_push_tls_address";
+     case DW_OP_GNU_uninit:
+       return "DW_OP_GNU_uninit";
      /* HP extensions. */ 
      case DW_OP_HP_is_value:
        return "DW_OP_HP_is_value";
*************** decode_locdesc (struct dwarf_block *blk,
*** 9204,9209 ****
--- 9206,9214 ----
  	    dwarf2_complex_location_expr_complaint ();
            break;
  
+ 	case DW_OP_GNU_uninit:
+ 	  break;
+ 
  	default:
  	  complaint (&symfile_complaints, _("unsupported stack op: '%s'"),
  		     dwarf_stack_op_name (op));
Index: gdb/value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.41
diff -c -3 -p -r1.41 value.c
*** gdb/value.c	13 Apr 2007 14:17:46 -0000	1.41
--- gdb/value.c	1 May 2007 22:10:44 -0000
*************** struct value
*** 157,162 ****
--- 157,165 ----
       actually exist in the program.  */
    char optimized_out;
  
+   /* If value is a variable, is it initialized or not.  */
+   int var_status;
+ 
    /* Actual contents of the value.  For use of this value; setting it
       uses the stuff above.  Not valid if lazy is nonzero.  Target
       byte-order.  We force it to be aligned properly for any possible
*************** allocate_value (struct type *type)
*** 232,237 ****
--- 235,241 ----
    val->embedded_offset = 0;
    val->pointed_to_offset = 0;
    val->modifiable = 1;
+   val->var_status = 1;  /* Default to initialized.  */
    return val;
  }
  
*************** using_struct_return (struct type *value_
*** 1691,1696 ****
--- 1695,1717 ----
  	  != RETURN_VALUE_REGISTER_CONVENTION);
  }
  
+ /* Set the var_status field in a value struct.  */
+ 
+ void
+ set_var_status (struct value *val, int status)
+ {
+   val->var_status = status;
+ }
+ 
+ 
+ /* Return the var_status field in a value struct.  */
+ 
+ int
+ value_var_status (struct value *val)
+ {
+   return val->var_status;
+ }
+ 
  void
  _initialize_values (void)
  {
Index: gdb/value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.96
diff -c -3 -p -r1.96 value.h
*** gdb/value.h	9 Jan 2007 17:58:59 -0000	1.96
--- gdb/value.h	1 May 2007 22:10:44 -0000
*************** extern int value_contents_equal (struct 
*** 193,198 ****
--- 193,202 ----
  extern int value_optimized_out (struct value *value);
  extern void set_value_optimized_out (struct value *value, int val);
  
+ /* */
+ extern int value_var_status (struct value *);
+ extern void set_var_status (struct value *, int);
+ 
  /* While the following fields are per- VALUE .CONTENT .PIECE (i.e., a
     single value might have multiple LVALs), this hacked interface is
     limited to just the first PIECE.  Expect further change.  */
Index: include/elf/dwarf2.h
===================================================================
RCS file: /cvs/src/src/include/elf/dwarf2.h,v
retrieving revision 1.19
diff -c -3 -p -r1.19 dwarf2.h
*** include/elf/dwarf2.h	2 Mar 2006 00:54:27 -0000	1.19
--- include/elf/dwarf2.h	1 May 2007 22:10:44 -0000
*************** enum dwarf_location_atom
*** 540,545 ****
--- 540,546 ----
      DW_OP_bit_piece = 0x9d,
      /* GNU extensions.  */
      DW_OP_GNU_push_tls_address = 0xe0,
+     DW_OP_GNU_uninit     = 0xf0,
      /* HP extensions.  */
      DW_OP_HP_unknown     = 0xe0, /* Ouch, the same as GNU_push_tls_address.  */
      DW_OP_HP_is_value    = 0xe1,

Re: [PATCH]: Tracking and reporting uninitialized variables

[Eli Zaretskii]

> From: Caroline Tice <ctice@apple.com>
> Date: Tue, 1 May 2007 17:14:39 -0700
> Cc: Caroline Tice <ctice@apple.com>
> 
> As part of some work I have been doing on improving debugging of
> optimized code, I have created a GCC patch that tags variables it
> believes are uninitialized with a new Dwarf op (an extension),
> DW_OP_GNU_uninit.  I have submitted that patch to the GCC patches
> list and am waiting for approval to commit it.  I have also created
> the following gdb patch to recognize the new op and inform the user
> when a variable the user requests to see is uninitialized.

Thanks.

What would be really nice is if you'd add a feature whereby I could
catch references to uninitialized variables, something like

   (gdb) rwatch foo if uninitialized

That would cause GDB to stop the program whenever it tries to use the
value of a variable that wasn't initialized.

Re: [PATCH]: Tracking and reporting uninitialized variables

[Eli Zaretskii]

> From: Caroline Tice <ctice@apple.com>
> Date: Wed, 2 May 2007 15:13:59 -0700
> 
> Question:  Are you saying that this is something I need to do before  
> the patch
> would be approved, or are you just saying this is something that would  
> be
> nice to have in addition once my patch has been committed?

The latter, I guess.

Ping! [PATCH]: Tracking and reporting uninitialized variables

[Caroline Tice]

On May 1, 2007, at 5:14 PM, Caroline Tice wrote:

>
> As part of some work I have been doing on improving debugging of  
> optimized code, I
> have created a GCC patch that tags variables it believes are  
> uninitialized with a new
> Dwarf op (an extension), DW_OP_GNU_uninit.   I have submitted that  
> patch to the
> GCC patches list and am waiting for approval to commit it.   I have  
> also created the
> following gdb patch to recognize the new op and inform the user when  
> a variable
> the user requests to see is uninitialized.
>
> I have tested this patch on some small testcases and I have run the  
> gdb dejagnu
> testsuite with no regressions.  I am new to submitting things to  
> this list, so if there
> is anything else I ought to have done, please let me know (kindly!).
>
> Is this patch okay to commit to gdb?
>
> -- Caroline Tice
> ctice@apple.com
>
>
>
> 2007-05-01  Caroline Tice  <ctice@apple.com>
>
>        * c-valprint.c (c_value_print):  If the var_status field of the
>        value struct is 0, print out "[uninitialized]" before the  
> value.
>        * dwarf2expr.c (add_piece): Make function non-static.
>        (unsigned_address_type): Likewise.
>        (signed_address_type): Likewise.
>        (execute_stack_op): Initialize ctx->var_status field; allow
>        DW_OP_GNU_uninit as legal op following a DW_OP_reg op or a
>        DW_OP_regx op; add case for DW_OP_GNU_uninit and update
>        ctx->var_status appropriately.
>        * dwarf2expr.h (struct dwarf_expr_context): New field,  
> var_status.
>        (unsigned_address_type): Add extern declaration.
>        (signed_address_type): Likewise.
>        (add_piece): Likewise.
>        * dwarf2loc.c (dwarf2_evaluate_loc_desc): Add call to
>        set_var_status.
>        * dwarf2read.c (dwarf_stack_op_name): Add case for  
> DW_OP_GNU_uninit.
>        (decode_locdesc): Add case for DW_OP_GNU_uninit.
>        * value.c (struct value):  New field, var_status.
>        (allocate_value): Initialize new field.
>        (set_var_status): New function.
>        (value_var_status): New function.
>        * value.h (value_var_status): New extern declaration.
>        (set_var_status): Likewise.
>        * include/elf/dwarf2.h: (enum dwarf_location_atom): Add new  
> DW_OP,
>        DW_OP_GNU_uninit.
>
>
> <fsf-gdb-patch.txt>

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Jim Blandy]

Caroline Tice <ctice@apple.com> writes:
>> As part of some work I have been doing on improving debugging of
>> optimized code, I
>> have created a GCC patch that tags variables it believes are
>> uninitialized with a new
>> Dwarf op (an extension), DW_OP_GNU_uninit.   I have submitted that
>> patch to the
>> GCC patches list and am waiting for approval to commit it.   I have
>> also created the
>> following gdb patch to recognize the new op and inform the user when
>> a variable
>> the user requests to see is uninitialized.
>>
>> I have tested this patch on some small testcases and I have run the
>> gdb dejagnu
>> testsuite with no regressions.  I am new to submitting things to
>> this list, so if there
>> is anything else I ought to have done, please let me know
>> (kindly!).

Hi, Caroline.  This seems like a nice patch.

In a multi-piece location expression, can each piece be individually
initialized or uninitialized?  If that's so, then there should also be
an 'initialized' member of 'struct dwarf_expr_piece', which gets set
appropriately for each piece in a multi-piece location expression.

Either way, the code for DW_OP_GNU_uninit should check that it's the
last opcode in the piece or in the entire expression, as the
DW_OP_reg* cases do.

I think the 'struct dwarf_expr_context' member should be named
simply 'initialized', instead of 'var_status'.  The 'struct value'
field should be named 'initialized', and the accessor functions should
be named 'value_initialized' and 'set_value_initialized'.  The comment
in value.h actually needs to be filled in; the description should be
thorough enough to allow someone who otherwise knows how GDB works to
use those functions, without reading their definitions.

I couldn't see from your patch why 'signed_address_type',
'unsigned_address_type', and 'add_piece' were made visible outside
dwarf2expr.c; that should be left out of the patch if it's not needed.

Have you filed a copyright assignment with the FSF?

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Jim Ingham]

Apple has a blanket copyright assignment for gdb.

Jim

On May 8, 2007, at 5:33 PM, Jim Blandy wrote:

> Have you filed a copyright assignment with the FSF?

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Jim Blandy]

Jim Ingham <jingham@apple.com> writes:
> Apple has a blanket copyright assignment for gdb.

Okay --- thanks.

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Caroline Tice]

On May 8, 2007, at 5:33 PM, Jim Blandy wrote:

>
> Caroline Tice <ctice@apple.com> writes:
>>> As part of some work I have been doing on improving debugging of
>>> optimized code, I
>>> have created a GCC patch that tags variables it believes are
>>> uninitialized with a new
>>> Dwarf op (an extension), DW_OP_GNU_uninit.   I have submitted that
>>> patch to the
>>> GCC patches list and am waiting for approval to commit it.   I have
>>> also created the
>>> following gdb patch to recognize the new op and inform the user when
>>> a variable
>>> the user requests to see is uninitialized.
>>>
>>> I have tested this patch on some small testcases and I have run the
>>> gdb dejagnu
>>> testsuite with no regressions.  I am new to submitting things to
>>> this list, so if there
>>> is anything else I ought to have done, please let me know
>>> (kindly!).
>
> Hi, Caroline.  This seems like a nice patch.
>
> In a multi-piece location expression, can each piece be individually
> initialized or uninitialized?  If that's so, then there should also be
> an 'initialized' member of 'struct dwarf_expr_piece', which gets set
> appropriately for each piece in a multi-piece location expression.
>

Okay, will do.  (Yes, I believe each piece can be individually
initialized or uninitialized.)

> Either way, the code for DW_OP_GNU_uninit should check that it's the
> last opcode in the piece or in the entire expression, as the
> DW_OP_reg* cases do.
>

Will do.

> I think the 'struct dwarf_expr_context' member should be named
> simply 'initialized', instead of 'var_status'.  The 'struct value'
> field should be named 'initialized', and the accessor functions should
> be named 'value_initialized' and 'set_value_initialized'.  The comment
> in value.h actually needs to be filled in; the description should be
> thorough enough to allow someone who otherwise knows how GDB works to
> use those functions, without reading their definitions.
>

Will do.

> I couldn't see from your patch why 'signed_address_type',
> 'unsigned_address_type', and 'add_piece' were made visible outside
> dwarf2expr.c; that should be left out of the patch if it's not needed.
>

I went back and checked; making them globally visible was actually
for a different patch I did.  Sorry; I will remove that from this patch.

> Have you filed a copyright assignment with the FSF?

Jim Ingham answered this one.

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Caroline Tice]

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

Okay, here is the modified patch.  I went back and double
checked  the variable tracking stuff in GCC and discovered
that there is NOT a way to mark individual pieces in a multi-piece
location expression as initialized or not; it's one initialized value  
for
the whole thing.  Therefore I did not add the initialized field to
the dwarf_expr_piece as suggested below.  But I did address
everything else.

I tested it by running it on a small test case I have
(with DW_OP_GNU_uninit ops in it), as well as running the
dejagnu testsuite with no regressions.  Is this modified patch okay
to commit to FSF GDB?

-- Caroline Tice
ctice@apple.com

2007-05-09  Caroline Tice  <ctice@apple.com>

         * c-valprint.c (c_value_print):  If the initialized field of  
the
         value struct is 0, print out "[uninitialized]" before the  
value.
         * dwarf2expr.c (execute_stack_op): Initialize ctx- 
 >initialized field;
         allow DW_OP_GNU_uninit as legal op following a DW_OP_reg op  
or a
         DW_OP_regx op; add case for DW_OP_GNU_uninit and update
         ctx->initialized appropriately. Verify no location op follows
         DW_OP_GNU_uninit.
         * dwarf2expr.h (struct dwarf_expr_context): New field,  
initialized.
         * dwarf2loc.c (dwarf2_evaluate_loc_desc): Add call to
         set_value_initialized.
         * dwarf2read.c (dwarf_stack_op_name): Add case for  
DW_OP_GNU_uninit.
         (decode_locdesc): Add case for DW_OP_GNU_uninit.
         * value.c (struct value):  New field, initialized.
         (allocate_value): Initialize new field.
         (set_value_initialized): New function.
         (value_initialized): New function.
         * value.h (value_initialized): New extern declaration.
         (set_value_initialized): Likewise.
         * include/elf/dwarf2.h: (enum dwarf_location_atom): Add new  
DW_OP,
         DW_OP_GNU_uninit.


[-- Attachment #2: fsf-gdb-patch2.txt --]
[-- Type: text/plain, Size: 7542 bytes --]

Index: gdb/c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.42
diff -c -3 -p -r1.42 c-valprint.c
*** gdb/c-valprint.c	26 Jan 2007 20:54:16 -0000	1.42
--- gdb/c-valprint.c	9 May 2007 20:53:21 -0000
*************** c_value_print (struct value *val, struct
*** 556,561 ****
--- 556,564 ----
  	}
      }
  
+   if (value_initialized (val) == 0)
+     fprintf_filtered (stream, " [uninitialized] ");
+ 
    if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
      {
        /* Attempt to determine real type of object */
Index: gdb/dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.20
diff -c -3 -p -r1.20 dwarf2expr.c
*** gdb/dwarf2expr.c	27 Apr 2007 22:34:56 -0000	1.20
--- gdb/dwarf2expr.c	9 May 2007 20:53:21 -0000
*************** execute_stack_op (struct dwarf_expr_cont
*** 284,289 ****
--- 284,290 ----
  		  gdb_byte *op_ptr, gdb_byte *op_end)
  {
    ctx->in_reg = 0;
+   ctx->initialized = 1;  /* Default is initialized.  */
  
    while (op_ptr < op_end)
      {
*************** execute_stack_op (struct dwarf_expr_cont
*** 410,416 ****
  	case DW_OP_reg29:
  	case DW_OP_reg30:
  	case DW_OP_reg31:
! 	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
  	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
  		   "used either alone or in conjuction with DW_OP_piece."));
  
--- 411,419 ----
  	case DW_OP_reg29:
  	case DW_OP_reg30:
  	case DW_OP_reg31:
! 	  if (op_ptr != op_end 
! 	      && *op_ptr != DW_OP_piece
! 	      && *op_ptr != DW_OP_GNU_uninit)
  	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
  		   "used either alone or in conjuction with DW_OP_piece."));
  
*************** execute_stack_op (struct dwarf_expr_cont
*** 731,736 ****
--- 734,748 ----
            }
            goto no_push;
  
+ 	case DW_OP_GNU_uninit:
+ 	  if (op_ptr != op_end
+ 	      && *op_ptr != DW_OP_piece)
+ 	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
+ 		   "used either alone or in conjuction with DW_OP_piece."));
+ 
+ 	  ctx->initialized = 0;
+ 	  goto no_push;
+ 
  	default:
  	  error (_("Unhandled dwarf expression opcode 0x%x"), op);
  	}
Index: gdb/dwarf2expr.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
retrieving revision 1.9
diff -c -3 -p -r1.9 dwarf2expr.h
*** gdb/dwarf2expr.h	9 Jan 2007 17:58:50 -0000	1.9
--- gdb/dwarf2expr.h	9 May 2007 20:53:21 -0000
*************** struct dwarf_expr_context
*** 76,81 ****
--- 76,84 ----
       will be on the expression stack.  */
    int in_reg;
  
+   /* Initialization status of variable.  */
+   int initialized;
+ 
    /* An array of pieces.  PIECES points to its first element;
       NUM_PIECES is its length.
  
Index: gdb/dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.39
diff -c -3 -p -r1.39 dwarf2loc.c
*** gdb/dwarf2loc.c	24 Jan 2007 22:04:48 -0000	1.39
--- gdb/dwarf2loc.c	9 May 2007 20:53:21 -0000
*************** dwarf2_evaluate_loc_desc (struct symbol 
*** 256,261 ****
--- 256,263 ----
        VALUE_ADDRESS (retval) = address;
      }
  
+   set_value_initialized (retval, ctx->initialized);
+ 
    free_dwarf_expr_context (ctx);
  
    return retval;
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.218
diff -c -3 -p -r1.218 dwarf2read.c
*** gdb/dwarf2read.c	18 Apr 2007 13:25:04 -0000	1.218
--- gdb/dwarf2read.c	9 May 2007 20:53:21 -0000
*************** dwarf_stack_op_name (unsigned op)
*** 8629,8634 ****
--- 8629,8636 ----
        return "DW_OP_bit_piece";
      case DW_OP_GNU_push_tls_address:
        return "DW_OP_GNU_push_tls_address";
+     case DW_OP_GNU_uninit:
+       return "DW_OP_GNU_uninit";
      /* HP extensions. */ 
      case DW_OP_HP_is_value:
        return "DW_OP_HP_is_value";
*************** decode_locdesc (struct dwarf_block *blk,
*** 9204,9209 ****
--- 9206,9214 ----
  	    dwarf2_complex_location_expr_complaint ();
            break;
  
+ 	case DW_OP_GNU_uninit:
+ 	  break;
+ 
  	default:
  	  complaint (&symfile_complaints, _("unsupported stack op: '%s'"),
  		     dwarf_stack_op_name (op));
Index: gdb/value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.41
diff -c -3 -p -r1.41 value.c
*** gdb/value.c	13 Apr 2007 14:17:46 -0000	1.41
--- gdb/value.c	9 May 2007 20:53:21 -0000
*************** struct value
*** 157,162 ****
--- 157,165 ----
       actually exist in the program.  */
    char optimized_out;
  
+   /* If value is a variable, is it initialized or not.  */
+   int initialized;
+ 
    /* Actual contents of the value.  For use of this value; setting it
       uses the stuff above.  Not valid if lazy is nonzero.  Target
       byte-order.  We force it to be aligned properly for any possible
*************** allocate_value (struct type *type)
*** 232,237 ****
--- 235,241 ----
    val->embedded_offset = 0;
    val->pointed_to_offset = 0;
    val->modifiable = 1;
+   val->initialized = 1;  /* Default to initialized.  */
    return val;
  }
  
*************** using_struct_return (struct type *value_
*** 1691,1696 ****
--- 1695,1716 ----
  	  != RETURN_VALUE_REGISTER_CONVENTION);
  }
  
+ /* Set the initialized field in a value struct.  */
+ 
+ void
+ set_value_initialized (struct value *val, int status)
+ {
+   val->initialized = status;
+ }
+ 
+ /* Return the initialized field in a value struct.  */
+ 
+ int
+ value_initialized (struct value *val)
+ {
+   return val->initialized;
+ }
+ 
  void
  _initialize_values (void)
  {
Index: gdb/value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.96
diff -c -3 -p -r1.96 value.h
*** gdb/value.h	9 Jan 2007 17:58:59 -0000	1.96
--- gdb/value.h	9 May 2007 20:53:21 -0000
*************** extern int value_contents_equal (struct 
*** 193,198 ****
--- 193,204 ----
  extern int value_optimized_out (struct value *value);
  extern void set_value_optimized_out (struct value *value, int val);
  
+ /* Set or return field indicating whether a variable is initialized or
+    not, based on DWARF location information supplied by the compiler. 
+    1 = initialized; 0 = uninitialized.  */
+ extern int value_initialized (struct value *);
+ extern void set_value_initialized (struct value *, int);
+ 
  /* While the following fields are per- VALUE .CONTENT .PIECE (i.e., a
     single value might have multiple LVALs), this hacked interface is
     limited to just the first PIECE.  Expect further change.  */
Index: include/elf/dwarf2.h
===================================================================
RCS file: /cvs/src/src/include/elf/dwarf2.h,v
retrieving revision 1.19
diff -c -3 -p -r1.19 dwarf2.h
*** include/elf/dwarf2.h	2 Mar 2006 00:54:27 -0000	1.19
--- include/elf/dwarf2.h	9 May 2007 20:53:22 -0000
*************** enum dwarf_location_atom
*** 540,545 ****
--- 540,546 ----
      DW_OP_bit_piece = 0x9d,
      /* GNU extensions.  */
      DW_OP_GNU_push_tls_address = 0xe0,
+     DW_OP_GNU_uninit     = 0xf0,
      /* HP extensions.  */
      DW_OP_HP_unknown     = 0xe0, /* Ouch, the same as GNU_push_tls_address.  */
      DW_OP_HP_is_value    = 0xe1,

[-- Attachment #3: Type: text/plain, Size: 2544 bytes --]


On May 9, 2007, at 10:36 AM, Caroline Tice wrote:

>
> On May 8, 2007, at 5:33 PM, Jim Blandy wrote:
>
>>
>> Caroline Tice <ctice@apple.com> writes:
>>>> As part of some work I have been doing on improving debugging of
>>>> optimized code, I
>>>> have created a GCC patch that tags variables it believes are
>>>> uninitialized with a new
>>>> Dwarf op (an extension), DW_OP_GNU_uninit.   I have submitted that
>>>> patch to the
>>>> GCC patches list and am waiting for approval to commit it.   I have
>>>> also created the
>>>> following gdb patch to recognize the new op and inform the user  
>>>> when
>>>> a variable
>>>> the user requests to see is uninitialized.
>>>>
>>>> I have tested this patch on some small testcases and I have run the
>>>> gdb dejagnu
>>>> testsuite with no regressions.  I am new to submitting things to
>>>> this list, so if there
>>>> is anything else I ought to have done, please let me know
>>>> (kindly!).
>>
>> Hi, Caroline.  This seems like a nice patch.
>>
>> In a multi-piece location expression, can each piece be individually
>> initialized or uninitialized?  If that's so, then there should  
>> also be
>> an 'initialized' member of 'struct dwarf_expr_piece', which gets set
>> appropriately for each piece in a multi-piece location expression.
>>
>
> Okay, will do.  (Yes, I believe each piece can be individually
> initialized or uninitialized.)
>
>> Either way, the code for DW_OP_GNU_uninit should check that it's the
>> last opcode in the piece or in the entire expression, as the
>> DW_OP_reg* cases do.
>>
>
> Will do.
>
>> I think the 'struct dwarf_expr_context' member should be named
>> simply 'initialized', instead of 'var_status'.  The 'struct value'
>> field should be named 'initialized', and the accessor functions  
>> should
>> be named 'value_initialized' and 'set_value_initialized'.  The  
>> comment
>> in value.h actually needs to be filled in; the description should be
>> thorough enough to allow someone who otherwise knows how GDB works to
>> use those functions, without reading their definitions.
>>
>
> Will do.
>
>> I couldn't see from your patch why 'signed_address_type',
>> 'unsigned_address_type', and 'add_piece' were made visible outside
>> dwarf2expr.c; that should be left out of the patch if it's not  
>> needed.
>>
>
> I went back and checked; making them globally visible was actually
> for a different patch I did.  Sorry; I will remove that from this  
> patch.
>
>> Have you filed a copyright assignment with the FSF?
>
> Jim Ingham answered this one.
>

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Jim Blandy]

Caroline Tice <ctice@apple.com> writes:
> I tested it by running it on a small test case I have
> (with DW_OP_GNU_uninit ops in it), as well as running the
> dejagnu testsuite with no regressions.  Is this modified patch okay
> to commit to FSF GDB?

Thanks for the revisions!  They look good.  I have a few more
comments, mostly just details.

> Index: gdb/c-valprint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/c-valprint.c,v
> retrieving revision 1.42
> diff -c -3 -p -r1.42 c-valprint.c
> *** gdb/c-valprint.c	26 Jan 2007 20:54:16 -0000	1.42
> --- gdb/c-valprint.c	9 May 2007 20:53:21 -0000
> *************** c_value_print (struct value *val, struct
> *** 556,561 ****
> --- 556,564 ----
>   	}
>       }
>   
> +   if (value_initialized (val) == 0)
> +     fprintf_filtered (stream, " [uninitialized] ");
> + 
>     if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
>       {
>         /* Attempt to determine real type of object */

Nit-picky suggestion: wouldn't it be nicer to say "if (!
value_initialized (val)) ..."?


> *************** execute_stack_op (struct dwarf_expr_cont
> *** 731,736 ****
> --- 734,748 ----
>             }
>             goto no_push;
>   
> + 	case DW_OP_GNU_uninit:
> + 	  if (op_ptr != op_end
> + 	      && *op_ptr != DW_OP_piece)
> + 	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
> + 		   "used either alone or in conjuction with DW_OP_piece."));
> + 
> + 	  ctx->initialized = 0;
> + 	  goto no_push;
> + 
>   	default:
>   	  error (_("Unhandled dwarf expression opcode 0x%x"), op);
>   	}

You said DW_OP_GNU_uninit doesn't apply to individual pieces.  So
shouldn't this code report an error whenever DW_OP_GNU_uninit isn't
the last opcode in the whole expression --- that is, even when it's
followed by DW_OP_piece?

> Index: gdb/dwarf2expr.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
> retrieving revision 1.9
> diff -c -3 -p -r1.9 dwarf2expr.h
> *** gdb/dwarf2expr.h	9 Jan 2007 17:58:50 -0000	1.9
> --- gdb/dwarf2expr.h	9 May 2007 20:53:21 -0000
> *************** struct dwarf_expr_context
> *** 76,81 ****
> --- 76,84 ----
>        will be on the expression stack.  */
>     int in_reg;
>   
> +   /* Initialization status of variable.  */
> +   int initialized;
> + 

Another nit-pick: the comment should actually explain the
interpretation of the value: "Non-zero if variable has been
initialized; zero otherwise."

> Index: gdb/value.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/value.h,v
> retrieving revision 1.96
> diff -c -3 -p -r1.96 value.h
> *** gdb/value.h	9 Jan 2007 17:58:59 -0000	1.96
> --- gdb/value.h	9 May 2007 20:53:21 -0000
> *************** extern int value_contents_equal (struct 
> *** 193,198 ****
> --- 193,204 ----
>   extern int value_optimized_out (struct value *value);
>   extern void set_value_optimized_out (struct value *value, int val);
>   
> + /* Set or return field indicating whether a variable is initialized or
> +    not, based on DWARF location information supplied by the compiler. 
> +    1 = initialized; 0 = uninitialized.  */
> + extern int value_initialized (struct value *);
> + extern void set_value_initialized (struct value *, int);
> + 
>   /* While the following fields are per- VALUE .CONTENT .PIECE (i.e., a
>      single value might have multiple LVALs), this hacked interface is
>      limited to just the first PIECE.  Expect further change.  */

A final nit-pick: value.h, value.c, and so on are meant to be
independent of any particular debugging format.  There should never be
DWARF dependencies here.  Your code does this right --- a generic
mechanism to be used by DWARF-specific code in the appropriate way ---
but the comment should also not suggest that it's DWARF-specific.
Thus, "... based on debugging information supplied by the compiler".

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Caroline Tice]

[-- Attachment #1: Type: text/plain, Size: 158 bytes --]

Okay,  I agree with your suggestions and have made the changes.  The
modified patch is attached.  Is this okay to commit?

-- Caroline Tice
ctice@apple.com



[-- Attachment #2: fsf-gdb-patch3.txt --]
[-- Type: text/plain, Size: 7539 bytes --]

Index: gdb/c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.42
diff -c -3 -p -r1.42 c-valprint.c
*** gdb/c-valprint.c	26 Jan 2007 20:54:16 -0000	1.42
--- gdb/c-valprint.c	17 May 2007 16:59:06 -0000
*************** c_value_print (struct value *val, struct
*** 556,561 ****
--- 556,564 ----
  	}
      }
  
+   if (!value_initialized (val))
+     fprintf_filtered (stream, " [uninitialized] ");
+ 
    if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
      {
        /* Attempt to determine real type of object */
Index: gdb/dwarf2expr.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.c,v
retrieving revision 1.20
diff -c -3 -p -r1.20 dwarf2expr.c
*** gdb/dwarf2expr.c	27 Apr 2007 22:34:56 -0000	1.20
--- gdb/dwarf2expr.c	17 May 2007 16:59:08 -0000
*************** execute_stack_op (struct dwarf_expr_cont
*** 284,289 ****
--- 284,290 ----
  		  gdb_byte *op_ptr, gdb_byte *op_end)
  {
    ctx->in_reg = 0;
+   ctx->initialized = 1;  /* Default is initialized.  */
  
    while (op_ptr < op_end)
      {
*************** execute_stack_op (struct dwarf_expr_cont
*** 410,416 ****
  	case DW_OP_reg29:
  	case DW_OP_reg30:
  	case DW_OP_reg31:
! 	  if (op_ptr != op_end && *op_ptr != DW_OP_piece)
  	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
  		   "used either alone or in conjuction with DW_OP_piece."));
  
--- 411,419 ----
  	case DW_OP_reg29:
  	case DW_OP_reg30:
  	case DW_OP_reg31:
! 	  if (op_ptr != op_end 
! 	      && *op_ptr != DW_OP_piece
! 	      && *op_ptr != DW_OP_GNU_uninit)
  	    error (_("DWARF-2 expression error: DW_OP_reg operations must be "
  		   "used either alone or in conjuction with DW_OP_piece."));
  
*************** execute_stack_op (struct dwarf_expr_cont
*** 731,736 ****
--- 734,747 ----
            }
            goto no_push;
  
+ 	case DW_OP_GNU_uninit:
+ 	  if (op_ptr != op_end)
+ 	    error (_("DWARF-2 expression error: DW_OP_GNU_unint must always "
+ 		   "be the very last op."));
+ 
+ 	  ctx->initialized = 0;
+ 	  goto no_push;
+ 
  	default:
  	  error (_("Unhandled dwarf expression opcode 0x%x"), op);
  	}
Index: gdb/dwarf2expr.h
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
retrieving revision 1.9
diff -c -3 -p -r1.9 dwarf2expr.h
*** gdb/dwarf2expr.h	9 Jan 2007 17:58:50 -0000	1.9
--- gdb/dwarf2expr.h	17 May 2007 16:59:08 -0000
*************** struct dwarf_expr_context
*** 76,81 ****
--- 76,85 ----
       will be on the expression stack.  */
    int in_reg;
  
+   /* Initialization status of variable: Non-zero if variable has been
+      initialized; zero otherwise.  */
+   int initialized;
+ 
    /* An array of pieces.  PIECES points to its first element;
       NUM_PIECES is its length.
  
Index: gdb/dwarf2loc.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2loc.c,v
retrieving revision 1.39
diff -c -3 -p -r1.39 dwarf2loc.c
*** gdb/dwarf2loc.c	24 Jan 2007 22:04:48 -0000	1.39
--- gdb/dwarf2loc.c	17 May 2007 16:59:08 -0000
*************** dwarf2_evaluate_loc_desc (struct symbol 
*** 256,261 ****
--- 256,263 ----
        VALUE_ADDRESS (retval) = address;
      }
  
+   set_value_initialized (retval, ctx->initialized);
+ 
    free_dwarf_expr_context (ctx);
  
    return retval;
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.218
diff -c -3 -p -r1.218 dwarf2read.c
*** gdb/dwarf2read.c	18 Apr 2007 13:25:04 -0000	1.218
--- gdb/dwarf2read.c	17 May 2007 16:59:08 -0000
*************** dwarf_stack_op_name (unsigned op)
*** 8629,8634 ****
--- 8629,8636 ----
        return "DW_OP_bit_piece";
      case DW_OP_GNU_push_tls_address:
        return "DW_OP_GNU_push_tls_address";
+     case DW_OP_GNU_uninit:
+       return "DW_OP_GNU_uninit";
      /* HP extensions. */ 
      case DW_OP_HP_is_value:
        return "DW_OP_HP_is_value";
*************** decode_locdesc (struct dwarf_block *blk,
*** 9204,9209 ****
--- 9206,9214 ----
  	    dwarf2_complex_location_expr_complaint ();
            break;
  
+ 	case DW_OP_GNU_uninit:
+ 	  break;
+ 
  	default:
  	  complaint (&symfile_complaints, _("unsupported stack op: '%s'"),
  		     dwarf_stack_op_name (op));
Index: gdb/value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.41
diff -c -3 -p -r1.41 value.c
*** gdb/value.c	13 Apr 2007 14:17:46 -0000	1.41
--- gdb/value.c	17 May 2007 16:59:08 -0000
*************** struct value
*** 157,162 ****
--- 157,165 ----
       actually exist in the program.  */
    char optimized_out;
  
+   /* If value is a variable, is it initialized or not.  */
+   int initialized;
+ 
    /* Actual contents of the value.  For use of this value; setting it
       uses the stuff above.  Not valid if lazy is nonzero.  Target
       byte-order.  We force it to be aligned properly for any possible
*************** allocate_value (struct type *type)
*** 232,237 ****
--- 235,241 ----
    val->embedded_offset = 0;
    val->pointed_to_offset = 0;
    val->modifiable = 1;
+   val->initialized = 1;  /* Default to initialized.  */
    return val;
  }
  
*************** using_struct_return (struct type *value_
*** 1691,1696 ****
--- 1695,1716 ----
  	  != RETURN_VALUE_REGISTER_CONVENTION);
  }
  
+ /* Set the initialized field in a value struct.  */
+ 
+ void
+ set_value_initialized (struct value *val, int status)
+ {
+   val->initialized = status;
+ }
+ 
+ /* Return the initialized field in a value struct.  */
+ 
+ int
+ value_initialized (struct value *val)
+ {
+   return val->initialized;
+ }
+ 
  void
  _initialize_values (void)
  {
Index: gdb/value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.96
diff -c -3 -p -r1.96 value.h
*** gdb/value.h	9 Jan 2007 17:58:59 -0000	1.96
--- gdb/value.h	17 May 2007 16:59:08 -0000
*************** extern int value_contents_equal (struct 
*** 193,198 ****
--- 193,204 ----
  extern int value_optimized_out (struct value *value);
  extern void set_value_optimized_out (struct value *value, int val);
  
+ /* Set or return field indicating whether a variable is initialized or
+    not, based on debugging information supplied by the compiler. 
+    1 = initialized; 0 = uninitialized.  */
+ extern int value_initialized (struct value *);
+ extern void set_value_initialized (struct value *, int);
+ 
  /* While the following fields are per- VALUE .CONTENT .PIECE (i.e., a
     single value might have multiple LVALs), this hacked interface is
     limited to just the first PIECE.  Expect further change.  */
Index: include/elf/dwarf2.h
===================================================================
RCS file: /cvs/src/src/include/elf/dwarf2.h,v
retrieving revision 1.19
diff -c -3 -p -r1.19 dwarf2.h
*** include/elf/dwarf2.h	2 Mar 2006 00:54:27 -0000	1.19
--- include/elf/dwarf2.h	17 May 2007 16:59:09 -0000
*************** enum dwarf_location_atom
*** 540,545 ****
--- 540,546 ----
      DW_OP_bit_piece = 0x9d,
      /* GNU extensions.  */
      DW_OP_GNU_push_tls_address = 0xe0,
+     DW_OP_GNU_uninit     = 0xf0,
      /* HP extensions.  */
      DW_OP_HP_unknown     = 0xe0, /* Ouch, the same as GNU_push_tls_address.  */
      DW_OP_HP_is_value    = 0xe1,

[-- Attachment #3: Type: text/plain, Size: 4136 bytes --]


On May 16, 2007, at 4:35 PM, Jim Blandy wrote:

>
> Caroline Tice <ctice@apple.com> writes:
>> I tested it by running it on a small test case I have
>> (with DW_OP_GNU_uninit ops in it), as well as running the
>> dejagnu testsuite with no regressions.  Is this modified patch okay
>> to commit to FSF GDB?
>
> Thanks for the revisions!  They look good.  I have a few more
> comments, mostly just details.
>
>> Index: gdb/c-valprint.c
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/c-valprint.c,v
>> retrieving revision 1.42
>> diff -c -3 -p -r1.42 c-valprint.c
>> *** gdb/c-valprint.c	26 Jan 2007 20:54:16 -0000	1.42
>> --- gdb/c-valprint.c	9 May 2007 20:53:21 -0000
>> *************** c_value_print (struct value *val, struct
>> *** 556,561 ****
>> --- 556,564 ----
>>  	}
>>      }
>>
>> +   if (value_initialized (val) == 0)
>> +     fprintf_filtered (stream, " [uninitialized] ");
>> +
>>    if (objectprint && (TYPE_CODE (type) == TYPE_CODE_CLASS))
>>      {
>>        /* Attempt to determine real type of object */
>
> Nit-picky suggestion: wouldn't it be nicer to say "if (!
> value_initialized (val)) ..."?
>
>
>> *************** execute_stack_op (struct dwarf_expr_cont
>> *** 731,736 ****
>> --- 734,748 ----
>>            }
>>            goto no_push;
>>
>> + 	case DW_OP_GNU_uninit:
>> + 	  if (op_ptr != op_end
>> + 	      && *op_ptr != DW_OP_piece)
>> + 	    error (_("DWARF-2 expression error: DW_OP_reg operations  
>> must be "
>> + 		   "used either alone or in conjuction with DW_OP_piece."));
>> +
>> + 	  ctx->initialized = 0;
>> + 	  goto no_push;
>> +
>>  	default:
>>  	  error (_("Unhandled dwarf expression opcode 0x%x"), op);
>>  	}
>
> You said DW_OP_GNU_uninit doesn't apply to individual pieces.  So
> shouldn't this code report an error whenever DW_OP_GNU_uninit isn't
> the last opcode in the whole expression --- that is, even when it's
> followed by DW_OP_piece?
>
>> Index: gdb/dwarf2expr.h
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/dwarf2expr.h,v
>> retrieving revision 1.9
>> diff -c -3 -p -r1.9 dwarf2expr.h
>> *** gdb/dwarf2expr.h	9 Jan 2007 17:58:50 -0000	1.9
>> --- gdb/dwarf2expr.h	9 May 2007 20:53:21 -0000
>> *************** struct dwarf_expr_context
>> *** 76,81 ****
>> --- 76,84 ----
>>       will be on the expression stack.  */
>>    int in_reg;
>>
>> +   /* Initialization status of variable.  */
>> +   int initialized;
>> +
>
> Another nit-pick: the comment should actually explain the
> interpretation of the value: "Non-zero if variable has been
> initialized; zero otherwise."
>
>> Index: gdb/value.h
>> ===================================================================
>> RCS file: /cvs/src/src/gdb/value.h,v
>> retrieving revision 1.96
>> diff -c -3 -p -r1.96 value.h
>> *** gdb/value.h	9 Jan 2007 17:58:59 -0000	1.96
>> --- gdb/value.h	9 May 2007 20:53:21 -0000
>> *************** extern int value_contents_equal (struct
>> *** 193,198 ****
>> --- 193,204 ----
>>  extern int value_optimized_out (struct value *value);
>>  extern void set_value_optimized_out (struct value *value, int val);
>>
>> + /* Set or return field indicating whether a variable is  
>> initialized or
>> +    not, based on DWARF location information supplied by the  
>> compiler.
>> +    1 = initialized; 0 = uninitialized.  */
>> + extern int value_initialized (struct value *);
>> + extern void set_value_initialized (struct value *, int);
>> +
>>  /* While the following fields are per- VALUE .CONTENT .PIECE  
>> (i.e., a
>>     single value might have multiple LVALs), this hacked interface is
>>     limited to just the first PIECE.  Expect further change.  */
>
> A final nit-pick: value.h, value.c, and so on are meant to be
> independent of any particular debugging format.  There should never be
> DWARF dependencies here.  Your code does this right --- a generic
> mechanism to be used by DWARF-specific code in the appropriate way ---
> but the comment should also not suggest that it's DWARF-specific.
> Thus, "... based on debugging information supplied by the compiler".

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Jim Blandy]

Caroline Tice <ctice@apple.com> writes:
> Okay,  I agree with your suggestions and have made the changes.  The
> modified patch is attached.  Is this okay to commit?

Yes, please do!

But first, add yourself to the "Write After Approval" section of
gdb/MAINTAINERS, with an appropriate ChangeLog entry, as a separate
commit from the patch.

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Caroline Tice]

It looks like someone needs to give me CVS write access to the GDB  
repository first (I already
have write access, after approval, to the GCC repository).  Whom  
should I contact about that?

-- Caroline Tice
ctice@apple.com

On May 17, 2007, at 5:00 PM, Jim Blandy wrote:

>
> Caroline Tice <ctice@apple.com> writes:
>> Okay,  I agree with your suggestions and have made the changes.  The
>> modified patch is attached.  Is this okay to commit?
>
> Yes, please do!
>
> But first, add yourself to the "Write After Approval" section of
> gdb/MAINTAINERS, with an appropriate ChangeLog entry, as a separate
> commit from the patch.

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Jim Blandy]

Caroline Tice <ctice@apple.com> writes:
> It looks like someone needs to give me CVS write access to the GDB
> repository first (I already
> have write access, after approval, to the GCC repository).  Whom
> should I contact about that?

Use our handy-dandy little form at
http://sourceware.org/cgi-bin/pdw/ps_form.cgi; list me as the person
who approved the request.

Re: Ping! [PATCH]: Tracking and reporting uninitialized variables

[Daniel Jacobowitz]

On Fri, May 18, 2007 at 10:05:08AM -0700, Jim Blandy wrote:
> 
> Caroline Tice <ctice@apple.com> writes:
> > It looks like someone needs to give me CVS write access to the GDB
> > repository first (I already
> > have write access, after approval, to the GCC repository).  Whom
> > should I contact about that?
> 
> Use our handy-dandy little form at
> http://sourceware.org/cgi-bin/pdw/ps_form.cgi; list me as the person
> who approved the request.

Actually, just send mail to overseers and copy Jim; that form is for
people who don't have an account yet.

-- 
Daniel Jacobowitz
CodeSourcery