[cfe-commits] [PATCH] PR14097, no debug info for artificial/'nodebug' methods

[Eric Christopher]

I've gone ahead and reverted it here:

	D	test/CodeGenCXX/debug-info-user-def.cpp
	M	test/CodeGenCXX/debug-lambda-expressions.cpp
	M	test/CodeGenCXX/debug-info-template-quals.cpp
	M	test/CodeGenCXX/debug-info-template-member.cpp
	M	lib/CodeGen/CGDebugInfo.cpp
W: -empty_dir: test/CodeGenCXX/debug-info-user-def.cpp
r166109 = 7c70a2300fbd713858c31cc9cbbe074bdc233412 (refs/remotes/origin/master)

Feel free to rebase some test suite changes on top of that.

Alexey: If you need anything else let me know.

Thanks!

-eric

On Wed, Oct 17, 2012 at 10:14 AM, Robinson, Paul
<Paul.Robinson at am.sony.com> wrote:
> My goal was reducing the size of .debug_info, however 158009 did not achieve
> this
>
> and I was trying to eliminate the bloat that it caused.
>
>
>
> If the artificial entries are useful for some other purpose then reverting
> 158009 is fine.
>
> If we do that, then my patch #1 becomes unnecessary.  Depending on what we
> decide
>
> 'nodebug' means, patch #2 may also be unnecessary, although I observe that
> there
>
> really are no tests for 'nodebug' on a method and we might want to hang onto
> that part.
>
>
>
>> What I don't need is the debug info for bodies of these functions (all the
>> local variables
>
>> etc.).
>
>
>
> When we are talking about class methods, what happens is that there is a
> declaration
>
> DIE which is a child of the class DIE.  This DIE has no code range.  When
> the method is
>
> defined, there is a definition DIE with code ranges, and DW_AT_specification
> pointing
>
> back to the declaration DIE.  (The problem with 158009 is that there was no
> declaration
>
> DIE, so Clang created one on the fly.)  So, if you are thinking of
> suppressing the
>
> definition DIE, you won't get code ranges for the artificial methods.  This
> is already
>
> the case for 'nodebug' methods, there is a declaration DIE but no definition
> DIE.
>
>
>
> I think this dual-DIE scheme does not apply to standalone functions.  It
> looks like there
>
> is no DIE at all for 'nodebug' standalone functions.
>
>
>
> I think most artificial functions/methods don't really have a lot of child
> DIEs?  They are
>
> usually pretty straightforward and don't declare local variables and so
> forth.  For an
>
> artificial constructor, if there is a DIE at all, you _do_ want to preserve
> the formal-parameter
>
> DIEs, but that's about all you get anyway.
>
>
>
> So if you want definition DIEs, but no children (other than parameters),
> maybe for the
>
> artificial/nodebug methods/functions we want to generate the
> declaration+defintion DIEs
>
> but then treat the bodies as if we have -gline-tables-only in effect?  Would
> that put the
>
> right code ranges on the subprogram DIEs?
>
> --paulr
>
>
>
> P.S. offsite the rest of the day, will try to check back this evening.
>
>
>
> ________________________________
> From: Alexey Samsonov [samsonov at google.com]
> Sent: Wednesday, October 17, 2012 9:39 AM
> To: Eric Christopher
> Cc: Robinson, Paul; cfe-commits at cs.uiuc.edu
>
> Subject: Re: [cfe-commits] [PATCH] PR14097, no debug info for
> artificial/'nodebug' methods
>
>
>
> On Wed, Oct 17, 2012 at 7:57 AM, Eric Christopher <echristo at gmail.com>
> wrote:
>>
>> > Yes it looks like our patches are at cross-purposes.
>> >
>> > I assume that your patch gets the desired effect because there is now a
>> > DIE that
>> > points to the subprogram's generated code?  That's not good for me,
>> > because if
>> > there's a class method definition DIE, it insists on having a
>> > declaration DIE too.
>> > I'm not sure whether the same is true for standalone functions.
>> >
>> > I wonder if there is a way we could produce an artificial subprogram
>> > DIE, but then
>> > have LLVM not actually emit it to the DWARF.  That is, suppress the
>> > excess DWARF
>> > in LLVM rather than in Clang.  If lives long enough to build the address
>> > ranges (and
>> > also .debug_line?) it would meet your needs, and then if it doesn't
>> > actually get emitted
>> > to .debug_info, it would meet my needs.
>
>
> What are your needs? Do you want to keep the .debug_info smaller, or this
> DIE may
> be malformed and cause problems for you? I'm somewhat afraid that data in
> .debug_line would not be enough for me. This depends on the way a tool maps
> instruction address to compile unit. One of the way is:
> 1) get the compile unit DIE
> 2) get a DWARF line table for this DIE
> 3) parse the table and build address ranges for this compile unit.
>
> This (step 3) is of course inefficient.
> Currently LLVM's DebugInfo lib does the following:
> 1) get the compile unit DIE
> 2) get all subprogram DIEs for this CU
> 3) collects address ranges for these subprograms.
>
> So, for such an approach, I need the debug_info entries for artificial
> functions and methods.
> generated by Clang. I even may need entries for functions with disabled
> debug
> info. What I don't need is the debug info for bodies of these functions (all
> the local variables
> etc.).
>
> Probably the best way is to use .debug_ranges section and add
> a single DW_AT_ranges into the compile unit DIE. In this way we'll be able
> to get all the instruction address ranges for compile unit without scanning
> any
> additional DIEs (or using weird .debug_aranges section).
>
>>
>>
>> I'm more than happy to revert r158009, I originally added it to reduce
>> the amount of debug information since often people would not be trying
>> to add break points or debug through an artificial function. Alexey's
>> patch and motivation, however, lead me to believe that this was
>> originally incorrect as we may want a backtrace with information and
>> line numbers that includes artificial functions like global
>> constructors.
>>
>> Either of you have an argument against reverting it?
>
>
> I have not.
>
>>
>>
>> -eric
>
>
>
> --
> Alexey Samsonov, MSK
>