teemperor created this revision.
teemperor added a reviewer: labath.
Herald added subscribers: lldb-commits, JDevlieghere, abidh.
Herald added a project: LLDB.
`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 returning the actual length of the buffer
we have written to (which is either
the buffer length or the length of the stop description, whatever is shorter).
Repository:
rLLDB LLDB
https://reviews.llvm.org/D72086
Files:
lldb/packages/Python/lldbsuite/test/python_api/thread/TestThreadAPI.py
lldb/source/API/SBThread.cpp
Index: lldb/source/API/SBThread.cpp
===================================================================
--- lldb/source/API/SBThread.cpp
+++ lldb/source/API/SBThread.cpp
@@ -327,8 +327,13 @@
if (stop_info_sp) {
const char *stop_desc = stop_info_sp->GetDescription();
if (stop_desc) {
- if (dst)
- return ::snprintf(dst, dst_len, "%s", stop_desc);
+ if (dst) {
+ ::snprintf(dst, dst_len, "%s", stop_desc);
+ // The string we return is either as long as buffer length minus
null
+ // terminator or the number of characters in the description
(depending
+ // which of these two is shorter).
+ return std::min(dst_len - 1, strlen(stop_desc));
+ }
else {
// NULL dst passed in, return the length needed to contain the
// description
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()."""
Index: lldb/source/API/SBThread.cpp
===================================================================
--- lldb/source/API/SBThread.cpp
+++ lldb/source/API/SBThread.cpp
@@ -327,8 +327,13 @@
if (stop_info_sp) {
const char *stop_desc = stop_info_sp->GetDescription();
if (stop_desc) {
- if (dst)
- return ::snprintf(dst, dst_len, "%s", stop_desc);
+ if (dst) {
+ ::snprintf(dst, dst_len, "%s", stop_desc);
+ // The string we return is either as long as buffer length minus null
+ // terminator or the number of characters in the description (depending
+ // which of these two is shorter).
+ return std::min(dst_len - 1, strlen(stop_desc));
+ }
else {
// NULL dst passed in, return the length needed to contain the
// description
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()."""
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
