[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it
jingham updated this revision to Diff 265632.
jingham added a comment.
Address review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80350/new/
https://reviews.llvm.org/D80350
Files:
lldb/include/lldb/lldb-enumerations.h
lldb/source/Expression/FunctionCaller.cpp
lldb/source/Expression/LLVMUserExpression.cpp
lldb/source/Target/Process.cpp
lldb/test/API/functionalities/thread/exit_during_expression/Makefile
lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
lldb/test/API/functionalities/thread/exit_during_expression/main.c
Index: lldb/test/API/functionalities/thread/exit_during_expression/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/main.c
@@ -0,0 +1,38 @@
+#include
+#include
+#include
+#include
+
+static unsigned int g_timeout = 200;
+
+int function_to_call() {
+
+ errno = 0;
+ while (1) {
+int result = usleep(g_timeout);
+if (errno != EINTR)
+ break;
+ }
+
+ pthread_exit((void *)10);
+
+ return 20; // Prevent warning
+}
+
+void *exiting_thread_func(void *unused) {
+ function_to_call(); // Break here and cause the thread to exit
+ return NULL;
+}
+
+int main() {
+ char *exit_ptr;
+ pthread_t exiting_thread;
+
+ pthread_create(_thread, NULL, exiting_thread_func, NULL);
+
+ pthread_join(exiting_thread, _ptr);
+ int ret_val = (int)exit_ptr;
+ usleep(g_timeout * 4); // Make sure in the "run all threads" case
+ // that we don't run past our breakpoint.
+ return ret_val;// Break here to make sure the thread exited.
+}
Index: lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
@@ -0,0 +1,106 @@
+"""
+Make sure that we handle an expression on a thread, if
+the thread exits while the expression is running.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class TestExitDuringExpression(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_exit_before_one_thread_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(True, True)
+
+@skipIfWindows
+def test_exit_before_one_thread_no_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(True, False)
+
+@skipIfWindows
+def test_exit_after_one_thread_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(False, True)
+
+@skipIfWindows
+def test_exit_after_one_thread_no_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(False, False)
+
+def setUp(self):
+TestBase.setUp(self)
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.build()
+
+def exiting_expression_test(self, before_one_thread_timeout , unwind):
+"""function_to_call sleeps for g_timeout microseconds, then calls pthread_exit.
+ This test calls function_to_call with an overall timeout of 500
+ microseconds, and a one_thread_timeout as passed in.
+ It also sets unwind_on_exit for the call to the unwind passed in.
+ This allows you to have the thread exit either before the one thread
+ timeout is passed. """
+
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+ "Break here and cause the thread to exit", self.main_source_file)
+
+# We'll continue to this breakpoint after running our expression:
+return_bkpt = target.BreakpointCreateBySourceRegex("Break here to make sure the thread exited", self.main_source_file)
+frame = thread.frames[0]
+tid = thread.GetThreadID()
+# Find the timeout:
+var_options = lldb.SBVariablesOptions()
+var_options.SetIncludeArguments(False)
+var_options.SetIncludeLocals(False)
+var_options.SetIncludeStatics(True)
+
+value_list = frame.GetVariables(var_options)
+g_timeout = value_list.GetFirstValueByName("g_timeout")
+self.assertTrue(g_timeout.IsValid(), "Found g_timeout")
+
+error = lldb.SBError()
+timeout_value = g_timeout.GetValueAsUnsigned(error)
+self.assertTrue(error.Success(), "Couldn't get timeout value: %s"%(error.GetCString()))
+
+one_thread_timeout = 0
+if (before_one_thread_timeout):
+
[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it
jingham marked an inline comment as done.
jingham added inline comments.
Comment at: lldb/source/Target/Process.cpp:4671
+ "the expression was running.",
+ thread_id);
+return eExpressionThreadVanished;
aprantl wrote:
> Is it useful to both log the message and have an almost similarly-worded
> diagnostic produced in LLVMUserExpression.cpp?
>
> ("Yes" is an acceptable answer)
The diagnostic will go the the user, and end up in the Error in the expression
result ValueObject. So that part we have to do. Putting it in the log will
make the message show up where it happens in the flow of events we're logging,
which is very handy. So IMO it is worthwhile doing the latter as well.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80350/new/
https://reviews.llvm.org/D80350
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it
aprantl added a comment.
This looks reasonable based on my limited knowledge.
Comment at: lldb/source/Target/Process.cpp:4671
+ "the expression was running.",
+ thread_id);
+return eExpressionThreadVanished;
Is it useful to both log the message and have an almost similarly-worded
diagnostic produced in LLVMUserExpression.cpp?
("Yes" is an acceptable answer)
Comment at: lldb/source/Target/Process.cpp:5534
+result_name = "eExpressionThreadVanished";
+ default:
+result_name = "";
If you remove the default case and initialize the result_name to "
"" at the top, you get a free compiler warning every time when someone
adds a new enumerator.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80350/new/
https://reviews.llvm.org/D80350
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it
jasonmolenda added a comment. I skimmed it, I guess the one question that comes to mind is how this will behave if we an operating system thread provider plugin active, where it may not actually list all of the threads that exist -- where we try to be tolerant of threads disappearing and then re-appearing later -- and the thread running an inferior function call got scheduled out while the function call was still happening. Pretty unlikely scenario, but I thought I'd throw it out there. Comment at: lldb/source/Expression/LLVMUserExpression.cpp:235 + "on which the expression was being run: 0x%" PRIx64 + " exited during its execution."); + return execution_result; I think you meant to include the tid in the Printf. Did this produce a warning? We should have decorated the Printf() method in this class with the thing that tells clang this is a printf functoin. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80350/new/ https://reviews.llvm.org/D80350 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it
jingham updated this revision to Diff 265608.
jingham added a comment.
clang-format
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80350/new/
https://reviews.llvm.org/D80350
Files:
lldb/include/lldb/lldb-enumerations.h
lldb/source/Expression/FunctionCaller.cpp
lldb/source/Expression/LLVMUserExpression.cpp
lldb/source/Target/Process.cpp
lldb/test/API/functionalities/thread/exit_during_expression/Makefile
lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
lldb/test/API/functionalities/thread/exit_during_expression/main.c
Index: lldb/test/API/functionalities/thread/exit_during_expression/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/main.c
@@ -0,0 +1,38 @@
+#include
+#include
+#include
+#include
+
+static unsigned int g_timeout = 200;
+
+int function_to_call() {
+
+ errno = 0;
+ while (1) {
+int result = usleep(g_timeout);
+if (errno != EINTR)
+ break;
+ }
+
+ pthread_exit((void *)10);
+
+ return 20; // Prevent warning
+}
+
+void *exiting_thread_func(void *unused) {
+ function_to_call(); // Break here and cause the thread to exit
+ return NULL;
+}
+
+int main() {
+ char *exit_ptr;
+ pthread_t exiting_thread;
+
+ pthread_create(_thread, NULL, exiting_thread_func, NULL);
+
+ pthread_join(exiting_thread, _ptr);
+ int ret_val = (int)exit_ptr;
+ usleep(g_timeout * 4); // Make sure in the "run all threads" case
+ // that we don't run past our breakpoint.
+ return ret_val;// Break here to make sure the thread exited.
+}
Index: lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
@@ -0,0 +1,106 @@
+"""
+Make sure that we handle an expression on a thread, if
+the thread exits while the expression is running.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class TestExitDuringExpression(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_exit_before_one_thread_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(True, True)
+
+@skipIfWindows
+def test_exit_before_one_thread_no_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(True, False)
+
+@skipIfWindows
+def test_exit_after_one_thread_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(False, True)
+
+@skipIfWindows
+def test_exit_after_one_thread_no_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(False, False)
+
+def setUp(self):
+TestBase.setUp(self)
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.build()
+
+def exiting_expression_test(self, before_one_thread_timeout , unwind):
+"""function_to_call sleeps for g_timeout microseconds, then calls pthread_exit.
+ This test calls function_to_call with an overall timeout of 500
+ microseconds, and a one_thread_timeout as passed in.
+ It also sets unwind_on_exit for the call to the unwind passed in.
+ This allows you to have the thread exit either before the one thread
+ timeout is passed. """
+
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+ "Break here and cause the thread to exit", self.main_source_file)
+
+# We'll continue to this breakpoint after running our expression:
+return_bkpt = target.BreakpointCreateBySourceRegex("Break here to make sure the thread exited", self.main_source_file)
+frame = thread.frames[0]
+tid = thread.GetThreadID()
+# Find the timeout:
+var_options = lldb.SBVariablesOptions()
+var_options.SetIncludeArguments(False)
+var_options.SetIncludeLocals(False)
+var_options.SetIncludeStatics(True)
+
+value_list = frame.GetVariables(var_options)
+g_timeout = value_list.GetFirstValueByName("g_timeout")
+self.assertTrue(g_timeout.IsValid(), "Found g_timeout")
+
+error = lldb.SBError()
+timeout_value = g_timeout.GetValueAsUnsigned(error)
+self.assertTrue(error.Success(), "Couldn't get timeout value: %s"%(error.GetCString()))
+
+one_thread_timeout = 0
+if (before_one_thread_timeout):
+
[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it
jingham created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
It is possible that the thread we are running a function on could exit while we
are waiting for the function call to return.
We sort of handled that case before, but it really only worked by accident
because the ThreadPlan still had a pointer to the Thread, and it hadn't
actually gone away when we touched it after stopping and finding that it had
exited. Now that ThreadPlans re-look up the thread after each stop, we were
handing out a null Thread pointer and crashing.
I moved the checking for vanished threads to the helper routine that handles
the stop event, added an expression result of eExpressionThreadVanished and
handle it properly in RunThreadPlan. I also added a test using a function that
just called pthread_exit. This crashed before these changes, and works
correctly after.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D80350
Files:
lldb/include/lldb/lldb-enumerations.h
lldb/source/Expression/FunctionCaller.cpp
lldb/source/Expression/LLVMUserExpression.cpp
lldb/source/Target/Process.cpp
lldb/test/API/functionalities/thread/exit_during_expression/Makefile
lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
lldb/test/API/functionalities/thread/exit_during_expression/main.c
Index: lldb/test/API/functionalities/thread/exit_during_expression/main.c
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/main.c
@@ -0,0 +1,42 @@
+#include
+#include
+#include
+#include
+
+static unsigned int g_timeout = 200;
+
+int
+function_to_call() {
+
+ errno = 0;
+ while(1) {
+int result = usleep(g_timeout);
+if (errno != EINTR)
+ break;
+ }
+
+ pthread_exit((void *) 10);
+
+ return 20; // Prevent warning
+}
+
+void *
+exiting_thread_func (void *unused) {
+ function_to_call(); // Break here and cause the thread to exit
+ return NULL;
+}
+
+int
+main()
+{
+ char *exit_ptr;
+ pthread_t exiting_thread;
+
+ pthread_create(_thread, NULL, exiting_thread_func, NULL);
+
+ pthread_join(exiting_thread, _ptr);
+ int ret_val = (int) exit_ptr;
+ usleep(g_timeout * 4); // Make sure in the "run all threads" case
+ // that we don't run past our breakpoint.
+ return ret_val; // Break here to make sure the thread exited.
+}
Index: lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
===
--- /dev/null
+++ lldb/test/API/functionalities/thread/exit_during_expression/TestExitDuringExpression.py
@@ -0,0 +1,106 @@
+"""
+Make sure that we handle an expression on a thread, if
+the thread exits while the expression is running.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+class TestExitDuringExpression(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+NO_DEBUG_INFO_TESTCASE = True
+
+@skipIfWindows
+def test_exit_before_one_thread_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(True, True)
+
+@skipIfWindows
+def test_exit_before_one_thread_no_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(True, False)
+
+@skipIfWindows
+def test_exit_after_one_thread_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(False, True)
+
+@skipIfWindows
+def test_exit_after_one_thread_no_unwind(self):
+"""Test the case where we exit within the one thread timeout"""
+self.exiting_expression_test(False, False)
+
+def setUp(self):
+TestBase.setUp(self)
+self.main_source_file = lldb.SBFileSpec("main.c")
+self.build()
+
+def exiting_expression_test(self, before_one_thread_timeout , unwind):
+"""function_to_call sleeps for g_timeout microseconds, then calls pthread_exit.
+ This test calls function_to_call with an overall timeout of 500
+ microseconds, and a one_thread_timeout as passed in.
+ It also sets unwind_on_exit for the call to the unwind passed in.
+ This allows you to have the thread exit either before the one thread
+ timeout is passed. """
+
+(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+ "Break here and cause the thread to exit", self.main_source_file)
+
+# We'll continue to this breakpoint after running our expression:
+return_bkpt = target.BreakpointCreateBySourceRegex("Break here to make sure the thread exited", self.main_source_file)
+frame
