Ping! [PATCH]: Tracking and reporting uninitialized variables

[Caroline Tice]

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.

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: fsf-gdb-patch2.txt
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20070509/0934926a/attachment.txt>
-------------- next part --------------

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.
>