Last Comment Bug 750498 - Support IPC Thread::SetName on Linux
: Support IPC Thread::SetName on Linux
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: IPC (show other bugs)
: unspecified
: All Linux
: -- enhancement (vote)
: mozilla15
Assigned To: Nicolas Silva [:nical]
:
Mentors:
http://sourceware.org/bugzilla/show_b...
Depends on: 752067
Blocks: 720778
  Show dependency treegraph
 
Reported: 2012-04-30 14:37 PDT by Nicolas Silva [:nical]
Modified: 2012-08-20 11:06 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Applies upstream change to Thread::SetName for thread names to be displayable in gdb on Linux. (2.79 KB, patch)
2012-04-30 14:37 PDT, Nicolas Silva [:nical]
cjones.bugs: review+
Details | Diff | Review
SetName calls prctl on all linux platforms including mobile. (2.26 KB, patch)
2012-05-01 15:10 PDT, Nicolas Silva [:nical]
cjones.bugs: review+
bgirard: feedback+
bgirard: checkin+
Details | Diff | Review

Description Nicolas Silva [:nical] 2012-04-30 14:37:23 PDT
Created attachment 619697 [details] [diff] [review]
Applies upstream change to Thread::SetName for thread names to be displayable in gdb on Linux.

Similar to bug 734685 Which implemented the feature on Mac, for desktop Linux this time.
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-04-30 23:28:33 PDT
Comment on attachment 619697 [details] [diff] [review]
Applies upstream change to Thread::SetName for thread names to be displayable in gdb on Linux.

bionic supports prctl(PR_SET_NAME), and so should maemo.  No need to exclude them here.  Leaving the original condition as |#ifndef OS_MACOSX| should be fine.

r=me with that change.
Comment 2 Nicolas Silva [:nical] 2012-05-01 07:02:53 PDT
(In reply to Chris Jones [:cjones] [:warhammer] from comment #1)
> bionic supports prctl(PR_SET_NAME), and so should maemo.  No need to exclude
> them here.  Leaving the original condition as |#ifndef OS_MACOSX| should be
> fine.

I tried this already and it breaks the build on android and b2g platforms because prctl apparently doesn't have the same signature (it takes a unsigned long int instead of a c string). 
I don't know about maemo because I don't think try servers include maemo build and I haven't found a wiki page explaining how to build for maemo. so I did not want to risk to break it. I'll ask romaxa about the maemo stuff.
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-01 11:21:42 PDT
  prctl(PR_SET_NAME, reinterpret_cast<uintptr_t>(name), 0, 0, 0);

ought to work on both.
Comment 4 Benoit Girard (:BenWa) 2012-05-01 13:39:36 PDT
I tried this on ICS and the threads doesn't get renamed :(.
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-01 13:47:07 PDT
How did you test?
Comment 6 Benoit Girard (:BenWa) 2012-05-01 13:50:14 PDT
Your changes and gdb's 'info threads'
Comment 7 Nicolas Silva [:nical] 2012-05-01 15:10:42 PDT
Created attachment 620086 [details] [diff] [review]
SetName calls prctl on all linux platforms including mobile.

This is the version with prctl(PR_SET_NAME, reinterpret_cast<uintptr_t>(name), 0, 0, 0); on all Linux platforms.

May not actually show the thread names in gdb for android (from BenWa's comment) but it doesn't break on try server so it's better than the previous attachment anyway.
Comment 8 Benoit Girard (:BenWa) 2012-05-01 15:17:17 PDT
Comment on attachment 620086 [details] [diff] [review]
SetName calls prctl on all linux platforms including mobile.

It compiled and ran fine so its worth checking in. Maybe later we can dig into why it didn't work on android.
Comment 9 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-05-01 16:19:40 PDT
I wouldn't be the least bit surprised if the crack-baby android gdb doesn't read thread names.  If attaching to a process that had already spawned threads worked reliably, we could check and see if gdb reads these names when it first looks for threads.

Would be worth poking at in jimdb.
Comment 10 Benoit Girard (:BenWa) 2012-05-03 07:52:25 PDT
Jim, any idea why setting the thread name wouldn't show up in GDB?
Comment 12 Ed Morley [:emorley] 2012-05-04 09:27:09 PDT
https://hg.mozilla.org/mozilla-central/rev/7cd83841136c
Comment 13 Jim Chen [:jchen] [:darchons] 2012-08-18 20:32:11 PDT
(In reply to Benoit Girard (:BenWa) from comment #10)
> Jim, any idea why setting the thread name wouldn't show up in GDB?

Sorry for the late reply. I think the issue is that gdbserver does not support thread names. Even on Linux, I don't see thread names when I switch from gdb to gdbserver. Thanks!
Comment 14 Benoit Girard (:BenWa) 2012-08-18 21:04:32 PDT
Thanks, filled http://sourceware.org/bugzilla/show_bug.cgi?id=14495
Comment 15 Benoit Girard (:BenWa) 2012-08-20 11:06:55 PDT
(In reply to Benoit Girard (:BenWa) from comment #14)
> Thanks, filled http://sourceware.org/bugzilla/show_bug.cgi?id=14495

Got an update from this bug, this is in progress:
This is part of http://sourceware.org/gdb/wiki/LocalRemoteFeatureParity

Note You need to log in before you can comment on or make changes to this bug.