918943 - [10.9] Duplicate symbol errors linking WebRTC when using the 10.9 SDK with --disable-optimize

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Steven Michaud [:smichaud] (Retired)]

If you work around bug 901348 and bug 917526, you get the following errors building current trunk with --disable-optimize and the 10.9 SDK specified in your mozconfig file:

...
ld: warning: could not create compact unwind for _ffi_call_unix64: does not use RBP or RSP based frame
duplicate symbol ___sigbits in:
    ../../media/webrtc/signaling/signaling_sipcc/gsm_sdp_crypto.o
    ../../media/webrtc/signaling/signaling_sipcc/lsm.o
duplicate symbol ___sigbits in:
    ../../media/webrtc/signaling/signaling_sipcc/gsm_sdp_crypto.o
    ../../media/webrtc/signaling/signaling_sipcc/ccsdp.o
duplicate symbol ___sigbits in:
    ../../media/webrtc/signaling/signaling_sipcc/gsm_sdp_crypto.o
    ../../media/webrtc/signaling/signaling_sipcc/sdp_access.o
duplicate symbol ___sigbits in:
    ../../media/webrtc/signaling/signaling_sipcc/gsm_sdp_crypto.o
    ../../media/webrtc/signaling/signaling_sipcc/sdp_attr.o
duplicate symbol ___sigbits in:
    ../../media/webrtc/signaling/signaling_sipcc/gsm_sdp_crypto.o
    ../../media/webrtc/signaling/signaling_sipcc/sdp_attr_access.o
duplicate symbol ___sigbits in:
    ../../media/webrtc/signaling/signaling_sipcc/gsm_sdp_crypto.o
    ../../media/webrtc/signaling/signaling_sipcc/sdp_token.o
duplicate symbol ___sigbits in:
    ../../media/webrtc/signaling/signaling_sipcc/gsm_sdp_crypto.o
    ../../media/webrtc/signaling/signaling_sipcc/ccsip_sdp.o
ld: 7 duplicate symbols for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[4]: *** [XUL] Error 1
make[3]: *** [libs] Error 2
make[2]: *** [default] Error 2
make[1]: *** [realbuild] Error 2
make: *** [build] Error 2

These errors happen not building WebRTC, but afterwards linking WebRTC's object files into the XUL binary.

You can work around bug 901348 by adding the following to your mozconfig:

ac_add_options --without-intl-api

Work around bug 917526 by applying its patch (attachment 807290 [details] [diff] [review]).

This, like bug 917526, is probably a clang bug.  I have a workaround for it, which I'll post shortly.

(Note that bug 884014 was fixed very recently on the trunk.  If you're not working with the latest trunk code, you'll also need to apply that bug's patch to see this bug's errors.)

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Actually you still get duplicate symbol errors (different ones) without --disable-optimize.

Comment 2

[Nomis101]

Attachment: Symbol errors

Today I've tried (with the current 10.9 patches applied) to build Thunderbird trunk on 10.9 with the 10.9 SDK. This was without --disable-optimize (a non-debug build). And I run into this duplicate symbol error. Is this the same webrtc error and related to this bug or is this something different?

Comment 3

[Steven Michaud [:smichaud] (Retired)]

> I have a workaround for it, which I'll post shortly.

Actually it's a bit more complicated than I thought.  I'll post my workaround next week.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Nomis101:

The simplest workaround is just to comment out this line:
https://hg.mozilla.org/mozilla-central/annotate/354c958ba7d4/media/webrtc/signaling/src/sipcc/core/sdp/sdp_os_defs.h#l30

Try building with that change.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #4)

You also need to apply my patch for bug 917526.

Comment 6

[Nomis101]

(In reply to Steven Michaud from comment #5)
> (Following up comment #4)
> 
> You also need to apply my patch for bug 917526.
Yes, Bug 917526 fixed my attached error (without commenting out the line from #4). I can now build successfully Firefox and Thunderbird trunk on 10.9 with the 10.9 SDK. :-)

Comment 7

[Steven Michaud [:smichaud] (Retired)]

(In reply to comment #6)

Not for me.  Even in current trunk (which contains my patch for bug 917526) I still see duplicate symbol errors trying to build with WebRTC (which is built by default) using the 10.9 SDK.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Attachment: Simple workaround

As I mentioned in comment #0, this is probably a clang bug.  But it has a very simple workaround -- this patch.

I know nothing about the SIPCC code, which seems to have been imported whole into the Mozilla tree from some other location.  I also don't know who *does* know about the SIPCC code.  But Adam, I found that you're the author of the following document:  https://wiki.mozilla.org/Media/WebRTC/SIPCC.  So I figure you're a good person to start with.

Why is the inline keyword defined away in sdp_os_defs.h?  Will it cause trouble if we stop defining it away?  Is there, perhaps, a small number of methods that should never be inlined, to whose declarations we could change to prevent that happening?

I could have my patch only have effect when compiling on OS X, or (perhaps, and with a lot of trouble) only when compiling with the 10.9 SDK.  But I thought it best to start with the simplest case.

Comment 9

[Steven Michaud [:smichaud] (Retired)]

(Following up comment #7)

Actually, in current trunk code (with my patch for bug 917526), I only see duplicate symbol errors compiling with --disable-optimize.

Comment 10

[Nomis101]

(In reply to Steven Michaud from comment #9)
> (Following up comment #7)
> 
> Actually, in current trunk code (with my patch for bug 917526), I only see
> duplicate symbol errors compiling with --disable-optimize.

That's why I have not seen this, I compiled with enabled optimize.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 812259 [details] [diff] [review]
Simple workaround

https://tbpl.mozilla.org/?tree=Try&rev=5b38a575c0d8

Windows builds fail with this patch, so we can't use it on that platform.

But I'd still like to get a response to the general idea of doing this.

Comment 12

[Ethan Hugg [:ehugg]]

I believe this line was put it to fix that exact Windows build problem.  I'm guessing that we would be better off putting an conditional around it based on __STDC_VERSION__ rather than OS type but I have not tried that yet.

Comment 13

[Adam Roach [:abr]]

Comment on attachment 812259 [details] [diff] [review]
Simple workaround

I'm going to lateral this to Ethan; I don't know why this was in the code in the first place, and he might have some history.

Comment 14

[Steven Michaud [:smichaud] (Retired)]

How about only defining away the inline keyword when __GNUC_STDC_INLINE__ is not defined?

At least with XCode's gcc/g++/clang, this is defined when we compile with "-std=gnu99" but not "-fgnu89-inline" (as we currently do on the Mac; see bug 917526).

This would (presumably) only define away the inline keyword when we compile with a non-GNU-compatible compiler, or when we specify "std=gnu99 -fgnu89-inline" with a GNU-compatible compiler.

Comment 15

[Ethan Hugg [:ehugg]]

Comment on attachment 812259 [details] [diff] [review]
Simple workaround


Sure, lets try using an #ifndef __GNUC_STDC_INLINE__ around this.  If it passes Try, then r? to me and I'll run it through some WebRTC tests to double-check that the runtime is unaffected.

Comment 16

[Steven Michaud [:smichaud] (Retired)]

Here goes:
https://tbpl.mozilla.org/?tree=Try&rev=b8f7e1e5803b

Comment 17

[Steven Michaud [:smichaud] (Retired)]

> https://tbpl.mozilla.org/?tree=Try&rev=b8f7e1e5803b

The results were very good -- there were no (genuine) failures of any kind.

Comment 18

[Steven Michaud [:smichaud] (Retired)]

Attachment: Workaround rev1

Comment 19

[Ethan Hugg [:ehugg]]

Comment on attachment 812877 [details] [diff] [review]
Workaround rev1

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

Looks good to me.  Ran it through some ad hoc tests on OSX.

Comment 20

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 812877 [details] [diff] [review]
Workaround rev1

Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/beacc621ec68

Comment 21

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/beacc621ec68