[RFC] powerpc: restore TOC when static longjmp to shared object
Commit Message
Hi Florian,
I've make the changes you asked for. Thank you for your review. Patch v4
is attached.
Regards
Em 04-06-2018 11:38, Florian Weimer escreveu:
> On 06/04/2018 04:34 PM, Rogerio Alves wrote:
>> diff --git a/setjmp/tst-setjmp-bug21895-static.c
>> b/setjmp/tst-setjmp-bug21895-static.c
>> new file mode 100644
>> index 0000000..6ab5340
>> --- /dev/null
>> +++ b/setjmp/tst-setjmp-bug21895-static.c
>
>> + /* Destroy TOC area on caller frame. */
>> + asm volatile(
>> + "li 14, 0\n\t"
>> + "std 14, 24(%0)"
>> + :
>> + : "r" (jb->__jmpbuf[20])
>> + );
>
> Sorry, this test is now very POWER-specific and needs to be moved into a
> POWER sysdeps directory (preferably the most generic one where the test
> is expected to be valid and pass).
>
> Thanks,
> Florian
>
Comments
Rogerio Alves <rcardoso@linux.vnet.ibm.com> writes:
> I've make the changes you asked for. Thank you for your review. Patch v4
> is attached.
I understand this patch solved all the requested changes.
I found some minor issues, which I fixed myself and pushed it.
> Subject: [PATCH v4] powerpc: Always restore TOC on longjmp.
We usually mention the bug report in the commit title, e.g.:
[PATCH v4] powerpc: Always restore TOC on longjmp [BZ #21895]
> This patch change longjmp to always restore the TOC pointer (r2 register)
> to the caller frame on powerpc. This is related to bug 21895[1] that reports
> a situation where you have a static longjmp to a shared object file.
>
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=21895
>
> 2018-05-16 Rogerio A. Cardoso <rcardoso@linux.vnet.ibm.com>
Added 2 spaces around your name.
We also mention the bug report in the ChangeLog entry, e.g.:
[BZ #21895]
>
> *sysdeps/powerpc/powerpc64/__longjmp-common.S: Remove condition code for
^
Missing space here.
> diff --git a/sysdeps/powerpc/powerpc64/setjmp-bug21895.c b/sysdeps/powerpc/powerpc64/setjmp-bug21895.c
> new file mode 100644
> index 0000000..9e65f23
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64/setjmp-bug21895.c
> @@ -0,0 +1,52 @@
> +/* Shared object part of test for setjmp interoperability with static
> + dlopen BZ #21895.
> + Copyright (C) 2017-2018 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <string.h>
> +#include <setjmp.h>
> +
> +/* Copy r1 adress to a local variable. */
> +#define GET_STACK_POINTER(sp) \
> + ({ \
> + asm volatile ("mr %0, 1\n\t" \
> + : "=r" (sp) \
> + ); \
Replaced 8 spaces with tabs.
> diff --git a/sysdeps/powerpc/powerpc64/tst-setjmp-bug21895-static.c b/sysdeps/powerpc/powerpc64/tst-setjmp-bug21895-static.c
> new file mode 100644
> index 0000000..235ba50
> --- /dev/null
> +++ b/sysdeps/powerpc/powerpc64/tst-setjmp-bug21895-static.c
> @@ -0,0 +1,81 @@
> +/* Test setjmp interoperability with static dlopen BZ #21895.
> + Copyright (C) 2017-2018 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <http://www.gnu.org/licenses/>. */
> +
> +#include <setjmp.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <dlfcn.h>
> +
> +/* Set TOC area pointed by sp to zero. */
> +#define SET_TOC_TO_ZERO(sp) \
> + ({ \
> + unsigned int zero = 0; \
> + asm volatile( \
Likewise.
> +/* Make sure the test will not stuck if jmp fails and fall into one of
> + for(;;). */
> +#define TIMEOUT 100
I don't think this test needs 100s. I confirmed the default timeout (20s) is
enough for this test and removed these 3 lines.
Thanks!
From da7c3e786e34390d9c2edda44333209076df995f Mon Sep 17 00:00:00 2001
From: Rogerio Alves <rcardoso@linux.vnet.ibm.com>
Date: Wed, 16 May 2018 14:20:53 -0500
Subject: [PATCH v4] powerpc: Always restore TOC on longjmp.
This patch change longjmp to always restore the TOC pointer (r2 register)
to the caller frame on powerpc. This is related to bug 21895[1] that reports
a situation where you have a static longjmp to a shared object file.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=21895
2018-05-16 Rogerio A. Cardoso <rcardoso@linux.vnet.ibm.com>
*sysdeps/powerpc/powerpc64/__longjmp-common.S: Remove condition code for
restore r2 on longjmp.
*sysdeps/powerpc/powerpc64/Makefile: Added tst-setjmp-bug21895-static to
test list.
Added rules to build test tst-setjmp-bug21895-static.
Added module setjmp-bug21895 and rules to build a shared object from it.
*sysdeps/powerpc/powerpc64/setjmp-bug21895.c: New test file.
*sysdeps/powerpc/powerpc64/tst-setjmp-bug21895-static.c: New test file.
---
Changes in v4: Per Florian and Tulio review. The tests now uses inline
asm specific for Power so the tests was moved to
sysdep/powerpc/powerpc64.
Changes in v3: as pointed out by Alexander Monakov we can't rely on
alloca/memset since gcc can optimize this out or it can save the
registers on the stack and the test will no longer make sense. Now I use
a small asm code to deliberately overwrite TOC area on caller frame with
zero. If the TOC is not restored by longjmp the test will fail.
Changes in v2: Per Adhemerval Zanella. Fix test fail using the suggestions
given. Fix changelog. Fix copyright and indentation for new tests. Change
tests to use libsupport instead old test-skeleton. Fix a extra space
in __longjmp-common.
sysdeps/powerpc/powerpc64/Makefile | 12 ++++
sysdeps/powerpc/powerpc64/__longjmp-common.S | 5 +-
sysdeps/powerpc/powerpc64/setjmp-bug21895.c | 52 ++++++++++++++
.../powerpc/powerpc64/tst-setjmp-bug21895-static.c | 81 ++++++++++++++++++++++
5 files changed, 146 insertions(+), 5 deletions(-)
create mode 100644 sysdeps/powerpc/powerpc64/setjmp-bug21895.c
create mode 100644 sysdeps/powerpc/powerpc64/tst-setjmp-bug21895-static.c
@@ -47,3 +47,15 @@ ifeq ($(subdir),gmon)
CFLAGS-mcount.c += $(no-special-regs)
sysdep_routines += ppc-mcount
endif
+
+ifeq ($(subdir),setjmp)
+tests += tst-setjmp-bug21895-static
+tests-static += tst-setjmp-bug21895-static
+modules-names += setjmp-bug21895
+
+$(objpfx)tst-setjmp-bug21895-static: $(common-objpfx)dlfcn/libdl.a
+$(objpfx)tst-setjmp-bug21895-static.out: $(objpfx)setjmp-bug21895.so
+
+tst-setjmp-bug21895-static-ENV = \
+ LD_LIBRARY_PATH=$(objpfx):$(common-objpfx):$(common-objpfx)setjmp:$(common-objpfx)elf
+endif
@@ -130,9 +130,6 @@ L(no_vmx):
ld r0,(JB_LR*8)(r3)
ld r14,((JB_GPRS+0)*8)(r3)
lfd fp14,((JB_FPRS+0)*8)(r3)
-#if defined SHARED && !IS_IN (rtld)
- std r2,FRAME_TOC_SAVE(r1) /* Restore the callers TOC save area. */
-#endif
ld r15,((JB_GPRS+1)*8)(r3)
lfd fp15,((JB_FPRS+1)*8)(r3)
ld r16,((JB_GPRS+2)*8)(r3)
@@ -152,7 +149,7 @@ L(no_vmx):
second argument (-4@4), and target address (8@0), respectively. */
LIBC_PROBE (longjmp, 3, 8@3, -4@4, 8@0)
mtlr r0
-/* std r2,FRAME_TOC_SAVE(r1) Restore the TOC save area. */
+ std r2,FRAME_TOC_SAVE(r1) /* Restore the TOC save area. */
ld r21,((JB_GPRS+7)*8)(r3)
lfd fp21,((JB_FPRS+7)*8)(r3)
ld r22,((JB_GPRS+8)*8)(r3)
new file mode 100644
@@ -0,0 +1,52 @@
+/* Shared object part of test for setjmp interoperability with static
+ dlopen BZ #21895.
+ Copyright (C) 2017-2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <string.h>
+#include <setjmp.h>
+
+/* Copy r1 adress to a local variable. */
+#define GET_STACK_POINTER(sp) \
+ ({ \
+ asm volatile ("mr %0, 1\n\t" \
+ : "=r" (sp) \
+ ); \
+ })
+
+jmp_buf jb;
+void (*bar)(jmp_buf, unsigned long);
+
+void
+lbar (unsigned long sp)
+{
+ bar(jb, sp);
+ for(;;);
+}
+
+void
+foo (void)
+{
+ unsigned long sp;
+ /* Copy r1 (stack pointer) to sp. It will be use later to get
+ TOC area. */
+ GET_STACK_POINTER(sp);
+ setjmp(jb);
+ lbar(sp);
+
+ for(;;);
+}
new file mode 100644
@@ -0,0 +1,81 @@
+/* Test setjmp interoperability with static dlopen BZ #21895.
+ Copyright (C) 2017-2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <setjmp.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <dlfcn.h>
+
+/* Set TOC area pointed by sp to zero. */
+#define SET_TOC_TO_ZERO(sp) \
+ ({ \
+ unsigned int zero = 0; \
+ asm volatile( \
+ "std %0, 24(%1)\n\t" \
+ :: "r" (zero), "r" (sp) \
+ ); \
+ })
+
+static void
+bar (jmp_buf jb, unsigned long sp)
+{
+ static int i;
+ if (i++==1)
+ exit(0); /* Success. */
+
+ /* This will set TOC are on caller frame (foo) to zero. __longjmp
+ must restore r2 otherwise a segmentation fault will happens after
+ it jumps back to foo. */
+ SET_TOC_TO_ZERO(sp);
+ longjmp(jb, i);
+}
+
+static int
+do_test (void)
+{
+ void *h = dlopen("setjmp-bug21895.so", RTLD_NOW);
+ if (!h)
+ {
+ puts(dlerror());
+ return 1;
+ }
+
+ void (*pfoo)(void) = dlsym(h, "foo");
+ if (!pfoo)
+ {
+ puts(dlerror());
+ return 1;
+ }
+
+ void (**ppbar)(jmp_buf, unsigned long) = dlsym(h, "bar");
+ if (!ppbar)
+ {
+ puts(dlerror());
+ return 1;
+ }
+
+ *ppbar = bar;
+ pfoo();
+
+ for(;;);
+}
+
+/* Make sure the test will not stuck if jmp fails and fall into one of
+ for(;;). */
+#define TIMEOUT 100
+#include <support/test-driver.c>
--
2.7.4