[Lldb-commits] [lldb] 22667e3 - Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Thu Jun 1 16:15:15 PDT 2023
Author: Jim Ingham
Date: 2023-06-01T16:15:06-07:00
New Revision: 22667e3220de5ead353a2148265d841644b63824
URL: https://github.com/llvm/llvm-project/commit/22667e3220de5ead353a2148265d841644b63824
DIFF: https://github.com/llvm/llvm-project/commit/22667e3220de5ead353a2148265d841644b63824.diff
LOG: Fix regex & startsWith name lookup in SBTarget::FindGlobalVariables
There were two bugs here.
eMatchTypeStartsWith searched for "symbol_name" by adding ".*" to the
end of the symbol name and treating that as a regex, which isn't
actually a regex for "starts with". The ".*" is in fact a no-op. When
we finally get to comparing the name, we compare against whatever form
of the name was in the accelerator table. But for C++ that might be
the mangled name. We should also try demangled names here, since most
users are going the see demangled not mangled names. I fixed these
two bugs and added a bunch of tests for FindGlobalVariables.
This change is in the DWARF parser code, so there may be a similar bug
in PDB, but the test for this was already skipped for Windows, so I
don't know about this.
You might theoretically need to do this Mangled comparison in
DWARFMappedHash::MemoryTable::FindByName
except when we have names we always chop them before looking them up
so I couldn't see any code paths that fail without that change. So I
didn't add that to this patch.
Differential Revision: https://reviews.llvm.org/D151940
Added:
Modified:
lldb/source/API/SBTarget.cpp
lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
lldb/test/API/lang/cpp/class_static/main.cpp
Removed:
################################################################################
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 980cb7788bf51..53af5b1d7a477 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1892,7 +1892,7 @@ SBValueList SBTarget::FindGlobalVariables(const char *name,
max_matches, variable_list);
break;
case eMatchTypeStartsWith:
- regexstr = llvm::Regex::escape(name) + ".*";
+ regexstr = "^" + llvm::Regex::escape(name) + ".*";
target_sp->GetImages().FindGlobalVariables(RegularExpression(regexstr),
max_matches, variable_list);
break;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
index f530993381a93..9b1497d955bcf 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
@@ -9,6 +9,8 @@
#include "HashedNameToDIE.h"
#include "llvm/ADT/StringRef.h"
+#include "lldb/Core/Mangled.h"
+
using namespace lldb_private::dwarf;
bool DWARFMappedHash::ExtractDIEArray(
@@ -423,7 +425,11 @@ DWARFMappedHash::MemoryTable::AppendHashDataForRegularExpression(
count * m_header.header_data.GetMinimumHashDataByteSize();
if (count > 0 && m_data.ValidOffsetForDataOfSize(*hash_data_offset_ptr,
min_total_hash_data_size)) {
- const bool match = regex.Execute(llvm::StringRef(strp_cstr));
+ // The name in the name table may be a mangled name, in which case we
+ // should also compare against the demangled version. The simplest way to
+ // do that is to use the Mangled class:
+ lldb_private::Mangled mangled_name((llvm::StringRef(strp_cstr)));
+ const bool match = mangled_name.NameMatches(regex);
if (!match && m_header.header_data.HashDataHasFixedByteSize()) {
// If the regex doesn't match and we have fixed size data, we can just
diff --git a/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py b/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
index 05c45142fec77..6fd4a8c9b3018 100644
--- a/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
+++ b/lldb/test/API/lang/cpp/class_static/TestStaticVariables.py
@@ -106,6 +106,20 @@ def test_with_run_command_complete(self):
],
)
+ def build_value_check(self, var_name, values):
+ children_1 = [ValueCheck(name = "x", value = values[0], type = "int"),
+ ValueCheck(name = "y", value = values[1], type = "int")]
+ children_2 = [ValueCheck(name = "x", value = values[2], type = "int"),
+ ValueCheck(name = "y", value = values[3], type = "int")]
+ elem_0 = ValueCheck(name = "[0]", value=None, type = "PointType",
+ children=children_1)
+ elem_1 = ValueCheck(name = "[1]", value=None, type = "PointType",
+ children=children_2)
+ value_check = ValueCheck(name=var_name, value = None, type = "PointType[2]",
+ children = [elem_0, elem_1])
+
+ return value_check
+
@expectedFailureAll(
compiler=["gcc"], bugnumber="Compiler emits incomplete debug info"
)
@@ -142,27 +156,30 @@ def test_with_python_api(self):
# in_scope_only => False
valList = frame.GetVariables(False, False, True, False)
- for val in valList:
+ # Build ValueCheckers for the values we're going to find:
+ value_check_A = self.build_value_check("A::g_points", ["1", "2", "11", "22"])
+ value_check_none = self.build_value_check("g_points", ["3", "4", "33", "44"])
+ value_check_AA = self.build_value_check("AA::g_points", ["5", "6", "55", "66"])
+
+ for val in valList:
self.DebugSBValue(val)
name = val.GetName()
- self.assertIn(name, ["g_points", "A::g_points"])
+ self.assertIn(name, ["g_points", "A::g_points", "AA::g_points"])
+
+ if name == "A::g_points":
+ self.assertEqual(val.GetValueType(), lldb.eValueTypeVariableGlobal)
+ value_check_A.check_value(self, val, "Got A::g_points right")
if name == "g_points":
self.assertEqual(val.GetValueType(), lldb.eValueTypeVariableStatic)
- self.assertEqual(val.GetNumChildren(), 2)
- elif name == "A::g_points":
+ value_check_none.check_value(self, val, "Got g_points right")
+ if name == "AA::g_points":
self.assertEqual(val.GetValueType(), lldb.eValueTypeVariableGlobal)
- self.assertEqual(val.GetNumChildren(), 2)
- child1 = val.GetChildAtIndex(1)
- self.DebugSBValue(child1)
- child1_x = child1.GetChildAtIndex(0)
- self.DebugSBValue(child1_x)
- self.assertEqual(child1_x.GetTypeName(), "int")
- self.assertEqual(child1_x.GetValue(), "11")
+ value_check_AA.check_value(self, val, "Got AA::g_points right")
# SBFrame.FindValue() should also work.
val = frame.FindValue("A::g_points", lldb.eValueTypeVariableGlobal)
self.DebugSBValue(val)
- self.assertEqual(val.GetName(), "A::g_points")
+ value_check_A.check_value(self, val, "FindValue also works")
# Also exercise the "parameter" and "local" scopes while we are at it.
val = frame.FindValue("argc", lldb.eValueTypeVariableArgument)
@@ -176,3 +193,37 @@ def test_with_python_api(self):
val = frame.FindValue("hello_world", lldb.eValueTypeVariableLocal)
self.DebugSBValue(val)
self.assertEqual(val.GetName(), "hello_world")
+
+ # We should also be able to get class statics from FindGlobalVariables.
+ # eMatchTypeStartsWith should only find A:: not AA::
+ val_list = target.FindGlobalVariables("A::", 10, lldb.eMatchTypeStartsWith)
+ self.assertEqual(val_list.GetSize(), 1, "Found only one match")
+ val = val_list[0]
+ value_check_A.check_value(self, val, "FindGlobalVariables starts with")
+
+ # Regex should find both
+ val_list = target.FindGlobalVariables("A::", 10, lldb.eMatchTypeRegex)
+ self.assertEqual(val_list.GetSize(), 2, "Found A & AA")
+ found_a = False
+ found_aa = False
+ for val in val_list:
+ name = val.GetName()
+ if name == "A::g_points":
+ value_check_A.check_value(self, val, "AA found by regex")
+ found_a = True
+ elif name == "AA::g_points":
+ value_check_AA.check_value(self, val, "A found by regex")
+ found_aa = True
+
+ self.assertTrue(found_a, "Regex search found A::g_points")
+ self.assertTrue(found_aa, "Regex search found AA::g_points")
+
+ # Normal search for full name should find one, but it looks like we don't match
+ # on identifier boundaries here yet:
+ val_list = target.FindGlobalVariables("A::g_points", 10, lldb.eMatchTypeNormal)
+ self.assertEqual(val_list.GetSize(), 2, "We aren't matching on name boundaries yet")
+
+ # Normal search for g_points should find 3 - FindGlobalVariables doesn't distinguish
+ # between file statics and globals:
+ val_list = target.FindGlobalVariables("g_points", 10, lldb.eMatchTypeNormal)
+ self.assertEqual(val_list.GetSize(), 3, "Found all three g_points")
diff --git a/lldb/test/API/lang/cpp/class_static/main.cpp b/lldb/test/API/lang/cpp/class_static/main.cpp
index e96443e865a8e..40a88029d98f1 100644
--- a/lldb/test/API/lang/cpp/class_static/main.cpp
+++ b/lldb/test/API/lang/cpp/class_static/main.cpp
@@ -21,23 +21,37 @@ class A
static PointType g_points[];
};
+// Make sure similar names don't confuse us:
+
+class AA
+{
+public:
+ static PointType g_points[];
+};
+
PointType A::g_points[] =
{
{ 1, 2 },
{ 11, 22 }
};
-
static PointType g_points[] =
{
{ 3, 4 },
{ 33, 44 }
};
+PointType AA::g_points[] =
+{
+ { 5, 6 },
+ { 55, 66 }
+};
+
int
main (int argc, char const *argv[])
{
const char *hello_world = "Hello, world!";
printf ("A::g_points[1].x = %i\n", A::g_points[1].x); // Set break point at this line.
+ printf ("AA::g_points[1].x = %i\n", AA::g_points[1].x);
printf ("::g_points[1].x = %i\n", g_points[1].x);
printf ("%s\n", hello_world);
return 0;
More information about the lldb-commits
mailing list