JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, labath, jasonmolenda, aprantl.
Herald added a subscriber: kristof.beyls.
Herald added a reviewer: shafik.
Herald added a project: All.
JDevlieghere requested review of this revision.
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.
https://reviews.llvm.org/D148846
Files:
lldb/include/lldb/Core/Mangled.h
lldb/source/Core/Mangled.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1980,7 +1980,7 @@
} else if (!func_undecorated_name.empty()) {
mangled.SetDemangledName(ConstString(func_undecorated_name));
} else if (!func_name.empty())
- mangled.SetValue(ConstString(func_name), false);
+ mangled.SetValue(ConstString(func_name));
return mangled;
}
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2417,7 +2417,7 @@
&frame_base)) {
Mangled func_name;
if (mangled)
- func_name.SetValue(ConstString(mangled), true);
+ func_name.SetValue(ConstString(mangled));
else if ((die.GetParent().Tag() == DW_TAG_compile_unit ||
die.GetParent().Tag() == DW_TAG_partial_unit) &&
Language::LanguageIsCPlusPlus(
@@ -2428,9 +2428,9 @@
// If the mangled name is not present in the DWARF, generate the
// demangled name using the decl context. We skip if the function is
// "main" as its name is never mangled.
- func_name.SetValue(ConstructDemangledNameFromDWARF(die), false);
+ func_name.SetValue(ConstructDemangledNameFromDWARF(die));
} else
- func_name.SetValue(ConstString(name), false);
+ func_name.SetValue(ConstString(name));
FunctionSP func_sp;
std::unique_ptr<Declaration> decl_up;
Index: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -246,7 +246,7 @@
if (auto record = FuncRecord::parse(*It)) {
Mangled func_name;
- func_name.SetValue(ConstString(record->Name), false);
+ func_name.SetValue(ConstString(record->Name));
addr_t address = record->Address + base;
SectionSP section_sp = list->FindSectionContainingFileAddress(address);
if (section_sp) {
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -3030,7 +3030,7 @@
// the first contains a directory and the
// second contains a full path.
sym[sym_idx - 1].GetMangled().SetValue(
- ConstString(symbol_name), false);
+ ConstString(symbol_name));
m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
add_nlist = false;
} else {
@@ -3076,7 +3076,7 @@
full_so_path += '/';
full_so_path += symbol_name;
sym[sym_idx - 1].GetMangled().SetValue(
- ConstString(full_so_path.c_str()), false);
+ ConstString(full_so_path.c_str()));
add_nlist = false;
m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
}
@@ -3468,17 +3468,13 @@
sym[sym_idx].GetMangled().SetDemangledName(
ConstString(symbol_name));
} else {
- bool symbol_name_is_mangled = false;
-
if (symbol_name && symbol_name[0] == '_') {
- symbol_name_is_mangled = symbol_name[1] == '_';
symbol_name++; // Skip the leading underscore
}
if (symbol_name) {
ConstString const_symbol_name(symbol_name);
- sym[sym_idx].GetMangled().SetValue(
- const_symbol_name, symbol_name_is_mangled);
+ sym[sym_idx].GetMangled().SetValue(const_symbol_name);
if (is_gsym && is_debug) {
const char *gsym_name =
sym[sym_idx]
@@ -3938,8 +3934,8 @@
if ((N_SO_index == sym_idx - 1) && ((sym_idx - 1) < num_syms)) {
// We have two consecutive N_SO entries where the first
// contains a directory and the second contains a full path.
- sym[sym_idx - 1].GetMangled().SetValue(ConstString(symbol_name),
- false);
+ sym[sym_idx - 1].GetMangled().SetValue(
+ ConstString(symbol_name));
m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
add_nlist = false;
} else {
@@ -3977,7 +3973,7 @@
full_so_path += '/';
full_so_path += symbol_name;
sym[sym_idx - 1].GetMangled().SetValue(
- ConstString(full_so_path.c_str()), false);
+ ConstString(full_so_path.c_str()));
add_nlist = false;
m_nlist_idx_to_sym_idx[nlist_idx] = sym_idx - 1;
}
@@ -4330,17 +4326,14 @@
ConstString(symbol_name_non_abi_mangled));
sym[sym_idx].GetMangled().SetDemangledName(ConstString(symbol_name));
} else {
- bool symbol_name_is_mangled = false;
if (symbol_name && symbol_name[0] == '_') {
- symbol_name_is_mangled = symbol_name[1] == '_';
symbol_name++; // Skip the leading underscore
}
if (symbol_name) {
ConstString const_symbol_name(symbol_name);
- sym[sym_idx].GetMangled().SetValue(const_symbol_name,
- symbol_name_is_mangled);
+ sym[sym_idx].GetMangled().SetValue(const_symbol_name);
}
}
Index: lldb/source/Core/Mangled.cpp
===================================================================
--- lldb/source/Core/Mangled.cpp
+++ lldb/source/Core/Mangled.cpp
@@ -90,23 +90,6 @@
b.GetName(ePreferMangled));
}
-// Set the string value in this objects. If "mangled" is true, then the mangled
-// named is set with the new value in "s", else the demangled name is set.
-void Mangled::SetValue(ConstString s, bool mangled) {
- if (s) {
- if (mangled) {
- m_demangled.Clear();
- m_mangled = s;
- } else {
- m_demangled = s;
- m_mangled.Clear();
- }
- } else {
- m_demangled.Clear();
- m_mangled.Clear();
- }
-}
-
void Mangled::SetValue(ConstString name) {
if (name) {
if (cstring_is_mangled(name.GetStringRef())) {
Index: lldb/include/lldb/Core/Mangled.h
===================================================================
--- lldb/include/lldb/Core/Mangled.h
+++ lldb/include/lldb/Core/Mangled.h
@@ -185,19 +185,6 @@
/// The number of bytes that this object occupies in memory.
size_t MemorySize() const;
- /// Set the string value in this object.
- ///
- /// If \a is_mangled is \b true, then the mangled named is set to \a name,
- /// else the demangled name is set to \a name.
- ///
- /// \param[in] name
- /// The already const version of the name for this object.
- ///
- /// \param[in] is_mangled
- /// If \b true then \a name is a mangled name, if \b false then
- /// \a name is demangled.
- void SetValue(ConstString name, bool is_mangled);
-
/// Set the string value in this object.
///
/// This version auto detects if the string is mangled by inspecting the
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
