diff mbox

RFC/RFA: MN10300: Fix handling of protected functions in shared libraries.

Message ID m3d3j9bfy5.fsf@redhat.com
State New
Headers show

Commit Message

Nick Clifton May 23, 2011, 3:21 p.m. UTC
Hi Alex, Hi Jeff, Hi Richard,

  Consider the following small test case:

    % cat test1.c
    extern int g (void) __attribute__ ((visibility("protected")));
    int f (void) { return g (); }

    % cat test2.c
    extern int g(void) __attribute__ ((visibility("protected")));
    int i;
    int g (void) { return i; }

    % gcc -fPIC -c test1.c test2.c
    % gcc -shared -o libtest_protected.so test1.o test2.o

  When compiled with a MN10300 toolchain based on the current FSF GCC
  and binutils sources the final link will fail with:
    
    test1.o: In function `f':
    test1.c:(.text+0x7): warning: error: inappropriate relocation type for shared library (did you forget -fpic?)

  The problem here is that GCC has decided that since "g" is protected
  it does not need a PLT entry.  But the linker has decided that since
  "g" is a function it does need a PLT entry (even though it is
  protected) so that function pointer comparison will work.  (See the
  definition of SYMBOL_REFERENCES_LOCAL in bfd/elf-bfd.h for more
  information on this).

  I have a small patch that fixes this problem (see below), but I am not
  sure if this is the correct solution.  As far as I can see, this is
  not an MN10300 specific problem however, so surely other targets ought
  to have similar problems ?  (But other targets do not seem to use
  targetm.binds_local_p to decide if a symbol is global or local.  cf/
  mn10300_encode_section_info).

  Anyway any advice or comments on the situation would be appreciated.

Cheers
  Nick

Comments

Jeff Law May 23, 2011, 4:16 p.m. UTC | #1
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/23/11 09:21, Nick Clifton wrote:
> Hi Alex, Hi Jeff, Hi Richard,
> 
>   Consider the following small test case:
> 
>     % cat test1.c
>     extern int g (void) __attribute__ ((visibility("protected")));
>     int f (void) { return g (); }
> 
>     % cat test2.c
>     extern int g(void) __attribute__ ((visibility("protected")));
>     int i;
>     int g (void) { return i; }
> 
>     % gcc -fPIC -c test1.c test2.c
>     % gcc -shared -o libtest_protected.so test1.o test2.o
> 
>   When compiled with a MN10300 toolchain based on the current FSF GCC
>   and binutils sources the final link will fail with:
>     
>     test1.o: In function `f':
>     test1.c:(.text+0x7): warning: error: inappropriate relocation type for shared library (did you forget -fpic?)
> 
>   The problem here is that GCC has decided that since "g" is protected
>   it does not need a PLT entry.  But the linker has decided that since
>   "g" is a function it does need a PLT entry (even though it is
>   protected) so that function pointer comparison will work.  (See the
>   definition of SYMBOL_REFERENCES_LOCAL in bfd/elf-bfd.h for more
>   information on this).
> 
>   I have a small patch that fixes this problem (see below), but I am not
>   sure if this is the correct solution.  As far as I can see, this is
>   not an MN10300 specific problem however, so surely other targets ought
>   to have similar problems ?  (But other targets do not seem to use
>   targetm.binds_local_p to decide if a symbol is global or local.  cf/
>   mn10300_encode_section_info).
> 
>   Anyway any advice or comments on the situation would be appreciated.
This isn't something I have much experience with; presumably we must
have the PLT entry even though "g" is considered protected?  If so, then
ISTM GCC will need to cope/adapt and your approach seems as good as any
other to me -- unless we wanted to move the code into a more generic
location.

jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJN2ohHAAoJEBRtltQi2kC7lpcH/ihLGKBhkB7PG2ZjkXZJBH2l
WMuG7lVDSKiveqLTCI5o4bXRue2Xt+cusYGn/+nUGv5oMfPZCb5qmypT58e0IvVM
w1oFks9X+0DyhuIQ0gD4BEbS+R36KkkMlnI4zGqpUq4J+i6XJFIrurw4DghJ22Pe
tU7P7C7+uUeAH5f/Be3QIx1MNl0BnruyzPRSrXzGnrg5DP6yyXdwE8SmAhqEuUvJ
UkTqT1HWDVgS1eN+lh8tHUUu2ygqoTELpVOwVkvTXP1qrJKmwgbV+3Tvu8jMfvYJ
olm2HWUINPX2zvlaqfYUNB+5gFifkqVg7JXOF+NUasaUZwPIiReGaxbsRSFt7Ug=
=zrdM
-----END PGP SIGNATURE-----
diff mbox

Patch

Index: gcc/config/mn10300/mn10300.c
===================================================================
--- gcc/config/mn10300/mn10300.c	(revision 174069)
+++ gcc/config/mn10300/mn10300.c	(working copy)
@@ -3315,8 +3315,30 @@ 
     }
 }
 
+static bool
+mn10300_binds_local (const_tree exp)
+{
+  bool local = default_binds_local_p (exp);
+
+  /* The default binds_local function will decide that protected functions
+     bind locally.  Whilst technically this is true, in practice in PIC mode
+     protected functions still need a PLT entry so that function pointer
+     comparison will work.  */
+  if (local
+      && flag_pic
+      && DECL_P (exp)
+      && TREE_CODE (exp) == FUNCTION_DECL
+      && DECL_VISIBILITY (exp) == VISIBILITY_PROTECTED)
+    return false;
+
+  return local;
+}
+
 /* Initialize the GCC target structure.  */
 
+#undef  TARGET_BINDS_LOCAL_P
+#define TARGET_BINDS_LOCAL_P mn10300_binds_local
+
 #undef  TARGET_MACHINE_DEPENDENT_REORG
 #define TARGET_MACHINE_DEPENDENT_REORG mn10300_reorg