319012 - Building with visibility hidden pragma creates position dependent code

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

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.so test.c
mh@namakemono:~$ objdump --private-headers test.so | 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.so test-nopragma.c
mh@namakemono:~$ objdump --private-headers test.so | 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:

Comment 1

[Benjamin Smedberg]

*** This bug has been marked as a duplicate of 307168 ***

Comment 2

[Benjamin Smedberg]

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

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

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.

Comment 4

[Benjamin Smedberg]

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)?

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

er, really? as per http://people.redhat.com/drepper/dsohowto.pdf all text relocations are rather a bad thing...

Comment 6

[Mike Hommey [:glandium]]

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.

Comment 7

[Mike Hommey [:glandium]]

Plus, there shouldn't be text relocations for hidden symbols, anyway.

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

did bug 334866 fix this?

Comment 9

[Benjamin Smedberg]

Possibly, though perhaps by accident: that bug was about class-level visibility modifiers, not builtins.

Comment 10

[Benjamin Smedberg]

Attachment: Check the assembly for "@PLT", rev. 1

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.

Comment 11

[Benjamin Smedberg]

Fixed on trunk.

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 220144 [details] [diff] [review]
Check the assembly for "@PLT", rev. 1

+                       rm -rf conftest.{c,S}

why recursive?

Comment 13

[Gergan Penkov]

and why grep -C ?

Comment 14

[:Gavin Sharp [email: [e-mail]]]

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

that was filed as bug 335949 (coincidentally?)

Comment 15

[Gergan Penkov]

thanks for pointing me the bug.
in fact it was pure coincidence :)

Comment 16

[Benjamin Smedberg]

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

Comment 17

[Mike Hommey [:glandium]]

It'd be nice to land that on mozilla1.8 branch.