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

[Caroline Tice <ctice@apple.com>]

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.