1375467 - mozalloc_abort.cpp - redefining abort() in C++ requires extern "C"

Status: RESOLVED FIXED

(Core :: Memory Allocator, defect)

Description

[Petr Sumbera]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170608105825

Steps to reproduce:

While building Firefox on Solaris:

gmake[1]: Entering directory '/var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc'
/usr/bin/g++ -std=gnu++11 -o Unified_cpp_memory_mozalloc0.o -c   -DNDEBUG=1 -DTRIMMED=1 -D_GNU_SOURCE -DIMPL_MFBT -DMOZ_HAS_MOZGLUE -I/var/tmp/firefox/memory/mozalloc -I/var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc -I/var/tmp/firefox/obj-x86_64-pc-solaris2.12/xpcom -I/var/tmp/firefox/memory/build -I/var/tmp/firefox/obj-x86_64-pc-solaris2.12/dist/include  -I/var/tmp/firefox/obj-x86_64-pc-solaris2.12/dist/include/nspr -I/var/tmp/firefox/obj-x86_64-pc-solaris2.12/dist/include/nss       -fPIC  -DMOZILLA_CLIENT -include /var/tmp/firefox/obj-x86_64-pc-solaris2.12/mozilla-config.h -MD -MP -MF .deps/Unified_cpp_memory_mozalloc0.o.pp  -Wall -Wc++11-compat -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wc++14-compat -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe  -g -O -fno-omit-frame-pointer     /var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc/Unified_cpp_memory_mozalloc0.cpp
In file included from /var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc/Unified_cpp_memory_mozalloc0.cpp:11:0:
/var/tmp/firefox/memory/mozalloc/mozalloc_abort.cpp: In function 'void abort()':
/var/tmp/firefox/memory/mozalloc/mozalloc_abort.cpp:71:16: error: 'void abort()' conflicts with a previous declaration
 void abort(void)
                ^
In file included from /usr/include/stdlib.h:12:0,
                 from /var/tmp/firefox/memory/mozalloc/mozalloc.cpp:45,
                 from /var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc/Unified_cpp_memory_mozalloc0.cpp:2:
/usr/include/iso/stdlib_iso.h:158:13: note: previous declaration 'void std::abort()'
 extern void abort(void) __NORETURN;
             ^
In file included from /var/tmp/firefox/memory/mozalloc/mozalloc_abort.cpp:19:0,
                 from /var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc/Unified_cpp_memory_mozalloc0.cpp:11:
/var/tmp/firefox/obj-x86_64-pc-solaris2.12/dist/include/mozilla/Assertions.h:227:18: error: call of overloaded 'abort()' is ambiguous
          ::abort(); \
                  ^
/var/tmp/firefox/obj-x86_64-pc-solaris2.12/dist/include/mozilla/Assertions.h:263:8: note: in expansion of macro 'MOZ_REALLY_CRASH'
        MOZ_REALLY_CRASH(__LINE__); \
        ^
/var/tmp/firefox/memory/mozalloc/mozalloc_abort.cpp:86:5: note: in expansion of macro 'MOZ_CRASH'
     MOZ_CRASH();
     ^
In file included from /usr/include/stdlib.h:12:0,
                 from /var/tmp/firefox/memory/mozalloc/mozalloc.cpp:45,
                 from /var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc/Unified_cpp_memory_mozalloc0.cpp:2:
/usr/include/iso/stdlib_iso.h:158:13: note: candidate: void std::abort()
 extern void abort(void) __NORETURN;
             ^
In file included from /var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc/Unified_cpp_memory_mozalloc0.cpp:11:0:
/var/tmp/firefox/memory/mozalloc/mozalloc_abort.cpp:71:6: note: candidate: void abort()
 void abort(void)
      ^
gmake[1]: *** [/var/tmp/firefox/config/rules.mk:1061: Unified_cpp_memory_mozalloc0.o] Error 1
gmake[1]: Leaving directory '/var/tmp/firefox/obj-x86_64-pc-solaris2.12/memory/mozalloc'



Actual results:

Here is simple test:

$ cat test.c
#include <stdio.h>
#include <cstdlib>

void abort () {}

$ g++ -c test.c
test.c: In function 'void abort()':
test.c:4:13: error: 'void abort()' conflicts with a previous declaration
 void abort () {}
             ^
In file included from /usr/include/stdlib.h:12:0,
                 from /usr/gcc/5/include/c++/5.4.0/cstdlib:72,
                 from test.c:2:
/usr/include/iso/stdlib_iso.h:158:13: note: previous declaration 'void std::abort()'
 extern void abort(void) __NORETURN;
             ^ 

Where this builds just fine on Linux.


Expected results:

Solaris headers import libc functions into C++ std namespace differently than how it happens on Linux. Both approaches are valid according to the langauge standards, but they can result in different implementation-specific behavior in different compilers. And the two approaches can uncover different bugs.

Linux:

extern void abort(void);
namespace std {
  using::abort;
}

Solaris:

namespace std {
   extern "C" void abort(void);   
}
using std::abort;

Redefining a system function after including the header file that declares it is not standard comforming code. Since this is C++ code and the intention is to define a C-ABI function, the correct way to write this code is to add extern "C" to the start of the last line:

   extern "C" void abort () {}

Comment 1

[Petr Sumbera]

Attachment: Bug1375467.patch

Comment 2

[Alon Zakai (:azakai)]

Comment on attachment 8880370 [details] [diff] [review]
Bug1375467.patch

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

This looks right to me, but I'm not a peer of this code and haven't done Gecko development for a long time, so I might be missing something.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15b99f4d26f6
Redefining abort() in C++ requires extern "C". r=glandium

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/15b99f4d26f6