Closed Bug 1226907 Opened 4 years ago Closed 4 years ago
Fix warnings in mozjemalloc and remove ALLOW
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
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 . 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  https://treeherder.mozilla.org/#/jobs?repo=try&revision=cdcc1b7dfd1e
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. :)
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+
You need to log in before you can comment on or make changes to this bug.