[Lldb-commits] [PATCH] D158785: [lldb] Add a "thread extrainfo" LC_NOTE for Mach-O corefiles, to store the thread IDs of the threads

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda marked 6 inline comments as done.
jasonmolenda added a comment.

Thanks for the feedback Alex & Jonas.  Jonas' comment that we have a lot of different methods manually stepping over load commands to find their LC_NOTEs was something I'd been meaning to deal with some day, but hadn't done yet.  I shouldn't complicate this patch by also doing that but ... I did.  Updating the patch in a minute.



================
Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:608-619
+        // If any threads have an unspecified thread id,
+        // find an unused number, use it instead.
+        tid_t current_unused_tid = 0;
+        for (uint32_t i = 0; i < num_threads; i++) {
+          if (tids[i] == LLDB_INVALID_THREAD_ID) {
+            while (used_tids.find(current_unused_tid) != used_tids.end()) {
+              current_unused_tid++;
----------------
JDevlieghere wrote:
> This code is (presumably on purpose) trying to find gaps in the available tids: for example if I had `[1, 2, ? ,4, ?, 6]` this is going to turn that into `[1, 2, (3), 4, (5), 6]`. If that property matters, it probably should be part of the comment. If it doesn't, you could exploit the fact that the `set` is sorted and just take the last value (6 for the previous example) and return `[1, 2, 4, 6, 7, 8]`, but again, I assume that's what you're trying to avoid here. 
I wasn't trying to maintain the order of the created tids, you're right I shouldn't bother being so fancy, just grab the highest tid number seen and increment past it.  I create the set by iterating over the vector, I'll just watch for the highest tid_t value in that loop instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158785/new/

https://reviews.llvm.org/D158785