[RFC] Debuginfo: Add default value support for subprogram arguments

[David Blaikie]

On Thu, Aug 14, 2014 at 10:22 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Thu, Aug 14, 2014 at 9:17 AM, Frédéric Riss <friss at apple.com> wrote:
>> Hi,
>>
>> This is an initial RFC regarding adding support for emitting default value
>> debug information for function/method arguments. Although I’d appreciate
>> initial review comments (patch attached), this is more about getting your
>> feedback on the approach, as it diverges a bit from the standard.
>
> Cool! I hadn't realized there was DWARF support for this (& even GCC
> doesn't seem to implement it - though I don't have a GCC ToT build
> around to test, just 4.8) - always annoying having to type "dump(0)"
> instead of just "dump()" when dumping some LLVM types that have
> optional "dump to this specified stream" support.
>
>> The Dwarf standard reads:
>>
>> A DW_AT_default_value attribute for a formal parameter entry. The value of
>> this attribute is a reference to the debugging information entry for a
>> variable or subroutine, or the value may be a constant. If the attribute
>> form is of class reference, the default value of the parameter is the value
>> of the referenced variable (which may be constant) or the value returned by
>> the referenced subroutine; a reference value of 0 means that no default
>> value has been specified. If the value is of class constant, that constant
>> is interpreted as a default value of the type of the formal parameter.
>>
>> For a constant form there is no way to express the absence of a default
>> value.
>>
>> As you can see, this text limits us to emitting information about default
>> values being:
>>  - simple constants (DW_FORM_data*)
>>  - global variable (DW_FORM_ref*)
>>  - calls to functions without arguments (DW_FORM_ref*)
>>
>> I tried to go a bit further and implement a more complete solution that
>> adds:
>>  - any link time constant (DW_FORM_block*): in this form, the block of data
>> represents the default value as it would be stored in the target memory
>
> How does this differ from the "simple constant" case above? What sort
> of constants can't be expressed with DW_FORM_data* that can be
> expressed with DW_FORM_block*?
>
>>  - arbitrary complex expressions (DW_FORM_str*): when no other of the above
>> forms fits, this dumps the default value expression as textual form. The
>> debugger can then evaluate this expression in the context of the declaration
>> if it has this capacity
>
> Sounds plausible.
>
>> First of all, would these kind of extensions be welcome? If you agree that
>> they are worthwhile, I’ll submit a request to augment the standard wording.
>
> I suspect the better place to start is on the DWARF committee, but I
> agree there's some chicken-and-egg problems there & it's helpful to
> have implementation buy-in, etc. (& I haven't participated in the
> DWARF committee - so I might be totally off there)
>
>> The attached patches seems to get at least the basics right, but 2 issues
>> are still bothering me:
>>
>>  - Forward declaration:
>>
>> int foo();
>>
>> void bar(int i = foo()) {return i;}
>>
>> int foo() {return 1;}
>>
>>
>> the default value references a forward decl of the function. When emitting
>> the debuginfo metadata in CGDebugInfo.cpp:EmitDeclare(), the init function
>> decl isn’t yet in the DeclCache.
>
> This ones my fault - I implemented DeclCache as a half-hearted way to
> support using declarations in C++, but you've found one of the
> limitations:
>
> namespace X {
> void foo();
> void bar() {
> }
> }
>
> using X::foo; // no debug info for this
> using X::bar; // debug info for this
>
> Currently clang has no smarts for producing debug info for function
> declarations on demand (and replacing them with definitions when a
> definition appears later) - except for the special case of
> declarations of member functions.
>
> If we add that functionality (to lazily create declarations, replace
> them with definitions when the latter follows the former, etc) we can
> fix/improve the using declaration support, and use it for features
> like this.
>
> Probably the right way to approach it is as I described (and means
> that the improvements can be made now/orthogonally to whenever the
> default argument support goes in) - by refactoring the clang support
> for emitting declarations (of global variables and non-member
> functions) under the goal of improving using declaration support, then
> it'll be there/ready/improved when the default argument support needs
> it.

I don't think Clang has infrastructure for emitting declarations of
global variables into the debug info metadata either. If/when you
plumb this through, it should be most of the work to fix
http://llvm.org/bugs/show_bug.cgi?id=14501 as well.

>
>> The current code falls back to emitting the
>> default_value attribute as the “foo()” string. This is part of the basic
>> stuff that could be expressed in standard Dwarf4, thus it would be nicer to
>> have it emitted as a reference to foo’s DIE.
>>
>>
>> - Relocatable address:
>>
>> const char *me = “fred”;
>> void foo(const char *str = me);
>>
>> the initialiser is a global address (function or data). The generated
>> metadata is alright. If I have clang dump assembly, it looks fine. However,
>> if I have clang emit the object file directly, it fails to relocate the
>> address of the initialiser in the debug information. The ObjectStreamer adds
>> fixups for the entries, but then nothing gets relocated. Any hints on how to
>> debug that kind of failure?
>
> Harder for me to guess - you've mentioned the assembly looks fine, but
> have you tried assembling that assembly? Is the resulting object file
> correct? If so, you should be able to observe this as a difference
> between compiling with and without -fno-integrated-assembler.
>
> If you observe a difference in behavior between the integrated and
> non-integrated assembler, then it's probably worth just exploring that
> as a bug with the integrated assembler, completely orthogonal to debug
> info.
>
> - David
>
>> The final version of the patch will of course feature doc update and
>> test(s).
>
> Haven't looked at the patches yet.
>
>> Thoughts?
>>
>> Thanks,
>> Fred
>>
>>
>>
>>