bgianfo updated this revision to Diff 100047.
bgianfo marked an inline comment as done.
bgianfo added a comment.
Address Pavel/Jim/Greg's feedback with the addition of a new test.
I followed Jim's advice and extended the existing num_threads suite
so that we start more thread3's. The new test_unique_stacks test case
was added to verify that we correct bucketing of the multiple thread3
call stacks executing across threads.
Thanks for the feedback thus far!
https://reviews.llvm.org/D33426
Files:
include/lldb/Target/Thread.h
packages/Python/lldbsuite/test/functionalities/thread/num_threads/TestNumThreads.py
packages/Python/lldbsuite/test/functionalities/thread/num_threads/main.cpp
source/Commands/CommandObjectThread.cpp
source/Core/Debugger.cpp
source/Target/Thread.cpp
Index: source/Target/Thread.cpp
===================================================================
--- source/Target/Thread.cpp
+++ source/Target/Thread.cpp
@@ -1912,39 +1912,42 @@
size_t Thread::GetStatus(Stream &strm, uint32_t start_frame,
uint32_t num_frames, uint32_t num_frames_with_source,
- bool stop_format) {
- ExecutionContext exe_ctx(shared_from_this());
- Target *target = exe_ctx.GetTargetPtr();
- Process *process = exe_ctx.GetProcessPtr();
- size_t num_frames_shown = 0;
- strm.Indent();
- bool is_selected = false;
- if (process) {
- if (process->GetThreadList().GetSelectedThread().get() == this)
- is_selected = true;
- }
- strm.Printf("%c ", is_selected ? '*' : ' ');
- if (target && target->GetDebugger().GetUseExternalEditor()) {
- StackFrameSP frame_sp = GetStackFrameAtIndex(start_frame);
- if (frame_sp) {
- SymbolContext frame_sc(
- frame_sp->GetSymbolContext(eSymbolContextLineEntry));
- if (frame_sc.line_entry.line != 0 && frame_sc.line_entry.file) {
- Host::OpenFileInExternalEditor(frame_sc.line_entry.file,
- frame_sc.line_entry.line);
+ bool stop_format, bool only_stacks) {
+
+ if (!only_stacks) {
+ ExecutionContext exe_ctx(shared_from_this());
+ Target *target = exe_ctx.GetTargetPtr();
+ Process *process = exe_ctx.GetProcessPtr();
+ strm.Indent();
+ bool is_selected = false;
+ if (process) {
+ if (process->GetThreadList().GetSelectedThread().get() == this)
+ is_selected = true;
+ }
+ strm.Printf("%c ", is_selected ? '*' : ' ');
+ if (target && target->GetDebugger().GetUseExternalEditor()) {
+ StackFrameSP frame_sp = GetStackFrameAtIndex(start_frame);
+ if (frame_sp) {
+ SymbolContext frame_sc(
+ frame_sp->GetSymbolContext(eSymbolContextLineEntry));
+ if (frame_sc.line_entry.line != 0 && frame_sc.line_entry.file) {
+ Host::OpenFileInExternalEditor(frame_sc.line_entry.file,
+ frame_sc.line_entry.line);
+ }
}
}
- }
- DumpUsingSettingsFormat(strm, start_frame, stop_format);
+ DumpUsingSettingsFormat(strm, start_frame, stop_format);
+ }
+ size_t num_frames_shown = 0;
if (num_frames > 0) {
strm.IndentMore();
const bool show_frame_info = true;
const char *selected_frame_marker = nullptr;
- if (num_frames == 1 ||
+ if (num_frames == 1 || only_stacks ||
(GetID() != GetProcess()->GetThreadList().GetSelectedThread()->GetID()))
strm.IndentMore();
else
Index: source/Core/Debugger.cpp
===================================================================
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -236,7 +236,7 @@
"when displaying thread information."},
{"thread-stop-format", OptionValue::eTypeFormatEntity, true, 0,
DEFAULT_THREAD_STOP_FORMAT, nullptr, "The default thread format "
- "string to usewhen displaying thread "
+ "string to use when displaying thread "
"information as part of the stop display."},
{"use-external-editor", OptionValue::eTypeBoolean, true, false, nullptr,
nullptr, "Whether to use an external editor or not."},
Index: source/Commands/CommandObjectThread.cpp
===================================================================
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -42,10 +42,40 @@
using namespace lldb_private;
//-------------------------------------------------------------------------
-// CommandObjectThreadBacktrace
+// CommandObjectIterateOverThreads
//-------------------------------------------------------------------------
class CommandObjectIterateOverThreads : public CommandObjectParsed {
+
+ class UniqueStack {
+
+ public:
+ UniqueStack(std::stack<Address> stack_frames, uint32_t thread_index_id)
+ : m_stack_frames(stack_frames) {
+ m_thread_index_ids.push_back(thread_index_id);
+ }
+
+ void AddThread(uint32_t thread_index_id) {
+ m_thread_index_ids.push_back(thread_index_id);
+ }
+
+ const std::vector<uint32_t>& GetUniqueThreadIndexIDs() const {
+ return m_thread_index_ids;
+ }
+
+ lldb::tid_t GetRepresentativeThread() const {
+ return m_thread_index_ids.front();
+ }
+
+ bool IsEqual(std::stack<Address> stack_frames) const {
+ return stack_frames == m_stack_frames;
+ }
+
+ protected:
+ std::vector<uint32_t> m_thread_index_ids;
+ std::stack<Address> m_stack_frames;
+ };
+
public:
CommandObjectIterateOverThreads(CommandInterpreter &interpreter,
const char *name, const char *help,
@@ -57,20 +87,23 @@
bool DoExecute(Args &command, CommandReturnObject &result) override {
result.SetStatus(m_success_return);
+ bool all_threads = false;
if (command.GetArgumentCount() == 0) {
Thread *thread = m_exe_ctx.GetThreadPtr();
if (!HandleOneThread(thread->GetID(), result))
return false;
return result.Succeeded();
+ } else if (command.GetArgumentCount() == 1) {
+ all_threads = ::strcmp(command.GetArgumentAtIndex(0), "all") == 0;
+ m_unique_stacks = ::strcmp(command.GetArgumentAtIndex(0), "unique") == 0;
}
// Use tids instead of ThreadSPs to prevent deadlocking problems which
// result from JIT-ing
// code while iterating over the (locked) ThreadSP list.
std::vector<lldb::tid_t> tids;
- if (command.GetArgumentCount() == 1 &&
- ::strcmp(command.GetArgumentAtIndex(0), "all") == 0) {
+ if (all_threads || m_unique_stacks) {
Process *process = m_exe_ctx.GetProcessPtr();
for (ThreadSP thread_sp : process->Threads())
@@ -108,20 +141,53 @@
}
}
- uint32_t idx = 0;
- for (const lldb::tid_t &tid : tids) {
- if (idx != 0 && m_add_return)
- result.AppendMessage("");
-
- if (!HandleOneThread(tid, result))
- return false;
+ if (m_unique_stacks) {
+ // Iterate over threads, finding unique stack buckets.
+ std::vector<UniqueStack> unique_stacks;
+ for (const lldb::tid_t &tid : tids) {
+ if (!BucketThread(tid, unique_stacks, result)) {
+ return false;
+ }
+ }
- ++idx;
+ // Write the thread id's and unique call stacks to the output stream
+ Stream &strm = result.GetOutputStream();
+ Process *process = m_exe_ctx.GetProcessPtr();
+ strm.IndentMore();
+ for (const UniqueStack& stack: unique_stacks) {
+ // List the common thread ID's
+ const std::vector<uint32_t>& thread_index_ids = stack.GetUniqueThreadIndexIDs();
+ strm.Printf("%lu thread(s) ", thread_index_ids.size());
+ for (const uint32_t &thread_index_id : thread_index_ids) {
+ strm.Printf("#%u ", thread_index_id);
+ }
+ strm.EOL();
+
+ // List the shared call stack for this set of threads
+ uint32_t representative_thread_id = stack.GetRepresentativeThread();
+ ThreadSP thread = process->GetThreadList().FindThreadByIndexID(representative_thread_id);
+ if (!HandleOneThread(thread->GetID(), result)) {
+ return false;
+ }
+ }
+ strm.IndentLess();
+ } else {
+ uint32_t idx = 0;
+ for (const lldb::tid_t &tid : tids) {
+ if (idx != 0 && m_add_return)
+ result.AppendMessage("");
+
+ if (!HandleOneThread(tid, result))
+ return false;
+
+ ++idx;
+ }
}
return result.Succeeded();
}
protected:
+
// Override this to do whatever you need to do for one thread.
//
// If you return false, the iteration will stop, otherwise it will proceed.
@@ -134,7 +200,47 @@
virtual bool HandleOneThread(lldb::tid_t, CommandReturnObject &result) = 0;
+ bool BucketThread(lldb::tid_t tid,
+ std::vector<UniqueStack>& unique_stacks,
+ CommandReturnObject &result) {
+ // Grab the corresponding thread for the given thread id.
+ Process *process = m_exe_ctx.GetProcessPtr();
+ Thread* thread = process->GetThreadList().FindThreadByID(tid).get();
+ if (thread == nullptr) {
+ result.AppendErrorWithFormat("Failed to process thread# %lu.\n", tid);
+ result.SetStatus(eReturnStatusFailed);
+ return false;
+ }
+
+ // Collect the each frame's address for this call-stack
+ std::stack<Address> stack_frames;
+ const uint32_t frame_count = thread->GetStackFrameCount();
+ for (uint32_t frame_index = 0; frame_index < frame_count; frame_index++) {
+ const lldb::StackFrameSP frame_sp = thread->GetStackFrameAtIndex(frame_index);
+ const Address frame_address = frame_sp->GetFrameCodeAddress();
+ stack_frames.push(frame_address);
+ }
+
+ // Try to match the threads stack to and existing entry.
+ bool found_match = false;
+ for (UniqueStack& unique_stack : unique_stacks) {
+ if (unique_stack.IsEqual(stack_frames)) {
+ found_match = true;
+ unique_stack.AddThread(thread->GetIndexID());
+ break;
+ }
+ }
+
+ // We failed to find an existing unique stack, so create a new one.
+ if (!found_match) {
+ UniqueStack new_unique_stack(stack_frames, thread->GetIndexID());
+ unique_stacks.push_back(new_unique_stack);
+ }
+ return true;
+ }
+
ReturnStatus m_success_return = eReturnStatusSuccessFinishResult;
+ bool m_unique_stacks = false;
bool m_add_return = true;
};
@@ -218,9 +324,9 @@
: CommandObjectIterateOverThreads(
interpreter, "thread backtrace",
"Show thread call stacks. Defaults to the current thread, thread "
- "indexes can be specified as arguments. Use the thread-index "
- "\"all\" "
- "to see all threads.",
+ "indexes can be specified as arguments.\n"
+ "Use the thread-index \"all\" to see all threads.\n"
+ "Use the thread-index \"unique\" to see threads grouped by unique call stacks.",
nullptr,
eCommandRequiresProcess | eCommandRequiresThread |
eCommandTryTargetAPILock | eCommandProcessMustBeLaunched |
@@ -270,11 +376,14 @@
Stream &strm = result.GetOutputStream();
+ // Only dump stack info if we processing unique stacks.
+ const bool only_stacks = m_unique_stacks;
+
// Don't show source context when doing backtraces.
const uint32_t num_frames_with_source = 0;
const bool stop_format = true;
if (!thread->GetStatus(strm, m_options.m_start, m_options.m_count,
- num_frames_with_source, stop_format)) {
+ num_frames_with_source, stop_format, only_stacks)) {
result.AppendErrorWithFormat(
"error displaying backtrace for thread: \"0x%4.4x\"\n",
thread->GetIndexID());
Index: packages/Python/lldbsuite/test/functionalities/thread/num_threads/main.cpp
===================================================================
--- packages/Python/lldbsuite/test/functionalities/thread/num_threads/main.cpp
+++ packages/Python/lldbsuite/test/functionalities/thread/num_threads/main.cpp
@@ -1,15 +1,17 @@
+#include <algorithm>
#include <condition_variable>
#include <mutex>
#include <thread>
+#include <vector>
std::mutex mutex;
std::condition_variable cond;
void *
thread3(void *input)
{
std::unique_lock<std::mutex> lock(mutex);
- cond.notify_all(); // Set break point at this line.
+ cond.notify_all(); // Set number_of_threads break point at this line.
return NULL;
}
@@ -38,13 +40,18 @@
std::thread thread_1(thread1, nullptr);
cond.wait(lock);
- std::thread thread_3(thread3, nullptr);
- cond.wait(lock);
+ std::vector<std::thread> thread_3s;
+ for (int i = 0; i < 10; i++) {
+ thread_3s.push_back(std::thread(thread3, nullptr));
+ }
+ cond.wait(lock); // Set unique_stacks break point at this line.
lock.unlock();
thread_1.join();
- thread_3.join();
+ std::for_each(thread_3s.begin(),
+ thread_3s.end(),
+ std::mem_fn(&std::thread::join));
return 0;
}
Index: packages/Python/lldbsuite/test/functionalities/thread/num_threads/TestNumThreads.py
===================================================================
--- packages/Python/lldbsuite/test/functionalities/thread/num_threads/TestNumThreads.py
+++ packages/Python/lldbsuite/test/functionalities/thread/num_threads/TestNumThreads.py
@@ -20,25 +20,26 @@
# Call super's setUp().
TestBase.setUp(self)
# Find the line number to break inside main().
- self.line = line_number('main.cpp', '// Set break point at this line.')
+ self.number_of_threads_line = line_number('main.cpp', '// Set number_of_threads break point at this line.')
+ self.unique_stacks_line = line_number('main.cpp', '// Set unique_stacks break point at this line.')
- def test(self):
+ def test_number_of_threads(self):
"""Test number of threads."""
self.build()
exe = os.path.join(os.getcwd(), "a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
# This should create a breakpoint with 1 location.
lldbutil.run_break_set_by_file_and_line(
- self, "main.cpp", self.line, num_expected_locations=1)
+ self, "main.cpp", self.number_of_threads_line, num_expected_locations=1)
- # The breakpoint list should show 3 locations.
+ # The breakpoint list should show 1 location.
self.expect(
"breakpoint list -f",
"Breakpoint location shown correctly",
substrs=[
"1: file = 'main.cpp', line = %d, exact_match = 0, locations = 1" %
- self.line])
+ self.number_of_threads_line])
# Run the program.
self.runCmd("run", RUN_SUCCEEDED)
@@ -59,3 +60,34 @@
self.assertTrue(
num_threads >= 4,
'Number of expected threads and actual threads do not match.')
+
+ def test_unique_stacks(self):
+ """Test backtrace unique with multiple threads executing the same stack."""
+ self.build()
+ exe = os.path.join(os.getcwd(), "a.out")
+ self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+ # This should create a breakpoint with 1 location.
+ lldbutil.run_break_set_by_file_and_line(
+ self, "main.cpp", self.unique_stacks_line, num_expected_locations=1)
+
+ # The breakpoint list should show 1 location.
+ self.expect(
+ "breakpoint list -f",
+ "Breakpoint location shown correctly",
+ substrs=[
+ "1: file = 'main.cpp', line = %d, exact_match = 0, locations = 1" %
+ self.unique_stacks_line])
+
+ # Run the program.
+ self.runCmd("run", RUN_SUCCEEDED)
+
+ # Stopped once.
+ self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+ substrs=["stop reason = breakpoint 1."])
+
+ # Now that we are stopped, we should have 10 threads waiting in the
+ # thread3 function. All of these threads should show as one stack.
+ self.expect("thread backtrace unique",
+ "Backtrace with unique stack shown correctly",
+ substrs=["10 thread(s) #4 #5 #6 #7 #8 #9 #10 #11 #12 #13"])
Index: include/lldb/Target/Thread.h
===================================================================
--- include/lldb/Target/Thread.h
+++ include/lldb/Target/Thread.h
@@ -1164,7 +1164,7 @@
size_t GetStatus(Stream &strm, uint32_t start_frame, uint32_t num_frames,
uint32_t num_frames_with_source,
- bool stop_format);
+ bool stop_format, bool only_stacks = false);
size_t GetStackFrameStatus(Stream &strm, uint32_t first_frame,
uint32_t num_frames, bool show_frame_info,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
