[Lldb-commits] [lldb] 61b6a4e - [lldb] Fix that SBThread.GetStopDescription is returning strings with uninitialized memory at the end.
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Tue Jan 14 00:35:23 PST 2020
Author: Raphael Isemann
Date: 2020-01-14T09:34:32+01:00
New Revision: 61b6a4e82653e1209126404d33ad20a268f55db1
URL: https://github.com/llvm/llvm-project/commit/61b6a4e82653e1209126404d33ad20a268f55db1
DIFF: https://github.com/llvm/llvm-project/commit/61b6a4e82653e1209126404d33ad20a268f55db1.diff
LOG: [lldb] Fix that SBThread.GetStopDescription is returning strings with uninitialized memory at the end.
Summary:
`SBThread.GetStopDescription` is a curious API as it takes a buffer length as a parameter that specifies
how many bytes the buffer we pass has. Then we fill the buffer until the specified length (or the length
of the stop description string) and return the string length. If the buffer is a nullptr however, we instead
return how many bytes we would have written to the buffer so that the user can allocate a buffer with
the right size and pass that size to a subsequent `SBThread.GetStopDescription` call.
Funnily enough, it is not possible to pass a nullptr via the Python SWIG bindings, so that might be the
first API in LLDB that is not only hard to use correctly but impossible to use correctly. The only way to
call this function via Python is to throw in a large size limit that is hopefully large enough to contain the
stop description (otherwise we only get the truncated stop description).
Currently passing a size limit that is smaller than the returned stop description doesn't cause the
Python bindings to return the stop description but instead the truncated stop description + uninitialized characters
at the end of the string. The reason for this is that we return the result of `snprintf` from the method
which returns the amount of bytes that *would* have been written (which is larger than the buffer).
This causes our Python bindings to return a string that is as large as full stop description but the
buffer that has been filled is only as large as the passed in buffer size.
This patch fixes this issue by just recalculating the string length in our buffer instead of relying on the wrong
return value. We also have to do this in a new type map as the old type map is also used for all methods
with the given argument pair `char *dst, size_t dst_len` (e.g. SBProcess.GetSTDOUT`). These methods have
different semantics for these arguments and don't null-terminate the returned buffer (they instead return the
size in bytes) so we can't change the existing typemap without breaking them.
Reviewers: labath, jingham
Reviewed By: labath
Subscribers: clayborg, shafik, abidh, JDevlieghere, lldb-commits
Tags: #lldb
Differential Revision: https://reviews.llvm.org/D72086
Added:
Modified:
lldb/bindings/interface/SBThread.i
lldb/bindings/python/python-typemaps.swig
lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
Removed:
################################################################################
diff --git a/lldb/bindings/interface/SBThread.i b/lldb/bindings/interface/SBThread.i
index 95b15b182ec2..66466b7947fd 100644
--- a/lldb/bindings/interface/SBThread.i
+++ b/lldb/bindings/interface/SBThread.i
@@ -127,7 +127,7 @@ public:
Pass only an (int)length and expect to get a Python string describing the
stop reason.") GetStopDescription;
size_t
- GetStopDescription (char *dst, size_t dst_len);
+ GetStopDescription (char *dst_or_null, size_t dst_len);
SBValue
GetStopReturnValue ();
diff --git a/lldb/bindings/python/python-typemaps.swig b/lldb/bindings/python/python-typemaps.swig
index 2ba380bdf0d5..bfd7ef9007d1 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -95,7 +95,6 @@
/* Typemap definitions to allow SWIG to properly handle char buffer. */
// typemap for a char buffer
-// See also SBThread::GetStopDescription.
%typemap(in) (char *dst, size_t dst_len) {
if (!PyInt_Check($input)) {
PyErr_SetString(PyExc_ValueError, "Expecting an integer");
@@ -113,7 +112,6 @@
%typemap(in) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
// Return the char buffer. Discarding any previous return result
-// See also SBThread::GetStopDescription.
%typemap(argout) (char *dst, size_t dst_len) {
Py_XDECREF($result); /* Blow away any previous result */
if (result == 0) {
@@ -132,6 +130,28 @@
%typemap(argout) (void *char_buf, size_t size) = (char *dst, size_t dst_len);
+// typemap for handling an snprintf-like API like SBThread::GetStopDescription.
+%typemap(in) (char *dst_or_null, size_t dst_len) {
+ if (!PyInt_Check($input)) {
+ PyErr_SetString(PyExc_ValueError, "Expecting an integer");
+ return NULL;
+ }
+ $2 = PyInt_AsLong($input);
+ if ($2 <= 0) {
+ PyErr_SetString(PyExc_ValueError, "Positive integer expected");
+ return NULL;
+ }
+ $1 = (char *) malloc($2);
+}
+%typemap(argout) (char *dst_or_null, size_t dst_len) {
+ Py_XDECREF($result); /* Blow away any previous result */
+ llvm::StringRef ref($1);
+ PythonString string(ref);
+ $result = string.release();
+ free($1);
+}
+
+
// typemap for an outgoing buffer
// See also SBEvent::SBEvent(uint32_t event, const char *cstr, uint32_t cstr_len).
// Ditto for SBProcess::PutSTDIN(const char *src, size_t src_len).
diff --git a/lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py b/lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
index 21c32fb257fa..912413852932 100644
--- a/lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
+++ b/lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
@@ -122,14 +122,20 @@ def get_stop_description(self):
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()."""
More information about the lldb-commits
mailing list