[lldb][Format] Unwrap references to C-strings when printing C-string summaries #174398
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesDepends on: (only last commit is relevant for this review) This makes the builtin summary for C-strings apply to references to C-strings too. Before: After: Full diff: https://github.com/llvm/llvm-project/pull/174398.diff 4 Files Affected:
diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp
index 8595f81964fd4..6d3faa5742d63 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -720,7 +720,7 @@ void FormatManager::LoadSystemFormatters() {
TypeSummaryImpl::Flags string_flags;
string_flags.SetCascades(true)
.SetSkipPointers(true)
- .SetSkipReferences(false)
+ .SetSkipReferences(true)
.SetDontShowChildren(true)
.SetDontShowValue(false)
.SetShowMembersOneLiner(false)
diff --git a/lldb/test/API/functionalities/data-formatter/stringprinter/Makefile b/lldb/test/API/functionalities/data-formatter/stringprinter/Makefile
new file mode 100644
index 0000000000000..4f79c0a900c3a
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/stringprinter/Makefile
@@ -0,0 +1,4 @@
+CXX_SOURCES := main.cpp
+CXXFLAGS_EXTRAS := -std=c++20
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/data-formatter/stringprinter/TestStringPrinter.py b/lldb/test/API/functionalities/data-formatter/stringprinter/TestStringPrinter.py
index 0e3bfc2733a56..8e537fa5fc122 100644
--- a/lldb/test/API/functionalities/data-formatter/stringprinter/TestStringPrinter.py
+++ b/lldb/test/API/functionalities/data-formatter/stringprinter/TestStringPrinter.py
@@ -1,7 +1,55 @@
-from lldbsuite.test import lldbinline
-from lldbsuite.test import decorators
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
-lldbinline.MakeInlineTest(
- __file__,
- globals(),
-)
+
+class TestStringPrinter(TestBase):
+ def test(self):
+ self.build()
+
+ self.addTearDownHook(
+ lambda x: x.runCmd("setting set escape-non-printables true")
+ )
+
+ lldbutil.run_to_source_breakpoint(
+ self, "Break here", lldb.SBFileSpec("main.cpp", False)
+ )
+
+ self.expect_var_path(
+ "charwithtabs",
+ summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"',
+ )
+ self.expect_var_path("a.data", summary='"FOOB"')
+ self.expect_var_path("b.data", summary=r'"FO\0B"')
+ self.expect_var_path("c.data", summary=r'"F\0O"')
+ self.expect_var_path("manytrailingnuls", summary=r'"F\0OO\0BA\0R"')
+
+ for c in ["", "const"]:
+ for v in ["", "volatile"]:
+ for s in ["", "unsigned"]:
+ summary = '"' + c + v + s + 'char"'
+ self.expect_var_path(c + v + s + "chararray", summary=summary)
+ # These should be printed normally
+ self.expect_var_path(c + v + s + "charstar", summary=summary)
+
+ Schar5 = self.expect_var_path(
+ "Schar5", children=[ValueCheck(name="x", value="0")]
+ )
+ self.assertIsNone(Schar5.GetSummary())
+ Scharstar = self.expect_var_path(
+ "Scharstar", children=[ValueCheck(name="x", value="0")]
+ )
+ self.assertIsNone(Scharstar.GetSummary())
+
+ self.runCmd("setting set escape-non-printables false")
+ self.expect_var_path(
+ "charwithtabs", summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"'
+ )
+ self.assertTrue(
+ self.frame().FindVariable("longconstcharstar").GetSummary().endswith('"...')
+ )
+
+ # FIXME: make "b.data" and "c.data" work sanely
+
+ self.expect("frame variable ref", substrs=['&ref = "Hello"'])
+ self.expect("frame variable refref", substrs=['&refref = "Hi"'])
diff --git a/lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp b/lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
index 6b39e4bf6e846..e4e28c94080f7 100644
--- a/lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -1,4 +1,4 @@
-#include <string>
+#include <cstdio>
#include <cstring>
struct A {
@@ -20,67 +20,67 @@ struct A {
MAKE_VARS();
MAKE_VARS(const);
-template<typename T>
-struct S {
+template <typename T> struct S {
int x = 0;
};
S<char[5]> Schar5;
S<char *> Scharstar;
-int main (int argc, char const *argv[])
-{
- const char manytrailingnuls[] = "F\0OO\0BA\0R\0\0\0\0";
- A a, b, c;
- // Deliberately write past the end of data to test that the formatter stops
- // at the end of array.
- memcpy(a.data, "FOOBAR", 7);
- memcpy(b.data, "FO\0BAR", 7);
- memcpy(c.data, "F\0O\0AR", 7);
- std::string stdstring("Hello\t\tWorld\nI am here\t\tto say hello\n"); //%self.addTearDownHook(lambda x: x.runCmd("setting set escape-non-printables true"))
- const char *charwithtabs = stdstring.c_str();
- std::string longstring(
-"I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
-" is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
-" a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
-" for science, or something"
- "I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
- " is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
- " a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
+int main(int argc, char const *argv[]) {
+ const char manytrailingnuls[] = "F\0OO\0BA\0R\0\0\0\0";
+ A a, b, c;
+ // Deliberately write past the end of data to test that the formatter stops
+ // at the end of array.
+ memcpy(a.data, "FOOBAR", 7);
+ memcpy(b.data, "FO\0BAR", 7);
+ memcpy(c.data, "F\0O\0AR", 7);
+ const char *charwithtabs = "Hello\t\tWorld\nI am here\t\tto say hello\n";
+ const char *longconstcharstar =
+ "I am a very long string; in fact I am longer than any reasonable length "
+ "that a string should be; quite long indeed; oh my, so many words; so "
+ "many letters; this is kind of like writing a poem; except in real life "
+ "all that is happening"
+ " is just me producing a very very long set of words; there is text "
+ "here, text there, text everywhere; it fills me with glee to see so much "
+ "text; all around me it's just letters, and symbols, and other pleasant "
+ "drawings that cause me"
+ " a large amount of joy upon visually seeing them with my eyes; well, "
+ "this is now a lot of letters, but it is still not enough for the "
+ "purpose of the test I want to test, so maybe I should copy and paste "
+ "this a few times, you know.."
" for science, or something"
- "I am a very long string; in fact I am longer than any reasonable length that a string should be; quite long indeed; oh my, so many words; so many letters; this is kind of like writing a poem; except in real life all that is happening"
- " is just me producing a very very long set of words; there is text here, text there, text everywhere; it fills me with glee to see so much text; all around me it's just letters, and symbols, and other pleasant drawings that cause me"
- " a large amount of joy upon visually seeing them with my eyes; well, this is now a lot of letters, but it is still not enough for the purpose of the test I want to test, so maybe I should copy and paste this a few times, you know.."
- " for science, or something"
- );
- const char* longconstcharstar = longstring.c_str();
- return 0; //% if self.TraceOn(): self.runCmd('frame variable')
- //%
- //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //% self.expect_var_path('charwithtabs', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //% self.expect_var_path("a.data", summary='"FOOB"')
- //% self.expect_var_path("b.data", summary=r'"FO\0B"')
- //% self.expect_var_path("c.data", summary=r'"F\0O"')
- //% self.expect_var_path("manytrailingnuls", summary=r'"F\0OO\0BA\0R"')
- //%
- //% for c in ["", "const"]:
- //% for v in ["", "volatile"]:
- //% for s in ["", "unsigned"]:
- //% summary = '"'+c+v+s+'char"'
- //% self.expect_var_path(c+v+s+"chararray", summary=summary)
- //% # These should be printed normally
- //% self.expect_var_path(c+v+s+"charstar", summary=summary)
- //% Schar5 = self.expect_var_path("Schar5",
- //% children=[ValueCheck(name="x", value="0")])
- //% self.assertIsNone(Schar5.GetSummary())
- //% Scharstar = self.expect_var_path("Scharstar",
- //% children=[ValueCheck(name="x", value="0")])
- //% self.assertIsNone(Scharstar.GetSummary())
- //%
- //% self.runCmd("setting set escape-non-printables false")
- //% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
- //% self.expect_var_path('charwithtabs', summary='"Hello\t\tWorld\nI am here\t\tto say hello\n"')
- //% self.assertTrue(self.frame().FindVariable('longstring').GetSummary().endswith('"...'))
- //% self.assertTrue(self.frame().FindVariable('longconstcharstar').GetSummary().endswith('"...'))
- // FIXME: make "b.data" and "c.data" work sanely
-}
+ "I am a very long string; in fact I am longer than any reasonable length "
+ "that a string should be; quite long indeed; oh my, so many words; so "
+ "many letters; this is kind of like writing a poem; except in real life "
+ "all that is happening"
+ " is just me producing a very very long set of words; there is text "
+ "here, text there, text everywhere; it fills me with glee to see so much "
+ "text; all around me it's just letters, and symbols, and other pleasant "
+ "drawings that cause me"
+ " a large amount of joy upon visually seeing them with my eyes; well, "
+ "this is now a lot of letters, but it is still not enough for the "
+ "purpose of the test I want to test, so maybe I should copy and paste "
+ "this a few times, you know.."
+ " for science, or something"
+ "I am a very long string; in fact I am longer than any reasonable length "
+ "that a string should be; quite long indeed; oh my, so many words; so "
+ "many letters; this is kind of like writing a poem; except in real life "
+ "all that is happening"
+ " is just me producing a very very long set of words; there is text "
+ "here, text there, text everywhere; it fills me with glee to see so much "
+ "text; all around me it's just letters, and symbols, and other pleasant "
+ "drawings that cause me"
+ " a large amount of joy upon visually seeing them with my eyes; well, "
+ "this is now a lot of letters, but it is still not enough for the "
+ "purpose of the test I want to test, so maybe I should copy and paste "
+ "this a few times, you know.."
+ " for science, or something";
+
+ const char *basic = "Hello";
+ const char *&ref = basic;
+ const char *&&refref = "Hi";
+ puts("Break here");
+
+ return 0;
+}
|
| string_flags.SetCascades(true) | ||
| .SetSkipPointers(true) | ||
| .SetSkipReferences(false) | ||
| .SetSkipReferences(true) |
Collaborator
There was a problem hiding this comment.
Closest I found to documentation for this is:
-r ( --skip-references )
Don't use this format for references-to-type objects.
So I think string_format was being used for references, but now is not.
I guess that this formatter didn't know how to remove the reference layer itself, and that's why it wasn't producing a summary. Now you've said not to use this for references, some generic reference to type handler kicks in and does a better job?
Member
Author
There was a problem hiding this comment.
Good point, I misread this option as the opposite. It seems like whatever summary provider is picked knows how to format the string through references.
Will check exactly what happens here
Member
Author
There was a problem hiding this comment.
Ok so what seems to happen is that when we SetSkipReferences then we don't apply this formatter to references to C-strings. So we don't print a summary for those at all. Previously the summary would have been <no value available>:
>>> lldb.frame.FindVariable("ref").GetSummary()
'<no value available>'
Now it would just not have a summary:
>>> lldb.frame.FindVariable("ref").GetSummary()
>>>
And because we skipped printing a summary (and it wasn't an error), we continue to printing the children:
llvm-project/lldb/source/DataFormatters/ValueObjectPrinter.cpp
Lines 123 to 129 in 5a33f99
The child is the (&ref = "hi"), which gets formatted with the char* system formatter. So the one-line child summary is what gives us the illusion of the root variable having a valid summary.
Note, currently LLDB doens't print the child not because <no value available> is viewed as a summary error (which it isn't), but because the system char* formatter sets SetDontShowChildren(true) on the summary flags and the summary wasn't empty.
TL;DR, what this patch does is "skip summaries for C-string references and let the child be printed instead".
Feels like a workaround for ${var%s} not accounting for references more than a proper fix. Let me see if I can fix it in a different way.
Member
Author
There was a problem hiding this comment.
Hmm the more I look into this the more it seems like the right thing to do is not apply this formatter for references and instead let the child be printed. That's similar to how the int& case also works it seems. I.e., we call PrintChildrenOneLiner which does the formatting of the pointee.
Member
Author
There was a problem hiding this comment.
Updated the PR description with some more details
Collaborator
|
Wasn't sure about the formatting of the result: But it does match any other reference to type: So that's fine. |
Michael137
commented
Jan 5, 2026
|
|
||
| # FIXME: make "b.data" and "c.data" work sanely | ||
|
|
||
| self.expect("frame variable ref", substrs=['&ref = "Hello"']) |
Member
Author
There was a problem hiding this comment.
We should also assert that GetSummary is now None (or a valid summary, depending on fix). And also we should do a ValueCheck on the child.
Michael137
added a commit
that referenced
this pull request
Jan 5, 2026
Printing references to C-strings doesn't work properly at the moment. This patch provides coverage for those cases and should fail once the underlying issue gets fixed (see #174398).
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 5, 2026
Printing references to C-strings doesn't work properly at the moment. This patch provides coverage for those cases and should fail once the underlying issue gets fixed (see llvm/llvm-project#174398).
Michael137
force-pushed
the
lldb/string-printer-cstr-refs
branch
from
3b1a2a7 to
4dc1f6f
Compare
adrian-prantl
approved these changes
Jan 5, 2026
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Jan 6, 2026
Printing references to C-strings doesn't work properly at the moment. This patch provides coverage for those cases and should fail once the underlying issue gets fixed (see llvm#174398). (cherry picked from commit e982b4f)
Michael137
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Jan 6, 2026
…summaries (llvm#174398) Depends on: * llvm#174385 (only last commit is relevant for this review) The `${var%s}` format isn't capable of formatting references to C-strings. So the summary for those becomes `<no value available>`. This patch prevents the system C-string formatter from applying to references, which means the summary for such types will be empty. This prompts LLDB to instead print the child, which is the referenced C-string. Before: ``` (lldb) v ref (const char *&) ref = 0x000000016fdfe960 <no value available> ``` After: ``` (lldb) v ref (const char *&) ref = 0x000000016fdfec40 (&ref = "hi") ``` An alternative would be to support references in the `ValueObject` dump methods. We assume C-string are pointers/arrays in a lot of places, so such a fix would be a more intrusive undertaking, and I'm not sure we would want to support references there in the first place. So for now I went with the fallback logic in this PR. (cherry picked from commit dca0880)
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jan 6, 2026
Printing references to C-strings doesn't work properly at the moment. This patch provides coverage for those cases and should fail once the underlying issue gets fixed (see llvm#174398).
navaneethshan
pushed a commit
to qualcomm/cpullvm-toolchain
that referenced
this pull request
Jan 8, 2026
Printing references to C-strings doesn't work properly at the moment. This patch provides coverage for those cases and should fail once the underlying issue gets fixed (see llvm/llvm-project#174398).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on:
(only last commit is relevant for this review)
The
${var%s}format isn't capable of formatting references to C-strings. So the summary for those becomes<no value available>. This patch prevents the system C-string formatter from applying to references, which means the summary for such types will be empty. This prompts LLDB to instead print the child, which is the referenced C-string.Before:
After:
An alternative would be to support references in the
ValueObjectdump methods. We assume C-string are pointers/arrays in a lot of places, so such a fix would be a more intrusive undertaking, and I'm not sure we would want to support references there in the first place. So for now I went with the fallback logic in this PR.