1389965 - Compiling with mingw-w64 based on GCC 6.4.0 fails with std::moz_xcalloc being not declared

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P3)

Description

[Georg Koppen]

While testing whether Tor Browser (which is based on ESR 52) can be compiled with GCC 6 I ran into the following errors:

/home/ubuntu/build/tor-browser/gfx/graphite2/src/MozGrMalloc.h:16:16: error: 'std::moz_xcalloc' has not been declared
 #define calloc moz_xcalloc
                ^
/home/ubuntu/build/tor-browser/gfx/graphite2/src/MozGrMalloc.h:15:16: error: 'std::moz_xmalloc' has not been declared
 #define malloc moz_xmalloc
                ^
/home/ubuntu/build/tor-browser/gfx/graphite2/src/MozGrMalloc.h:17:17: error: 'std::moz_xrealloc' has not been declared
 #define realloc moz_xrealloc
                 ^
make[5]: *** [NameTable.o] Error 1
make[5]: Leaving directory `/home/ubuntu/build/tor-browser/obj-mingw/gfx/graphite2/src'
make[4]: *** [gfx/graphite2/src/target] Error 2

Comment 1

[Georg Koppen]

Jacek, IIRC you had a patch for this issue. Do you feel it is mature enough to fix this problem and get merged? If so, could you attach it and get someone to review it? Thanks.

Comment 2

[Jacek Caban]

Attachment: patch.diff

Sure, I just used inline wrappers instead of macros to forward calls. Unfortunately, my mingw build is broken with current m-c, so I can't test the patch.

Comment 3

[Georg Koppen]

It fixes the compilation for me at least when building on an ESR 52 base.

Comment 4

[Bas Schouten (:bas.schouten)]

I suppose this depends on if we want to stray from upstream in graphite.

Comment 5

[Jonathan Kew [:jfkthame]]

The MozGrMalloc.h file doesn't come from upstream, it's a local Gecko addition. So we're free to modify it if that will help portability, or whatever...

Comment 6

[Tom Ritter [:tjr]]

Attachment: Bug 1389965 Redefine macros to inline functions to fix MinGW build

Review commit: https://reviewboard.mozilla.org/r/185146/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/185146/

Comment 7

[Tom Ritter [:tjr]]

Jacek's patch does resolve this, but after looking at the code I'm not quite sure why.  My best guess is that is has something to do with the std:: namespace that MinGW is saying, but that's mostly a guess.

Comment 8

[Jonathan Kew [:jfkthame]]

Comment on attachment 8913755 [details]
Bug 1389965 Redefine macros to inline functions to fix MinGW build

https://reviewboard.mozilla.org/r/185146/#review190302

::: gfx/graphite2/src/MozGrMalloc.h:15
(Diff revision 1)
>  // predictable, safe OOM crashes rather than relying on the code to handle
>  // allocation failures reliably.
>  
>  #include "mozilla/mozalloc.h"
>  
> -#define malloc moz_xmalloc
> +inline void *malloc(size_t size)

nit: the `*` should be attached to `void` rather than to the function name, please (here and below).

::: gfx/graphite2/src/MozGrMalloc.h:20
(Diff revision 1)
> -#define malloc moz_xmalloc
> -#define calloc moz_xcalloc
> -#define realloc moz_xrealloc
> +inline void *malloc(size_t size)
> +{
> +    return moz_xmalloc(size);
> +}
> +
> +inline void *malloc(size_t nmemb, size_t size)

Oops, I think this should be `calloc`!

Comment 9

[Tom Ritter [:tjr]]

Comment on attachment 8913755 [details]
Bug 1389965 Redefine macros to inline functions to fix MinGW build

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/185146/diff/1-2/

Comment 10

[Tom Ritter [:tjr]]

Fixed.

MinGW Try Run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f8f31734048bdfa4b92d5c5cff211d19c826253d

Comment 11

[Tom Ritter [:tjr]]

Vanilla Try Run:  https://treeherder.mozilla.org/#/jobs?repo=try&revision=2131c443d4c53fee78bf40d15684fdc8579a681d

Comment 12

[Jonathan Kew [:jfkthame]]

Comment on attachment 8913755 [details]
Bug 1389965 Redefine macros to inline functions to fix MinGW build

https://reviewboard.mozilla.org/r/185146/#review192196

LGTM, thanks.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9eada0c91208
Redefine macros to inline functions to fix MinGW build r=jfkthame

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/9eada0c91208

Comment 15

[Petr Sumbera]

This seems to broke Solaris build like this:

106:45.02 /usr/bin/g++ -std=gnu++11 -o NameTable.o -c -I/scratch/firefox/obj-x86_64-pc-solaris2.11/dist/stl_wrappers -I/scratch/firefox/obj-x86_64-pc-solaris2.11/dist/system_wrappers -include /scratch/firefox/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DGRAPHITE2_STATIC '-DPACKAGE_VERSION="moz"' '-DPACKAGE_BUGREPORT="http://bugzilla.mozilla.org/"' -DGRAPHITE2_NFILEFACE -DGRAPHITE2_NTRACING -DGRAPHITE2_NSEGCACHE '-DGRAPHITE2_CUSTOM_HEADER="MozGrMalloc.h"' -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/scratch/firefox/gfx/graphite2/src -I/scratch/firefox/obj-x86_64-pc-solaris2.11/gfx/graphite2/src -I/scratch/firefox/obj-x86_64-pc-solaris2.11/dist/include -I/scratch/firefox/obj-x86_64-pc-solaris2.11/dist/include/nspr -I/scratch/firefox/obj-x86_64-pc-solaris2.11/dist/include/nss  -fPIC  -DMOZILLA_CLIENT -include /scratch/firefox/obj-x86_64-pc-solaris2.11/mozilla-config.h -MD -MP -MF .deps/NameTable.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 -Wno-error=free-nonheap-object -Wformat -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe  -g -O -fno-omit-frame-pointer     /scratch/firefox/gfx/graphite2/src/NameTable.cpp
106:45.53 In file included from /scratch/firefox/gfx/graphite2/src/inc/Main.h:33:0,
106:45.54                  from /scratch/firefox/gfx/graphite2/src/NameTable.cpp:27:
106:45.54 /scratch/firefox/gfx/graphite2/src/MozGrMalloc.h: In function 'void* malloc(std::size_t)':
106:45.54 /scratch/firefox/gfx/graphite2/src/MozGrMalloc.h:15:32: error: 'void* malloc(std::size_t)' conflicts with a previous declaration
106:45.54  inline void* malloc(size_t size)
106:45.54                                 ^
106:45.54 In file included from /usr/include/stdlib.h:12:0,
106:45.55                  from /scratch/firefox/obj-x86_64-pc-solaris2.11/dist/system_wrappers/stdlib.h:3,
106:45.55                  from /usr/gcc/5/include/c++/5.4.0/cstdlib:72,
106:45.55                  from /scratch/firefox/obj-x86_64-pc-solaris2.11/dist/system_wrappers/cstdlib:3,
106:45.55                  from /scratch/firefox/obj-x86_64-pc-solaris2.11/dist/stl_wrappers/cstdlib:44,
106:45.55                  from /scratch/firefox/gfx/graphite2/src/inc/Main.h:29,
106:45.56                  from /scratch/firefox/gfx/graphite2/src/NameTable.cpp:27:
106:45.56 /usr/include/iso/stdlib_iso.h:179:14: note: previous declaration 'void* std::malloc(std::size_t)'
106:45.57  extern void *malloc(size_t);
106:45.57               ^
106:45.57 In file included from /scratch/firefox/gfx/graphite2/src/inc/Main.h:33:0,
106:45.57                  from /scratch/firefox/gfx/graphite2/src/NameTable.cpp:27:
106:45.57 /scratch/firefox/gfx/graphite2/src/MozGrMalloc.h: In function 'void* calloc(std::size_t, std::size_t)':
106:45.57 /scratch/firefox/gfx/graphite2/src/MozGrMalloc.h:20:46: error: 'void* calloc(std::size_t, std::size_t)' conflicts with a previous declaration
106:45.61  inline void* calloc(size_t nmemb, size_t size)
106:45.61                                               ^

Comment 16

[Tom Ritter [:tjr]]

(In reply to Petr Sumbera from comment #15)
> This seems to broke Solaris build like this:

Can we just use an #if defined(SOLARIS) to restore the old behavior?

Comment 17

[Petr Sumbera]

Probably #if defined(XP_SOLARIS). But don't know in first place why other systems are not affected.

I build with gcc 5.4. But in future newer gcc should be used too. What was original problem? Some background please.

Comment 18

[Tom Ritter [:tjr]]

(In reply to Petr Sumbera from comment #17)
> Probably #if defined(XP_SOLARIS). But don't know in first place why other
> systems are not affected.
> 
> I build with gcc 5.4. But in future newer gcc should be used too. What was
> original problem? Some background please.

It's not clear what the root cause of the issue is/was, but the definitions of moz_xcalloc and friends could not be found. The fix worked on all of the platforms in TaskCluster though, so we applied it globally.

Comment 19

[Petr Sumbera]

Ok. There was already similar issue:
https://bugzilla.mozilla.org/show_bug.cgi?id=1375467

Basically adding following helps:  extern "C"

Something like this:

diff -r 5eba13f5b3a6 gfx/graphite2/src/MozGrMalloc.h
--- a/gfx/graphite2/src/MozGrMalloc.h   Sun Oct 08 16:10:05 2017 +0200
+++ b/gfx/graphite2/src/MozGrMalloc.h   Tue Oct 10 10:41:23 2017 +0000
@@ -12,17 +12,17 @@

 #include "mozilla/mozalloc.h"

-inline void* malloc(size_t size)
+extern "C" inline void* malloc(size_t size)
 {
     return moz_xmalloc(size);
 }

-inline void* calloc(size_t nmemb, size_t size)
+extern "C" inline void* calloc(size_t nmemb, size_t size)
 {
     return moz_xcalloc(nmemb, size);
 }

-inline void* realloc(void *ptr, size_t size)
+extern "C" inline void* realloc(void *ptr, size_t size)
 {
     return moz_xrealloc(ptr, size);
 }

Comment 20

[Jacek Caban]

The root of mingw problem is that mingw stdlib.h has following line:
using std::calloc;
which gets hurt by calloc macro.

The patch adding extern "C" looks right to me.