Bug 15574 - Gold is overly pedantic for dynamic references to symbols with hidden visibility
Summary: Gold is overly pedantic for dynamic references to symbols with hidden visibility
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: gold (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Cary Coutant
URL:
Keywords:
: 18596 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-06-04 20:52 UTC by law
Modified: 2015-07-20 16:29 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description law 2013-06-04 20:52:20 UTC
$ cat t.c
#include <stdio.h>
#include <dlfcn.h>

void foo(void) __attribute__((visibility("hidden")));
void foo(void) {
  puts("In executable: foo - before forwarding to DSO");
  ((void(*)(void))dlsym(RTLD_DEFAULT,"foo"))();
  puts("In executable: foo - after forwarding to DSO");
}

void bar(void);

int main() {
  foo();
  bar();
}

$ cat u.c
#include <stdio.h>

void foo(void);
void bar(void) {
  puts("In DSO: bar");
  foo();
}

$ cat v.c
#include <stdio.h>

void foo(void) {
  puts("In DSO: foo");
}


$ gcc v.c -fPIC -shared -olibv.so
$ gcc u.c -fPIC -shared -olibu.so
$ gcc -D_GNU_SOURCE t.c -L. -lu -lv -ldl -Wl,-rpath,`pwd`


When linking with the bfd linker, there is no error or warning.  However, linking with gold produces:

/usr/bin/ld: warning: hidden symbol 'foo' in /tmp/ccAF4VIi.o is referenced by DSO ./libu.so


Note we have two definitions of "foo".  One with default visibilty in libv and one that is hidden (main executable) and a reference to foo with default visibility (libu). 


From the ELF standard:

---
 None of the visibility attributes affects resolution of symbols within an executable or shared object during link-editing -- such resolution is controlled by the binding type. Once the link-editor has chosen its resolution, these attributes impose two requirements, both based on the fact that references in the code being linked may have been optimized to take advantage of the attributes.

    * First, all of the non-default visibility attributes, when applied to a symbol reference, imply that a definition to satisfy that reference must be provided within the current executable or shared object. If such a symbol reference has no definition within the component being linked, then the reference must have STB_WEAK binding and is resolved to zero.

    * Second, if any reference to or definition of a name is a symbol with a non-default visibility attribute, the visibility attribute must be propagated to the resolving symbol in the linked object. If different visibility attributes are specified for distinct references to or definitions of a symbol, the most constraining visibility attribute must be propagated to the resolving symbol in the linked object. The attributes, ordered from least to most constraining, are: STV_PROTECTED, STV_HIDDEN and STV_INTERNAL.
---

Note carefully that visibility attributes do not affect symbol resolution (which is controlled by the binding type).  Based on that wording I believe the warning from gold is unnecessary and confusing.
Comment 1 John Marino 2014-05-12 14:17:43 UTC
Is there a patch or workaround for this?

I definitely want to turn off this warning.

I'm still seeing this on binutils 2.24 gold linker.
Comment 2 Cary Coutant 2014-06-12 17:27:07 UTC
We print this warning before we see the shared object that contains the non-hidden definition. Since gold (unlike BFD ld) does not try to emulate the full binding behavior of the dynamic linker, there's really no way we can tell for sure if there's going to be a non-hidden definition at runtime, so I think the only feasible way to eliminate the false positives is to eliminate this warning entirely.
Comment 3 John Marino 2014-06-12 18:10:14 UTC
This is exactly what I ended up doing, by the way:

--- gold/resolve.cc.orig	2013-11-04 15:33:39.000000000 +0000
+++ gold/resolve.cc
@@ -276,8 +276,7 @@ Symbol_table::resolve(Sized_symbol<size>
       to->set_in_reg();
     }
   else if (st_shndx == elfcpp::SHN_UNDEF
-           && (to->visibility() == elfcpp::STV_HIDDEN
-               || to->visibility() == elfcpp::STV_INTERNAL))
+           && (to->visibility() == elfcpp::STV_INTERNAL))
     {
       // A dynamic object cannot reference a hidden or internal symbol
       // defined in another object.
Comment 4 Cary Coutant 2014-06-12 18:34:10 UTC
> --- gold/resolve.cc.orig    2013-11-04 15:33:39.000000000 +0000
> +++ gold/resolve.cc
> @@ -276,8 +276,7 @@ Symbol_table::resolve(Sized_symbol<size>
>        to->set_in_reg();
>      }
>    else if (st_shndx == elfcpp::SHN_UNDEF
> -           && (to->visibility() == elfcpp::STV_HIDDEN
> -               || to->visibility() == elfcpp::STV_INTERNAL))
> +           && (to->visibility() == elfcpp::STV_INTERNAL))
>      {
>        // A dynamic object cannot reference a hidden or internal symbol
>        // defined in another object.

With your patch, the hidden symbol will end up in the dynamic symbol
table. See PR 10471. I think the proper patch would be to remove the
warning, but still return immediately.

-cary
Comment 5 Cary Coutant 2015-07-20 15:34:03 UTC
*** Bug 18596 has been marked as a duplicate of this bug. ***
Comment 6 Sourceware Commits 2015-07-20 16:19:43 UTC
The master branch has been updated by Cary Coutant <ccoutant@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c20ceeb226168ffd84078ef74d890c2b7f69a435

commit c20ceeb226168ffd84078ef74d890c2b7f69a435
Author: Yiran Wang <yiran@google.com>
Date:   Mon Jul 20 08:47:57 2015 -0700

    Remove warning about references from shared objects to hidden symbols.
    
    gold/
    	PR gold/15574
    	* resolve.cc (Symbol_table): Remove warning about references
    	from shared objects to hidden symbols.
    	* testsuite/Makefile.am (hidden_test): Add hidden_test.syms.
    	* testsuite/Makefile.in: Regenerate.
    	* testsuite/hidden_test.sh: Check dynamic symbol table; update
    	expected error messages.
Comment 7 Cary Coutant 2015-07-20 16:23:28 UTC
Fixed on master.