diff mbox

powerpc help needed: Defining hidden alias for __sigsetjmp

Message ID db51debb-cf84-5f5a-6659-eb9ad1ea72e4@redhat.com
State New
Headers show

Commit Message

Florian Weimer Nov. 14, 2016, 2:44 p.m. UTC 
I'm trying to create a hidden alias for __sigsetjmp on powerpc.  The 
problem is that powerpc uses the generic <setjmp.h> wrapper, which has a 
libc_hidden_proto for it, but the implementation of that redirect never 
happens.

I came up with the attached patch.  It adds a definition of the hidden 
redirect to libc.so, as expected.  But in addition to that, I get this 
ABI difference on powerpc32:

-GLIBC_2.3.4 __longjmp F

This does not happen on powerpc64.

I really can't explain that.  Any ideas?

Florian

Comments

Andreas Schwab Nov. 14, 2016, 3:37 p.m. UTC | #1 
On Nov 14 2016, Florian Weimer <fweimer@redhat.com> wrote:

> I came up with the attached patch.  It adds a definition of the hidden

> redirect to libc.so, as expected.  But in addition to that, I get this ABI

> difference on powerpc32:

>

> -GLIBC_2.3.4 __longjmp F


Are you sure this is the only change you did?  setjmp.* does not contain
anything related to longjmp.  I couldn't reproduce that.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Florian Weimer Nov. 14, 2016, 5:34 p.m. UTC | #2 
On 11/14/2016 04:37 PM, Andreas Schwab wrote:
> On Nov 14 2016, Florian Weimer <fweimer@redhat.com> wrote:

>

>> I came up with the attached patch.  It adds a definition of the hidden

>> redirect to libc.so, as expected.  But in addition to that, I get this ABI

>> difference on powerpc32:

>>

>> -GLIBC_2.3.4 __longjmp F

>

> Are you sure this is the only change you did?  setjmp.* does not contain

> anything related to longjmp.  I couldn't reproduce that.


Thanks.  It only happens in conjunction with this patch:

   <https://sourceware.org/ml/libc-alpha/2016-11/msg00465.html>

This patch adds a reference to __longjmp to libc.so.  With it, 
libc_pic.os has this:

  8296: 00017024    492 FUNC    GLOBAL HIDDEN         2 
__longjmp@@GLIBC_2.3.4
  9342: 00017210    184 FUNC    GLOBAL DEFAULT        2 __longjmp@GLIBC_2.0

And libc.so has this:

  5208: 00039c04    492 FUNC    LOCAL  DEFAULT       11 
__longjmp@@GLIBC_2.3.4
  5758: 00039df0    184 FUNC    LOCAL  DEFAULT       11 __longjmp@GLIBC_2.0

It turns out that libc.map lists _longjmp only, based on the entry in 
sysdeps/powerpc/Versions.

setjmp/__longjmp.os has:

    11: 00000000    492 FUNC    GLOBAL DEFAULT        1 __vmx__longjmp
    14: 000001ec    184 FUNC    GLOBAL DEFAULT        1 __novmx__longjmp
    15: 00000000    492 FUNC    GLOBAL DEFAULT        1 
__longjmp@@GLIBC_2.3.4
    16: 000001ec    184 FUNC    GLOBAL DEFAULT        1 __longjmp@GLIBC_2.0

setjmp/longjmp.os has:

    21: 00000000    116 FUNC    GLOBAL DEFAULT        1 __vmx__libc_longjmp
    22: 00000000    116 FUNC    GLOBAL DEFAULT        1 
__vmx__libc_siglongjmp
    23: 00000000    116 FUNC    WEAK   DEFAULT        1 __vmx_longjmp
    24: 00000000    116 FUNC    WEAK   DEFAULT        1 __vmxlongjmp
    25: 00000000    116 FUNC    WEAK   DEFAULT        1 __vmxsiglongjmp
    30: 00000000    116 FUNC    GLOBAL HIDDEN         1 
__GI___vmx__libc_longjmp
    31: 00000000    116 FUNC    GLOBAL DEFAULT        1 
__libc_longjmp@@GLIBC_PRIVATE
    32: 00000000    116 FUNC    GLOBAL DEFAULT        1 
__libc_siglongjmp@@GLIBC_PRIVATE
    33: 00000000    116 FUNC    WEAK   DEFAULT        1 
_longjmp@@GLIBC_2.3.4
    34: 00000000    116 FUNC    WEAK   DEFAULT        1 longjmp@@GLIBC_2.3.4
    35: 00000000    116 FUNC    WEAK   DEFAULT        1 
siglongjmp@@GLIBC_2.3.4

So the __GI_* aliases are missing there, too.

The internal reference to __longjmp seems to cause ld to drop the 
exported symbol version.  Adding __longjmp to sysdeps/powerpc/Versions 
does not change this.  Neither does adding

libc_hidden_ver (__vmx__libc_longjmp, __longjmp)

to sysdeps/powerpc/longjmp.c.  Okay, that's because the definition comes 
from sysdeps/powerpc/powerpc32/fpu/__longjmp-common.S.  But changing 
that file to contain:

versioned_symbol (libc, __vmx__longjmp, __longjmp, GLIBC_2_3_4); 

# define __longjmp_symbol  __vmx__longjmp
# include "__longjmp-common.S"
libc_hidden_ver (__vmx__libc_longjmp, __longjmp)

does not help, either.

Maybe this is relevant.  __longjmp is declared *hidden*:

extern void __longjmp (__jmp_buf __env, int __val)
      __attribute__ ((__noreturn__)) attribute_hidden;

So maybe that's why a reference to it causes ld to drop the export.

I wonder if this indeed the root cause.  It would mean this is a 
spurious ABI addition.  Can we drop __longjmp from the ABI in this way? 
Or do we have to fix this up using a powerpc-specific alias?

This happens on no other architecture for which we have abilist files. 
It was introduced into the abilist file here:

commit 0741d64c9140aa205f9df8ebee80ca0bcb018445
Author: Andreas Schwab <schwab@linux-m68k.org>
Date:   Tue May 1 01:12:54 2012 +0200

     Update powerpc ABI data

Thanks,
Florian
Andreas Schwab Nov. 15, 2016, 9:06 a.m. UTC | #3 
On Nov 14 2016, Florian Weimer <fweimer@redhat.com> wrote:

> This patch adds a reference to __longjmp to libc.so.  With it, libc_pic.os

> has this:

>

>  8296: 00017024    492 FUNC    GLOBAL HIDDEN         2

> __longjmp@@GLIBC_2.3.4

>  9342: 00017210    184 FUNC    GLOBAL DEFAULT        2 __longjmp@GLIBC_2.0

>

> And libc.so has this:

>

>  5208: 00039c04    492 FUNC    LOCAL  DEFAULT       11

> __longjmp@@GLIBC_2.3.4

>  5758: 00039df0    184 FUNC    LOCAL  DEFAULT       11 __longjmp@GLIBC_2.0

>

> It turns out that libc.map lists _longjmp only, based on the entry in

> sysdeps/powerpc/Versions.


So it looks like a linker bug that the symbol was exported before,
probably related to the fact that it is a default version.  But it is
also a bug that __longjmp has a version in the first place.

> setjmp/__longjmp.os has:

>

>    11: 00000000    492 FUNC    GLOBAL DEFAULT        1 __vmx__longjmp

>    14: 000001ec    184 FUNC    GLOBAL DEFAULT        1 __novmx__longjmp

>    15: 00000000    492 FUNC    GLOBAL DEFAULT        1

> __longjmp@@GLIBC_2.3.4

>    16: 000001ec    184 FUNC    GLOBAL DEFAULT        1 __longjmp@GLIBC_2.0

>

> setjmp/longjmp.os has:

>

>    21: 00000000    116 FUNC    GLOBAL DEFAULT        1 __vmx__libc_longjmp

>    22: 00000000    116 FUNC    GLOBAL DEFAULT        1

> __vmx__libc_siglongjmp

>    23: 00000000    116 FUNC    WEAK   DEFAULT        1 __vmx_longjmp

>    24: 00000000    116 FUNC    WEAK   DEFAULT        1 __vmxlongjmp

>    25: 00000000    116 FUNC    WEAK   DEFAULT        1 __vmxsiglongjmp

>    30: 00000000    116 FUNC    GLOBAL HIDDEN         1

> __GI___vmx__libc_longjmp

>    31: 00000000    116 FUNC    GLOBAL DEFAULT        1

> __libc_longjmp@@GLIBC_PRIVATE

>    32: 00000000    116 FUNC    GLOBAL DEFAULT        1

> __libc_siglongjmp@@GLIBC_PRIVATE

>    33: 00000000    116 FUNC    WEAK   DEFAULT        1

> _longjmp@@GLIBC_2.3.4

>    34: 00000000    116 FUNC    WEAK   DEFAULT        1 longjmp@@GLIBC_2.3.4

>    35: 00000000    116 FUNC    WEAK   DEFAULT        1

> siglongjmp@@GLIBC_2.3.4


The version is coming from sysdeps/powerpc/powerpc32/fpu/__longjmp.S,
and was added in commit 5c76ff279f.  Clearly this was a genuine error,
and the versioned alias in sysdeps/powerpc/powerpc32/__longjmp.S has
later been removed in commit 9b9ef82358 (without fixing the fpu
version).

> So the __GI_* aliases are missing there, too.


There is no hidden_proto for __longjmp, only attribute_hidden, so this
is correct.  It is already an internal-only function.

> I wonder if this indeed the root cause.  It would mean this is a spurious

> ABI addition.  Can we drop __longjmp from the ABI in this way? Or do we

> have to fix this up using a powerpc-specific alias?


I'm not sure.  Nobody should be using __longjmp, it's not declared or
used in any public header.

> This happens on no other architecture for which we have abilist files. It

> was introduced into the abilist file here:

>

> commit 0741d64c9140aa205f9df8ebee80ca0bcb018445

> Author: Andreas Schwab <schwab@linux-m68k.org>

> Date:   Tue May 1 01:12:54 2012 +0200

>

>     Update powerpc ABI data


It's unfortunate that Jakub didn't fix the fpu version back in 2004.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Florian Weimer Nov. 15, 2016, 2:17 p.m. UTC | #4 
On 11/15/2016 10:06 AM, Andreas Schwab wrote:
> On Nov 14 2016, Florian Weimer <fweimer@redhat.com> wrote:

>

>> This patch adds a reference to __longjmp to libc.so.  With it, libc_pic.os

>> has this:

>>

>>  8296: 00017024    492 FUNC    GLOBAL HIDDEN         2

>> __longjmp@@GLIBC_2.3.4

>>  9342: 00017210    184 FUNC    GLOBAL DEFAULT        2 __longjmp@GLIBC_2.0

>>

>> And libc.so has this:

>>

>>  5208: 00039c04    492 FUNC    LOCAL  DEFAULT       11

>> __longjmp@@GLIBC_2.3.4

>>  5758: 00039df0    184 FUNC    LOCAL  DEFAULT       11 __longjmp@GLIBC_2.0

>>

>> It turns out that libc.map lists _longjmp only, based on the entry in

>> sysdeps/powerpc/Versions.

>

> So it looks like a linker bug that the symbol was exported before,

> probably related to the fact that it is a default version.  But it is

> also a bug that __longjmp has a version in the first place.


I've asked for clarification of the linker behavior:

   <https://sourceware.org/ml/binutils/2016-11/msg00180.html>

 From the binutils perspective, it may work as designed.  It may be a 
mismatch between the assembler (which does not know about a mere 
reference to a hidden symbol), and the C/C++ source, where visibility 
attributes are supported on declarations (and declarations in one 
translation unit are not supposed to affect how other translation units 
are compiled/linked).

>> ABI addition.  Can we drop __longjmp from the ABI in this way? Or do we

>> have to fix this up using a powerpc-specific alias?

>

> I'm not sure.  Nobody should be using __longjmp, it's not declared or

> used in any public header.


I don't see a reference in codesearch.debian.org (the existing ones are 
definitions in other libcs).  I don't have ppc RPMs indexed in my symbol 
database, unfortunately.

I suspect this is something for the powerpc maintainers to decide.

Thanks,
Florian
diff mbox

Patch

powerpc: Add hidden definition for __sigsetjmp

2016-11-14  Florian Weimer  <fweimer@redhat.com>

	* sysdeps/powerpc/powerpc64/setjmp-common.S (__GI___sigsetjmp):
	Define.
	* sysdeps/powerpc/powerpc32/setjmp.S (__sigsetjmp): Add hidden
	definition.

diff --git a/sysdeps/powerpc/powerpc32/fpu/setjmp.S b/sysdeps/powerpc/powerpc32/fpu/setjmp.S
index 6a4016c..de6cdcf 100644
--- a/sysdeps/powerpc/powerpc32/fpu/setjmp.S
+++ b/sysdeps/powerpc/powerpc32/fpu/setjmp.S
@@ -32,6 +32,7 @@  versioned_symbol (libc, __vmx__sigsetjmp, __sigsetjmp, GLIBC_2_3_4)
 # define __sigsetjmp_symbol __vmx__sigsetjmp
 # define __sigjmp_save_symbol __vmx__sigjmp_save
 # include "setjmp-common.S"
+libc_hidden_ver (__vmx__sigsetjmp, __sigsetjmp)
 
 # if defined SHARED && SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_3_4)
 #  define __NO_VMX__
diff --git a/sysdeps/powerpc/powerpc32/setjmp.S b/sysdeps/powerpc/powerpc32/setjmp.S
index 88f7f82..285d40c 100644
--- a/sysdeps/powerpc/powerpc32/setjmp.S
+++ b/sysdeps/powerpc/powerpc32/setjmp.S
@@ -31,6 +31,7 @@  versioned_symbol (libc, __vmx__sigsetjmp, __sigsetjmp, GLIBC_2_3_4)
 # define __sigsetjmp_symbol __vmx__sigsetjmp
 # define __sigjmp_save_symbol __vmx__sigjmp_save
 # include "setjmp-common.S"
+libc_hidden_ver (__vmx__sigsetjmp, __sigsetjmp)
 
 # if defined SHARED && SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_3_4)
 #  define __NO_VMX__
diff --git a/sysdeps/powerpc/powerpc64/setjmp-common.S b/sysdeps/powerpc/powerpc64/setjmp-common.S
index 83361f5..b5de49e 100644
--- a/sysdeps/powerpc/powerpc64/setjmp-common.S
+++ b/sysdeps/powerpc/powerpc64/setjmp-common.S
@@ -232,3 +232,14 @@  L(no_vmx):
 	blr
 #endif
 END (__sigsetjmp_symbol)
+
+#if defined SHARED && !IS_IN (rtld) && !defined __NO_VMX__
+/* When called from within libc we need a special version of __sigsetjmp
+   that saves r2 since the call won't go via a plt call stub.  See
+   bugz #269.  */
+ENTRY (__GI___sigsetjmp)
+	std r2,FRAME_TOC_SAVE(r1) /* Save the callers TOC in the save area.  */
+	CALL_MCOUNT 1
+	b JUMPTARGET (GLUE(__sigsetjmp_symbol,_ent))
+END (__GI___sigsetjmp)
+#endif