Closed Bug 1130164 Opened 7 years ago Closed 6 years ago

build with musl libc: tools/profiler: unknown type name 'u_int64_t'

Categories

(Core :: Gecko Profiler, defect)

Other
Other
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: felix.janda, Assigned: felix.janda)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; U; Unix; en-US) AppleWebKit/537.15 (KHTML, like Gecko) Chrome/24.0.1295.0 Safari/537.15 Surf/0.6

Steps to reproduce:

Try to build firefox on a musl libc based system. (http://musl-libc.org)


Actual results:

Compile failures in tools/profiler/UnwinderThread2.cpp and tools/profiler/local_debug_info_symbolizer.cc because of missing u_int64_t


Expected results:

Successful compilation
Attached patch Proposed fixSplinter Review
Component: Untriaged → Gecko Profiler
Product: Firefox → Core
Attachment #8560087 - Flags: review?(bgirard)
Comment on attachment 8560087 [details] [diff] [review]
Proposed fix

This should be defined in the breakpad headers. It would be better to find out why your environment isn't including the proper file instead of changing this.

We could also take this patch to be pragmatic. I'll leave it up to Julian since it's his code.
Attachment #8560087 - Flags: review?(bgirard) → review?(jseward)
Grepping u_int64_t on the tree (with the patch applied) gives (only):

./toolkit/crashreporter/google-breakpad/src/tools/mac/crash_report/crash_report.mm:      u_int64_t instruction = frame->instruction;
./toolkit/crashreporter/google-breakpad/src/tools/mac/crash_report/crash_report.mm:  u_int64_t main_address = 0;
./toolkit/crashreporter/google-breakpad/src/tools/mac/crash_report/crash_report.mm:    u_int64_t base_address = module->base_address();
./toolkit/crashreporter/google-breakpad/src/third_party/linux/include/gflags/gflags.h:typedef u_int64_t uint64;
./toolkit/crashreporter/google-breakpad/src/third_party/glog/src/windows/glog/logging.h:typedef u_int64_t uint64;
./toolkit/crashreporter/google-breakpad/src/third_party/glog/src/glog/logging.h.in:typedef u_int64_t uint64;
./dom/plugins/base/nptypes.h:  typedef u_int64_t uint64_t;
./netwerk/sctp/src/user_uma.h:  u_int64_t       uz_allocs;      /* Total number of allocations */
./netwerk/sctp/src/user_uma.h:  u_int64_t       uz_frees;       /* Total number of frees */
./netwerk/sctp/src/user_uma.h:  u_int64_t       uz_fails;       /* Total number of alloc failures */


I don't understand how the typedefs in glog/logging.h or gflags/gflags.h would get included.


Anyway, musl has u_int64_t in <sys/types.h>. I guess that <sys/types.h> on other systems is included implicitly from other headers (e.g. <stdio.h>) and that is why it is working for them.


As I understand mozilla is the upstream for tools/profiler, so I don't see what could be wrong with consistently using <stdint.h> types. (mozilla-central/tools/profiler/UnwinderThread2.cpp uses uint64_t also without the patch, e.g. in the definition of AMD64Regs)
Comment on attachment 8560087 [details] [diff] [review]
Proposed fix

Review of attachment 8560087 [details] [diff] [review]:
-----------------------------------------------------------------

This seems fine to me.  I don't see that moving to be more stdint.h
compliant is a bad thing.
Attachment #8560087 - Flags: review?(jseward) → review+
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → felix.janda
Keywords: checkin-needed
can we get a try run for this change ? thanks!
Flags: needinfo?(felix.janda)
Keywords: checkin-needed
I don't have commit access to the try trees.
Should I request access or can someone submit the job for me? Thanks!
Flags: needinfo?(felix.janda)
Maybe Julian can help with a try run
Flags: needinfo?(jseward)
Looks pretty good to me.
https://hg.mozilla.org/mozilla-central/rev/a446b4e60744
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.