Bug 772817 - 4.7 regression: unable to build mame, linking failure: undefined reference to foo
Summary: 4.7 regression: unable to build mame, linking failure: undefined reference to...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: gcc
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelinek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-01-10 02:14 UTC by Julian Sikorski
Modified: 2012-01-10 15:59 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-01-10 15:21:43 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
differences between 4.6.2 and 4.7.0 versions of <new> (2.64 KB, patch)
2012-01-10 11:49 UTC, Julian Sikorski
no flags Details | Diff

Description Julian Sikorski 2012-01-10 02:14:50 UTC
Description of problem:
I was trying to build mame (an rpmfusion package) with gcc-4.7. I have
managed to get it to build, but it fails at the linking stage:

obj/sdl/libocore.a(sdlsocket.o): In function `operator new(unsigned long)':
/builddir/build/BUILD/mame-0.144u5/src/emu/emualloc.h:114: undefined
reference to `malloc_file_line(unsigned long, char const*, int)'
obj/sdl/libocore.a(sdlsocket.o): In function `operator delete(void*)':
/builddir/build/BUILD/mame-0.144u5/src/emu/emualloc.h:131: undefined
reference to `free_file_line(void*, char const*, int)'
collect2: error: ld returned 1 exit status

The same code builds with gcc-4.6 just fine. Were there any changes in
that regard?

Full build log:
http://belegdol.fedorapeople.org/build.log

Version-Release number of selected component (if applicable):
gcc-4.7.0-0.5.fc17

How reproducible:
always

Steps to Reproduce:
1. Fetch the source RPM from https://share.ols.inode.at/HA19LXBVKCKBMUFIVMLO7TRLY6SYZR6G36PN41IN
2. Try to rebuild it
  
Actual results:
build fails

Expected results:
build works

Additional info:
Petr Machata has kindly provided the initial analysis:

The problem boils down to the following reproducer:

    #include <new>
    #include <cstddef>

    void *malloc_file_line(size_t size, const char *file, int line);

    __attribute__((always_inline)) inline void *
    operator new(std::size_t size) {
     return malloc_file_line(size, __null, 0);
    }

    struct item {
      void add() { new item; }
    };

    int main(int argc, char *argv[]) {
      return 0;
    }

# g++ ble.cc
/tmp/ccwc7vSd.o: In function `operator new(unsigned long)':
ble.cc:(.text._Znwm[_Znwm]+0x1e): undefined reference to `malloc_file_line(unsigned long, char const*, int)'

If #include <new> is removed, the problem goes away.  I suspect there
might be another declarations of new operator in that header file, and
the two are not going well together.  Would you be so kind as to open a
bug against gcc?  It looks like something that responsible parties
should take a look at, what with all the "inline" and "always_inline"
stuff that GCC seems to be ignoring.

To work around your immediate problem, I think you might #define
NO_MEM_TRACKING before including emu.h, like this:

diff -up mame-0.144u5/src/osd/sdl/sdlsocket.c\~ mame-0.144u5/src/osd/sdl/sdlsocket.c
--- mame-0.144u5/src/osd/sdl/sdlsocket.c~	2012-01-10 00:34:48.000000000 +0100
+++ mame-0.144u5/src/osd/sdl/sdlsocket.c	2012-01-10 02:13:51.293943347 +0100
@@ -23,6 +23,7 @@
 #endif
 #include <errno.h>
 
+#define NO_MEM_TRACKING
 #include "emu.h"
 #include "sdlfile.h"

It then proceeds for a bit before hitting a C++11 correctness problem.
You might consider smuggling in -std=c++98 and alarm upstream, it seems
GCC 4.7 defaults to C++11 and the code is not ready for this.

Comment 1 Jakub Jelinek 2012-01-10 07:34:48 UTC
g++ doesn't default to -std=c++11 yet, it defaults to -std=gnu++98.
And your reproducer is bogus, because malloc_file_line isn't defined anywhere.
You need a definition for that somewhere, and as it isn't prototyped with extern "C", it needs to be a C++ symbol too.
So, please look first which source file is supposed to provide that symbol, which does that when you compile with 4.6 and what changed that it doesn't provide it anymore with gcc 4.7.

Comment 2 Julian Sikorski 2012-01-10 09:40:58 UTC
I think this function is defined in src/emu/emualloc.h, but then my coding skills are negligible so I might be talking nonsense:
http://mamedev.org/source/src/emu/emualloc.h.html
As for the reproducer, Petr, could you please comment on that?

Comment 3 Kevin Kofler 2012-01-10 10:32:53 UTC
No, the function is not defined in emualloc.h, it's only declared as a prototype, i.e. a forward declaration. There's no actual definition there.

Comment 4 Julian Sikorski 2012-01-10 10:50:18 UTC
I see. I am not sure where to look though, since the only references to malloc_file_line and free_file_line google returns come back to mame.

Comment 5 Julian Sikorski 2012-01-10 11:49:13 UTC
Created attachment 551819 [details]
differences between 4.6.2 and 4.7.0 versions of <new>

The function is defined in
http://mamedev.org/source/src/emu/emualloc.c.html
I am attaching the differences in the header file. I tried fiddling with extern "C", but so far without success.

Comment 6 Kevin Kofler 2012-01-10 12:30:16 UTC
So the difference is probably the externally_visible attribute which got added in the prototype for operator new(size_t), which keeps the function from being optimized away even though it is marked always_inline and not used.

So I think the code already didn't work as intended with g++ 4.6.2, the replaced new function wasn't being used at all, which is why the unresolved reference went unnoticed.

Comment 7 Julian Sikorski 2012-01-10 12:39:35 UTC
Would the fix be to rename it?

Comment 8 Petr Machata 2012-01-10 12:56:33 UTC
I prompted opening of a bug report because there doesn't seem to be any reason to emit body of operator new.  It's declared as inline and only called from inline function whose body is also not emitted.  That's a change from GCC 4.6.  Apparently the flags defined in <new> at operator new (__externally_visible__?) take precedence.  If it's not a bug, then so much the better.

(Adding in libemu as a dependency is not an option, that leads to circular dependencies.  Julian, just use that #define hack that I posted.)

Julian, about that c++11 thing, it turns out that they are all _warnings_, but -Werror turns them into errors.  I missed that detail before.  I think the sane way out is to build with make NOWERROR=1, otherwise you'd have to plow through it all and either turn off individual warnings or fix actual offending code.

Comment 9 Petr Machata 2012-01-10 13:00:05 UTC
Kevin, I think it's just included coincidentally.  They need some declarations, but the chain of includes also brings in the operator new.  They never actually use any of that code (the file is in fact a .c file, just massaged through the c++ compiler) in the module itself.  It was okay up until now, but it breaks now that GCC emits the (actually unused) function body.

Comment 10 Julian Sikorski 2012-01-10 13:29:13 UTC
NOWERROR=1 is actually used by the RPM spec, you must have tried with plain make.
I can also confirm that your workaround has fixed the linking issue. I'll forward these findings to upstream.

Comment 11 Jakub Jelinek 2012-01-10 15:21:43 UTC
(In reply to comment #8)
> I prompted opening of a bug report because there doesn't seem to be any reason
> to emit body of operator new.  It's declared as inline and only called from
> inline function whose body is also not emitted.  That's a change from GCC 4.6. 
> Apparently the flags defined in <new> at operator new (__externally_visible__?)
> take precedence.  If it's not a bug, then so much the better.

I don't think it is a bug.  operator new is quite special and the externally_visible attribute on it is certainly desirable, the C++ standard requires that you can override the operator new.  See PR50594.

As for -Werror, every GCC major version adds or removes some warnings, if something is built with -Werror, you need to be prepared to adjust your code, either to fix up real code bugs or workaround false positive warnings, or disable individual warnings from erroring out (-Wno-error=warningname).

Comment 12 Petr Machata 2012-01-10 15:59:26 UTC
(In reply to comment #10)
> NOWERROR=1 is actually used by the RPM spec, you must have tried with plain
> make.

Yes, that's true.


Note You need to log in before you can comment on or make changes to this bug.