[PATCH] This phabricator revision is the merge of 4 patches that aim to provide resolving of AT_abstract_origin and AT_specification attributes.

[Alexey Samsonov]

================
Comment at: lib/DebugInfo/DWARFDebugInfoEntry.cpp:319
@@ +318,3 @@
+      Offset >= U->getNextUnitOffset()) {
+    U = U->getContext().getCompileUnitForOffset(Offset);
+    assert(U && "Invalid DIE referenced.");
----------------
dblaikie wrote:
> samsonov wrote:
> > dblaikie wrote:
> > > samsonov wrote:
> > > > dblaikie wrote:
> > > > > Is "getCompileUnitForOffset" cheap enough that we should just always call it? (& maybe pass in the context to this function, rather than the DWARFUnit, at that point)
> > > > This part makes no sense to me. We definitely should somehow ensure that "this" is in a fact a DIE in "U". When you call DWARFDebugInfoEntry::getSubroutineName(), you certainly should know which unit this DIE belongs to. How can it be not so?
> > > LLVM under LTO will generate DIE references across the CUs (in an effort to reduce debug info size - eg: rather than putting a type (even a declaration) in multiple CUs, one CU just references directly into the other CU to reference the type defined there) - so you actually don't know.
> > OK, makes sense. But you still are supposed to know which Unit a given DIE belongs to. You can't even extract it otherwise: DWARFDebugInfoEntry::extractFast() takes a pointer to Unit as an input parameter. So, in case of cross-CU references, and, for instance, getSubroutineName you should do the following:
> >   1) assume "this" is a DIE contained in "U".
> >   2) if DIE has name attribute, return it
> >   3) try to fetch name from specification:
> >     a) read DW_AT_specification reference SpecRef
> >     b) calculate SpecU this SpecRef corresponds to
> >     c) call SpecDIE.extractFast(SpecU, &SpecRef)
> >     d) call SpecDIE.getSubroutineName(SpecU)
> > 
> @samsanov:
> 
> Yeah, what you're suggesting is something along the lines of what I was trying to describe when I said:
> 
> "Is "getCompileUnitForOffset" cheap enough that we should just always call it? (& maybe pass in the context to this function, rather than the DWARFUnit, at that point)" - essentially, as you described in (3(b)) - always compute the CU from the ref, rather than assuming/verifying if it's the same as the current one.
I still see no benefit in passing DWARFContext here - you will always know the DWARFUnit for the DIE before you can issue the call to getSubroutineName, why not pass this DWARFUnit there, and keep it consistent with the rest various DWARFDebugInfoEntryMinimal accessors?

http://reviews.llvm.org/D5208