Skip to content

Commit 10f3f06

Browse files
kastiglionekastiglione
authored
[lldb] Improve summary string handling of dollar chars (#98190)
This improves the handling of `$` (dollar) characters in summary strings in the following ways: 1. When a `$` is not followed by an open paren (`{`), it should be treated as a literal character and preserved in the output. Previously, the dollar would be consumed by the parser and not shown in the output. 2. When a `$` is the last character of a format string, this change eliminates the infinite loop lldb would enter into. rdar://131392446
1 parent ef89e3e commit 10f3f06

File tree

4 files changed

+155
-132
lines changed

4 files changed

+155
-132
lines changed

‎lldb/source/Core/FormatEntity.cpp‎

Lines changed: 126 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -2170,154 +2170,148 @@ static Status ParseInternal(llvm::StringRef &format, Entry &parent_entry,
21702170
} break;
21712171

21722172
case '$':
2173-
if (format.size() == 1) {
2174-
// '$' at the end of a format string, just print the '$'
2173+
format = format.drop_front(); // Skip the '$'
2174+
if (format.empty() || format.front() != '{') {
2175+
// Print '$' when not followed by '{'.
21752176
parent_entry.AppendText("$");
21762177
} else {
2177-
format = format.drop_front(); // Skip the '$'
2178-
2179-
if (format[0] == '{') {
2180-
format = format.drop_front(); // Skip the '{'
2181-
2182-
llvm::StringRef variable, variable_format;
2183-
error = FormatEntity::ExtractVariableInfo(format, variable,
2184-
variable_format);
2185-
if (error.Fail())
2186-
return error;
2187-
bool verify_is_thread_id = false;
2188-
Entry entry;
2189-
if (!variable_format.empty()) {
2190-
entry.printf_format = variable_format.str();
2191-
2192-
// If the format contains a '%' we are going to assume this is a
2193-
// printf style format. So if you want to format your thread ID
2194-
// using "0x%llx" you can use: ${thread.id%0x%llx}
2195-
//
2196-
// If there is no '%' in the format, then it is assumed to be a
2197-
// LLDB format name, or one of the extended formats specified in
2198-
// the switch statement below.
2199-
2200-
if (entry.printf_format.find('%') == std::string::npos) {
2201-
bool clear_printf = false;
2202-
2203-
if (entry.printf_format.size() == 1) {
2204-
switch (entry.printf_format[0]) {
2205-
case '@': // if this is an @ sign, print ObjC description
2206-
entry.number = ValueObject::
2207-
eValueObjectRepresentationStyleLanguageSpecific;
2208-
clear_printf = true;
2209-
break;
2210-
case 'V': // if this is a V, print the value using the default
2211-
// format
2212-
entry.number =
2213-
ValueObject::eValueObjectRepresentationStyleValue;
2214-
clear_printf = true;
2215-
break;
2216-
case 'L': // if this is an L, print the location of the value
2217-
entry.number =
2218-
ValueObject::eValueObjectRepresentationStyleLocation;
2219-
clear_printf = true;
2220-
break;
2221-
case 'S': // if this is an S, print the summary after all
2222-
entry.number =
2223-
ValueObject::eValueObjectRepresentationStyleSummary;
2224-
clear_printf = true;
2225-
break;
2226-
case '#': // if this is a '#', print the number of children
2227-
entry.number =
2228-
ValueObject::eValueObjectRepresentationStyleChildrenCount;
2229-
clear_printf = true;
2230-
break;
2231-
case 'T': // if this is a 'T', print the type
2232-
entry.number =
2233-
ValueObject::eValueObjectRepresentationStyleType;
2234-
clear_printf = true;
2235-
break;
2236-
case 'N': // if this is a 'N', print the name
2237-
entry.number =
2238-
ValueObject::eValueObjectRepresentationStyleName;
2239-
clear_printf = true;
2240-
break;
2241-
case '>': // if this is a '>', print the expression path
2242-
entry.number = ValueObject::
2243-
eValueObjectRepresentationStyleExpressionPath;
2244-
clear_printf = true;
2245-
break;
2246-
}
2178+
format = format.drop_front(); // Skip the '{'
2179+
2180+
llvm::StringRef variable, variable_format;
2181+
error = FormatEntity::ExtractVariableInfo(format, variable,
2182+
variable_format);
2183+
if (error.Fail())
2184+
return error;
2185+
bool verify_is_thread_id = false;
2186+
Entry entry;
2187+
if (!variable_format.empty()) {
2188+
entry.printf_format = variable_format.str();
2189+
2190+
// If the format contains a '%' we are going to assume this is a
2191+
// printf style format. So if you want to format your thread ID
2192+
// using "0x%llx" you can use: ${thread.id%0x%llx}
2193+
//
2194+
// If there is no '%' in the format, then it is assumed to be a
2195+
// LLDB format name, or one of the extended formats specified in
2196+
// the switch statement below.
2197+
2198+
if (entry.printf_format.find('%') == std::string::npos) {
2199+
bool clear_printf = false;
2200+
2201+
if (entry.printf_format.size() == 1) {
2202+
switch (entry.printf_format[0]) {
2203+
case '@': // if this is an @ sign, print ObjC description
2204+
entry.number = ValueObject::
2205+
eValueObjectRepresentationStyleLanguageSpecific;
2206+
clear_printf = true;
2207+
break;
2208+
case 'V': // if this is a V, print the value using the default
2209+
// format
2210+
entry.number =
2211+
ValueObject::eValueObjectRepresentationStyleValue;
2212+
clear_printf = true;
2213+
break;
2214+
case 'L': // if this is an L, print the location of the value
2215+
entry.number =
2216+
ValueObject::eValueObjectRepresentationStyleLocation;
2217+
clear_printf = true;
2218+
break;
2219+
case 'S': // if this is an S, print the summary after all
2220+
entry.number =
2221+
ValueObject::eValueObjectRepresentationStyleSummary;
2222+
clear_printf = true;
2223+
break;
2224+
case '#': // if this is a '#', print the number of children
2225+
entry.number =
2226+
ValueObject::eValueObjectRepresentationStyleChildrenCount;
2227+
clear_printf = true;
2228+
break;
2229+
case 'T': // if this is a 'T', print the type
2230+
entry.number = ValueObject::eValueObjectRepresentationStyleType;
2231+
clear_printf = true;
2232+
break;
2233+
case 'N': // if this is a 'N', print the name
2234+
entry.number = ValueObject::eValueObjectRepresentationStyleName;
2235+
clear_printf = true;
2236+
break;
2237+
case '>': // if this is a '>', print the expression path
2238+
entry.number =
2239+
ValueObject::eValueObjectRepresentationStyleExpressionPath;
2240+
clear_printf = true;
2241+
break;
22472242
}
2243+
}
22482244

2249-
if (entry.number == 0) {
2250-
if (FormatManager::GetFormatFromCString(
2251-
entry.printf_format.c_str(), entry.fmt)) {
2252-
clear_printf = true;
2253-
} else if (entry.printf_format == "tid") {
2254-
verify_is_thread_id = true;
2255-
} else {
2256-
error.SetErrorStringWithFormat("invalid format: '%s'",
2257-
entry.printf_format.c_str());
2258-
return error;
2259-
}
2245+
if (entry.number == 0) {
2246+
if (FormatManager::GetFormatFromCString(
2247+
entry.printf_format.c_str(), entry.fmt)) {
2248+
clear_printf = true;
2249+
} else if (entry.printf_format == "tid") {
2250+
verify_is_thread_id = true;
2251+
} else {
2252+
error.SetErrorStringWithFormat("invalid format: '%s'",
2253+
entry.printf_format.c_str());
2254+
return error;
22602255
}
2261-
2262-
// Our format string turned out to not be a printf style format
2263-
// so lets clear the string
2264-
if (clear_printf)
2265-
entry.printf_format.clear();
22662256
}
2267-
}
22682257

2269-
// Check for dereferences
2270-
if (variable[0] == '*') {
2271-
entry.deref = true;
2272-
variable = variable.drop_front();
2258+
// Our format string turned out to not be a printf style format
2259+
// so lets clear the string
2260+
if (clear_printf)
2261+
entry.printf_format.clear();
22732262
}
2263+
}
22742264

2275-
error = ParseEntry(variable, &g_root, entry);
2276-
if (error.Fail())
2277-
return error;
2265+
// Check for dereferences
2266+
if (variable[0] == '*') {
2267+
entry.deref = true;
2268+
variable = variable.drop_front();
2269+
}
22782270

2279-
llvm::StringRef entry_string(entry.string);
2280-
if (entry_string.contains(':')) {
2281-
auto [_, llvm_format] = entry_string.split(':');
2282-
if (!llvm_format.empty() && !LLVMFormatPattern.match(llvm_format)) {
2283-
error.SetErrorStringWithFormat("invalid llvm format: '%s'",
2284-
llvm_format.data());
2285-
return error;
2286-
}
2271+
error = ParseEntry(variable, &g_root, entry);
2272+
if (error.Fail())
2273+
return error;
2274+
2275+
llvm::StringRef entry_string(entry.string);
2276+
if (entry_string.contains(':')) {
2277+
auto [_, llvm_format] = entry_string.split(':');
2278+
if (!llvm_format.empty() && !LLVMFormatPattern.match(llvm_format)) {
2279+
error.SetErrorStringWithFormat("invalid llvm format: '%s'",
2280+
llvm_format.data());
2281+
return error;
22872282
}
2283+
}
22882284

2289-
if (verify_is_thread_id) {
2290-
if (entry.type != Entry::Type::ThreadID &&
2291-
entry.type != Entry::Type::ThreadProtocolID) {
2292-
error.SetErrorString("the 'tid' format can only be used on "
2293-
"${thread.id} and ${thread.protocol_id}");
2294-
}
2285+
if (verify_is_thread_id) {
2286+
if (entry.type != Entry::Type::ThreadID &&
2287+
entry.type != Entry::Type::ThreadProtocolID) {
2288+
error.SetErrorString("the 'tid' format can only be used on "
2289+
"${thread.id} and ${thread.protocol_id}");
22952290
}
2291+
}
22962292

2297-
switch (entry.type) {
2298-
case Entry::Type::Variable:
2299-
case Entry::Type::VariableSynthetic:
2300-
if (entry.number == 0) {
2301-
if (entry.string.empty())
2302-
entry.number =
2303-
ValueObject::eValueObjectRepresentationStyleValue;
2304-
else
2305-
entry.number =
2306-
ValueObject::eValueObjectRepresentationStyleSummary;
2307-
}
2308-
break;
2309-
default:
2310-
// Make sure someone didn't try to dereference anything but ${var}
2311-
// or ${svar}
2312-
if (entry.deref) {
2313-
error.SetErrorStringWithFormat(
2314-
"${%s} can't be dereferenced, only ${var} and ${svar} can.",
2315-
variable.str().c_str());
2316-
return error;
2317-
}
2293+
switch (entry.type) {
2294+
case Entry::Type::Variable:
2295+
case Entry::Type::VariableSynthetic:
2296+
if (entry.number == 0) {
2297+
if (entry.string.empty())
2298+
entry.number = ValueObject::eValueObjectRepresentationStyleValue;
2299+
else
2300+
entry.number =
2301+
ValueObject::eValueObjectRepresentationStyleSummary;
2302+
}
2303+
break;
2304+
default:
2305+
// Make sure someone didn't try to dereference anything but ${var}
2306+
// or ${svar}
2307+
if (entry.deref) {
2308+
error.SetErrorStringWithFormat(
2309+
"${%s} can't be dereferenced, only ${var} and ${svar} can.",
2310+
variable.str().c_str());
2311+
return error;
23182312
}
2319-
parent_entry.AppendEntry(std::move(entry));
23202313
}
2314+
parent_entry.AppendEntry(std::move(entry));
23212315
}
23222316
break;
23232317
}

‎lldb/test/API/functionalities/data-formatter/special-chars/Makefile‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
C_SOURCES = main.c
2+
include Makefile.rules

‎lldb/test/API/functionalities/data-formatter/special-chars/TestSummaryStringSpecialChars.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import lldb
2+
from lldbsuite.test.decorators import *
3+
from lldbsuite.test.lldbtest import *
4+
from lldbsuite.test import lldbutil
5+
6+
7+
class TestCase(TestBase):
8+
def test_summary_string_with_bare_dollar_char(self):
9+
self.build()
10+
lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
11+
self.runCmd("type summary add --summary-string '$ $CASH $' --no-value Dollars")
12+
self.expect("v cash", startstr="(Dollars) cash = $ $CASH $")
13+
14+
def test_summary_string_with_bare_dollar_char_before_var(self):
15+
self.build()
16+
lldbutil.run_to_source_breakpoint(self, "break here", lldb.SBFileSpec("main.c"))
17+
self.runCmd("type summary add --summary-string '$${var}' --no-value Dollars")
18+
self.expect("v cash", startstr="(Dollars) cash = $99")

‎lldb/test/API/functionalities/data-formatter/special-chars/main.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#include <stdio.h>
2+
3+
typedef int Dollars;
4+
5+
int main() {
6+
Dollars cash = 99;
7+
printf("break here: %d\n", cash);
8+
return 0;
9+
}

0 commit comments

Comments
 (0)