307168 - Need to clone NSPR X86-64 GCC configure fix to main configure (hidden visibility)

Status: VERIFIED FIXED

(Firefox Build System :: General, defect, P2)

Description

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

Trunk fails to build on X86-64 (using FC3 with gcc version 3.4.2 20041017 (Red
Hat 3.4.2-6.fc3) with a relocation failure for R_X86_64_PC32.  The error comes
in zlib.

The fix from bug 293438 for nspr configure(.in) needs to be cloned to the trunk
configure (and configure.in).

Patch to be added

Comment 1

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

I'd like to improve on the patch from nspr to let the compiler optimize fully in
the non-x86-64 case.  We should consider doing this to bug 293438 as well.

Comment 2

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

Attachment: Path for root & NSPR configure(.in)

Comment 3

[Wan-Teh Chang]

Comment on attachment 195110 [details] [diff] [review]
Path for root & NSPR configure(.in)

I'm happy with the current code in NSPR's configure script.
It uses the technique documented in
http://www.nedprod.com/programs/gccvisibility.html.

I understand Brian Ryner's technique is better, but it is
not documented.

Comment 4

[Benjamin Smedberg]

Attachment: Use -fvisibility=hidden first, then fall back to the #pragma

This patch fixes my FC4 x86_64 with stock gcc4.0.1: it uses -fvisibility=hidden
first, if available, and only then falls back to use the #pragma if
-fvisibility=hidden is not available. It continues to use the system wrappers
in all cases.

Comment 5

[Benjamin Smedberg]

oh, and for some reason I don't seem to need this patch for NSPR, just for the
main tree (the error for me only occurs for special gcc builtins like memset).

Comment 6

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

Looks good, but shouldn't we include mods to configure as well, since it's in CVS?

Comment 7

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

I assume the reason you don't *need* to patch NSPR is that the patch from bug
293438 is checked in, which is a simpler solution.  It might still be a good
idea  to do the more complex patch you did to root configure(.in).

Comment 8

[Benjamin Smedberg]

cltbld auto-commits configure, and I generally don't include it in patches for
review anyway. You're welcome to steal shaver's r? (I'd still like to get bryner
to do a review).

Comment 9

[Wan-Teh Chang]

Comment on attachment 195257 [details] [diff] [review]
Use -fvisibility=hidden first, then fall back to the #pragma

This patch doesn't make sense.	The system header wrappers
use GCC visibility pragmas, so how can you set
WRAP_SYSTEM_INCLUDES=1 without checking for GCC visibility
pragma support?

Also, you check for -fvisibility=hidden support but don't
use that compiler flag if it's supported, and you check for
the less optimal -fvisibility=hidden support before you
check for the more optimal visibility pragma support, which
is the wrong order.

Comment 10

[Benjamin Smedberg]

> This patch doesn't make sense.	The system header wrappers
> use GCC visibility pragmas, so how can you set
> WRAP_SYSTEM_INCLUDES=1 without checking for GCC visibility
> pragma support?

If -fvisibility=hidden is supported, we also know that pragmas are supported
(historically).

> Also, you check for -fvisibility=hidden support but don't
> use that compiler flag if it's supported, and you check for

Whoops! This patch went through several revisions, and this sucks ;-)

> the less optimal -fvisibility=hidden support before you
> check for the more optimal visibility pragma support, which

On the contrary. The -fvisibility=hidden support is the highly preferred
solution, and the pragmas are only supported to aid backwards compatibility. In
this case the pragmas are exposing bugs in gcc on x86_64 that don't show up with
-fvisibility=hidden.

Comment 11

[Wan-Teh Chang]

There is an undocumented benefit for using the system
header wrappers + gcc_hidden.h method.  I asked Brian
Ryner to explain that benefit to me and summarized my
understanding of that benefit in the comment I added to
nsprpub/configure.in:

  To work around a build problem on Linux x86-64 (Bugzilla bug
  307168), we use the -fvisibility=hidden flag.  This flag is less
  optimal than #pragma GCC visibility push(hidden) because the flag
  assumes that symbols defined outside the current source file have
  the default visibility.  This has the advantage that we don't need
  to wrap system header files, but has the disadvantage that calls
  to hidden symbols defined in other source files cannot be
  optimized by the compiler.  The -fvisibility=hidden flag does
  hide and export symbols correctly.

In the end, I chose to use the -fvisibility=hidden flag (without
wrapping system headers), which is the method described in
http://www.nedprod.com/programs/gccvisibility.html.

Comment 12

[Benjamin Smedberg]

ok, I found the GCC patch which fixes this bug, see
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20297

I'm inclined to perhaps introduce a configure test to workaround that bug
specifically.

Comment 13

[Benjamin Smedberg]

Attachment: autoconf check for the GCC bug

ok, instead of substituting -fvisibility=hidden (wtc is right about how
-fvisibility=hidden is less effective than it ought to be), this does a
configure-time check for the GCC bug, and disables the pragmas if gcc doesn't
stand up. I've verified that this works correctly for "bad" and "good" GCCs.

Comment 14

[Wan-Teh Chang]

Comment on attachment 195469 [details] [diff] [review]
autoconf check for the GCC bug

>+        if test "$ac_cv_have_visibility_builtin_bug" = "no"; then
>+          AC_DEFINE(HAVE_VISIBILITY_PRAGMA)
>+          VISIBILITY_FLAGS='-I$(DIST)/include/system_wrappers -include $(topsrcdir)/config/gcc_hidden.h'
>+          WRAP_SYSTEM_INCLUDES=1
>+        fi

It would be nice to do the second best thing, -fvisibility=hidden,
if the compiler has visibility builtin bug.  So I suggest adding an
else block:

	  AC_DEFINE(HAVE_VISIBILITY_PRAGMA)
	  if test "$ac_cv_have_visibility_builtin_bug" = "no"; then
	    VISIBILITY_FLAGS='-I$(DIST)/include/system_wrappers -include
$(topsrcdir)/config/gcc_hidden.h'
	    WRAP_SYSTEM_INCLUDES=1
	  else
	    VISIBILITY_FLAGS="-fvisibility=hidden"
	    WRAP_SYSTEM_INCLUDES=
	  fi

Comment 15

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

Comment on attachment 195469 [details] [diff] [review]
autoconf check for the GCC bug

Though I believe this will work, I'd like to see it with WTC's "else" case
added.	Once that's in, I'll happily + it.

I'll hand-run a test with that (including WTC's).  Because of the darn bug
104642 (the "you can't use a modern autoconf" bug), I have to play "human
autoconf" to test patches without configure.  I really would prefer if the
configure change were included in the patch.  There's another reason - we'd in
general like to move towards more complete/correct patches so that 3rd-party
packagers (distros) can more easily vet/certify changes, especially on
long-lived (stable) branches.

BTW, WTC - again, should we make this change to NSPR?

Comment 16

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

No joy currently (with WTC's mod):
./fb-opt-static/dist/bin/firefox-bin: symbol lookup error:
/home/jesup/src/trunk/mozilla/fb-opt-static/dist/bin/components/libxpconnect.so:
undefined symbol: XPTI_GetInterfaceInfoManager

I'll try again without it.

Update: my drive went into read-only mode after hitting some errors, so I'm out
of commission for donig x86-64 testing for a while.

Comment 17

[Benjamin Smedberg]

*** Bug 307812 has been marked as a duplicate of this bug. ***

Comment 18

[Wan-Teh Chang]

Comment on attachment 195469 [details] [diff] [review]
autoconf check for the GCC bug

>+__attribute__ ((visibility ("default"))) void Func() {
>+  char c[[100]];
>+  memset(c, 0, sizeof(c));
>+}

We may want to do something to prevent the compiler from
optimizing away the memset call.  Perhaps something like
this:

  __attribute__ ((visibility ("default"))) void
  Func(void *s, size_t n)
  {
    memset(s, 0, n);
  }

I can adapt the Mozilla patch for NSPR myself.	If you write
a patch for NSPR, I'll be happy to review it and check it in
(but probably on the NSPR trunk and client branch only since
I need to stablize the NSPR for MOZILLA_1_8_BRANCH now).

Comment 19

[Benjamin Smedberg]

Attachment: ifdef the macro based on the attribute, not the pragma

WTC, this is the only patch that I'd like to get in NSPR trunk/client/1.8

Comment 20

[Benjamin Smedberg]

Attachment: autoconf check for the GCC bug, rev. 2

It turns out that many times DSO_LDOPTS contained makefile variable references
"$@" which were being misinterpreted. This patch moves those references to
MKSHLIB/MKCSHLIB so that we can use DSO_LDOPTS directly.

Cls, you don't really need to review the configure test logic or the #ifdef
changes, I'm mainly worried about the configure variables.

Comment 21

[Benjamin Smedberg]

wtc, just for reference, I tried to port this to NSPR but the GCC patch is wrong
somehow, see http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20297#c10

Comment 22

[Wan-Teh Chang]

Comment on attachment 195872 [details] [diff] [review]
ifdef the macro based on the attribute, not the pragma

Should we rename this macro HAVE_VISIBILITY_ATTRIBUTE?

Thank you for trying to port the autoconf test to NSPR.

Comment 23

[Wan-Teh Chang]

Just to elaborate on my suggestion of renaming the
HAVE_VISIBILITY_HIDDEN_ATTRIBUTE macro:

I think the name HAVE_VISIBILITY_ATTRIBUTE is more
accurate.  My Web search for HAVE_VISIBILITY_ATTRIBUTE
generated pages of search results, which seem to suggest
that many(?) other projects are using HAVE_VISIBILITY_ATTRIBUTE.
In contrast, the Web search for HAVE_VISIBILITY_HIDDEN_ATTRIBUTE
only generated one page of search results (all are Mozilla
source files).

Comment 24

[Benjamin Smedberg]

I don't mind renaming it, but I'd like to do it as a second step since I've got
multiple trees with this stuff in it.

Comment 25

[Brian Ryner (not reading)]

Comment on attachment 195872 [details] [diff] [review]
ifdef the macro based on the attribute, not the pragma

This is not correct, though perhaps it's relying on an undocumented assumption.
 The earlier versions of gcc to support visibility("hidden") do _not_ support
visibility("default").	Because the default visibility declaration is only
needed if the #pragma is used to push hidden visibility, I just rolled the two
checks together.

Comment 26

[Benjamin Smedberg]

argh. The problem is that we're now building with -fvisibility=hidden and we
need the visibility(default), but the pragmas are still broken. I guess I'll
need another configure check or something :-(

Comment 27

[Benjamin Smedberg]

Attachment: ifdef the macro based on the attribute, not the pragma, rev. 2

NSPR doesn't use visibility(hidden) at all, so I significantly simplified the
configure test to check for -fvisibility=hidden and use
HAVE_VISIBILITY_ATTRIBUTE

Comment 28

[Benjamin Smedberg]

Attachment: autoconf check for the GCC bug, rev. 3

This sets the HAVE_VISIBILITY_HIDDEN_ATTRIBUTE and HAVE_VISIBILITY_ATTRIBUTE
separately.

Comment 29

[Wan-Teh Chang]

Comment on attachment 196045 [details] [diff] [review]
ifdef the macro based on the attribute, not the pragma, rev. 2

The configure test checks for the -fvisibility flag but
defines the HAVE_VISIBILITY_ATTRIBUTE macro.  The name
of the macro does not match the check performed.  The
comment should also be updated to describe the new configure
test.  In any case, I'm not sure if we should remove the
currently unused configure tests, because I am still
interested in using the visibility pragmas when the Linux
x86-64 linking problem is fixed.

In pr/include/prtypes.h, I want to write the block of code
in question like this:

  /* GCC 3.3 and later support the visibility attribute. */
  #if (__GNUC__ >= 4) ||
      (__GNUC__ == 3 && __GNUC_MINOR__ >= 3)
  #define PR_VISIBILITY_DEFAULT __attribute__((visibility("default")))
  #else
  #define PR_VISIBILITY_DEFAULT
  #endif

I looked at the GCC 3.0 - 4.0 manuals and found that the
visibility attribute is first documented in the GCC 3.3
manual.

The reason is that I want to maintain the property that NSPR
users don't need to define any macros when including NSPR
headers.

My suggested change of testing the GCC version works provided
that no one (individual or Linux distribution) has backported
the visibility pragmas or the -fvisibility flag to their special
build of GCC 3.2.x or earlier, because the visibility pragmas
and the -fvisibility flag are what make it crucial to define
PR_VISIBILITY_DEFAULT to be __attribute__((visibility("default"))).

-fvisilibility is new in GCC 4.0.0, so I doubt anyone would
backport it to GCC 3.2.x or earlier.  (Red Hat backported it
to its special build of GCC 3.4.x.)  I believe that the visibility
pragmas were added in the same GCC patch, but I'll need bryner to
confirm that.

Comment 30

[Benjamin Smedberg]

Attachment: Use NS_COM instead of PR_EXTERN/IMPLEMENT [checked in]

Comment 31

[Darin Fisher]

Comment on attachment 196149 [details] [diff] [review]
Use NS_COM instead of PR_EXTERN/IMPLEMENT [checked in]

r=darin

Comment 32

[Benjamin Smedberg]

Comment on attachment 196149 [details] [diff] [review]
Use NS_COM instead of PR_EXTERN/IMPLEMENT [checked in]

checked in on trunk. I'd like this on the 1.8 branch (very low risk), since
right now the NSPR and mozilla systems are tied together in a rather fragile
way.

Comment 33

[Benjamin Smedberg]

*** Bug 309088 has been marked as a duplicate of this bug. ***

Comment 34

[tensor5]

Hi,
I applied the last two patch of the above list, but I still get this error:

c++ -I/usr/include -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith
-Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy
-Wno-non-virtual-dtor -Wno-long-long -pedantic -fshort-wchar -pthread -pipe 
-DNDEBUG -DTRIMMED -O3 -fomit-frame-pointer -march=athlon64 -fPIC -shared
-Wl,-h,libmozpango-thaix.so -o libmozpango-thaix.so  thai-x.o      
-lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0
-lXinerama -lXi -lXrandr -lXext -lXcursor -lXfixes -lcairo -lpangoft2-1.0
-lfontconfig -lfreetype -lz -lpango-1.0 -lm -lXrender -lX11 -lgobject-2.0
-lgmodule-2.0 -ldl -lglib-2.0   -L../../../../dist/lib -lmozpango  -ldl -lm
/usr/bin/ld: ../../../../dist/lib/libmozpango.a(glyphstring.o): relocation
R_X86_64_32 against `a local symbol' can not be used when making a shared
object; recompile with -fPIC
../../../../dist/lib/libmozpango.a: could not read symbols: Bad value
collect2: ld returned 1 exit status
make[5]: *** [libmozpango-thaix.so] Error 1
make[5]: Leaving directory `/tmp/mozilla-build/intl/ctl/src/thaiShaper'

is it related to your bug? It is a failure for R_X86_64_32 instead of R_X86_64_PC32.

Thanks

Comment 35

[Benjamin Smedberg]

> I applied the last two patch of the above list, but I still get this error:

Did you run autoconf and reconfigure after applying the patch? I am currently
not building with pango (or at least I don't think I am), so we may need to do
additional work there.

Comment 36

[Michael Ward]

I applied the latest patches, and now I get:

gmake[5]: Entering directory `/root/mozilla/fb-opt-static/xpcom/tools/registry'
c++  -I/usr/X11R6/include -fno-rtti -fno-exceptions -Wall -Wconversion
-Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy
-Wno-non-virtual-dtor -Wno-long-long -pedantic -fshort-wchar -pthread -pipe 
-DNDEBUG -DTRIMMED -O -o regxpcom regxpcom.o     -L../../../dist/bin
-L../../../dist/lib ../../../dist/lib/libxpcomglue.a -L../../../dist/lib -lplds4
-lplc4 -lnspr4 -lpthread -ldl  -ldl -lm
../../../dist/bin/libplds4.so: undefined reference to `PR_Free'
../../../dist/bin/libplds4.so: undefined reference to `PR_Malloc'
../../../dist/bin/libplds4.so: undefined reference to `PR_CallOnce'
../../../dist/bin/libplc4.so: undefined reference to `PR_Calloc'
../../../dist/bin/libplds4.so: undefined reference to `PR_Lock'
../../../dist/bin/libplc4.so: undefined reference to `PR_GetSpecialFD'
../../../dist/bin/libplc4.so: undefined reference to `PR_GetError'
../../../dist/bin/libplds4.so: undefined reference to `PR_Unlock'
../../../dist/bin/libplds4.so: undefined reference to `PR_DestroyLock'
../../../dist/bin/libplc4.so: undefined reference to `PR_SetError'
../../../dist/bin/libplc4.so: undefined reference to `PR_fprintf'
../../../dist/bin/libplc4.so: undefined reference to `PR_GetOSError'
../../../dist/bin/libplds4.so: undefined reference to `PR_CeilingLog2'
../../../dist/bin/libplds4.so: undefined reference to `PR_NewLock'
collect2: ld returned 1 exit status
gmake[5]: *** [regxpcom] Error 1

Comment 37

[Zibi Braniecki [:zbraniecki][:gandalf]]

after applying patch from attachment 196045 [details] [diff] [review] and 196046 I got this error:
http://rafb.net/paste/results/ldmV4Q27.html

Comment 38

[Benjamin Smedberg]

Comment on attachment 196149 [details] [diff] [review]
Use NS_COM instead of PR_EXTERN/IMPLEMENT [checked in]

NS_COM patch checked in on 1.8 branch.

Comment 39

[Mike Hommey [:glandium]]

note that the gcc bug mentionned here also applies on powerpc. See bug #301936.
Using -fvisibility=hidden in that case solves the problem

Comment 40

[Mike Hommey [:glandium]]

Please consider simply using the -fvisibility=hidden option whenever you use
gcc4. There is actually another bug with the pragma visibility : the output is
not position independant code, in spite of the -fPIC option beeing specified.

Comment 41

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In my gcc version, the general bug is present but unfortunately the configure
test passes and we try to use hidden visibility.

Using built-in specs.
Target: x86_64-suse-linux
Configured with: ../configure --enable-threads=posix --prefix=/usr
--with-local-prefix=/usr/local --infodir=/usr/share/info --mandir=/usr/share/man
--libdir=/usr/lib64 --libexecdir=/usr/lib64
--enable-languages=c,c++,objc,f95,java,ada --disable-checking
--with-gxx-include-dir=/usr/include/c++/4.0.2 --enable-java-awt=gtk
--disable-libjava-multilib --with-slibdir=/lib64 --with-system-zlib
--enable-shared --enable-__cxa_atexit --without-system-libunwind
--host=x86_64-suse-linux
Thread model: posix
gcc version 4.0.2 20050901 (prerelease) (SUSE Linux)

Comment 42

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Here, I needed to add -O2 to get the configure test to reproduce the bug:

                     if ! ${CC-cc} ${DSO_PIC_CFLAGS} ${DSO_LDOPTS} -O2 -o
conftest.so conftest.c >/dev/null 2>&1; then

Comment 43

[Simon Strandman]

I'm using gcc 4.0.2 with patches for gcc pr19664, pr20297 and pr19520. The
testcase from 20297 passes without problems. Still beta2 and current CVS fails
to build. Any ideas what might be wrong?

x86_64-pc-linux-gnu-gcc -DGENTOO_NSPLUGINS_DIR=\"/usr/lib64/nsplugins\"
-DGENTOO_NSBROWSER_PLUGINS_DIR=\"/usr/lib64/nsbrowser/plugins\"   -W -Wno-unused
-Wpointer-arith -Wcast-align -Wno-long-long -march=athlon64 -pipe -fPIC
-Wno-return-type -w -pthread -pipe  -DNDEBUG -DTRIMMED -ffunction-sections -Os
-fPIC -shared -Wl,-h -Wl,libmozjs.so -Wl,-R/usr/lib64/mozilla-firefox -o
libmozjs.so  jsapi.o jsarena.o jsarray.o jsatom.o jsbool.o jscntxt.o jsdate.o
jsdbgapi.o jsdhash.o jsdtoa.o jsemit.o jsexn.o jsfun.o jsgc.o jshash.o
jsinterp.o jslock.o jslog2.o jslong.o jsmath.o jsnum.o jsobj.o jsopcode.o
jsparse.o jsprf.o jsregexp.o jsscan.o jsscope.o jsscript.o jsstr.o jsutil.o
jsxdrapi.o jsxml.o prmjtime.o    -Wl,-O1  -Wl,-R/usr/lib64/mozilla-firefox   -lm
-ldl -L../../dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl -ldl -lm
/usr/lib/gcc/x86_64-pc-linux-gnu/4.0.2/../../../../x86_64-pc-linux-gnu/bin/ld:
warning: creating a DT_TEXTREL in object.
/usr/lib/gcc/x86_64-pc-linux-gnu/4.0.2/../../../../x86_64-pc-linux-gnu/bin/ld:
jsapi.o: relocation R_X86_64_PC32 against `memcpy@@GLIBC_2.2.5' can not be used
when making a shared object; recompile with -fPIC
/usr/lib/gcc/x86_64-pc-linux-gnu/4.0.2/../../../../x86_64-pc-linux-gnu/bin/ld:
final link failed: Bad value
collect2: ld returned 1 exit status
gmake[3]: *** [libmozjs.so] Error 1
gmake[3]: Leaving directory
`/var/tmp/portage/mozilla-firefox-9999/work/mozilla/js/src'
gmake[2]: *** [libs] Error 2
gmake[2]: Leaving directory `/var/tmp/portage/mozilla-firefox-9999/work/mozilla/js'
gmake[1]: *** [tier_2] Error 2
gmake[1]: Leaving directory `/var/tmp/portage/mozilla-firefox-9999/work/mozilla'
make: *** [default] Error 2

The 9999 version means that it's a CVS build in gentoo.

Binutils is version 2.16.1 and glibc is 2.3.5. 

Comment 44

[Robert Kaiser]

roc:
It's know that (open)SUSE 10.0 ships with that gcc bug, see
https://bugzilla.novell.com/show_bug.cgi?id=83908
I suspect others will soon as well, and note that it also happens with 32-bit
architecture, so we'll run into that problem more and more :(

(In reply to comment #43)
> I'm using gcc 4.0.2 with patches for gcc pr19664, pr20297 and pr19520. The
> testcase from 20297 passes without problems. Still beta2 and current CVS fails
> to build. Any ideas what might be wrong?

I guess you should check if your failure is different from what bsmedberg
reported in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20297 and if not, report
it to gcc people, as it might very well still be their bug, and it might show
once again that the fix for 20297 is not enough, which gcc people are already
suspecting anyways, if you follow the discussion starting with
http://gcc.gnu.org/ml/gcc-patches/2005-03/msg00248.html

Comment 45

[Gergan Penkov]

(In reply to comment #43)
> I'm using gcc 4.0.2 with patches for gcc pr19664, pr20297 and pr19520. The
> testcase from 20297 passes without problems. Still beta2 and current CVS fails
> to build. Any ideas what might be wrong?
> 
> x86_64-pc-linux-gnu-gcc -DGENTOO_NSPLUGINS_DIR=\"/usr/lib64/nsplugins\"
> -DGENTOO_NSBROWSER_PLUGINS_DIR=\"/usr/lib64/nsbrowser/plugins\"   -W -Wno-unused
> -Wpointer-arith -Wcast-align -Wno-long-long -march=athlon64 -pipe -fPIC
> -Wno-return-type -w -pthread -pipe  -DNDEBUG -DTRIMMED -ffunction-sections -Os
> -fPIC -shared -Wl,-h -Wl,libmozjs.so -Wl,-R/usr/lib64/mozilla-firefox -o
> libmozjs.so  jsapi.o jsarena.o jsarray.o jsatom.o jsbool.o jscntxt.o jsdate.o
> jsdbgapi.o jsdhash.o jsdtoa.o jsemit.o jsexn.o jsfun.o jsgc.o jshash.o
> jsinterp.o jslock.o jslog2.o jslong.o jsmath.o jsnum.o jsobj.o jsopcode.o
> jsparse.o jsprf.o jsregexp.o jsscan.o jsscope.o jsscript.o jsstr.o jsutil.o
> jsxdrapi.o jsxml.o prmjtime.o    -Wl,-O1  -Wl,-R/usr/lib64/mozilla-firefox   -lm
> -ldl -L../../dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl -ldl -lm
> /usr/lib/gcc/x86_64-pc-linux-gnu/4.0.2/../../../../x86_64-pc-linux-gnu/bin/ld:
> warning: creating a DT_TEXTREL in object.
> /usr/lib/gcc/x86_64-pc-linux-gnu/4.0.2/../../../../x86_64-pc-linux-gnu/bin/ld:
> jsapi.o: relocation R_X86_64_PC32 against `memcpy@@GLIBC_2.2.5' can not be used
> when making a shared object; recompile with -fPIC
> /usr/lib/gcc/x86_64-pc-linux-gnu/4.0.2/../../../../x86_64-pc-linux-gnu/bin/ld:
> final link failed: Bad value
> collect2: ld returned 1 exit status
> gmake[3]: *** [libmozjs.so] Error 1
> gmake[3]: Leaving directory
> `/var/tmp/portage/mozilla-firefox-9999/work/mozilla/js/src'
> gmake[2]: *** [libs] Error 2
> gmake[2]: Leaving directory
`/var/tmp/portage/mozilla-firefox-9999/work/mozilla/js'
> gmake[1]: *** [tier_2] Error 2
> gmake[1]: Leaving directory `/var/tmp/portage/mozilla-firefox-9999/work/mozilla'
> make: *** [default] Error 2
> 
> The 9999 version means that it's a CVS build in gentoo.
> 
> Binutils is version 2.16.1 and glibc is 2.3.5. 
remove the  "&& [[ $(gcc-major-version) -eq 3 ]]" from the following line in the
ebuild 
if [[ ${ARCH} == amd64 ]] && [[ $(gcc-major-version) -eq 3 ]]; then
I thought it is bug only in gcc-3, probably the next ebuilds will remove the
check for the gcc version, till it is corrected here or in gentoo gcc-builds:
the patch works around this same bug-issue and could be found here:
http://groups.google.com/group/netscape.public.mozilla.builds/msg/a1427b0beb76525d

Comment 46

[Simon Strandman]

Yes i know how to work around it but I rather see this problem fixed than doing that. I still haven't got any response from the gcc guys but I guess they are busy finishing gcc 4.1.

Comment 47

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It would be nice to get some review on benjamin's patch (with the addition of -O2 on the test). Patching all my local trees is tedious.

Comment 48

[Robert O'Callahan (:roc) (email my personal email if necessary)]

BTW the reason everyone is shipping with this bug is because the bug will not be fixed in any gcc 4.0.x release.

Comment 49

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Oh, I just noticed I have another local change to fix a bug in benjamin's patch:

+    if test "$ac_cv_visibility_pragma" = "yes"; then
+      AC_DEFINE(HAVE_VISIBILITY_ATTRIBUTE)

should be

+      AC_DEFINE(HAVE_VISIBILITY_PRAGMA)

Comment 50

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

These patches need updating...

As for your last comment, ROC, it makes sense but if so, the patches for everything else need to change (they reference HAVE_VISIBILITY_ATTRIBUTE which now does not exist).

The patch needs to include configure, since configure is under CVS (for good reasons), and also patches should include all changes that go into CVS.

Comment 51

[Benjamin Smedberg]

HAVE_VIBISILITY_ATTRIBUTE should *not* be HAVE_VISIBILITY_PRAGMA.

It is Build Config module policy that patches should *not* include configure, only configure.in. configure is automatically committed by cltbld when configure.in changes.

Comment 52

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Ah, sorry about that. I found a use of _PRAGMA but it must have been my incorrect application of the patch.

I still need -O2 in the test though. And I still really want this checked in!

Comment 53

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

(In reply to comment #51)
> HAVE_VIBISILITY_ATTRIBUTE should *not* be HAVE_VISIBILITY_PRAGMA.

  Unless I misread the patch, the check preceding that line checks for the pragma.  So, why shouldn't it be HAVE_VISIBILITY_PRAGMA?  If HAVE_VISIBILITY_ATTRIBUTE is a subset of HAVE_VISIBILITY_HIDDEN_ATTRIBUTE, it should be done so and tested so.  The uses of HAVE_VISIBILITY_ATTRIBUTE don't use hidden, which makes sense - they use "__attribute__((visibility ("default")))".  But the test that sets that define is a test for the pragma.

> It is Build Config module policy that patches should *not* include configure,
> only configure.in. configure is automatically committed by cltbld when
> configure.in changes.

Ok, it makes sense that we have a known system generate configure - but it makes it impossible for someone to test a patch to configure.in without the exact "right" autoconf environment.  Perhaps people should attach a sample configure (not to be checked in) in a separate attachment to match the configure.in changes so others can test without having to install autoconf 2.13.

Comment 54

[Benjamin Smedberg]

Attachment: autoconf check for the GCC bug, rev. 4

Comment 55

[Benjamin Smedberg]

Just for explanation, this adds -O2 to the pragma-bug check, and it separates out the check for visibility("hidden") from the check for the pragma.

Comment 56

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 201517 [details] [diff] [review]
autoconf check for the GCC bug, rev. 4

Looks good to me but I'm no build expert. Let's ask caillon (or if he doesn't respond, Wolfgang)

Comment 57

[Mike Hommey [:glandium]]

Please also check for the "position (in)dependant" bug with pragma visibility. See comment #40 and http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=331460 for a testcase of the bug.

Comment 58

[Wolfgang Rosenauer [:wolfiR]]

Comment on attachment 201517 [details] [diff] [review]
autoconf check for the GCC bug, rev. 4

looks good to me

Comment 59

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #57)
> Please also check for the "position (in)dependant" bug with pragma visibility.
> See comment #40 and http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=331460 for
> a testcase of the bug.

How about we check this in, and file a followup bug to detect that issue

Comment 60

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

I agree, lets get this in.

Looks good to me now too, but I can't test it (because of the lack of autoconf 2.13).

Comment 61

[Benjamin Smedberg]

Fixed on trunk with a bustage fix for mac MKSHLIB.

Comment 62

[Mike Kaply [:mkaply]]

FYI, OS/2 needed the fix that was done for darwin as well.

Comment 63

[Benjamin Smedberg]

*** Bug 314836 has been marked as a duplicate of this bug. ***

Comment 64

[Zibi Braniecki [:zbraniecki][:gandalf]]

I was able to build Fx trunk on my amd64 gentoo machine.

Comment 65

[Benjamin Smedberg]

*** Bug 319012 has been marked as a duplicate of this bug. ***

Comment 66

[Benjamin Smedberg]

Comment on attachment 201517 [details] [diff] [review]
autoconf check for the GCC bug, rev. 4

I'd like to pull this in for ffox2 (with the darwin/OS2 fixes), it's not no-risk but it's baked on trunk and it will be good to make our sources build out of the box on linux distros.

Comment 67

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I just tried a 1.8 branch build with SUSE10 gcc 4.1.something and it failed with this patch. The problem is that HAVE_VISIBILITY_PRAGMA is not defined outside of nsprpub, which means PR_IMPLEMENT() and its synonyms do not cause their symbols to be exported.

Comment 68

[Wan-Teh Chang]

roc: you need to also apply the NSPR patch I described
in comment 29.  It is best to generate the patch using CVS
because the patch in comment 29 is missing a backslash for
line continuation.

cvs diff -u -r3.30 -r3.31 mozilla/nsprpub/pr/include/prtypes.h

Comment 69

[Benjamin Smedberg]

Comment on attachment 201517 [details] [diff] [review]
autoconf check for the GCC bug, rev. 4

WTC, I believe that I can put this on the 1.8 branch without changing NSPR, can you verify that there aren't changes between NSPR on the 1.8 branch and NSPR on trunk that would affect this patch?

Comment 70

[Wan-Teh Chang]

Comment on attachment 201517 [details] [diff] [review]
autoconf check for the GCC bug, rev. 4

Ben, I confirm that MOZILLA_1_8_BRANCH already has
the NSPR change required by this patch (the patch
attachment 199459 [details] [diff] [review] in bug 312361).

Comment 71

[Kai Engert [:KaiE:]]

I'm checking this in now, as I need it badly, as my primary work is a x86_64 machine. I understand this has all the required approvals. I also verified it makes my build work. In addition to the attached patch, I'll check in the two bustage fixes for darwin and os/2.