[PATCH] D31474: Update suspended threads info to be compatible with darwin

[Francis Ricci via Phabricator via llvm-commits]

fjricci added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_stoptheworld_mac.cc:32-40
+  thread_identifier_info_data_t info;
+  mach_msg_type_number_t info_count = THREAD_IDENTIFIER_INFO_COUNT;
+  kern_return_t err = thread_info(thread, THREAD_IDENTIFIER_INFO,
+                                  (thread_info_t)&info, &info_count);
+  if (err != KERN_SUCCESS) {
+    VReport(1, "Error - unable to get thread ident for a thread\n");
+    return 0;
----------------
kubamracek wrote:
> fjricci wrote:
> > kubamracek wrote:
> > > Does this return the same value as `pthread_threadid_np`?  Can we use it instead?  Or even better, use `GetTid` from sanitizer_mac.cc?  Calling `thread_info` here sounds very expensive.
> > > 
> > > Why not store the thread_id's in the array directly?  If we really need to know both `thread_t` and `thread_id`, then we should have a struct that contains both and store the struct in the array.
> > I think it's reasonable to use a struct instead. There is a fair amount of existing code that requires the `thread_id`, but the code for accessing register info will require the `thread_t`, so there's definitely a need for both, especially if it's expensive to convert.
> I don't think it's expensive to convert, but we should make sure to only do the conversion once per thread.
Does that go for `pthread_threadid_np` as well? I don't think there's a good way to cache the values and ensure it's only called once, other than using a struct to store both.


https://reviews.llvm.org/D31474