[lldb] Add alternative SBThread::GetStopDescription #165379
Conversation
the function signature for `GetStopDescription` is `lldb::SBThread::GetStopDescription(char *dst_or_null, size_t len)` to get a description you need to call the function first time to get the buffer size. and again to get the description. This is little worse from the python size as the signature is `lldb.SBThread.GetStopDescription(int: len) -> list[str]` the user has to pass the max size as possible with no way of checking if it is enough. This patch adds a new api `lldb.SBThread.GetStopDescription(desc: lldb.SBStream()) -> bool` `bool lldb::SBThread::GetStopDescription(lldb::SBStream &description)` which handles this case.
|
@llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) Changesthe function signature for This is little worse from the python size as the signature is This patch adds a new api Adds test case for lua. Full diff: https://github.com/llvm/llvm-project/pull/165379.diff 8 Files Affected:
diff --git a/lldb/bindings/lua/lua-typemaps.swig b/lldb/bindings/lua/lua-typemaps.swig
index 56756936a532c..26163c1ad3328 100644
--- a/lldb/bindings/lua/lua-typemaps.swig
+++ b/lldb/bindings/lua/lua-typemaps.swig
@@ -121,9 +121,16 @@ LLDB_NUMBER_TYPEMAP(enum SWIGTYPE);
$1 = (char *)malloc($2);
}
+// Remove the default type check for this match.
+// because if the match function has an overload and a typemap,
+// it will typecheck against the original function instead of the
+// typemap.
+%typemap(typecheck, precedence=SWIG_TYPECHECK_POINTER) (char *dst_or_null, size_t dst_len) ""
+
%typemap(argout) (char *dst_or_null, size_t dst_len) {
lua_pop(L, 1); // Blow away the previous result
- lua_pushlstring(L, (const char *)$1, $result);
+ llvm::StringRef ref($1);
+ lua_pushlstring(L, (const char *)$1, ref.size());
free($1);
// SWIG_arg was already incremented
}
diff --git a/lldb/bindings/python/python-typemaps.swig b/lldb/bindings/python/python-typemaps.swig
index 715914fe745f8..a7fa40445b5d0 100644
--- a/lldb/bindings/python/python-typemaps.swig
+++ b/lldb/bindings/python/python-typemaps.swig
@@ -224,6 +224,13 @@ AND call SWIG_fail at the same time, because it will result in a double free.
}
$1 = (char *)malloc($2);
}
+
+// Remove the default type check for this match.
+// because if the match function has an overload and a typemap,
+// it will typecheck against the original function instead of the
+// typemap.
+%typemap(typecheck, precedence=SWIG_TYPECHECK_POINTER) (char *dst_or_null, size_t dst_len) ""
+
%typemap(argout) (char *dst_or_null, size_t dst_len) {
Py_XDECREF($result); /* Blow away any previous result */
llvm::StringRef ref($1);
diff --git a/lldb/include/lldb/API/SBThread.h b/lldb/include/lldb/API/SBThread.h
index e9fe5858d125e..2411dfd376519 100644
--- a/lldb/include/lldb/API/SBThread.h
+++ b/lldb/include/lldb/API/SBThread.h
@@ -81,6 +81,14 @@ class LLDB_API SBThread {
SBThreadCollection
GetStopReasonExtendedBacktraces(InstrumentationRuntimeType type);
+ /// Gets a human-readable description of why the thread stopped.
+ ///
+ /// \param stream Output stream to receive the stop description text
+ /// \return
+ /// true if obtained and written to the stream,
+ // false if there was an error retrieving the description.
+ bool GetStopDescription(lldb::SBStream &stream) const;
+
size_t GetStopDescription(char *dst_or_null, size_t dst_len);
SBValue GetStopReturnValue();
diff --git a/lldb/source/API/SBThread.cpp b/lldb/source/API/SBThread.cpp
index 4e4aa48bc9a2e..f58a1b52afa91 100644
--- a/lldb/source/API/SBThread.cpp
+++ b/lldb/source/API/SBThread.cpp
@@ -239,11 +239,34 @@ SBThread::GetStopReasonExtendedBacktraces(InstrumentationRuntimeType type) {
return threads;
}
-size_t SBThread::GetStopDescription(char *dst, size_t dst_len) {
- LLDB_INSTRUMENT_VA(this, dst, dst_len);
+bool SBThread::GetStopDescription(lldb::SBStream &stream) const {
+ LLDB_INSTRUMENT_VA(this, stream);
+
+ if (!m_opaque_sp)
+ return false;
+
+ llvm::Expected<StoppedExecutionContext> exe_ctx =
+ GetStoppedExecutionContext(m_opaque_sp);
+ if (!exe_ctx) {
+ LLDB_LOG_ERROR(GetLog(LLDBLog::API), exe_ctx.takeError(), "{0}");
+ return false;
+ }
+
+ if (!exe_ctx->HasThreadScope())
+ return false;
+
+ Stream &strm = stream.ref();
+ const std::string stop_desc = exe_ctx->GetThreadPtr()->GetStopDescription();
+ strm.PutCString(stop_desc);
+
+ return true;
+}
+
+size_t SBThread::GetStopDescription(char *dst_or_null, size_t dst_len) {
+ LLDB_INSTRUMENT_VA(this, dst_or_null, dst_len);
- if (dst)
- *dst = 0;
+ if (dst_or_null)
+ *dst_or_null = 0;
llvm::Expected<StoppedExecutionContext> exe_ctx =
GetStoppedExecutionContext(m_opaque_sp);
@@ -259,8 +282,8 @@ size_t SBThread::GetStopDescription(char *dst, size_t dst_len) {
if (thread_stop_desc.empty())
return 0;
- if (dst)
- return ::snprintf(dst, dst_len, "%s", thread_stop_desc.c_str()) + 1;
+ if (dst_or_null)
+ return ::snprintf(dst_or_null, dst_len, "%s", thread_stop_desc.c_str()) + 1;
// NULL dst passed in, return the length needed to contain the
// description.
diff --git a/lldb/test/API/lua_api/TestThreadAPI.lua b/lldb/test/API/lua_api/TestThreadAPI.lua
new file mode 100644
index 0000000000000..5a38d0ba9192f
--- /dev/null
+++ b/lldb/test/API/lua_api/TestThreadAPI.lua
@@ -0,0 +1,25 @@
+_T = require('lua_lldb_test').create_test('TestThreadAPI')
+
+function _T:TestGetStopDescription()
+ local target = self:create_target()
+ local breakpoint = target:BreakpointCreateByName("main", "a.out")
+ assertTrue(breakpoint:IsValid() and breakpoint:GetNumLocations() == 1)
+
+ local process = target:LaunchSimple({ 'arg1', 'arg2' }, nil, nil)
+ local thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+ assertNotNil(thread)
+ assertTrue(thread:IsValid())
+
+ assertEqual("breakpoint", thread:GetStopDescription(string.len("breakpoint") + 1))
+ assertEqual("break", thread:GetStopDescription(string.len("break") + 1))
+ assertEqual("b", thread:GetStopDescription(string.len("b") + 1))
+ assertEqual("breakpoint 1.1", thread:GetStopDescription(string.len("breakpoint 1.1") + 100))
+
+ -- Test stream variation
+ local stream = lldb.SBStream()
+ assertTrue(thread:GetStopDescription(stream))
+ assertNotNil(stream)
+ assertEqual("breakpoint 1.1", stream:GetData())
+end
+
+os.exit(_T:run())
diff --git a/lldb/test/API/python_api/default-constructor/sb_thread.py b/lldb/test/API/python_api/default-constructor/sb_thread.py
index 34eb3db852c38..4252fa0321fff 100644
--- a/lldb/test/API/python_api/default-constructor/sb_thread.py
+++ b/lldb/test/API/python_api/default-constructor/sb_thread.py
@@ -10,6 +10,7 @@ def fuzz_obj(obj):
obj.GetStopReasonDataCount()
obj.GetStopReasonDataAtIndex(100)
obj.GetStopDescription(256)
+ obj.GetStopDescription(lldb.SBStream())
obj.GetThreadID()
obj.GetIndexID()
obj.GetName()
diff --git a/lldb/test/API/python_api/thread/TestThreadAPI.py b/lldb/test/API/python_api/thread/TestThreadAPI.py
index 5583434a742a9..acad7583eec19 100644
--- a/lldb/test/API/python_api/thread/TestThreadAPI.py
+++ b/lldb/test/API/python_api/thread/TestThreadAPI.py
@@ -138,6 +138,11 @@ def get_stop_description(self):
"breakpoint 1.1", thread.GetStopDescription(len("breakpoint 1.1") + 100)
)
+ # Test the stream variation
+ stream = lldb.SBStream()
+ self.assertTrue(thread.GetStopDescription(stream))
+ self.assertEqual("breakpoint 1.1", stream.GetData())
+
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()."""
exe = self.getBuildArtifact(exe_name)
diff --git a/lldb/tools/lldb-rpc-gen/RPCCommon.cpp b/lldb/tools/lldb-rpc-gen/RPCCommon.cpp
index 7af6ee37f248b..5405f77b31d96 100644
--- a/lldb/tools/lldb-rpc-gen/RPCCommon.cpp
+++ b/lldb/tools/lldb-rpc-gen/RPCCommon.cpp
@@ -118,6 +118,7 @@ static constexpr llvm::StringRef MethodsWithPointerPlusLen[] = {
"_ZN4lldb8SBTarget15GetInstructionsEyPKvm",
"_ZN4lldb8SBTarget25GetInstructionsWithFlavorEyPKcPKvm",
"_ZN4lldb8SBThread18GetStopDescriptionEPcm",
+ "_ZNK4lldb8SBThread18GetStopDescriptionERNS_8SBStreamE",
// The below mangled names are used for dummy methods in shell tests
// that test the emitters' output. If you're adding any new mangled names
// from the actual SB API to this list please add them above.
|
| // because if the match function has an overload and a typemap, | ||
| // it will typecheck against the original function instead of the | ||
| // typemap. | ||
| %typemap(typecheck, precedence=SWIG_TYPECHECK_POINTER) (char *dst_or_null, size_t dst_len) "" |
There was a problem hiding this comment.
Might be worth mentioning that this is required because the other overload has a numinputs=1 typemap. And the generated dispatching function will try to type check that as a char*, where in actuality it's an integer (correct me if I'm wrong).
Does this mean it removes the type-check for all the other APIs with this signature too? Is that an issue?
There was a problem hiding this comment.
Updated the comment.
Does this mean it removes the type-check for all the other APIs with this signature too? Is that an issue?
Only if it matches the argument name and type. But now no other function does.
could not find a way to limit it to a class without redefining the class.
There was a problem hiding this comment.
I also see:
// For lldb::SBFileSpec::GetPath
%typemap(in) (char *dst_path, size_t dst_len) = (char *dst_or_null, size_t dst_len);
%typemap(argout) (char *dst_path, size_t dst_len) = (char *dst_or_null, size_t dst_len);
I'm not too familiar with SWIG, so correct me if I'm wrong. This just copies the typemap, but not the typecheck itself? And regardless, is typecheck only ever applicable to these dispatch functions? In which case, GetPath wouldn't really be affected by that even if it did copy the typecheck?
Unrelated: looks like SBFileSpec::GetPath suffers from the same kind of API problem where you have to supply a fixed size. Should we add an SBStream overload here too? In which case, copying the typemap would be beneficial anyways.
There was a problem hiding this comment.
This just copies the typemap, but not the typecheck itself
Yes
, is typecheck only ever applicable to these dispatch functions?
No because it copied for only typemap(in) and type map(argout)
SBFileSpec::GetPath suffers from the same kind of API problem where you have to supply a fixed size. Should we add an SBStream overload here too?
Would make sense to do so
I think you mean test case as in whole new test case, but at first this read as if this was a Lua only feature. You have in fact added Python coverage as well. |
|
I don't feel strongly against this (since passing an |
The two "calls" is now amortised depending on the scenario They have a |
5 participants
the function signature for
GetStopDescriptionislldb::SBThread::GetStopDescription(char *dst_or_null, size_t len).To get a description you need to call the function first time to get the buffer size. a second time to get the description.
This is little worse from the python size as the signature is
lldb.SBThread.GetStopDescription(int: len) -> list[str]the user has to pass the max size as possible with no way of checking if it is enough.This patch adds a new api
lldb.SBThread.GetStopDescription(desc: lldb.SBStream()) -> boolbool lldb::SBThread::GetStopDescription(lldb::SBStream &description)which handles this case.Adds new Test case for lua.