Closed Bug 1261392 Opened 4 years ago Closed 4 years ago

gettid is not wrapped by most libc implementations on Linux

Categories

(Core :: Gecko Profiler, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: charles, Assigned: charles)

Details

Attachments

(1 file, 2 obsolete files)

Attached file platform-gettid.patch (obsolete) —
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.
Component: Untriaged → Gecko Profiler
Product: Firefox → Core
Reformat patch in line with Mozilla requirements.
Attachment #8737248 - Attachment is obsolete: true
Attachment #8739704 - Flags: review?(bgirard)
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+
Thanks. This works on my machine but please could you push it to the Try server for me?
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3b0f4de4300

Back to you. Once it's green please request landing.
Flags: needinfo?(charles)
Flags: needinfo?(charles)
Keywords: checkin-needed
Assignee: nobody → charles
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
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)
Attachment #8740283 - Flags: review?(bgirard) → review+
Please could you push it to the Try server again? Including Android this time :)
Flags: needinfo?(bgirard)
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)
Thank you
Flags: needinfo?(charles)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bc44cbe93310
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.