diff mbox

[RFC] powerpc: restore TOC when static longjmp to shared object

Message ID 106d886e-719e-c2a8-fd98-39e5ff5ebf36@linux.vnet.ibm.com
State Committed
Delegated to: Tulio Magno Quites Machado Filho
Headers show

Commit Message

rcardoso@linux.vnet.ibm.com June 21, 2018, 7:14 p.m. UTC
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

Tulio Magno Quites Machado Filho July 16, 2018, 7:11 p.m. UTC | #1
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!
diff mbox

Patch

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

diff --git a/sysdeps/powerpc/powerpc64/Makefile b/sysdeps/powerpc/powerpc64/Makefile
index 9d15db0..a0bd0c9 100644
--- a/sysdeps/powerpc/powerpc64/Makefile
+++ b/sysdeps/powerpc/powerpc64/Makefile
@@ -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
diff --git a/sysdeps/powerpc/powerpc64/__longjmp-common.S b/sysdeps/powerpc/powerpc64/__longjmp-common.S
index 0e10b8d..99c17c5 100644
--- a/sysdeps/powerpc/powerpc64/__longjmp-common.S
+++ b/sysdeps/powerpc/powerpc64/__longjmp-common.S
@@ -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)
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)               \
+  );                          \
+ })
+
+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(;;);
+}
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(             \
+      "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