Closed Bug 319012 Opened 16 years ago Closed 16 years ago

Building with visibility hidden pragma creates position dependent code


(Firefox Build System :: General, defect)

Not set


(Not tracked)



(Reporter: glandium, Assigned: benjamin)




(1 file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; ja-JP; rv:1.8) Gecko/20051118 Debian/1.4.99+1.5rc3.dfsg-1 Firefox/1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; ja-JP; rv:1.8) Gecko/20051118 Debian/1.4.99+1.5rc3.dfsg-1 Firefox/1.5

Suggested in bug #307168 to file a new bug for that.

Due to a bug in gcc, if you use the visibility pragma to set the hidden visibility, instead of using -fvisibility=hidden, the resulting binary will be position dependent code.

The following is a testcase:
#pragma GCC visibility push(hidden)
#pragma GCC visibility push(default)
#include <string.h>
#pragma GCC visibility pop

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

This gives position dependent code:
mh@namakemono:~$ gcc -fPIC -shared -o test.c
mh@namakemono:~$ objdump --private-headers | grep TEXTREL
  TEXTREL     0x0

If you removing all pragmas in the source code, and build with
-fvisibility=hidden, you get:
mh@namakemono:~$ gcc -fPIC -shared -fvisibility=hidden -o test-nopragma.c
mh@namakemono:~$ objdump --private-headers | grep TEXTREL

Mozilla should just be building with -fvisibility=hidden in all cases, instead of the current situation, or at least check for that bug (it might be fixed some time in gcc 4.1)

Reproducible: Always

Steps to Reproduce:

*** This bug has been marked as a duplicate of 307168 ***
Closed: 16 years ago
Resolution: --- → DUPLICATE
A separate bug for what? The configure test is in place on the trunk and it works properly for me... are you trying this on branch or trunk?

Reopen this if you mean something different than the configure test which is already in place. The pragmas provide significant advantages over -fvisibility=hidden
reopening, this bug is about normal x86 too, as far as I can tell. the bug isn't that builds don't work, but that there are text relocations, i.e. that the code for the libraries isn't PIC.
Resolution: DUPLICATE → ---
1) relocations to hidden symbols (Func in the example code) should be text relocations, that's why we have hidden visibility
2) relocations to normal symbols (memset in the example) should be PIC relocations

Are you saying that we're generating a text relocation to "memset"? That is what my configure test is checking for and that should fail at link-time, no (and be caught by the existing configure test)?
er, really? as per all text relocations are rather a bad thing...
Benjamin, what you seem to not get is that my testcase shows that in one case of hidden symbols we get text relocations (with pragma) and in the other, we don't (with -fvisibility=hidden). The result is supposed to be the same between the two builds, but it's not.
Plus, there shouldn't be text relocations for hidden symbols, anyway.
Possibly, though perhaps by accident: that bug was about class-level visibility modifiers, not builtins.
Assignee: nobody → benjamin
Ever confirmed: true
The underlying cause of this is that text relocations fail to link on x86_64, but they will link on x86. (Nobody has explained this to me properly... is there a linker flag that will force them to fail at link time on x86?) This patch checks for @PLT in the assembly, instead of checking for completed linkage.
Attachment #220144 - Flags: review?(bryner)
Attachment #220144 - Flags: review?(bryner) → review+
Fixed on trunk.
Closed: 16 years ago16 years ago
Resolution: --- → FIXED
Comment on attachment 220144 [details] [diff] [review]
Check the assembly for "@PLT", rev. 1

+                       rm -rf conftest.{c,S}

why recursive?
and why grep -C ?
(In reply to comment #13)
> and why grep -C ?

that was filed as bug 335949 (coincidentally?)
thanks for pointing me the bug.
in fact it was pure coincidence :)
*** Bug 344821 has been marked as a duplicate of this bug. ***
It'd be nice to land that on mozilla1.8 branch.
Depends on: 353150
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.