profiler does not build with musl libc

RESOLVED FIXED in Firefox 69

Status

()

defect
P2
normal
RESOLVED FIXED
2 years ago
Last month

People

(Reporter: mozilla, Assigned: anarchy)

Tracking

Trunk
mozilla69
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 affected, firefox54 ?, firefox55 ?, firefox69 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Posted patch fix_gettid_musl.patch (obsolete) — Splinter Review
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_3) AppleWebKit/600.5.17 (KHTML, like Gecko) Version/8.0.5 Safari/600.5.17
Build ID: 20170319182602

Steps to reproduce:

Building firefox with musl libc on a linux system.


Actual results:

gettid(2) is not defined.

```
/builddir/firefox-53.0/tools/profiler/core/platform-linux.cc: In static member function 'static Thread::tid_t Thread::GetCurrentId()':
/builddir/firefox-53.0/tools/profiler/core/platform-linux.cc:120:17: error: 'gettid' was not declared in this scope
   return gettid();
                 ^
/builddir/firefox-53.0/tools/profiler/core/platform-linux.cc: In function 'void* SignalSender(void*)':
/builddir/firefox-53.0/tools/profiler/core/platform-linux.cc:307:34: error: 'gettid' was not declared in this scope
   DebugOnly<int> my_tid = gettid();
                                  ^

/builddir/firefox-53.0/tools/profiler/lul/LulMain.cpp: In constructor 'lul::LUL::LUL(void (*)(const char*))':
/builddir/firefox-53.0/tools/profiler/lul/LulMain.cpp:841:27: error: 'gettid' was not declared in this scope
   , mAdminThreadId(gettid())
                           ^
/builddir/firefox-53.0/tools/profiler/lul/LulMain.cpp: In destructor 'lul::LUL::~LUL()':
/builddir/firefox-53.0/tools/profiler/lul/LulMain.cpp:833:37: error: 'gettid' was not declared in this scope
                    getpid(), gettid(), this, (_str));   \
                                     ^
/builddir/firefox-53.0/tools/profiler/lul/LulMain.cpp:852:3: note: in expansion of macro 'LUL_LOG'
   LUL_LOG("LUL::~LUL: Destroyed object");
   ^~~~~~~
/builddir/firefox-53.0/tools/profiler/lul/LulMain.cpp: In member function 'void lul::LUL::EnableUnwinding()':
/builddir/firefox-53.0/tools/profiler/lul/LulMain.cpp:833:37: error: 'gettid' was not declared in this scope
                    getpid(), gettid(), this, (_str));   \
                                     ^
/builddir/firefox-53.0/tools/profiler/lul/LulMain.cpp:888:3: note: in expansion of macro 'LUL_LOG'
   LUL_LOG("LUL::EnableUnwinding");
   ^~~~~~~
```


Expected results:

`gettid(2)` should be defined for all libcs as discussed in Bug 1261392.
Blocks: 1287392
Component: Untriaged → General
Product: Firefox → Core
Version: 53 Branch → Trunk
Flags: needinfo?(tlee)
Assignee: nobody → mozilla
I think you are requesting for a feedback against the patch.
You should not replace __GLIBC__ with __linux__ because, may, some libc
for Linux have defined gettid().
So, what you should do is to use
 #if defined(__GLIBC__) || defined(__MUSL__)
or some thing similar instead of
 #if defined(__GLIBC__)

Unfortunately, musl libc seems refusing to define __MUSL__ or alike.
So, you need to find a way to detect if it is using musl libc.
Flags: needinfo?(tlee)
Since no libc on linux except bionic has gettid, it seems easier to check whether we are not on Android:
#if defined(__linux__) || !defined(__BIONIC__)
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Please review so we can get this resolved once and for all.

Assignee: mozilla → anarchy
Attachment #8860125 - Attachment is obsolete: true
Attachment #9070133 - Flags: data-review?(jseward)
Attachment #9070133 - Flags: data-review?(jseward) → data-review?(n.nethercote)

The change seems fine, as long as it passes tests. However, a while back Mozilla switched to using Phabricator for submitting and reviewing patches. Details are here: https://wiki.mozilla.org/Phabricator. So the submission will have to go through there. Sorry for the inconvenience!

Attachment #9070133 - Flags: data-review?(n.nethercote)

gettid is not supported by any libc in linux so use the wrapper

Component: General → Gecko Profiler
Priority: -- → P2

BIONIC is only platform that actually supports gettid. Easiest
solution is to check for linux and disable for BIONIC platform. This
includes the change requested by Gerald to keep the two profilers sync'd.

Pushed by wkocher@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1e2f7d1f21ea
gettid wrapper is not provided by any libc in linux r=njn
Status: ASSIGNED → RESOLVED
Closed: Last month
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in before you can comment on or make changes to this bug.