Closed Bug 366844 Opened 17 years ago Closed 17 years ago

[configure] visibility pragma support broken

Categories

(Firefox Build System :: General, defect)

1.8 Branch
x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: guenther, Assigned: guenther)

References

Details

(Keywords: fixed1.8.1.4)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.4) Gecko/20060607 Firefox/1.5.0.4
Build Identifier: 

The configure test for the visibility pragma seems to be broken. In my case it claims the pragma to be supported, which breaks the build. Explicitely disabling it makes the build happy again.

Firefox 2.0.0.1

gcc (GCC) 3.4.3 (Mandrakelinux 10.2 3.4.3-7mdk)
GNU ld version 2.15.92.0.2 20040927

$ uname -s -m -p -i
Linux i686 Intel(R) Pentium(R) M processor 1.73GHz unknown

Note: I had no such issue building FF 1.5.x.x on the very same machine and system. I first encountered this issue since FF 2.0.


Reproducible: Always

Steps to Reproduce:
Build FF... ;)

Actual Results:  
The relevant configure report for the *broken* build, will attach the build error to not break lines:

 checking for visibility(hidden) attribute... yes
 checking for visibility(default) attribute... yes
 checking for visibility pragma support... yes
 checking For x86_64 gcc visibility bug with builtins (GCC bug 20297)... no

Configure output after explicitely *disabling* visibility pragma support by adding "ac_cv_visibility_pragma=no" to mozconfig-firefox:

 checking for visibility(hidden) attribute... yes
 checking for visibility(default) attribute... yes
 checking for visibility pragma support... (cached) no



If you need any more details, please feel free to ask.
Product: Firefox → Core
QA Contact: build.config → build-config
Version: unspecified → 1.8 Branch
Attached file build error
The User-Agent: reported above of course is not the one I am talking about, but the currently used one. ;)

I found the workaround here:
http://benjamin.smedbergs.us/blog/2005-10-27/gcc-40-workaround
http://benjamin.smedbergs.us/blog/2006-03-28/gcc-and-visibility-one-step-forward-hit-a-brick-wall/

This seems not to be limited to GCC 4 only. The "unresolvable relocation" is common to my issue and the one in these links, although for different symbols (and FF versions).
Does the trunk build properly? I think that there are trunk configure patches which were not backported to the branches.
Some details of a *different* user, who got a similar relocation error:

$ uname -a
Linux domo-debian 2.6.19.1.2007-01-04 #1 PREEMPT Thu Jan 4 11:18:32 EET 2007 x86_64 GNU/Linux

$ gcc -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --program-suffix=-4.1 --enable-__cxa_atexit --enable-clocale=gnu --enable-libstdcxx-debug --enable-mpfr --enable-checking=release x86_64-linux-gnu
Thread model: posix
gcc version 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)

$ ld --version
GNU ld version 2.17 Debian GNU/Linux


The build error (straight from my IRC logs):

g++ -I/home/domas/garnome/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 -I/home/domas/garnome/include -L/home/domas/garnome/lib -O2 -pipe -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -O -fPIC -shared -Wl,-z,defs -Wl,-h,libxpcom_core.so -o libxpcom_core.so  pldhash.o nsCOMPtr.o nsComponentManagerUtils.o nsDebug.o nsID.o nsIInterfaceRequestorUtils.o nsINIParser.o nsMemory.o nsTraceRefcnt.o nsWeakReference.o nsGREGlue.o nsVersionComparator.o nsTHashtable.o nsTArray.o nsGenericFactory.o nsXPComInit.o nsStringAPI.o    -Wl,--export-dynamic -L/home/domas/garnome/lib   -Wl,--whole-archive ../../dist/lib/libxpcomds_s.a ../../dist/lib/libxpcomio_s.a ../../dist/lib/libxpcomcomponents_s.a ../../dist/lib/libxpcomthreads_s.a ../../dist/lib/libxpcomproxy_s.a ../../dist/lib/libxpcombase_s.a ../../dist/lib/libxptcall.a ../../dist/lib/libxptinfo.a ../../dist/lib/libxpt.a ../../dist/lib/libxptcmd.a ../../dist/lib/libstring_s.a  -Wl,--no-whole-archive  -L../../dist/lib -lplds4 -lplc4 -lnspr4 -L/home/domas/garnome/lib -lpthread -ldl -ldl -lm
/usr/bin/ld: nsCOMPtr.o: relocation R_X86_64_PC32 against `nsGetServiceByContractIDWithError::operator()(nsID const&, void**) const' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: Bad value


Adding "ac_cv_visibility_pragma=no" to mozconfig-firefox fixed the issue for him, too.
(In reply to comment #3)
> Does the trunk build properly? I think that there are trunk configure patches
> which were not backported to the branches.

Ah, the one who provided the workaround himself. :)

I did not try this with trunk, just 2.0 and 2.0.0.1. I will try to give trunk a test the next days.


Btw, are there any downsides of disabling this pragma? Any impact?
The pragma, when it works correctly, reduces the number of dynamic relocations in mozilla code and improves startup time significantly... somewhere between 5-15% in some tests.
I see, thanks Benjamin. :)

What we (the GARNOME Team) did, is this:  Get the latest Firefox sources from CVS HEAD, identify the visibility pragma test in configure.in and substitute the one from 2.0.0.1 with the latest code. Find an old autoconf 2.13 ;) and regenerate configure. Results from running ./configure:

 checking for visibility(hidden) attribute... yes
 checking for visibility(default) attribute... yes
 checking for visibility pragma support... yes
 checking For gcc visibility bug with class-level attributes (GCC bug 26905)...
  yes
 checking For x86_64 gcc visibility bug with builtins (GCC bug 20297)... no

Unlike the current 2.0.0.1 test, latest code identifies a gcc visibility bug to be present.


[Currently running the build to verify... Will report back later.]

So, given this upstream code works fine, what are the odds for this to be backported soon, like... 2.0.0.2? :)


Btw, I can provide the patch I used for this on request. ;)
Confirmed. Build finished successfully with the visibility pragma test from HEAD.

(Again, this still is Firefix 2.0.0.1, I just changed some short part of configure to use the latest code from HEAD.)
If you can provide a backport patch then yes, I will consider it. But please note that when I made the configure changes there were code changes required as well. I don't think a patch that touches just configure will be sufficient.

Just a note, when attaching patches here please don't diff configure, only configure.in
Status: UNCONFIRMED → NEW
Ever confirmed: true
Great. :)  I will test the patch with the other user (see comment 4) first. If it works as expected, I'll attach the proposed patch for review here.

Thanks, Benjamin.
Confirmed, the configure.in patch works for the other user (see comment 4, totally different system) as well.

The original configure results for the relevant visibility pragma tests are identical to the ones in comment 0. With the patch in place and regenerating configure, the results are identical to comment 7. Build finished successfully, without the need to edit mozconfig.
As mentoined in comment 7, this is a straightforward backport of the latest configure tests for the GCC visibility bugs. I believe the patch to be isolated and not require further code changes, since it simply extends the tests to identify GCC visibility bugs and requires both to not be present.
Benjamin, did you have a chance to review the patch yet?

(FWIW, a hand crafted configure patch based on this one just has been added to GARNOME trunk.)
Comment on attachment 251463 [details] [diff] [review]
proposed fix, backported GCC visibility bug tests

Karsten, in generally you should set the review? flag to me to request review. I'm setting the flag now to add it to my queue.
Attachment #251463 - Flags: review?(benjamin)
Oops. Thanks for the info. I'm not that used to the Mozilla bugzilla but the GNOME way of handling bugs... I will remember this bit. :)
Comment on attachment 251463 [details] [diff] [review]
proposed fix, backported GCC visibility bug tests

Yeah, I had already ported the other bits. I really mean to be requesting approval1.8.1.3?, but that flag isn't available so I'm setting 1.8.1.2 and I'll let the drivers set the correct flags.
Attachment #251463 - Flags: review?(benjamin)
Attachment #251463 - Flags: review+
Attachment #251463 - Flags: approval1.8.1.2?
Flags: wanted1.8.1.x+
Comment on attachment 251463 [details] [diff] [review]
proposed fix, backported GCC visibility bug tests

bsmedberg:  Clearing the blocking flag for 1.8.1.2 to get this off our radar for the upcoming release.

If you want this in 1.8.1.3, please renominate when those flags are available. Thanks.
Attachment #251463 - Flags: approval1.8.1.2?
Comment on attachment 251463 [details] [diff] [review]
proposed fix, backported GCC visibility bug tests

Requesting approval1.8.1.3? as per comment 16.
Attachment #251463 - Flags: approval1.8.1.3?
Also requesting blocking1.8.1.3?, since I have been told on IRC that this is the way to go. :)
Flags: blocking1.8.1.3?
Karsten: assigning to you to push this in. Please get someone to check this in on the trunk for sanity-checking before we approve the patch. Not blocking any release, but we will approve this build change once it's been vetted for unexpected bustage on the trunk.
Assignee: nobody → guenther
Flags: blocking1.8.1.4? → blocking1.8.1.4-
Whiteboard: needs trunk checkin
Daniel, I'm not sure if you get you. Maybe it's just the subtle lingo difference between GNOME and Mozilla bugzilla. ;)

Please note that I do not have commit permissions. Which the assigning is about, no? I'd need someone to do this on my behalf, please. I can provide the missing ChangeLog bit, if you want me to.  Benjamin? :)

Also, this patch is a straight backport from trunk (aka HEAD). See comment 7 and comment 12. This code is already part of trunk, and thus committing and sanity- checking this particular patch on trunk won't really work.

As I understand it, Benjamnin already reviewed the patch, and even meant to request approval1.8.1.3? himself, which unfortunately hasn't been available at that stage. I'm kind of confused how to go on and finally get this patch backported.


Regarding "not blocking": Sorry, if this was wrong. I just set it according to the advice in #firefox. I'm not used to the Mozilla way of processing patches.
Dan, this code is already landed on trunk, and should be safe for the branch.
Comment on attachment 251463 [details] [diff] [review]
proposed fix, backported GCC visibility bug tests

approved for 1.8.1.4, a=dveditz for release-drivers
Attachment #251463 - Flags: approval1.8.1.4? → approval1.8.1.4+
Whiteboard: needs trunk checkin
Fixed on MOZILLA_1_8_BRANCH
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: fixed1.8.1.4
Resolution: --- → FIXED
This might have caused a build problem on SeaMonkey Mac, see my post in m.d.builds
Robert, does trunk have the same issue for a long time already?

Please note, that this patch is a straight backport from trunk of some configure-time detection magic, which has been used in trunk before.
Still, the same library apparently links fine on trunk but doesn't on branch. Something clearly _is_ different, and it started happening the first time configure was run by that box after the checkin for this bug.
See the newsgroup for the details.
Blocks: 377210
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.