[Lldb-commits] [lldb] 3339000 - We don't require users to type out the full context of a function, for
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Thu May 12 12:39:36 PDT 2022
Author: Jim Ingham
Date: 2022-05-12T12:39:28-07:00
New Revision: 3339000e0bda696c2e29173d15958c0a4978a143
URL: https://github.com/llvm/llvm-project/commit/3339000e0bda696c2e29173d15958c0a4978a143
DIFF: https://github.com/llvm/llvm-project/commit/3339000e0bda696c2e29173d15958c0a4978a143.diff
LOG: We don't require users to type out the full context of a function, for
symbol name matches. Instead, we extract the incoming path's base
name, look up all the symbols with that base name, and then compare
the rest of the context that the user provided to make sure it
matches. However, we do this comparison using just a strstr. So for
instance:
break set -n foo::bar
will match not only "a::foo::bar" but "notherfoo::bar". The former is
pretty clearly the user's intent, but I don't think the latter is, and
results in breakpoints picking up too many matches.
This change adds a Language::DemangledNameContainsPath API which can
do a language aware match against the path provided. If the language
doesn't provide this we fall back to the strstr (though that's changed
to StringRef::contains in the patch).
Differential Revision: https://reviews.llvm.org/D124579
Added:
Modified:
lldb/include/lldb/Target/Language.h
lldb/source/Core/Module.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
lldb/source/Target/Language.cpp
lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
lldb/test/API/functionalities/breakpoint/cpp/main.cpp
lldb/test/API/functionalities/return-value/TestReturnValue.py
lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py
lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/Language.h b/lldb/include/lldb/Target/Language.h
index ce2d273a82770..fa79aaee05748 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -217,6 +217,17 @@ class Language : public PluginInterface {
std::string &prefix,
std::string &suffix);
+ // When looking up functions, we take a user provided string which may be a
+ // partial match to the full demangled name and compare it to the actual
+ // demangled name to see if it matches as much as the user specified. An
+ // example of this is if the user provided A::my_function, but the
+ // symbol was really B::A::my_function. We want that to be
+ // a match. But we wouldn't want this to match AnotherA::my_function. The
+ // user is specifying a truncated path, not a truncated set of characters.
+ // This function does a language-aware comparison for those purposes.
+ virtual bool DemangledNameContainsPath(llvm::StringRef path,
+ ConstString demangled) const;
+
// if a language has a custom format for printing variable declarations that
// it wants LLDB to honor it should return an appropriate closure here
virtual DumpValueObjectOptions::DeclPrintingHelper GetDeclPrintingHelper();
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index e89d5ec401788..7160c2386efd6 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -739,13 +739,25 @@ void Module::LookupInfo::Prune(SymbolContextList &sc_list,
while (i < sc_list.GetSize()) {
if (!sc_list.GetContextAtIndex(i, sc))
break;
- ConstString full_name(sc.GetFunctionName());
- if (full_name &&
- ::strstr(full_name.GetCString(), m_name.GetCString()) == nullptr) {
- sc_list.RemoveContextAtIndex(i);
- } else {
- ++i;
+
+ llvm::StringRef user_name = m_name.GetStringRef();
+ bool keep_it = true;
+ Language *language = Language::FindPlugin(sc.GetLanguage());
+ // If the symbol has a language, then let the language make the match.
+ // Otherwise just check that the demangled name contains the user name.
+ if (language)
+ keep_it = language->DemangledNameContainsPath(m_name.GetStringRef(),
+ sc.GetFunctionName());
+ else {
+ llvm::StringRef full_name = sc.GetFunctionName().GetStringRef();
+ // We always keep unnamed symbols:
+ if (!full_name.empty())
+ keep_it = full_name.contains(user_name);
}
+ if (keep_it)
+ ++i;
+ else
+ sc_list.RemoveContextAtIndex(i);
}
}
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 56daf75ee0faf..08b6d89e55f7a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -268,6 +268,41 @@ std::string CPlusPlusLanguage::MethodName::GetScopeQualifiedName() {
return res;
}
+bool CPlusPlusLanguage::MethodName::ContainsPath(llvm::StringRef path) {
+ if (!m_parsed)
+ Parse();
+ // If we can't parse the incoming name, then just check that it contains path.
+ if (m_parse_error)
+ return m_full.GetStringRef().contains(path);
+
+ llvm::StringRef identifier;
+ llvm::StringRef context;
+ std::string path_str = path.str();
+ bool success
+ = CPlusPlusLanguage::ExtractContextAndIdentifier(path_str.c_str(),
+ context,
+ identifier);
+ if (!success)
+ return m_full.GetStringRef().contains(path);
+
+ if (identifier != GetBasename())
+ return false;
+ // Incoming path only had an identifier, so we match.
+ if (context.empty())
+ return true;
+ // Incoming path has context but this method does not, no match.
+ if (m_context.empty())
+ return false;
+
+ llvm::StringRef haystack = m_context;
+ if (!haystack.consume_back(context))
+ return false;
+ if (haystack.empty() || !isalnum(haystack.back()))
+ return true;
+
+ return false;
+}
+
bool CPlusPlusLanguage::IsCPPMangledName(llvm::StringRef name) {
// FIXME!! we should really run through all the known C++ Language plugins
// and ask each one if this is a C++ mangled name
@@ -280,6 +315,12 @@ bool CPlusPlusLanguage::IsCPPMangledName(llvm::StringRef name) {
return true;
}
+bool CPlusPlusLanguage::DemangledNameContainsPath(llvm::StringRef path,
+ ConstString demangled) const {
+ MethodName demangled_name(demangled);
+ return demangled_name.ContainsPath(path);
+}
+
bool CPlusPlusLanguage::ExtractContextAndIdentifier(
const char *name, llvm::StringRef &context, llvm::StringRef &identifier) {
if (MSVCUndecoratedNameParser::IsMSVCUndecoratedName(name))
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
index 5547864a37639..53a01cfc4799d 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -55,6 +55,8 @@ class CPlusPlusLanguage : public Language {
llvm::StringRef GetArguments();
llvm::StringRef GetQualifiers();
+
+ bool ContainsPath(llvm::StringRef path);
protected:
void Parse();
@@ -105,6 +107,9 @@ class CPlusPlusLanguage : public Language {
static llvm::StringRef GetPluginNameStatic() { return "cplusplus"; }
bool SymbolNameFitsToLanguage(Mangled mangled) const override;
+
+ bool DemangledNameContainsPath(llvm::StringRef path,
+ ConstString demangled) const override;
ConstString
GetDemangledFunctionNameWithoutArguments(Mangled mangled) const override;
diff --git a/lldb/source/Target/Language.cpp b/lldb/source/Target/Language.cpp
index eee1ff1512d95..6df36aeeb7b72 100644
--- a/lldb/source/Target/Language.cpp
+++ b/lldb/source/Target/Language.cpp
@@ -428,6 +428,14 @@ bool Language::GetFormatterPrefixSuffix(ValueObject &valobj,
return false;
}
+bool Language::DemangledNameContainsPath(llvm::StringRef path,
+ ConstString demangled) const {
+ // The base implementation does a simple contains comparision:
+ if (path.empty())
+ return false;
+ return demangled.GetStringRef().contains(path);
+}
+
DumpValueObjectOptions::DeclPrintingHelper Language::GetDeclPrintingHelper() {
return nullptr;
}
diff --git a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py b/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
index be21c6eea138b..8edd852cd8ece 100644
--- a/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
+++ b/lldb/test/API/functionalities/breakpoint/cpp/TestCPPBreakpointLocations.py
@@ -28,7 +28,7 @@ def verify_breakpoint_locations(self, target, bp_dict):
self.assertEquals(
bp.GetNumLocations(),
len(names),
- "Make sure we find the right number of breakpoint locations")
+ "Make sure we find the right number of breakpoint locations for {}".format(name))
bp_loc_names = list()
for bp_loc in bp:
@@ -47,9 +47,9 @@ def breakpoint_id_tests(self):
target = self.dbg.CreateTarget(exe)
self.assertTrue(target, VALID_TARGET)
bp_dicts = [
- {'name': 'func1', 'loc_names': ['a::c::func1()', 'b::c::func1()']},
- {'name': 'func2', 'loc_names': ['a::c::func2()', 'c::d::func2()']},
- {'name': 'func3', 'loc_names': ['a::c::func3()', 'b::c::func3()', 'c::d::func3()']},
+ {'name': 'func1', 'loc_names': ['a::c::func1()', 'aa::cc::func1()', 'b::c::func1()']},
+ {'name': 'func2', 'loc_names': ['a::c::func2()', 'aa::cc::func2()', 'c::d::func2()']},
+ {'name': 'func3', 'loc_names': ['a::c::func3()', 'aa::cc::func3()', 'b::c::func3()', 'c::d::func3()']},
{'name': 'c::func1', 'loc_names': ['a::c::func1()', 'b::c::func1()']},
{'name': 'c::func2', 'loc_names': ['a::c::func2()']},
{'name': 'c::func3', 'loc_names': ['a::c::func3()', 'b::c::func3()']},
diff --git a/lldb/test/API/functionalities/breakpoint/cpp/main.cpp b/lldb/test/API/functionalities/breakpoint/cpp/main.cpp
index f352084e74447..088e33c6a7c73 100644
--- a/lldb/test/API/functionalities/breakpoint/cpp/main.cpp
+++ b/lldb/test/API/functionalities/breakpoint/cpp/main.cpp
@@ -24,6 +24,29 @@ namespace a {
c::~c() {}
}
+namespace aa {
+ class cc {
+ public:
+ cc();
+ ~cc();
+ void func1()
+ {
+ puts (__PRETTY_FUNCTION__);
+ }
+ void func2()
+ {
+ puts (__PRETTY_FUNCTION__);
+ }
+ void func3()
+ {
+ puts (__PRETTY_FUNCTION__);
+ }
+ };
+
+ cc::cc() {}
+ cc::~cc() {}
+}
+
namespace b {
class c {
public:
@@ -62,11 +85,15 @@ namespace c {
int main (int argc, char const *argv[])
{
a::c ac;
+ aa::cc aac;
b::c bc;
c::d cd;
ac.func1();
ac.func2();
ac.func3();
+ aac.func1();
+ aac.func2();
+ aac.func3();
bc.func1();
bc.func3();
cd.func2();
diff --git a/lldb/test/API/functionalities/return-value/TestReturnValue.py b/lldb/test/API/functionalities/return-value/TestReturnValue.py
index f2f50385b8bd7..517ed46a4fe9f 100644
--- a/lldb/test/API/functionalities/return-value/TestReturnValue.py
+++ b/lldb/test/API/functionalities/return-value/TestReturnValue.py
@@ -237,7 +237,7 @@ def return_and_test_struct_value(self, func_name):
# Set the breakpoint, run to it, finish out.
bkpt = self.target.BreakpointCreateByName(func_name)
- self.assertTrue(bkpt.GetNumResolvedLocations() > 0)
+ self.assertTrue(bkpt.GetNumResolvedLocations() > 0, "Got wrong number of locations for {0}".format(func_name))
self.process.Continue()
diff --git a/lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py b/lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py
index 5c76d7aabf1bc..e2cebf0209104 100644
--- a/lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py
+++ b/lldb/test/API/macosx/indirect_symbol/TestIndirectSymbols.py
@@ -98,7 +98,7 @@ def test_with_python_api(self):
# make sure we are again in out target function.
break_reexported = target.BreakpointCreateByName(
"reexport_to_indirect")
- self.assertTrue(break_reexported, VALID_BREAKPOINT)
+ self.assertEqual(break_reexported.GetNumLocations(), 1, VALID_BREAKPOINT)
# Now continue should take us to the second call through the indirect
# symbol:
diff --git a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
index 20abd5a87e1b4..bf887fb0777c5 100644
--- a/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ b/lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -123,6 +123,37 @@ TEST(CPlusPlusLanguage, MethodNameParsing) {
}
}
+TEST(CPlusPlusLanguage, ContainsPath) {
+ CPlusPlusLanguage::MethodName
+ reference_1(ConstString("int foo::bar::func01(int a, double b)"));
+ CPlusPlusLanguage::MethodName
+ reference_2(ConstString("int foofoo::bar::func01(std::string a, int b)"));
+ CPlusPlusLanguage::MethodName reference_3(ConstString("int func01()"));
+ CPlusPlusLanguage::MethodName
+ reference_4(ConstString("bar::baz::operator bool()"));
+
+ EXPECT_TRUE(reference_1.ContainsPath("func01"));
+ EXPECT_TRUE(reference_1.ContainsPath("bar::func01"));
+ EXPECT_TRUE(reference_1.ContainsPath("foo::bar::func01"));
+ EXPECT_FALSE(reference_1.ContainsPath("func"));
+ EXPECT_FALSE(reference_1.ContainsPath("baz::func01"));
+ EXPECT_FALSE(reference_1.ContainsPath("::bar::func01"));
+ EXPECT_FALSE(reference_1.ContainsPath("::foo::baz::func01"));
+ EXPECT_FALSE(reference_1.ContainsPath("foo::bar::baz::func01"));
+
+ EXPECT_TRUE(reference_2.ContainsPath("foofoo::bar::func01"));
+ EXPECT_FALSE(reference_2.ContainsPath("foo::bar::func01"));
+
+ EXPECT_TRUE(reference_3.ContainsPath("func01"));
+ EXPECT_FALSE(reference_3.ContainsPath("func"));
+ EXPECT_FALSE(reference_3.ContainsPath("bar::func01"));
+
+ EXPECT_TRUE(reference_4.ContainsPath("operator bool"));
+ EXPECT_TRUE(reference_4.ContainsPath("baz::operator bool"));
+ EXPECT_TRUE(reference_4.ContainsPath("bar::baz::operator bool"));
+ EXPECT_FALSE(reference_4.ContainsPath("az::operator bool"));
+}
+
TEST(CPlusPlusLanguage, ExtractContextAndIdentifier) {
struct TestCase {
std::string input;
More information about the lldb-commits
mailing list