Page MenuHomePhabricator

[FormatEntity] Add mangled function name support
ClosedPublic

Authored by mib on Dec 9 2019, 5:05 PM.

Details

Summary

Add function.mangled-name key for FormatEntity to show the mangled
function names in backtraces.

rdar://54088244

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

Diff Detail

Event Timeline

mib created this revision.Dec 9 2019, 5:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 9 2019, 5:05 PM
mib added a reviewer: Restricted Project.Dec 9 2019, 5:06 PM
jasonmolenda marked an inline comment as done.Dec 9 2019, 5:09 PM
jasonmolenda added a subscriber: jasonmolenda.
jasonmolenda added inline comments.
lldb/test/Shell/Settings/TestFrameFormatMangling.test
10

Is this mangled name going to be the same on windows?

JDevlieghere added inline comments.
lldb/include/lldb/Target/Language.h
146 ↗(On Diff #232973)

eNameMangled?

lldb/source/Core/FormatEntity.cpp
1754

Let's be consistent with the braces. Either all single-line statements should have them, or none.

1782

Early return?

1800

This looks odd, are you missing a closing brace?

lldb/test/Shell/Settings/Inputs/main.cpp
10

This looks not clang formatted. I'm pretty sure I added a .clang-format file for the shell tests that should match the one used by LLDB but without the 80 col limit.

lldb/test/Shell/Settings/TestFrameFormatMangling.test
8

Can we simplify the format? For example, I don't think the color matters here.

mib updated this revision to Diff 232976.Dec 9 2019, 5:15 PM

Mark test as unsupported for windows.
Run clang-format.

xiaobai added a subscriber: xiaobai.Dec 9 2019, 5:15 PM
xiaobai added inline comments.
lldb/test/Shell/Settings/Inputs/main.cpp
96

nit: comment unneeded.

lldb/test/Shell/Settings/TestFrameFormatMangling.test
10

This will definitely be different on windows, so this will fail.

jingham added a subscriber: jingham.Dec 9 2019, 5:20 PM

If you make the frame format such that it is easy to pull the mangled & demangled names out of the format output (e.g. #${function.mangled-name}# ???${function.name}) then you could check that the two names are different, and for extra credit you could use "lang c++ demangle" to demangle it and match it against the regular mangled name...

labath added a subscriber: labath.Dec 10 2019, 2:15 AM

No opinion on the code, but I think the test needs to be cleaned up _a lot_.

lldb/test/Shell/Settings/Inputs/main.cpp
2

This file is about 70 lines longer than necessary to test this functionality, and it contains a #include statement in the middle of the file which is completely bogus

lldb/test/Shell/Settings/TestFrameFormatMangling.test
3

The -std argument doesn't seem relevant here.

4

-x not needed

mib updated this revision to Diff 233189.Dec 10 2019, 1:44 PM
mib marked 4 inline comments as done.

Remove unnecessary Language FunctionNameRepresention changes.
Rewrote test file. Refactored frame-format FormatEntity.

mib marked an inline comment as done.Dec 10 2019, 2:14 PM
mib marked 5 inline comments as done.
JDevlieghere added inline comments.Dec 11 2019, 1:57 PM
lldb/source/Core/FormatEntity.cpp
1763

No else after return

1768

You can write this as

if(Block *inline_block = sc->block->GetContainingInlinedBlock()) {
1771

Same here

lldb/test/Shell/Settings/Inputs/main.cpp
2

Tests don't need the license header

14

This tests seems way to complex for what you are testing. Unless another test is reusing this please make it as simple as possible.

lldb/test/Shell/Settings/TestFrameFormatMangling.test
11

If you're going to regex match the frame PC, why not remove it from the frame format?

mib updated this revision to Diff 233463.Dec 11 2019, 3:47 PM
mib marked 5 inline comments as done.
This comment was removed by mib.
mib updated this revision to Diff 233464.Dec 11 2019, 3:52 PM

Simplifying the test.

mib updated this revision to Diff 233465.Dec 11 2019, 3:54 PM

Run clang-format.

mib updated this revision to Diff 233473.Dec 11 2019, 5:04 PM
This revision is now accepted and ready to land.Dec 11 2019, 5:05 PM
teemperor accepted this revision.Dec 11 2019, 11:19 PM
This revision was automatically updated to reflect the committed changes.