This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Let Mangled decide whether a name is mangled or not
ClosedPublic

Authored by JDevlieghere on Apr 20 2023, 2:48 PM.

Details

Summary

We have a handful of places in LLDB where we try to outsmart the logic in Mangled to determine whether a string is mangled or not. There's at least one place (*) where we were getting this wrong and this causes a subtle bug. The cstring_is_mangled is cheap enough that we should always rely on it to determine whether a string is mangled or not.

(*) ObjectFileMachO assumes that a symbol that starts with a double underscore (such as __pthread_kill) is mangled. That's mostly harmless, until you want to use function.name-without-args in your frame format. For this formatter, we call Symbol::GetNameNoArguments() which is a wrapper around Mangled::GetName(Mangled::ePreferDemangledWithoutArguments). The latter will first try using the appropriate language plugin to get the demangled name without arguments, but if that fails, it falls back to returning the demangled name. Because we forced Mangled to treat the symbol as a mangled name while it's not, there's no demangled name, and your frames don't show symbols at all. Alternatively, I could've worked around the issue by checking if we have a demangeld at all, but that's just working around the bug.

Diff Detail

Event Timeline

JDevlieghere created this revision.Apr 20 2023, 2:48 PM
Herald added a project: Restricted Project. · View Herald Transcript
JDevlieghere requested review of this revision.Apr 20 2023, 2:48 PM

Add simple test

aprantl accepted this revision.Apr 21 2023, 8:36 AM

It seems like we can quickly and unambiguously decide whether a name is mangled or not by looking at the first characters so that seems fine to me. Does Mangled try all Language plugins to decide?
This seems like a risky change, but I think it's going into the right direction.

This revision is now accepted and ready to land.Apr 21 2023, 8:36 AM
aprantl added inline comments.Apr 21 2023, 8:38 AM
lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
2

why?

8

I guess this is a no debug info test?

lldb/test/API/macosx/format/main.c
3

why?

JDevlieghere marked 3 inline comments as done.Apr 21 2023, 10:11 AM
JDevlieghere added inline comments.
lldb/test/API/macosx/format/TestFunctionNameWithoutArgs.py
2

I assume the question is rhetoric, but on the off-chance it's not: I copy-pasted it from another test. Same for the other "why?".

This revision was automatically updated to reflect the committed changes.
JDevlieghere marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2023, 10:23 AM