Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lldb] Print mangled names with verbose break list #84071

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

felipepiovezan
Copy link
Contributor

When debugging LLDB itself, it can often be useful to know the mangled name of the function where a breakpoint is set. Since the --verbose setting of break --list is aimed at debugging LLDB, this patch makes it so that the mangled name is also printed in that mode.

Note about testing: since mangling is not the same on Windows and Linux, the test refrains from hardcoding mangled names.

When debugging LLDB itself, it can often be useful to know the mangled name of
the function where a breakpoint is set. Since the `--verbose` setting of `break
--list` is aimed at debugging LLDB, this patch makes it so that the mangled name
is also printed in that mode.

Note about testing: since mangling is not the same on Windows and Linux, the
test refrains from hardcoding mangled names.
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 5, 2024

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

When debugging LLDB itself, it can often be useful to know the mangled name of the function where a breakpoint is set. Since the --verbose setting of break --list is aimed at debugging LLDB, this patch makes it so that the mangled name is also printed in that mode.

Note about testing: since mangling is not the same on Windows and Linux, the test refrains from hardcoding mangled names.


Full diff: https://github.com/llvm/llvm-project/pull/84071.diff

2 Files Affected:

  • (modified) lldb/source/Breakpoint/BreakpointLocation.cpp (+3)
  • (modified) lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py (+9)
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index f7b8ca1f5506f3..045ed145cc7c49 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -524,6 +524,9 @@ void BreakpointLocation::GetDescription(Stream *s,
           s->EOL();
           s->Indent("function = ");
           s->PutCString(sc.function->GetName().AsCString("<unknown>"));
+          s->EOL();
+          s->Indent("mangled function = ");
+          s->PutCString(sc.function->GetMangled().GetMangledName().AsCString("<unknown>"));
         }
 
         if (sc.line_entry.line > 0) {
diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py b/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
index 129290909029a1..c3b911145db3a6 100644
--- a/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
+++ b/lldb/test/API/functionalities/breakpoint/breakpoint_options/TestBreakpointOptions.py
@@ -90,6 +90,15 @@ def breakpoint_options_language_test(self):
             num_expected_locations=1,
         )
 
+        self.expect(
+            "breakpoint list -v",
+            "Verbose breakpoint list contains mangled names",
+            substrs=[
+                "function = ns::func"
+                "mangled function ="
+            ],
+        )
+
         # This should create a breakpoint with 0 locations.
         lldbutil.run_break_set_by_symbol(
             self,

Copy link

github-actions bot commented Mar 5, 2024

✅ With the latest revision this PR passed the Python code formatter.

Copy link

github-actions bot commented Mar 5, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@@ -524,6 +524,9 @@ void BreakpointLocation::GetDescription(Stream *s,
s->EOL();
s->Indent("function = ");
s->PutCString(sc.function->GetName().AsCString("<unknown>"));
s->EOL();
s->Indent("mangled function = ");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to emit "mangled function" for C which doesn't have mangled names? It might be nicer to check whether the GetMangledName returns a mangled name, and only print it if it does? Having:

function = main
mangled function = main

looks a little silly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's doable, I'll have a look. (in the C case, we print "unknown" with the current impl, which is arguably more silly)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually don't feel super strongly about this one, however.

Copy link
Collaborator

@jimingham jimingham Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we really shouldn't print "unknown". You could make the fallback "", since there can't be a mangled name that's that, and then if you get back "" don't print the line.

@@ -90,6 +90,15 @@ def breakpoint_options_language_test(self):
num_expected_locations=1,
)
Copy link
Collaborator

@jimingham jimingham Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run_break_set_by_symbol returns the break ID of the new breakpoint. You could look it up, get its location, get the function from the SBSymbolContext at the breakpoint location's address and get the function's mangled name from there. That way you could do a mangling scheme independent test that you produced the right mangled name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! this works really well

Copy link
Collaborator

@clayborg clayborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to print the mangled name of the symbol as well? We are doing it for the function...

Comment on lines 528 to 529
s->Indent("mangled function = ");
s->PutCString(sc.function->GetMangled().GetMangledName().AsCString("<unknown>"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (sc.function->GetMangled().GetMangledName()) {
  s->Indent("mangled function = ");
  s->PutCString(sc.function->GetMangled().GetMangledName().AsCString("<unknown>"));
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does SymbolContext::GetFunctionName do the right thing here? It will also find the innermost inlined function if the SC is in some inlining and return that as well.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it does, you could just call it twice, once with the prefer mangled false, and once true.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be nice in verbose mode to see the whole chain of functions from the concrete function down to the most inlined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will also find the innermost inlined function if the SC is in some inlining and return that as well.

From an inspection of its implementation, it is not as well, but rather instead.

It might be nice in verbose mode to see the whole chain of functions from the concrete function down to the most inlined?

I agree it would be nice, but I couldn't find any readily available APIs that go past the first inline context, so this is probably better done as a follow up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might be able to reuse bool SymbolContext::DumpStopContext, this is what is plain break list uses

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does look like SymbolContext::DumpStopContext does what we want. Will the output fit well into the "breakpoint list --verbose" output? Or will it look too different?

I would be fine to just print the inline function name if the output is too different.

FYI: Any lldb_private::Block may or may not have inlined function info. You can always call:

Block *inlined_block = block->GetContainingInlinedBlock();

If block is an inlined function it will return block, else it will get the parent block and see if any parent blocks contain inline function info. If it returns NULL, then block isn't a or contained in an inline block.

Getting an inline call stack from this information is tricky and is done in SymbolContext::DumpStopContext() correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that you mention it, I think we kind of have to use DumpStopContext. As it stands, the verbose output is missing information that the non-verbose output has, which feels wrong.

But let's merge this one first, since doing the above requires changing existing pieces, and this PR is about adding a new piece.

Copy link
Collaborator

@clayborg clayborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for just including the mangled name.

Copy link
Collaborator

@jimingham jimingham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this looks fine to me for adding the mangled name.

@felipepiovezan felipepiovezan merged commit 0497c77 into llvm:main Mar 7, 2024
4 checks passed
@felipepiovezan felipepiovezan deleted the felipe/verbose_bps branch March 7, 2024 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants