[PATCH] D30526: [Support] Add functions to get and set thread name.

[Pavel Labath via Phabricator via llvm-commits]

labath requested changes to this revision.
labath added a comment.
This revision now requires changes to proceed.





================
Comment at: llvm/lib/Support/Threading.cpp:155
+#else
+void llvm::set_thread_name(StringRef Name) {
+  // Make sure the input is null terminated.
----------------
It might be overkill, but you could potentially save a copy if you use the `Twine.toNullTerminatedStringRef` trick


================
Comment at: llvm/lib/Support/Threading.cpp:248
+  raw_svector_ostream Stream(Path);
+  Stream << "/proc/" << static_cast<uint64_t>(::pthread_self()) << "/comm";
+
----------------
This is not correct. pthread_self returns a pthread_t, which bears no relation to the OS-level thread ID's. You'd need to use `syscall(SYS_gettid)` here. Also according to the docs <http://man7.org/linux/man-pages/man3/pthread_setname_np.3.html> it seems a better file to read would be `/proc/self/task/<tid>/comm`. As was as I know these files always have the same contents, but maybe the other one is deprecated (?)

However, the main issue here is that  the set and get code are wildly out of sync. There is a `pthread_getname_np` pair to the `pthread_setname_np` (see manpage above), so we should either use that here (guarded by the same ifdefs), or switch the setting code to write directly to the file as well (these functions are basically documented as utilities for reading/writing the file).

I'm sorry I am dumping this on you, but I think the llvm bar for this should be higher than in lldb. I can do this next week if you don't feel comfortable touching that.


https://reviews.llvm.org/D30526