Closed Bug 1226907 Opened 4 years ago Closed 4 years ago

Fix warnings in mozjemalloc and remove ALLOW_COMPILER_WARNINGS


(Core :: Memory Allocator, defect)

Not set



Tracking Status
firefox45 --- fixed


(Reporter: cpeterson, Assigned: cpeterson)


(Blocks 1 open bug)



(2 files)

1 clang warning on OS X:

> memory/mozjemalloc/jemalloc.c:438:9: error: '__DECONST' macro redefined [-Werror,-Wmacro-redefined]
> #define __DECONST(type, var) ((type)(uintptr_t)(const void *)(var))
> /Applications/ note: previous definition is here
> #define __DECONST(type, var)    __CAST_AWAY_QUALIFIER(var, const, type)

1 gcc warning on Android:

> memory/mozjemalloc/jemalloc.c:6047:2: error: implicit declaration of function 'pthread_atfork' [-Werror=implicit-function-declaration]
> pthread_atfork(_malloc_prefork, _malloc_postfork, _malloc_postfork);

I would have liked to limit the forward declaration of pthread_atfork() to #if ANDROID_VERSION < 21 but the ANDROID_VERSION macro is not #defined in mozglue. There is a precedent for forward declaring pthread_atfork() in LogAlloc.cpp to suppress another -Wimplicit-function-declaration warning:

Fix 1 MSVC warning on Win64. reg_size is 64-bit size_t but 1U is 32-bit unsigned int:

> Warning: C4334 in memory\mozjemalloc\jemalloc.c: '<<' : result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)

Suppress 1 MSVC warning (originally reported in bug 558163):

> memory/mozjemalloc/jemalloc.c(6782) : warning C4273: '_recalloc' : inconsistent dll linkage
> memory/mozjemalloc/jemalloc.c(6809) : warning C4273: '_expand' : inconsistent dll linkage
> memory/mozjemalloc/jemalloc.c(6818) : warning C4273: '_msize' : inconsistent dll linkage
Attachment #8690454 - Flags: review?(mh+mozilla)
Have you tried this on try server? When I looked at this directory on Linux I saw warnings about unchecked return values from _write() and strerror_r()...
(In reply to Nicholas Nethercote [:njn] from comment #1)
> Have you tried this on try server? When I looked at this directory on Linux
> I saw warnings about unchecked return values from _write() and
> strerror_r()...

Yes. The Linux try builders successfully built this change [1]. The problem you see is that some newer Linux distros have system headers with warn_unused_result annotations, e.g. bug 1113669 and bug 1119403.

I don't have a Linux machine. If you send me a list of the mozjemalloc warnings you see (mach warnings-list | grep mozjemalloc), I can fix or suppress them.

Or I could exclude gcc from my change like:


If it's passing on try server that's good enough for me. Thanks.
I think those warnings should be addressed somehow, either fixing them or allowing warnings for gcc. I don't want developers to have any excuse for building without --enable-warnings-as-errors locally just because they have newer distro version than the try servers. :)
Blocks: buildwarning
Attachment #8690454 - Flags: review?(mh+mozilla) → review+
Part 2: To breaking the builds of people who are building with newer glibc headers with the warn_unused_result annotations, check the return values from _write and strerror_r.
Attachment #8693150 - Flags: review?(n.nethercote)
Attachment #8693150 - Flags: review?(n.nethercote) → review+
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Duplicate of this bug: 502904
Duplicate of this bug: 942833
You need to log in before you can comment on or make changes to this bug.