[Lldb-commits] [PATCH] D111399: [lldb] Make char[N] formatters respect the end of the array (PR44649)
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8093c2ea574b: [lldb] Make char[N] formatters respect the end
of the array (PR44649) (authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D111399?vs=378162=378616#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111399/new/
https://reviews.llvm.org/D111399
Files:
lldb/source/DataFormatters/FormatManager.cpp
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -1,7 +1,18 @@
#include
+#include
+
+struct A {
+ char data[4];
+ char overflow[4];
+};
int main (int argc, char const *argv[])
{
+A a, b;
+// 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);
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* constcharstar = stdstring.c_str();
std::string longstring(
@@ -20,12 +31,15 @@
);
const char* longconstcharstar = longstring.c_str();
return 0; //% if self.TraceOn(): self.runCmd('frame variable')
- //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary()
== '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //%
self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() ==
'"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //% self.runCmd("setting set escape-non-printables false")
- //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary()
== '"Hello\t\tWorld\nI am here\t\tto say hello\n"')
- //%
self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() ==
'"Hello\t\tWorld\nI am here\t\tto say hello\n"')
+//% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am
here\\t\\tto say hello\\n"')
+//% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI
am here\\t\\tto say hello\\n"')
+//% 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('constcharstar', 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('"...'))
+//% self.expect_var_path("a.data", summary='"FOOB"')
+// FIXME: Should this be "FO\0B" instead?
+//% self.expect_var_path("b.data", summary='"FO"')
}
Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -722,7 +722,7 @@
new StringSummaryFormat(string_flags, "${var%s}"));
lldb::TypeSummaryImplSP string_array_format(
- new StringSummaryFormat(string_array_flags, "${var%s}"));
+ new StringSummaryFormat(string_array_flags, "${var%char[]}"));
RegularExpression any_size_char_arr(llvm::StringRef("char \\[[0-9]+\\]"));
Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -1,7 +1,18 @@
#include
+#include
+
+struct A {
+ char data[4];
+ char overflow[4];
+};
int main (int argc, char const *argv[])
{
+A a, b;
+// 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);
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* constcharstar = stdstring.c_str();
std::string longstring(
@@ -20,12 +31,15 @@
);
const char* longconstcharstar = longstring.c_str();
return 0; //% if self.TraceOn(): self.runCmd('frame variable')
- //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary() == '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //% self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() == '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //% self.runCmd("setting set escape-non-printables false")
- //%
[Lldb-commits] [PATCH] D111399: [lldb] Make char[N] formatters respect the end of the array (PR44649)
labath added inline comments.
Comment at:
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:39
+ //% self.expect_var_path('constcharstar', 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('"...'))
teemperor wrote:
> If you align the start of these comments when merging this, then I'll address
> you from now on as "Pavel the Great".
uh-oh
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111399/new/
https://reviews.llvm.org/D111399
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111399: [lldb] Make char[N] formatters respect the end of the array (PR44649)
teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
LGTM, but maybe give the test comment a FIXME.
Comment at:
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:39
+ //% self.expect_var_path('constcharstar', 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('"...'))
If you align the start of these comments when merging this, then I'll address
you from now on as "Pavel the Great".
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111399/new/
https://reviews.llvm.org/D111399
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111399: [lldb] Make char[N] formatters respect the end of the array (PR44649)
labath added inline comments.
Comment at:
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp:42-43
+//% self.expect_var_path("a.data", summary='"FOOB"')
+// Should this be "FO\0B" instead?
+//% self.expect_var_path("b.data", summary='"FO"')
}
This would be consistent with gdb as well, and it was the preferred result of
last years irc straw poll, but it will require a different fix.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111399/new/
https://reviews.llvm.org/D111399
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D111399: [lldb] Make char[N] formatters respect the end of the array (PR44649)
labath created this revision.
labath added reviewers: teemperor, jingham.
labath requested review of this revision.
Herald added a project: LLDB.
I believe this is a more natural behavior, and it also matches what gdb
does.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D111399
Files:
lldb/source/DataFormatters/FormatManager.cpp
lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -1,7 +1,18 @@
#include
+#include
+
+struct A {
+ char data[4];
+ char overflow[4];
+};
int main (int argc, char const *argv[])
{
+A a, b;
+// 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);
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* constcharstar = stdstring.c_str();
std::string longstring(
@@ -20,12 +31,15 @@
);
const char* longconstcharstar = longstring.c_str();
return 0; //% if self.TraceOn(): self.runCmd('frame variable')
- //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary()
== '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //%
self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() ==
'"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+ //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am
here\\t\\tto say hello\\n"')
+ //% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI
am here\\t\\tto say hello\\n"')
//% self.runCmd("setting set escape-non-printables false")
- //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary()
== '"Hello\t\tWorld\nI am here\t\tto say hello\n"')
- //%
self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() ==
'"Hello\t\tWorld\nI am here\t\tto say hello\n"')
+ //% self.expect_var_path('stdstring', summary='"Hello\t\tWorld\nI am
here\t\tto say hello\n"')
+ //% self.expect_var_path('constcharstar', 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('"...'))
+//% self.expect_var_path("a.data", summary='"FOOB"')
+// Should this be "FO\0B" instead?
+//% self.expect_var_path("b.data", summary='"FO"')
}
Index: lldb/source/DataFormatters/FormatManager.cpp
===
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -722,7 +722,7 @@
new StringSummaryFormat(string_flags, "${var%s}"));
lldb::TypeSummaryImplSP string_array_format(
- new StringSummaryFormat(string_array_flags, "${var%s}"));
+ new StringSummaryFormat(string_array_flags, "${var%char[]}"));
RegularExpression any_size_char_arr(llvm::StringRef("char \\[[0-9]+\\]"));
Index: lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
+++ lldb/test/API/functionalities/data-formatter/stringprinter/main.cpp
@@ -1,7 +1,18 @@
#include
+#include
+
+struct A {
+ char data[4];
+ char overflow[4];
+};
int main (int argc, char const *argv[])
{
+A a, b;
+// 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);
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* constcharstar = stdstring.c_str();
std::string longstring(
@@ -20,12 +31,15 @@
);
const char* longconstcharstar = longstring.c_str();
return 0; //% if self.TraceOn(): self.runCmd('frame variable')
- //% self.assertTrue(self.frame().FindVariable('stdstring').GetSummary() == '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
- //% self.assertTrue(self.frame().FindVariable('constcharstar').GetSummary() == '"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+ //% self.expect_var_path('stdstring', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
+ //% self.expect_var_path('constcharstar', summary='"Hello\\t\\tWorld\\nI am here\\t\\tto say hello\\n"')
//% self.runCmd("setting set escape-non-printables false")
