[Lldb-commits] [PATCH] D27258: Use Timeout<> in Process::RunThreadPlan
This revision was automatically updated to reflect the committed changes.
Closed by commit rL288326: Use Timeout<> in Process::RunThreadPlan (authored by
labath).
Changed prior to commit:
https://reviews.llvm.org/D27258?vs=79737&id=79886#toc
Repository:
rL LLVM
https://reviews.llvm.org/D27258
Files:
lldb/trunk/source/Target/Process.cpp
Index: lldb/trunk/source/Target/Process.cpp
===
--- lldb/trunk/source/Target/Process.cpp
+++ lldb/trunk/source/Target/Process.cpp
@@ -13,6 +13,7 @@
#include
// Other libraries and framework includes
+#include "llvm/Support/ScopedPrinter.h"
// Project includes
#include "Plugins/Process/Utility/InferiorCallPOSIX.h"
#include "lldb/Breakpoint/BreakpointLocation.h"
@@ -4799,6 +4800,45 @@
};
} // anonymous namespace
+static microseconds
+GetOneThreadExpressionTimeout(const EvaluateExpressionOptions &options) {
+ const milliseconds default_one_thread_timeout(250);
+
+ // If the overall wait is forever, then we don't need to worry about it.
+ if (options.GetTimeoutUsec() == 0) {
+if (options.GetOneThreadTimeoutUsec() != 0)
+ return microseconds(options.GetOneThreadTimeoutUsec());
+return default_one_thread_timeout;
+ }
+
+ // If the one thread timeout is set, use it.
+ if (options.GetOneThreadTimeoutUsec() != 0)
+return microseconds(options.GetOneThreadTimeoutUsec());
+
+ // Otherwise use half the total timeout, bounded by the
+ // default_one_thread_timeout.
+ return std::min(default_one_thread_timeout,
+microseconds(options.GetTimeoutUsec()) / 2);
+}
+
+static Timeout
+GetExpressionTimeout(const EvaluateExpressionOptions &options,
+ bool before_first_timeout) {
+ // If we are going to run all threads the whole time, or if we are only
+ // going to run one thread, we can just return the overall timeout.
+ if (!options.GetStopOthers() || !options.GetTryAllThreads())
+return ConvertTimeout(microseconds(options.GetTimeoutUsec()));
+
+ if (before_first_timeout)
+return GetOneThreadExpressionTimeout(options);
+
+ if (options.GetTimeoutUsec() == 0)
+return llvm::None;
+ else
+return microseconds(options.GetTimeoutUsec()) -
+ GetOneThreadExpressionTimeout(options);
+}
+
ExpressionResults
Process::RunThreadPlan(ExecutionContext &exe_ctx,
lldb::ThreadPlanSP &thread_plan_sp,
@@ -4879,6 +4919,16 @@
}
}
+ // Make sure the timeout values make sense. The one thread timeout needs to be
+ // smaller than the overall timeout.
+ if (options.GetOneThreadTimeoutUsec() != 0 && options.GetTimeoutUsec() != 0 &&
+ options.GetTimeoutUsec() < options.GetOneThreadTimeoutUsec()) {
+diagnostic_manager.PutString(eDiagnosticSeverityError,
+ "RunThreadPlan called with one thread "
+ "timeout greater than total timeout");
+return eExpressionSetupError;
+ }
+
StackID ctx_frame_id = selected_frame_sp->GetStackID();
// N.B. Running the target may unset the currently selected thread and frame.
@@ -4985,67 +5035,20 @@
// that we have to halt the target.
bool do_resume = true;
bool handle_running_event = true;
-const uint64_t default_one_thread_timeout_usec = 25;
// This is just for accounting:
uint32_t num_resumes = 0;
-uint32_t timeout_usec = options.GetTimeoutUsec();
-uint32_t one_thread_timeout_usec;
-uint32_t all_threads_timeout_usec = 0;
-
// If we are going to run all threads the whole time, or if we are only
-// going to run one thread,
-// then we don't need the first timeout. So we set the final timeout, and
-// pretend we are after the
-// first timeout already.
-
-if (!options.GetStopOthers() || !options.GetTryAllThreads()) {
+// going to run one thread, then we don't need the first timeout. So we
+// pretend we are after the first timeout already.
+if (!options.GetStopOthers() || !options.GetTryAllThreads())
before_first_timeout = false;
- one_thread_timeout_usec = 0;
- all_threads_timeout_usec = timeout_usec;
-} else {
- uint32_t option_one_thread_timeout = options.GetOneThreadTimeoutUsec();
-
- // If the overall wait is forever, then we only need to set the one thread
- // timeout:
- if (timeout_usec == 0) {
-if (option_one_thread_timeout != 0)
- one_thread_timeout_usec = option_one_thread_timeout;
-else
- one_thread_timeout_usec = default_one_thread_timeout_usec;
- } else {
-// Otherwise, if the one thread timeout is set, make sure it isn't
-// longer than the overall timeout,
-// and use it, otherwise use half the total timeout, bounded by the
-// default_one_thread_timeout_usec.
-uint64_t computed_one_thread_timeout;
-if (option_one_thread_
[Lldb-commits] [PATCH] D27258: Use Timeout<> in Process::RunThreadPlan
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That looks correct to me as well. Thanks for taking the time to clean this up. Comment at: source/Target/Process.cpp:4926-4928 +diagnostic_manager.PutString(eDiagnosticSeverityError, + "RunThreadPlan called without one thread " + "timeout greater than total timeout"); without -> with The error was in the original. https://reviews.llvm.org/D27258 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27258: Use Timeout<> in Process::RunThreadPlan
labath created this revision.
labath added a reviewer: jingham.
labath added a subscriber: lldb-commits.
Since the function is way too big already, I tried at least to factor out the
timeout computation stuff into a separate function. I've tried to make the new
code semantically equivalent, and it also makes sense when I look at it as a
done
deal, but I could use another pair of eyes for this.
https://reviews.llvm.org/D27258
Files:
source/Target/Process.cpp
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -13,6 +13,7 @@
#include
// Other libraries and framework includes
+#include "llvm/Support/ScopedPrinter.h"
// Project includes
#include "Plugins/Process/Utility/InferiorCallPOSIX.h"
#include "lldb/Breakpoint/BreakpointLocation.h"
@@ -4799,6 +4800,45 @@
};
} // anonymous namespace
+static microseconds
+GetOneThreadExpressionTimeout(const EvaluateExpressionOptions &options) {
+ const milliseconds default_one_thread_timeout(250);
+
+ // If the overall wait is forever, then we don't need to worry about it.
+ if (options.GetTimeoutUsec() == 0) {
+if (options.GetOneThreadTimeoutUsec() != 0)
+ return microseconds(options.GetOneThreadTimeoutUsec());
+return default_one_thread_timeout;
+ }
+
+ // If the one thread timeout is set, use it.
+ if (options.GetOneThreadTimeoutUsec() != 0)
+return microseconds(options.GetOneThreadTimeoutUsec());
+
+ // Otherwise use half the total timeout, bounded by the
+ // default_one_thread_timeout.
+ return std::min(default_one_thread_timeout,
+microseconds(options.GetTimeoutUsec()) / 2);
+}
+
+static Timeout
+GetExpressionTimeout(const EvaluateExpressionOptions &options,
+ bool before_first_timeout) {
+ // If we are going to run all threads the whole time, or if we are only
+ // going to run one thread, we can just return the overall timeout.
+ if (!options.GetStopOthers() || !options.GetTryAllThreads())
+return ConvertTimeout(microseconds(options.GetTimeoutUsec()));
+
+ if (before_first_timeout)
+return GetOneThreadExpressionTimeout(options);
+
+ if (options.GetTimeoutUsec() == 0)
+return llvm::None;
+ else
+return microseconds(options.GetTimeoutUsec()) -
+ GetOneThreadExpressionTimeout(options);
+}
+
ExpressionResults
Process::RunThreadPlan(ExecutionContext &exe_ctx,
lldb::ThreadPlanSP &thread_plan_sp,
@@ -4879,6 +4919,16 @@
}
}
+ // Make sure the timeout values make sense. The one thread timeout needs to be
+ // smaller than the overall timeout.
+ if (options.GetOneThreadTimeoutUsec() != 0 && options.GetTimeoutUsec() != 0 &&
+ options.GetTimeoutUsec() < options.GetOneThreadTimeoutUsec()) {
+diagnostic_manager.PutString(eDiagnosticSeverityError,
+ "RunThreadPlan called without one thread "
+ "timeout greater than total timeout");
+return eExpressionSetupError;
+ }
+
StackID ctx_frame_id = selected_frame_sp->GetStackID();
// N.B. Running the target may unset the currently selected thread and frame.
@@ -4985,67 +5035,20 @@
// that we have to halt the target.
bool do_resume = true;
bool handle_running_event = true;
-const uint64_t default_one_thread_timeout_usec = 25;
// This is just for accounting:
uint32_t num_resumes = 0;
-uint32_t timeout_usec = options.GetTimeoutUsec();
-uint32_t one_thread_timeout_usec;
-uint32_t all_threads_timeout_usec = 0;
-
// If we are going to run all threads the whole time, or if we are only
-// going to run one thread,
-// then we don't need the first timeout. So we set the final timeout, and
-// pretend we are after the
-// first timeout already.
-
-if (!options.GetStopOthers() || !options.GetTryAllThreads()) {
+// going to run one thread, then we don't need the first timeout. So we
+// pretend we are after the first timeout already.
+if (!options.GetStopOthers() || !options.GetTryAllThreads())
before_first_timeout = false;
- one_thread_timeout_usec = 0;
- all_threads_timeout_usec = timeout_usec;
-} else {
- uint32_t option_one_thread_timeout = options.GetOneThreadTimeoutUsec();
-
- // If the overall wait is forever, then we only need to set the one thread
- // timeout:
- if (timeout_usec == 0) {
-if (option_one_thread_timeout != 0)
- one_thread_timeout_usec = option_one_thread_timeout;
-else
- one_thread_timeout_usec = default_one_thread_timeout_usec;
- } else {
-// Otherwise, if the one thread timeout is set, make sure it isn't
-// longer than the overall timeout,
-// and use it, otherwise use half the total timeout, bounded by the
-// default_one_thread_ti
