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

[Alexey Samsonov]

On Wed, Oct 17, 2012 at 9:48 PM, Eric Christopher <echristo at gmail.com>wrote:

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

Yes, I think I still need a review for patch for PR13942.

https://codereview.appspot.com/6720044/


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



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121024/0cfa70b4/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: issue6720044_2002.diff
Type: application/octet-stream
Size: 7511 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121024/0cfa70b4/attachment.obj>