[Lldb-commits] [PATCH] D72086: [lldb] Fix that SBThread.GetStopDescription is returning strings with uninitialized memory at the end.

[Raphael Isemann via Phabricator via lldb-commits]

teemperor updated this revision to Diff 236542.
teemperor edited the summary of this revision.
teemperor added a comment.

- Move change to SWIG type map.
- Fixed some type in the test.

The API never emulated the snprintf behavior as it includes a NULL byte when we pass a nullptr buffer (and that's why it has the inconsistent ` + 1` in the else branch). We get this whole thing even more wrong as we add the NULL byte to the snprintf result in the other branch (at the end of the method). Anyway, I changed this in the type map now and just use `strlen` on the result string which is simpler (and doesn't rely on any of our bogus return values).

I'll make another patch where we can discuss what this function is supposed to return.


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

https://reviews.llvm.org/D72086

Files:
  lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
  lldb/scripts/Python/python-typemaps.swig


Index: lldb/scripts/Python/python-typemaps.swig
===================================================================
--- lldb/scripts/Python/python-typemaps.swig
+++ lldb/scripts/Python/python-typemaps.swig
@@ -121,7 +121,8 @@
       $result = string.release();
       Py_INCREF($result);
    } else {
-      llvm::StringRef ref(static_cast<const char*>($1), result);
+      const char *cstr = static_cast<const char*>($1);
+      llvm::StringRef ref(cstr, strlen(cstr));
       PythonString string(ref);
       $result = string.release();
    }
Index: lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
+++ lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
@@ -122,14 +122,20 @@
         self.assertTrue(
             thread.IsValid(),
             "There should be a thread stopped due to breakpoint")
-        #self.runCmd("process status")
-
-        # Due to the typemap magic (see lldb.swig), we pass in an (int)length to GetStopDescription
-        # and expect to get a Python string as the return object!
-        # The 100 is just an arbitrary number specifying the buffer size.
-        stop_description = thread.GetStopDescription(100)
-        self.expect(stop_description, exe=False,
-                    startstr='breakpoint')
+
+        # Get the stop reason. GetStopDescription expects that we pass in the size of the description
+        # we expect plus an additional byte for the null terminator.
+
+        # Test with a buffer that is exactly as large as the expected stop reason.
+        self.assertEqual("breakpoint 1.1", thread.GetStopDescription(len('breakpoint 1.1') + 1))
+
+        # Test some smaller buffer sizes.
+        self.assertEqual("breakpoint", thread.GetStopDescription(len('breakpoint') + 1))
+        self.assertEqual("break", thread.GetStopDescription(len('break') + 1))
+        self.assertEqual("b", thread.GetStopDescription(len('b') + 1))
+
+        # Test that we can pass in a much larger size and still get the right output.
+        self.assertEqual("breakpoint 1.1", thread.GetStopDescription(len('breakpoint 1.1') + 100))
 
     def step_out_of_malloc_into_function_b(self, exe_name):
         """Test Python SBThread.StepOut() API to step out of a malloc call where the call site is at function b()."""


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72086.236542.patch
Type: text/x-patch
Size: 2442 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200107/e16324c9/attachment.bin>