1226907 - Fix warnings in mozjemalloc and remove ALLOW_COMPILER_WARNINGS

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: fix-mozjemalloc-warnings.patch

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/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/usr/include/sys/cdefs.h:327:9: 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: 

https://hg.mozilla.org/mozilla-central/annotate/tip/memory/replace/logalloc/LogAlloc.cpp#l48


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

Comment 1

[Nicholas Nethercote [inactive]]

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()...

Comment 2

[Chris Peterson [:cpeterson]]

(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 moz.build change like:

  if CONFIG['GCC_CXX'] and not CONFIG['CLANG_CXX']:
    ALLOW_COMPILER_WARNINGS = True

[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdcc1b7dfd1e

Comment 3

[Nicholas Nethercote [inactive]]

If it's passing on try server that's good enough for me. Thanks.

Comment 4

[Chris Peterson [:cpeterson]]

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. :)

Comment 5

[Chris Peterson [:cpeterson]]

Attachment: part-2-use-unused-return-values.patch

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.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ab8e51f00ce
https://hg.mozilla.org/integration/mozilla-inbound/rev/dec5fee37b12

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5ab8e51f00ce
https://hg.mozilla.org/mozilla-central/rev/dec5fee37b12