Page MenuHomePhabricator

Add option for emitting dbg info for call site parameters
Needs ReviewPublic

Authored by djtodoro on Feb 11 2019, 3:09 AM.

Details

Summary

Generating of all info about call sites and call site parameters will be restricted with the option until all testing and patching is done.

Example of using the option:

clang -g -O2 -femit-param-entry-values test.c

In addition, when the option is set add flag all_call_sites
in a subprogram in order to support GNU extension of DWARF as well.

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

Diff Detail

Event Timeline

djtodoro created this revision.Feb 11 2019, 3:09 AM
djtodoro retitled this revision from [clang] Add option for emitting dbg info for call sites to Add option for emitting dbg info for call sites.
djtodoro edited the summary of this revision. (Show Details)Feb 11 2019, 3:14 AM
djtodoro added a subscriber: NikolaPrica.
djtodoro added a project: debug-info.
djtodoro edited reviewers, added: cfe-commits; removed: llvm-commits.
djtodoro added a subscriber: petarj.
djtodoro edited the summary of this revision. (Show Details)Feb 11 2019, 4:48 AM
djtodoro updated this revision to Diff 195188.Mon, Apr 15, 8:26 AM
djtodoro edited the summary of this revision. (Show Details)

-Rebase
-Add all_call_sites flag in the case of GNU extensions

aprantl added inline comments.Mon, Apr 15, 8:28 PM
include/clang/Driver/Options.td
919

I assume that this is the same -f option that GCC uses?

djtodoro marked an inline comment as done.Mon, Apr 15, 11:42 PM
djtodoro added inline comments.
include/clang/Driver/Options.td
919

Actually in GCC production of call_site and call_site_parameters debug info is enabled by default.
Production of entry_values is implemented on top of variable's value tracking system and there is no particular option just for entry_values.

Basically, we introduce this option as experimental one. As soon as we test everything we should get rid of this or turn it ON by default (and maybe add '-fno-emit-param-entry-values' in order to have an option for disabling the functionality). That will be ideal scenario.

I guess I'm not clear what your final goal is for the option. Keep it, even though GCC doesn't have one like it? Eliminate it? Please clearly state what you intend to have in the end, and what you might have in the short term in case that is different.

include/clang/Driver/CC1Options.td
362 ↗(On Diff #195188)

I think this is not necessary, see comment on Options.td.

include/clang/Driver/Options.td
919

By convention, the name of the option should start with 'f' and match the option spelling (with hyphens changed to underscore).

921

I believe that by marking this with [CC1Option] cc1 will automatically understand it and you won't need to define a separate option in CC1Options.td.

And if we plan to enable it by default, too, perhaps not adding a driver option (CC1 only) is preferable, since we need to maintain compatibility with driver options indefinitely.

djtodoro marked 2 inline comments as done.Wed, Apr 17, 7:31 AM

@probinson @aprantl Thanks a lot for your comments!

Let's clarify some things. I'm sorry about the confusion.

Initial patch for the functionality can be restricted by this option (like we pushed here), since LLDB doesn't read this type of debug info (yet).
The plan is, when LLDB has all necessary info and we are sure that everything works fine during using this info in debugging sessions, we can turn it to ON by default.

Does that make sense ? :)

include/clang/Driver/Options.td
921

I can't remember what problem we occurred with this on 4.0, but we definitely don't need both definitions.
Thanks for this!

djtodoro updated this revision to Diff 195568.Wed, Apr 17, 7:35 AM
djtodoro retitled this revision from Add option for emitting dbg info for call sites to Add option for emitting dbg info for call site parameters.

-Refactor
-Remove CC1 def

djtodoro updated this revision to Diff 195571.Wed, Apr 17, 7:54 AM

-Fix comments

@probinson @aprantl Thanks a lot for your comments!

Let's clarify some things. I'm sorry about the confusion.

Initial patch for the functionality can be restricted by this option (like we pushed here), since LLDB doesn't read this type of debug info (yet).
The plan is, when LLDB has all necessary info and we are sure that everything works fine during using this info in debugging sessions, we can turn it to ON by default.

Does that make sense ? :)

Yes. And I agree with Adrian, this should be a CC1-only option; it can be default-off for now and maybe debugger-tuning specific later.