860222 - stun_build.o: requires dynamic R_X86_64_PC32 reloc against 'isascii' which may overflow at runtime; recompile with -fPIC

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[Olli Pettay [:smaug][[e-mail]]]

/usr/bin/ld: error: /home/smaug/mozilla/hg/inbound/ff_build/toolkit/library/../../media/mtransport/third_party/nICEr/nicer_nicer/src/stun/stun_build.o: requires dynamic R_X86_64_PC32 reloc against 'isascii' which may overflow at runtime; recompile with -fPIC
/usr/bin/ld: error: hidden symbol 'isascii' is not defined locally
/usr/bin/ld: error: hidden symbol 'isascii' is not defined locally

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

olli: for reference, OS and compiler/rev?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

clang 3.2 + gold.

gcc 4.7 + gold seems to work

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

64bit linux (Fedora 18)

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Ok, that's odd, since we're including <ctype.h> (I thought we weren't but just checked and we are).  And while isascii() is rare in the tree, isdigit() is all over the place.  But isascii is a little different in <ctype.h>; it depends on stuff like:

if defined __USE_SVID || defined __USE_MISC || defined __USE_XOPEN

To decide whether to use internal impls (__isascii():
#define	__isascii(c)	(((c) & ~0x7f) == 0)	/* If C is a 7 bit value.  */
or an external impl, it appears.  My guess is clang somehow is using different #defines.  You might need to look at the preprocessor output.

glandium? ideas?

Comment 5

[Mihai Sucan [:msucan]]

Attachment: stun_build.i

As requested by glandium on #developers.

Comment 6

[Mike Hommey [:glandium]]

There is no declaration of isascii at all in the preprocessed file, so clang ends up using an implicit extern declaration of isascii, with visibility hidden because of -fvisibility=hidden. Which then fails to link against libc's isascii. gcc doesn't have the problem because it is either inlining an implementation, or using an implicit declaration with an explicit default visibility. That /might/ be a clang bug, especially if gcc is doing the latter, but the root problem is using isascii, which is an obsolete BSD extension.

Comment 7

[Mike Hommey [:glandium]]

Note this breaks ASAN builds.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

So, this is almost certainly due to none of 
#if defined __USE_SVID || defined __USE_MISC || defined __USE_XOPEN
being defined (per above), one of which apparently is for gcc.  (See /usr/include/ctype.h).

Apparently (hard to find hard info, but per MSFT) __isascii is a posix replacement for isascii().  I'll try that.

Comment 9

[Mike Hommey [:glandium]]

(In reply to Randell Jesup [:jesup] from comment #8)
> Apparently (hard to find hard info, but per MSFT) __isascii is a posix
> replacement for isascii().  I'll try that.

The is no mention of it I can find in POSIX.1-2008.
http://pubs.opengroup.org/onlinepubs/9699919799/

Comment 10

[Nicholas Nethercote [inactive]]

Can we just implement it ourselves?  Doesn't seem hard.

       isascii()
              checks whether c is a 7-bit unsigned char value that  fits  into
              the ASCII character set.

|c <= 0 && c <= 127| seems like it would do it, though I'd check it exhaustively against an existing isascii() implementation if I were to implement it myself.

Comment 11

[Mike Hommey [:glandium]]

(In reply to Nicholas Nethercote [:njn] from comment #10)
> |c <= 0 && c <= 127| seems like it would do it

See comment 4 for the __isascii implementation from ctype.h

Comment 12

[Randell Jesup [:jesup] (needinfo me)]

Attachment: use our own isascii() for sanitizing TURN passwords

https://tbpl.mozilla.org/?tree=Try&rev=0223de8a9b6e

Comment 13

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 736461 [details] [diff] [review]
use our own isascii() for sanitizing TURN passwords

I'll get rid of the tab.

Some platforms have __isascii(); mac (for one) does not.

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/e41c1fc65901

Comment 15

[Nicholas Nethercote [inactive]]

Local clang 3.2 builds are working again for me.  Thanks!

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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

Comment 17

[StevenLee[:slee]]

I built on my Ubuntu and got the following errors. Is there anyone having this problem?

 0:52.14 ../../../build/unix/gold/ld: error: /home/steven/workspace/b2g/mozilla-central/objdir-desktop-debug/toolkit/library/build/../../../content/media/webrtc/Unified_cpp_content_media_webrtc0.o: requires dynamic R_X86_64_PC32 reloc against 'mozilla::MediaEngine::DEFAULT_169_VIDEO_WIDTH' which may overflow at runtime; recompile with -fPIC
 0:52.14 ../../../build/unix/gold/ld: error: /home/steven/workspace/b2g/mozilla-central/objdir-desktop-debug/toolkit/library/build/../../../content/media/webrtc/Unified_cpp_content_media_webrtc0.o: requires dynamic R_X86_64_PC32 reloc against 'mozilla::MediaEngine::DEFAULT_169_VIDEO_HEIGHT' which may overflow at runtime; recompile with -fPIC
 0:52.14 ../../../build/unix/gold/ld: error: read-only segment has dynamic relocations
 0:52.14 /home/steven/workspace/b2g/mozilla-central/content/media/webrtc/MediaEngine.h:158: error: undefined reference to 'mozilla::MediaEngine::DEFAULT_169_VIDEO_WIDTH'
 0:52.14 /home/steven/workspace/b2g/mozilla-central/content/media/webrtc/MediaEngine.h:158: error: undefined reference to 'mozilla::MediaEngine::DEFAULT_43_VIDEO_WIDTH'
 0:52.14 /home/steven/workspace/b2g/mozilla-central/content/media/webrtc/MediaEngine.h:163: error: undefined reference to 'mozilla::MediaEngine::DEFAULT_169_VIDEO_HEIGHT'
 0:52.14 /home/steven/workspace/b2g/mozilla-central/content/media/webrtc/MediaEngine.h:163: error: undefined reference to 'mozilla::MediaEngine::DEFAULT_43_VIDEO_HEIGHT'11

Comment 18

[StevenLee[:slee]]

Sorry, I posted the message on the wrong bug. I filed another bug for it.