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()."""
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()."""
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits