Closed
Bug 1261392
Opened 8 years ago
Closed 8 years ago
gettid is not wrapped by most libc implementations on Linux
Categories
(Core :: Gecko Profiler, defect)
Core
Gecko Profiler
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: charles, Assigned: charles)
Details
Attachments
(1 file, 2 obsolete files)
1.04 KB,
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Firefox/45.0 Build ID: 20160321181141 Steps to reproduce: Attempted to build Firefox against musl libc on Linux. Actual results: /.../mozilla-central/tools/profiler/core/platform-linux.cc: In static member function 'static Thread::tid_t Thread::GetCurrentId()': /.../mozilla-central/tools/profiler/core/platform-linux.cc:124:17: error: 'gettid' was not declared in this scope return gettid(); Expected results: The gettid function in tools/profiler/core/platform.h should have been defined. A pre-processor macro is used to conditionally define the function only when we are building against glibc. However, glibc is not the only libc implementation on Linux that does not wrap the gettid syscall. In fact, I don't know of any that do. I have checked: glibc eglibc bionic dietlibc klibc newlib musl uClibc and none of them provide gettid. The best solution would be to add a test for gettid to the configure script. However, since I don't know of any Linux libc implementations that wrap gettid, it might be acceptable just to test defined(__linux__) rather than defined(__GLIBC__). I've attached that as a patch.
Updated•8 years ago
|
Component: Untriaged → Gecko Profiler
Product: Firefox → Core
Assignee | ||
Comment 1•8 years ago
|
||
Reformat patch in line with Mozilla requirements.
Attachment #8737248 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8739704 -
Flags: review?(bgirard)
Comment 2•8 years ago
|
||
Comment on attachment 8739704 [details] [diff] [review] Define gettid for all Linux builds Review of attachment 8739704 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Thank you!
Attachment #8739704 -
Flags: review?(bgirard) → review+
Assignee | ||
Comment 3•8 years ago
|
||
Thanks. This works on my machine but please could you push it to the Try server for me?
Comment 5•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3b0f4de4300 Back to you. Once it's green please request landing.
Flags: needinfo?(charles)
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(charles)
Keywords: checkin-needed
Updated•8 years ago
|
Assignee: nobody → charles
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment 7•8 years ago
|
||
Backed out for Android bustage. https://hg.mozilla.org/integration/mozilla-inbound/rev/5192b8cce2d9 https://treeherder.mozilla.org/logviewer.html#?job_id=25481949&repo=mozilla-inbound
Assignee | ||
Comment 8•8 years ago
|
||
Sorry, I checked bionic and thought it didn't provide gettid. It's not in this file: https://android.googlesource.com/platform/bionic/+/master/libc/SYSCALLS.TXT However, I now see that it is in here: https://android.googlesource.com/platform/bionic/+/master/libc/include/unistd.h I've re-checked all the other Linux libc implementations and still don't think any others provide gettid - just bionic.
Attachment #8739704 -
Attachment is obsolete: true
Attachment #8740283 -
Flags: review?(bgirard)
Updated•8 years ago
|
Attachment #8740283 -
Flags: review?(bgirard) → review+
Assignee | ||
Comment 9•8 years ago
|
||
Please could you push it to the Try server again? Including Android this time :)
Flags: needinfo?(bgirard)
Comment 11•8 years ago
|
||
Opps, sorry for the delay. I started preparing the push and got pulled away. https://treeherder.mozilla.org/#/jobs?repo=try&revision=55fe96497ce4 ni? as a reminder to check the result.
Flags: needinfo?(bgirard) → needinfo?(charles)
Comment 13•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc44cbe93310
Keywords: checkin-needed
Comment 14•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bc44cbe93310
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•