This is an archive of the discontinued LLVM Phabricator instance.

Command-line options to set debugger "target"
AbandonedPublic

Authored by probinson on Mar 24 2015, 6:22 PM.

Details

Summary

The Clang side of the debugger "target" patches. Slightly repurpose -ggdb and
add -glldb and -gsce to set GDB, LLDB, or PS4 as the "debugger target."
This is thematically consistent with GCC options such as -ggdb and -gvms.
No Clang functionality depends on this yet, although conditionally importing
anonymous namespaces (in review) could use this instead of a triple test.

The LLVM part is available at http://reviews.llvm.org/D8506

I'll whip up the obvious -### driver test but thought I'd put this out now.

Diff Detail

Event Timeline

probinson updated this revision to Diff 22624.Mar 24 2015, 6:22 PM
probinson retitled this revision from to Command-line options to set debugger "target".
probinson updated this object.
probinson edited the test plan for this revision. (Show Details)
probinson added reviewers: echristo, dblaikie, aprantl.
probinson added a subscriber: Unknown Object (MLST).

Reposting the initial comment as I hit "save" before setting reviewers/subscribers...

The Clang side of the debugger "target" patches. Slightly repurpose -ggdb and
add -glldb and -gsce to set GDB, LLDB, or PS4 as the "debugger target."
This is thematically consistent with GCC options such as -ggdb and -gvms.
No Clang functionality depends on this yet, although conditionally importing
anonymous namespaces (in review) could use this instead of a triple test.

The LLVM part is available at http://reviews.llvm.org/D8506

I'll whip up the obvious -### driver test but thought I'd put this out now.

dblaikie added inline comments.Mar 25 2015, 11:00 AM
include/clang/Driver/Options.td
1051

Is SCE usually pronounced with each letter separately, or as a word ("ski" or similar)? (wondering whether "an SCE target" or "a SCE target" is correct)

include/clang/Frontend/CodeGenOptions.def
159

I take it that GDB is the default here? But ignored in the frontend or driver in favor of platform-specific defaults?

lib/Frontend/CompilerInvocation.cpp
408

Should this handling be up in the driver? Then it should be easy to write tests for the defaults.

probinson added inline comments.Mar 25 2015, 2:47 PM
include/clang/Driver/Options.td
1051

SCE = ess see ee (Sony Computer Entertainment)
There is a European division, SCEE, which I have heard pronounced as "ski."

include/clang/Frontend/CodeGenOptions.def
159

Well, it seems to be customary to put a default here, and then initialize the field explicitly elsewhere anyway without relying on the default value. I didn't exhaustively check every field, but eyeballing a handful it was certainly the pattern.
The most entertaining case was DwarfVersion.

lib/Frontend/CompilerInvocation.cpp
408

No, it should be here, so we can specify a debugger target for -cc1 tests.
There aren't any use-cases for that yet, but Katya's import-anonymous-namespace patch could use it, once it goes in.

echristo edited edge metadata.Mar 25 2015, 2:48 PM

You could handle some of the DW_AT_APPLE foo things that way?

-eric

dblaikie added inline comments.Mar 25 2015, 2:51 PM
lib/Frontend/CompilerInvocation.cpp
408

Sorry - I mean specifically the cases a few lines down that test isOSDarwin and isPS4CPU - I imagine those should be handled in the driver (sorry, I looked at this block of code & assumed it all dealt with defaults, etc) - and this code should then be simpler and just set the debugger based on which debugger flag is passed down (& possibly rely on the default if no debugger is specified)

You could handle some of the DW_AT_APPLE foo things that way?

-eric

That would be on the LLVM side, and somebody would have to decide whether debugger-target or a triple check is the more appropriate condition. But, yeah.

probinson added inline comments.Mar 25 2015, 3:01 PM
lib/Frontend/CompilerInvocation.cpp
408

Oh, I see. Yes, that would make sense.

probinson updated this revision to Diff 22692.Mar 25 2015, 7:02 PM
probinson edited edge metadata.

Update the debug-options.c test to include the debugger target.
Move target-specific defaulting decisions into the driver.
Update how options are handled in the driver (yay tests!).

dblaikie added inline comments.Mar 27 2015, 9:25 AM
test/Driver/debug-options.c
32

Given that -g<debugger> and -g<debug level> are orthogonal (they are, right? Judging by your change they look orthogonal) we don't need to test all the combinations of them. I'd either just test them separately (g1, g2, g3, ggdb, glldb, gsce) or if want to really try to minimize the number of test cases, test them in ad-hoc pairs (g1+ggdb, g2+glldb, etc.) - though that can make things a bit harder to maintain later on (trying to decide if the pairing is significant or not, etc).

aprantl edited edge metadata.May 6 2015, 2:57 PM

The new options should probably also be mentioned in the man page (docs/tools/clang.pod).

probinson abandoned this revision.Dec 18 2015, 12:28 PM

Split into separate CC1 and Driver parts for easier reviewing.