Skip to content

Commit 22667e3

Browse files
committed
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
1 parent 0a043e6 commit 22667e3

File tree

4 files changed

+86
-15
lines changed

4 files changed

+86
-15
lines changed

lldb/source/API/SBTarget.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1892,7 +1892,7 @@ SBValueList SBTarget::FindGlobalVariables(const char *name,
18921892
max_matches, variable_list);
18931893
break;
18941894
case eMatchTypeStartsWith:
1895-
regexstr = llvm::Regex::escape(name) + ".*";
1895+
regexstr = "^" + llvm::Regex::escape(name) + ".*";
18961896
target_sp->GetImages().FindGlobalVariables(RegularExpression(regexstr),
18971897
max_matches, variable_list);
18981898
break;

lldb/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include "HashedNameToDIE.h"
1010
#include "llvm/ADT/StringRef.h"
1111

12+
#include "lldb/Core/Mangled.h"
13+
1214
using namespace lldb_private::dwarf;
1315

1416
bool DWARFMappedHash::ExtractDIEArray(
@@ -423,7 +425,11 @@ DWARFMappedHash::MemoryTable::AppendHashDataForRegularExpression(
423425
count * m_header.header_data.GetMinimumHashDataByteSize();
424426
if (count > 0 && m_data.ValidOffsetForDataOfSize(*hash_data_offset_ptr,
425427
min_total_hash_data_size)) {
426-
const bool match = regex.Execute(llvm::StringRef(strp_cstr));
428+
// The name in the name table may be a mangled name, in which case we
429+
// should also compare against the demangled version. The simplest way to
430+
// do that is to use the Mangled class:
431+
lldb_private::Mangled mangled_name((llvm::StringRef(strp_cstr)));
432+
const bool match = mangled_name.NameMatches(regex);
427433

428434
if (!match && m_header.header_data.HashDataHasFixedByteSize()) {
429435
// If the regex doesn't match and we have fixed size data, we can just

lldb/test/API/lang/cpp/class_static/TestStaticVariables.py

Lines changed: 63 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,20 @@ def test_with_run_command_complete(self):
106106
],
107107
)
108108

109+
def build_value_check(self, var_name, values):
110+
children_1 = [ValueCheck(name = "x", value = values[0], type = "int"),
111+
ValueCheck(name = "y", value = values[1], type = "int")]
112+
children_2 = [ValueCheck(name = "x", value = values[2], type = "int"),
113+
ValueCheck(name = "y", value = values[3], type = "int")]
114+
elem_0 = ValueCheck(name = "[0]", value=None, type = "PointType",
115+
children=children_1)
116+
elem_1 = ValueCheck(name = "[1]", value=None, type = "PointType",
117+
children=children_2)
118+
value_check = ValueCheck(name=var_name, value = None, type = "PointType[2]",
119+
children = [elem_0, elem_1])
120+
121+
return value_check
122+
109123
@expectedFailureAll(
110124
compiler=["gcc"], bugnumber="Compiler emits incomplete debug info"
111125
)
@@ -142,27 +156,30 @@ def test_with_python_api(self):
142156
# in_scope_only => False
143157
valList = frame.GetVariables(False, False, True, False)
144158

145-
for val in valList:
159+
# Build ValueCheckers for the values we're going to find:
160+
value_check_A = self.build_value_check("A::g_points", ["1", "2", "11", "22"])
161+
value_check_none = self.build_value_check("g_points", ["3", "4", "33", "44"])
162+
value_check_AA = self.build_value_check("AA::g_points", ["5", "6", "55", "66"])
163+
164+
for val in valList:
146165
self.DebugSBValue(val)
147166
name = val.GetName()
148-
self.assertIn(name, ["g_points", "A::g_points"])
167+
self.assertIn(name, ["g_points", "A::g_points", "AA::g_points"])
168+
169+
if name == "A::g_points":
170+
self.assertEqual(val.GetValueType(), lldb.eValueTypeVariableGlobal)
171+
value_check_A.check_value(self, val, "Got A::g_points right")
149172
if name == "g_points":
150173
self.assertEqual(val.GetValueType(), lldb.eValueTypeVariableStatic)
151-
self.assertEqual(val.GetNumChildren(), 2)
152-
elif name == "A::g_points":
174+
value_check_none.check_value(self, val, "Got g_points right")
175+
if name == "AA::g_points":
153176
self.assertEqual(val.GetValueType(), lldb.eValueTypeVariableGlobal)
154-
self.assertEqual(val.GetNumChildren(), 2)
155-
child1 = val.GetChildAtIndex(1)
156-
self.DebugSBValue(child1)
157-
child1_x = child1.GetChildAtIndex(0)
158-
self.DebugSBValue(child1_x)
159-
self.assertEqual(child1_x.GetTypeName(), "int")
160-
self.assertEqual(child1_x.GetValue(), "11")
177+
value_check_AA.check_value(self, val, "Got AA::g_points right")
161178

162179
# SBFrame.FindValue() should also work.
163180
val = frame.FindValue("A::g_points", lldb.eValueTypeVariableGlobal)
164181
self.DebugSBValue(val)
165-
self.assertEqual(val.GetName(), "A::g_points")
182+
value_check_A.check_value(self, val, "FindValue also works")
166183

167184
# Also exercise the "parameter" and "local" scopes while we are at it.
168185
val = frame.FindValue("argc", lldb.eValueTypeVariableArgument)
@@ -176,3 +193,37 @@ def test_with_python_api(self):
176193
val = frame.FindValue("hello_world", lldb.eValueTypeVariableLocal)
177194
self.DebugSBValue(val)
178195
self.assertEqual(val.GetName(), "hello_world")
196+
197+
# We should also be able to get class statics from FindGlobalVariables.
198+
# eMatchTypeStartsWith should only find A:: not AA::
199+
val_list = target.FindGlobalVariables("A::", 10, lldb.eMatchTypeStartsWith)
200+
self.assertEqual(val_list.GetSize(), 1, "Found only one match")
201+
val = val_list[0]
202+
value_check_A.check_value(self, val, "FindGlobalVariables starts with")
203+
204+
# Regex should find both
205+
val_list = target.FindGlobalVariables("A::", 10, lldb.eMatchTypeRegex)
206+
self.assertEqual(val_list.GetSize(), 2, "Found A & AA")
207+
found_a = False
208+
found_aa = False
209+
for val in val_list:
210+
name = val.GetName()
211+
if name == "A::g_points":
212+
value_check_A.check_value(self, val, "AA found by regex")
213+
found_a = True
214+
elif name == "AA::g_points":
215+
value_check_AA.check_value(self, val, "A found by regex")
216+
found_aa = True
217+
218+
self.assertTrue(found_a, "Regex search found A::g_points")
219+
self.assertTrue(found_aa, "Regex search found AA::g_points")
220+
221+
# Normal search for full name should find one, but it looks like we don't match
222+
# on identifier boundaries here yet:
223+
val_list = target.FindGlobalVariables("A::g_points", 10, lldb.eMatchTypeNormal)
224+
self.assertEqual(val_list.GetSize(), 2, "We aren't matching on name boundaries yet")
225+
226+
# Normal search for g_points should find 3 - FindGlobalVariables doesn't distinguish
227+
# between file statics and globals:
228+
val_list = target.FindGlobalVariables("g_points", 10, lldb.eMatchTypeNormal)
229+
self.assertEqual(val_list.GetSize(), 3, "Found all three g_points")

lldb/test/API/lang/cpp/class_static/main.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,23 +21,37 @@ class A
2121
static PointType g_points[];
2222
};
2323

24+
// Make sure similar names don't confuse us:
25+
26+
class AA
27+
{
28+
public:
29+
static PointType g_points[];
30+
};
31+
2432
PointType A::g_points[] =
2533
{
2634
{ 1, 2 },
2735
{ 11, 22 }
2836
};
29-
3037
static PointType g_points[] =
3138
{
3239
{ 3, 4 },
3340
{ 33, 44 }
3441
};
3542

43+
PointType AA::g_points[] =
44+
{
45+
{ 5, 6 },
46+
{ 55, 66 }
47+
};
48+
3649
int
3750
main (int argc, char const *argv[])
3851
{
3952
const char *hello_world = "Hello, world!";
4053
printf ("A::g_points[1].x = %i\n", A::g_points[1].x); // Set break point at this line.
54+
printf ("AA::g_points[1].x = %i\n", AA::g_points[1].x);
4155
printf ("::g_points[1].x = %i\n", g_points[1].x);
4256
printf ("%s\n", hello_world);
4357
return 0;

0 commit comments

Comments
 (0)