Page MenuHomePhabricator

[DwarfDebug] Dump call site debug info into DWARF
AbandonedPublic

Authored by djtodoro on Feb 11 2019, 4:01 AM.

Details

Summary

Dump DWARF information about call sites and call site parameters into debug info sections.

Also. add handling of DW_OP_entry_value emitting.

For now, we use GNU extensions, since LLVM has portion of dumping call site information for tail calls (with DWARF 5 support). It can stay this way until we test, merge and patch every peace of the feature.

Authors: @asowda, @NikolaPrica, @djtodoro, @ivanbaev

Diff Detail

Event Timeline

djtodoro created this revision.Feb 11 2019, 4:01 AM
djtodoro edited the summary of this revision. (Show Details)
djtodoro added a subscriber: NikolaPrica.
djtodoro added a project: debug-info.
djtodoro added a subscriber: petarj.
djtodoro updated this revision to Diff 186453.Feb 12 2019, 6:42 AM

If we wanted to support the DWARF 5 variants are there any semantic differences, or do we just need to rename the attributes?

If we wanted to support the DWARF 5 variants are there any semantic differences, or do we just need to rename the attributes?

@aprantl Yes, it will take only DWARF symbols (tag, attributes, operand) renaming.

djtodoro updated this revision to Diff 186614.Feb 13 2019, 2:49 AM
  • Rename: VariableNotChanged ===> ArgumentNotModified
  • Refactor test cases
dstenb added a subscriber: dstenb.Feb 13 2019, 8:27 AM
dstenb added inline comments.
lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
39

Although it is a short function which is easy to skim through, I think it would be helpful to somehow signal that it only applies for non-entry values, e.g. through amending the comment or changing the name (although I don't have any suggestions for the latter).

Alternatively, maybe the function can be left as it was before this patch, and instead the calls to {add,drop}RegDescribedVar can be wrapped with that conditional?

72

Since isDescribedByReg has been changed to ignore entry values, the InlinedEntity will not be added to RegVars, so unless I have overlooked something I don't think we should get here with an entry value. Maybe this should be an assertion?

djtodoro marked 2 inline comments as done.Feb 14 2019, 3:40 AM
djtodoro added inline comments.
lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.cpp
39

Thanks! Definitely we should change something here.

72

Oh, this is dead code for sure. We didn't have this in initial patch. We'll remove this. Thanks!

djtodoro updated this revision to Diff 187191.Feb 17 2019, 10:44 PM
  • Change a comment for isDescribedByReg()
  • Remove a dead code
vsk added inline comments.Feb 21 2019, 11:25 AM
lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
1214

Any chance to share code with DwarfCompileUnit::constructCallSiteEntryDIE?

probinson added inline comments.Feb 21 2019, 11:37 AM
lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
1214

I don't understand this comment. DW_AT_call_origin is for the call-site stuff, DW_AT_abstract_origin is for inlined instances. call_origin is not a "replacement" for abstract_origin.

vsk added inline comments.Feb 21 2019, 11:45 AM
lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
1214

Sorry, didn't mean to conflate abstract_origin vs. call_origin here. What I'm getting at is that DwarfCompileUnit::constructCallSiteDIE and the pre-existing constructCallSiteEntryDIE both do some work at each call site. I'm just wondering if it's feasible to have one entry point for that sort of work.

probinson added inline comments.Feb 21 2019, 11:55 AM
lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
1214

Hi Vedant, actually I was asking about the TODO comment in the source, not commenting on your comment.

djtodoro marked 2 inline comments as done.Feb 22 2019, 1:06 AM
djtodoro added inline comments.
lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
1214

Any chance to share code with DwarfCompileUnit::constructCallSiteEntryDIE?

@vsk Thanks for your comment!

All of this, for now, works under an experimental option. We generate GNU DWARF extensions, and we plan to add also DWARF 5 support as well.
There is still discussion on RFC how we should proceed with the final implementation.

We saw that you implemented a portion of dumping DWARF 5 symbols for call sites, but in order to support debug call site parameters, we would need to introduce new meta instructions etc. (as you can see in other patches) and place the production of that info somewhere earlier in backend pipeline. Also, there will be support for indirect calls through the meta instruction that holds the register.

So, the point is that we will include the part of this in 'DwarfCompileUnit::constructCallSiteEntryDIE' , but also in order to avoid redundant functionality, we will need to change 'DwarfDebug::constructCallSiteEntryDIEs’.

Let me know what you think. :)

1214

I don't understand this comment. DW_AT_call_origin is for the call-site stuff, DW_AT_abstract_origin is for inlined instances. call_origin is >not a "replacement" for abstract_origin.

@probinson Thanks for your comment!

As I wrote in the previous comment, we use GNU DWARF extensions for describing debug info about call sites and call site parameters in DWARF sections (for now).

Since 'call_origin' is used in DWARF 5 proposal, we leave it on the side, but we plan to implement that as well.

For DWARF 4 support, even in GCC, there were no GNU extensions for every symbol needed for the feature. For example, instead of 'call_pc' for marking address after a call instruction 'low_pc' DWARF attribute was used for that. Also, instead of 'call_origin', that is actually reference of called subprogram of direct calls, 'abstract_origin' was used. In addition, GNU GDB debugger is aware how to consume it in right way.

djtodoro abandoned this revision.Apr 15 2019, 8:11 AM