Bug 13600

Summary: protected visibility creates bogus relocation
Product: binutils Reporter: Richard Biener <rguenth>
Component: ldAssignee: Not yet assigned to anyone <unassigned>
Status: NEW ---    
Severity: normal CC: amodra, bugdal, hjl.tools, sam
Priority: P2    
Version: 2.22   
Target Milestone: ---   
See Also: https://sourceware.org/bugzilla/show_bug.cgi?id=584
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19520
Host: Target:
Build: Last reconfirmed:

Description Richard Biener 2012-01-17 12:32:11 UTC
.file   "t1.c"
        .text
        .globl  foo
        .protected      foo
        .type   foo, @function
foo:
.LFB0:
        .cfi_startproc
        pushq   %rbp
        .cfi_def_cfa_offset 16
        .cfi_offset 6, -16
        movq    %rsp, %rbp
        .cfi_def_cfa_register 6
        leaq    foo(%rip), %rax
        popq    %rbp
        .cfi_def_cfa 7, 8
        ret
        .cfi_endproc
.LFE0:
        .size   foo, .-foo
        .ident  "GCC: (GNU) 4.7.0 20120116 (experimental)"
        .section        .note.GNU-stack,"",@progbits

does not link:

/usr/lib64/gcc/x86_64-suse-linux/4.3/../../../../x86_64-suse-linux/bin/ld: /tmp/cc43SbHJ.o: relocation R_X86_64_PC32 against protected symbol `foo' can not be used when making a shared object
/usr/lib64/gcc/x86_64-suse-linux/4.3/../../../../x86_64-suse-linux/bin/ld: final link failed: Bad value
collect2: error: ld returned 1 exit status


C testcase:

__attribute__((visibility("protected"))) void * foo (void) { return (void *)foo; }

gcc t1.c -o libt1.so -shared -fPIC.

Sounds similar to PR584, but this time with a self-reference.
Comment 1 H.J. Lu 2012-01-17 17:25:44 UTC
I think it is a gcc bug:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19520
Comment 2 Alan Modra 2012-01-18 01:47:22 UTC
I think this is a ld bug.  For protected foo, leaq foo(%rip),%rax ought to resolve to the local foo function address.  No dynamic reloc necessary.
Comment 3 Richard Biener 2012-01-18 09:13:09 UTC
(In reply to comment #1)
> I think it is a gcc bug:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19520

That one mixes in the issue of comparing function addresses of protected
symbols - that's sth entirely different (and a QOI issue, which is
probably why ICC generates the more funky variant,
see http://gcc.gnu.org/ml/gcc-bugs/2012-01/msg01892.html).
Comment 4 H.J. Lu 2012-01-18 16:52:10 UTC
Both ld and ld.so go extra efforts to make sure that
the same function pointer value is used for protected
function in the entire process.  If we drop this requirement,
we can simplify ld and ld.so quite a bit.
Comment 5 Richard Biener 2012-01-19 09:16:32 UTC
(In reply to comment #4)
> Both ld and ld.so go extra efforts to make sure that
> the same function pointer value is used for protected
> function in the entire process.  If we drop this requirement,
> we can simplify ld and ld.so quite a bit.

Well, if we make sure ld.so resolves the GOT entry to the function
address instead of the PLT then that will work - in the module
the protected symbol binds locally we get it resolved to the function
address directly anyway (see GCC assembly from the testcase).

Sounds easier allover the place, no?
Comment 6 Alan Modra 2012-01-19 10:00:36 UTC
In comment #2 I was forgetting what we do in an executable.  In the main executable (which might be non-PIC), if we take the address of foo and the address of foo is written into a read-only section (.text or .rodata or somesuch) then we have a problem.  You've got three choices:
a) either the address location needs a dynamic relocation which results in that page of the executable being non-shared, or
b) we need the hack of making the address of foo be the plt code for foo in the executable, with all the pain in ld.so that causes, or
c) you ensure that function addresses never appear in read-only sections.

At the moment we do (b), so I was wrong to say this was a ld bug unless it's true that x86_64 gcc already does (c).
Comment 7 rguenther 2012-01-19 10:22:22 UTC
On Thu, 19 Jan 2012, amodra at gmail dot com wrote:

> http://sourceware.org/bugzilla/show_bug.cgi?id=13600
> 
> --- Comment #6 from Alan Modra <amodra at gmail dot com> 2012-01-19 10:00:36 UTC ---
> In comment #2 I was forgetting what we do in an executable.  In the main
> executable (which might be non-PIC), if we take the address of foo and the
> address of foo is written into a read-only section (.text or .rodata or
> somesuch) then we have a problem.  You've got three choices:
> a) either the address location needs a dynamic relocation which results in that
> page of the executable being non-shared, or
> b) we need the hack of making the address of foo be the plt code for foo in the
> executable, with all the pain in ld.so that causes, or
> c) you ensure that function addresses never appear in read-only sections.
> 
> At the moment we do (b), so I was wrong to say this was a ld bug unless it's
> true that x86_64 gcc already does (c).

Why is it not a ld bug that it doesn't do what GCC asks it to do?
That is, bind foo locally?  It sure can do that.  Whether what GCC
asks it to do is "bad" is a completely different issue, no?

For the issue you bring up you are thinking of

extern void foo (void);
const void *p = (void *)foo;

?  Regardless of if the app is PIC, PIE or not GCC generates

.globl p
        .data
        .align 8
        .type   p, @object
        .size   p, 8
p:
        .quad   foo

thus, it doesn't put p into .rodata (it needs a relocation after all).

So, how would you put those function address into a readonly section?
Comment 8 Alan Modra 2012-01-19 10:50:45 UTC
const void *const p = (void *)foo; might be more interesting.  Or

const struct blah {
  int (*f) ();
  int x,y,z,w;
} f = { .f = printf };
Comment 9 rguenther 2012-01-19 11:05:40 UTC
On Thu, 19 Jan 2012, amodra at gmail dot com wrote:

> http://sourceware.org/bugzilla/show_bug.cgi?id=13600
> 
> --- Comment #8 from Alan Modra <amodra at gmail dot com> 2012-01-19 10:50:45 UTC ---
> const void *const p = (void *)foo; might be more interesting.  Or
> 
> const struct blah {
>   int (*f) ();
>   int x,y,z,w;
> } f = { .f = printf };

Both go to .rodata.  But that of course has the same problems, whether
foo's defintion is protected or default?
Comment 10 Alan Modra 2012-01-19 11:57:15 UTC
That's why you make the address of foo the plt entry for foo in the executable.  The address is fixed at link time.  However, it does mean that address of a default or protected visibility foo in a shared lib must have a relocation.
Comment 11 rguenther 2012-01-19 12:04:23 UTC
On Thu, 19 Jan 2012, amodra at gmail dot com wrote:

> http://sourceware.org/bugzilla/show_bug.cgi?id=13600
> 
> --- Comment #10 from Alan Modra <amodra at gmail dot com> 2012-01-19 11:57:15 UTC ---
> That's why you make the address of foo the plt entry for foo in the executable.

Well, if I put that constant into a shared library _and_ I put one into
the executable I'm still lost, no?

Btw, with -fPIC or -fPIE GCC puts the constants in .data.rel.ro
instead.

>  The address is fixed at link time.  However, it does mean that address of a
> default or protected visibility foo in a shared lib must have a relocation.

Which means a lost optimization (we need to go through the GOT here).

But well, I guess it's supposed to pay off in the executable which
requires only one relocation for each address-taken function.

Still, if GCC tells ld to resolve the address of a protected visibility
foo in a shared lib without a relocation why can't it simply do so
as surely it is techincally possible?
Comment 12 Alan Modra 2012-01-19 12:49:02 UTC
> Well, if I put that constant into a shared library _and_ I put one into
> the executable I'm still lost, no?

No, because shared libraries must be -fpic/PIC and as you note

> Btw, with -fPIC or -fPIE GCC puts the constants in .data.rel.ro
> instead.

and .data.rel.ro can have relocs.

> Which means a lost optimization (we need to go through the GOT here).

Choose your poison.  :)

> Still, if GCC tells ld to resolve the address of a protected visibility
> foo in a shared lib without a relocation why can't it simply do so
> as surely it is techincally possible?

It is possible, but will break function pointer comparisons if a pointer to foo is passed between the shared lib and executable in either direction, since the address of foo in the shared lib and the address of the same foo in the executable will be different.
Comment 13 rguenther 2012-01-19 13:22:03 UTC
On Thu, 19 Jan 2012, amodra at gmail dot com wrote:

> http://sourceware.org/bugzilla/show_bug.cgi?id=13600
> 
> --- Comment #12 from Alan Modra <amodra at gmail dot com> 2012-01-19 12:49:02 UTC ---
> > Well, if I put that constant into a shared library _and_ I put one into
> > the executable I'm still lost, no?
> 
> No, because shared libraries must be -fpic/PIC and as you note
> 
> > Btw, with -fPIC or -fPIE GCC puts the constants in .data.rel.ro
> > instead.
> 
> and .data.rel.ro can have relocs.
> 
> > Which means a lost optimization (we need to go through the GOT here).
> 
> Choose your poison.  :)
> 
> > Still, if GCC tells ld to resolve the address of a protected visibility
> > foo in a shared lib without a relocation why can't it simply do so
> > as surely it is techincally possible?
> 
> It is possible, but will break function pointer comparisons if a pointer to foo
> is passed between the shared lib and executable in either direction, since the
> address of foo in the shared lib and the address of the same foo in the
> executable will be different.

True.  Just GCC did it that way since forever ... I don't have older ld
to verify if only now ld rejects this behavior (there is certainly a set
of programs that were not affected by this "bug" of GCC that now ld 
breaks).

We'll fix the GCC issue on the GCC side.

Richard.
Comment 14 Rich Felker 2012-05-28 15:27:40 UTC
Binutils' 2.17 ld did not have this issue; it accepted the relocations and generated working binaries (albeit with the function pointer mismatch issue, which is a GCC issue, not binutils' responsibility). I think rejecting semantically valid ELF relocations solely for the purpose of preventing potentially-buggy code generated by GCC from getting linked is bad policy, and the behavior should be reverted. GCC can be fixed to lookup protected function pointers through the GOT, and perhaps even add a new visibility variant for the old behavior (which is highly desirable if you don't intend to pass function pointers in and out of the library).
Comment 15 Rich Felker 2012-10-12 19:48:59 UTC
Ping. Is anybody willing to look at this?

This bug has been open 10 months now and fixing it is as simple as removing the offending code that's enforcing policy to protest a GCC bug. The relocations are semantically valid in ELF (even if the object code was not correctly generated from the C, which is GCC's responsibility), so binutils should allow them to be processed.

Short of a good argument why the current behavior should be kept, I think the change should just be reverted. I can go lookup the commit where it changed and make a patch reverting it if anybody is willing to look at it (and I might do this anyway since I want this patch for local use, and people packaging musl-based distributions/base-systems are also interested in the patch which would allow us to safely use -fvisibility=protected to achieve significant size and performance gains).
Comment 16 Jackie Rosen 2014-02-16 16:59:04 UTC Comment hidden (spam)