[Lldb-commits] [PATCH] D33426: Introduce new command: thread backtrace unique

Greg Clayton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue May 30 08:37:06 PDT 2017


clayborg added a comment.

Clicked submit too early with last comments. After seeing how we previously needed to create a temp UniqueStack just to do the lookup, and also how UniqueStack make its thread index ID list mutable, a better solution is to use a std::map as shown in the inlined comments.

One issue with this fix that we will need to fix is we will need to disable showing variable values in the stack backtrace. The default frame format is:

  (lldb) settings show frame-format
  frame-format (format-string) = "frame #${frame.index}: ${frame.pc}{ ${module.file.basename}{`${function.name-with-args}{${frame.no-debug}${function.pc-offset}}}}{ at ${line.file.basename}:${line.number}}{${function.is-optimized} [opt]}\n"

The main problem is we have "${function.name-with-args}" in the format string. We must detect we are trying to unique stacks and not try to print any thread variables otherwise we will not be showing the correct information to the user. This would mean piping the fact we are trying to unique stacks into the FormatEntity::Format() function in FormatEntity.h/.cpp and just don't show the variables in this case.



================
Comment at: source/Commands/CommandObjectThread.cpp:50
+
+  class UniqueStack {
+
----------------
After seeing how we have to create a temp UniqueStack so we can search for it in our set, we can do this a bit cleaner. We can get rid of this class if we use a std::map that maps from the unique stack load addresses to the thread ID list:

```
std::map<std::vector<lldb::addr_t>, std::vector<uint32_t>> unique_stacks;
```

So more changes below...


================
Comment at: source/Commands/CommandObjectThread.cpp:148
+      // Iterate over threads, finding unique stack buckets.
+      std::set<UniqueStack> unique_stacks;
+      for (const lldb::tid_t &tid : tids) {
----------------
Change to:
```
std::map<std::vector<lldb::addr_t>, std::vector<uint32_t>> unique_stacks;
```


================
Comment at: source/Commands/CommandObjectThread.cpp:159
+      strm.IndentMore();
+      for (const UniqueStack& stack: unique_stacks) {
+        // List the common thread ID's
----------------
change to:
```
for (const auto &stacks_tids: unique_stacks) {
```


================
Comment at: source/Commands/CommandObjectThread.cpp:161
+        // 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());
----------------
change to:
```
const std::vector<uint32_t>& thread_index_ids = stacks_tids.second;



================
Comment at: source/Commands/CommandObjectThread.cpp:169
+        // 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);
----------------
change to:

```
// Since all threads have the same call stack we can just pick the first one.
uint32_t representative_thread_id = thread_index_ids.front();
```


================
Comment at: source/Commands/CommandObjectThread.cpp:206
+  bool BucketThread(lldb::tid_t tid,
+                    std::set<UniqueStack>& unique_stacks,
+                    CommandReturnObject &result) {
----------------
```
std::map<std::vector<lldb::addr_t>, std::vector<uint32_t>> &unique_stacks
```


================
Comment at: source/Commands/CommandObjectThread.cpp:227
+    uint32_t thread_index_id = thread->GetIndexID();
+    UniqueStack new_unique_stack(stack_frames, thread_index_id);
+
----------------
Remove this.


================
Comment at: source/Commands/CommandObjectThread.cpp:230
+    // Try to match the threads stack to and existing entry.
+    std::set<UniqueStack>::iterator matching_stack = unique_stacks.find(new_unique_stack);
+    if (matching_stack != unique_stacks.end()) {
----------------
```
auto matching_stack = unique_stacks.find(stack_frames);
```


================
Comment at: source/Commands/CommandObjectThread.cpp:231-235
+    if (matching_stack != unique_stacks.end()) {
+        matching_stack->AddThread(thread_index_id);
+    } else { 
+      unique_stacks.insert(new_unique_stack);
+    }
----------------
Can probably replace the entire if/else with:
```
unique_stacks[stack_frames].push_back(thread_index_id);
```


https://reviews.llvm.org/D33426





More information about the lldb-commits mailing list