Skip to content

[lldb-vscode] Show value addresses in a short format #66534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lldb/include/lldb/API/SBType.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ class SBType {

uint64_t GetByteSize();

/// \return
/// Whether the type is a pointer or a reference.
bool IsPointerOrReferenceType();

bool IsPointerType();

bool IsReferenceType();
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/API/SBType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,10 @@ uint64_t SBType::GetByteSize() {
return 0;
}

bool SBType::IsPointerOrReferenceType() {
return IsPointerType() || IsReferenceType();
}

bool SBType::IsPointerType() {
LLDB_INSTRUMENT_VA(this);

Expand Down
2 changes: 1 addition & 1 deletion lldb/test/API/tools/lldb-vscode/evaluate/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,6 @@ int main(int argc, char const *argv[]) {
my_bool_vec.push_back(true);
my_bool_vec.push_back(false); // breakpoint 6
my_bool_vec.push_back(true); // breakpoint 7

return 0;
}
107 changes: 93 additions & 14 deletions lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None):
('"%s" value "%s" doesn\'t start with' ' "%s")')
% (key, actual_value, verify_value),
)
if "notstartswith" in verify_dict:
verify = verify_dict["notstartswith"]
for key in verify:
verify_value = verify[key]
actual_value = actual[key]
startswith = actual_value.startswith(verify_value)
self.assertFalse(
startswith,
('"%s" value "%s" starts with' ' "%s")')
% (key, actual_value, verify_value),
)
if "contains" in verify_dict:
verify = verify_dict["contains"]
for key in verify:
Expand Down Expand Up @@ -155,6 +166,7 @@ def do_test_scopes_variables_setVariable_evaluate(
"argv": {
"equals": {"type": "const char **"},
"startswith": {"value": "0x"},
"notstartswith": {"value": "0x0"},
"hasVariablesReference": True,
},
"pt": {
Expand All @@ -166,8 +178,53 @@ def do_test_scopes_variables_setVariable_evaluate(
"buffer": {"children": buffer_children},
},
},
"pt_ptr": {
"equals": {"type": "PointType *"},
"startswith": {"value": "0x"},
"notstartswith": {"value": "0x0"},
"hasVariablesReference": True,
},
"another_pt_ptr": {
"equals": {"type": "PointType *"},
"startswith": {"value": "<null>"},
"hasVariablesReference": True,
},
"x": {"equals": {"type": "int"}},
"some_int": {
"equals": {
"type": "int",
"value": "10",
},
},
"some_int_ptr": {
"equals": {"type": "int *"},
"startswith": {"value": "0x"},
"notstartswith": {"value": "0x0"},
},
"another_int_ptr": {
"equals": {
"type": "int *",
"value": "<null>",
},
},
}
if enableAutoVariableSummaries:
verify_locals["pt_ptr"] = {
"equals": {"type": "PointType *"},
"hasVariablesReference": True,
"children": {
"x": {"equals": {"type": "int", "value": "11"}},
"y": {"equals": {"type": "int", "value": "22"}},
"buffer": {"children": buffer_children},
},
}
verify_locals["some_int_ptr"] = {
"equals": {
"type": "int *",
"value": "20",
},
}

verify_globals = {
"s_local": {"equals": {"type": "float", "value": "2.25"}},
"::g_global": {"equals": {"type": "int", "value": "123"}},
Expand Down Expand Up @@ -297,9 +354,9 @@ def do_test_scopes_variables_setVariable_evaluate(

verify_locals["argc"]["equals"]["value"] = "123"
verify_locals["pt"]["children"]["x"]["equals"]["value"] = "111"
verify_locals["x @ main.cpp:17"] = {"equals": {"type": "int", "value": "89"}}
verify_locals["x @ main.cpp:19"] = {"equals": {"type": "int", "value": "42"}}
verify_locals["x @ main.cpp:21"] = {"equals": {"type": "int", "value": "72"}}
verify_locals["x @ main.cpp:23"] = {"equals": {"type": "int", "value": "89"}}
verify_locals["x @ main.cpp:25"] = {"equals": {"type": "int", "value": "42"}}
verify_locals["x @ main.cpp:27"] = {"equals": {"type": "int", "value": "72"}}

self.verify_variables(verify_locals, self.vscode.get_local_variables())

Expand All @@ -310,29 +367,29 @@ def do_test_scopes_variables_setVariable_evaluate(
)

self.assertTrue(
self.vscode.request_setVariable(1, "x @ main.cpp:17", 17)["success"]
self.vscode.request_setVariable(1, "x @ main.cpp:23", 17)["success"]
)
self.assertTrue(
self.vscode.request_setVariable(1, "x @ main.cpp:19", 19)["success"]
self.vscode.request_setVariable(1, "x @ main.cpp:25", 19)["success"]
)
self.assertTrue(
self.vscode.request_setVariable(1, "x @ main.cpp:21", 21)["success"]
self.vscode.request_setVariable(1, "x @ main.cpp:27", 21)["success"]
)

# The following should have no effect
self.assertFalse(
self.vscode.request_setVariable(1, "x @ main.cpp:21", "invalid")["success"]
self.vscode.request_setVariable(1, "x @ main.cpp:27", "invalid")["success"]
)

verify_locals["x @ main.cpp:17"]["equals"]["value"] = "17"
verify_locals["x @ main.cpp:19"]["equals"]["value"] = "19"
verify_locals["x @ main.cpp:21"]["equals"]["value"] = "21"
verify_locals["x @ main.cpp:23"]["equals"]["value"] = "17"
verify_locals["x @ main.cpp:25"]["equals"]["value"] = "19"
verify_locals["x @ main.cpp:27"]["equals"]["value"] = "21"

self.verify_variables(verify_locals, self.vscode.get_local_variables())

# The plain x variable shold refer to the innermost x
self.assertTrue(self.vscode.request_setVariable(1, "x", 22)["success"])
verify_locals["x @ main.cpp:21"]["equals"]["value"] = "22"
verify_locals["x @ main.cpp:27"]["equals"]["value"] = "22"

self.verify_variables(verify_locals, self.vscode.get_local_variables())

Expand All @@ -349,9 +406,9 @@ def do_test_scopes_variables_setVariable_evaluate(
names = [var["name"] for var in locals]
# The first shadowed x shouldn't have a suffix anymore
verify_locals["x"] = {"equals": {"type": "int", "value": "17"}}
self.assertNotIn("x @ main.cpp:17", names)
self.assertNotIn("x @ main.cpp:19", names)
self.assertNotIn("x @ main.cpp:21", names)
self.assertNotIn("x @ main.cpp:23", names)
self.assertNotIn("x @ main.cpp:25", names)
self.assertNotIn("x @ main.cpp:27", names)

self.verify_variables(verify_locals, locals)

Expand Down Expand Up @@ -421,10 +478,32 @@ def do_test_scopes_and_evaluate_expansion(self, enableAutoVariableSummaries: boo
},
},
},
"pt_ptr": {
"equals": {"type": "PointType *"},
"hasVariablesReference": True,
"missing": ["indexedVariables"],
},
"another_pt_ptr": {
"equals": {"type": "PointType *"},
"hasVariablesReference": True,
"missing": ["indexedVariables"],
},
"x": {
"equals": {"type": "int"},
"missing": ["indexedVariables"],
},
"some_int": {
"equals": {"type": "int"},
"missing": ["indexedVariables"],
},
"some_int_ptr": {
"equals": {"type": "int *"},
"missing": ["indexedVariables"],
},
"another_int_ptr": {
"equals": {"type": "int *"},
"missing": ["indexedVariables"],
},
}
self.verify_variables(verify_locals, locals)

Expand Down
6 changes: 6 additions & 0 deletions lldb/test/API/tools/lldb-vscode/variables/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ int test_indexedVariables();
int main(int argc, char const *argv[]) {
static float s_local = 2.25;
PointType pt = { 11,22, {0}};
PointType *pt_ptr = new PointType{11, 22, {0}};
PointType *another_pt_ptr = nullptr;
for (int i=0; i<BUFFER_SIZE; ++i)
pt.buffer[i] = i;

int some_int = 10;
int *some_int_ptr = new int{20};
int *another_int_ptr = nullptr;
int x = s_global - g_global - pt.y; // breakpoint 1
{
int x = 42;
Expand Down
21 changes: 19 additions & 2 deletions lldb/tools/lldb-vscode/JSONUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) {
if (!g_vsc.enable_auto_variable_summaries)
return false;

if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType())
if (!v.GetType().IsPointerOrReferenceType())
return false;

// If we are referencing a pointer, we don't dereference to avoid confusing
Expand Down Expand Up @@ -228,7 +228,24 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object,
strm << "<error: " << error.GetCString() << ">";
} else {
auto tryDumpSummaryAndValue = [&strm](lldb::SBValue value) {
llvm::StringRef val = value.GetValue();
std::string val;
// Whenever the value is a non-synthetic address, we format it ourselves
// to use as few chars as possible because the variables pane on VS Code
// is by default narrow.
if (!value.IsSynthetic() && value.GetType().IsPointerOrReferenceType()) {
lldb::addr_t address = value.GetValueAsUnsigned(LLDB_INVALID_ADDRESS);
if (address == LLDB_INVALID_ADDRESS) {
val = "<invalid address>";
} else if (address == 0) {
val = "<null>";
} else {
llvm::raw_string_ostream os(val);
os << llvm::format_hex(address, 0);
}
Comment on lines +237 to +244
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't mess with the values or try to display them in any fancy way, I would always use:

llvm::raw_string_ostream os(val);
os << llvm::format_hex(address, 0);

Users might encode -1 into their pointers as a special value and you wouldn't want to see "" as the value. Also I would rather see "0x0" instead of "".

} else {
val = llvm::StringRef(value.GetValue()).str();
}

llvm::StringRef summary = value.GetSummary();
if (!val.empty()) {
strm << val;
Expand Down