[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG177dd63c8d74: Data formatters: fix detection of C strings 
(authored by jarin, committed by teemperor).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76650/new/

https://reviews.llvm.org/D76650

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
  
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp


Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 


Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin updated this revision to Diff 252276.
jarin marked 2 inline comments as done.
jarin added a comment.

Addressed reviewer comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76650/new/

https://reviews.llvm.org/D76650

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
  
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp


Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 


Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = u8"";
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect_expr("s", result_summary='""')
+self.expect_expr("(const char*)s", result_summary='""')
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin added a comment.

Thanks for the review! Could you possibly land this for me?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76650/new/

https://reviews.llvm.org/D76650



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

[Raphael Isemann via Phabricator via lldb-commits]

teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for tracking this down! I only have a minor comment about the test 
character.




Comment at: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py:18
+self.expect("expr s", substrs=['"é"'])
+self.expect("expr (const char*)s", substrs=['"é"'])

`expect_expr` is the modern way to check this:
```
lang=python
self.expect_expr("s", result_summary='""')
self.expect_expr("(const char*)s", result_summary='""')
```



Comment at: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp:2
+int main() {
+  const char *s = "é";
+  return 0; // break here

Could you make this a string like `u8""`? "é" is also a valid character in a 
lot of extended-ASCII character sets (e.g., Latin-1) and we probably should 
take some unicode-exclusive character and make sure it's utf-8.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76650/new/

https://reviews.llvm.org/D76650



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76650: Data formatters: fix detection of C strings

[Jaroslav Sevcik via Phabricator via lldb-commits]

jarin created this revision.
jarin added a reviewer: teemperor.
jarin added a project: LLDB.
Herald added a subscriber: lldb-commits.

Detection of C strings does not work well for pointers. If the value object 
holding a (char*) pointer does not have an address (e.g., if it is a temp), the 
value is not considered a C string and its formatting is left to 
DumpDataExtractor rather than the special handling in  
ValueObject::DumpPrintableRepresentation. This leads to inconsistent outputs, 
e.g., in escaping non-ASCII characters. See the test for an example; the second 
test expectation is not met (without this patch). With this patch, the C string 
detection only insists that the pointer value is valid. The patch makes the 
code consistent with how the pointer is obtained in 
ValueObject::ReadPointedString.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76650

Files:
  lldb/source/Core/ValueObject.cpp
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
  
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
  lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp


Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = "é";
+  return 0; // break here
+}
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect("expr s", substrs=['"é"'])
+self.expect("expr (const char*)s", substrs=['"é"'])
Index: 
lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);
   return (cstr_address != LLDB_INVALID_ADDRESS);
 }
 


Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/main.cpp
@@ -0,0 +1,4 @@
+int main() {
+  const char *s = "é";
+  return 0; // break here
+}
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/TestCstringUnicode.py
@@ -0,0 +1,18 @@
+# coding=utf8
+
+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CstringUnicodeTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_cstring_unicode(self):
+self.build()
+lldbutil.run_to_source_breakpoint(self, "// break here",
+lldb.SBFileSpec("main.cpp", False))
+self.expect("expr s", substrs=['"é"'])
+self.expect("expr (const char*)s", substrs=['"é"'])
Index: lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
===
--- /dev/null
+++ lldb/test/API/functionalities/data-formatter/cstring-utf8-summary/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -764,7 +764,7 @@
 return true;
   addr_t cstr_address = LLDB_INVALID_ADDRESS;
   AddressType cstr_address_type = eAddressTypeInvalid;
-  cstr_address = GetAddressOf(true, _address_type);
+  cstr_address = GetPointerValue(_address_type);