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

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

[Rogerio Alves]

[-- Attachment #1: Type: text/plain, Size: 11945 bytes --]

I've attached patch v2 to this email with the fixes suggested by Adhemerval.

One last issue is regard to the copyright on the tests. I put a 
copyright there but, the test is based on a test the user reported in 
bugzilla. I've adapted to libsupport and changed some of the code. What 
should I do in that case? I have to get the permission from the author 
first since the test is a derivative work? Some of the tests doesn't 
have copyright, and some of them mention the person who reports the test 
like stdio-common/tstscanf.c but I can't find a standard.


Em 16-05-2018 21:42, libc-alpha-digest-help@sourceware.org escreveu:
> 
> On 16/05/2018 16:36, Rogerio Alves wrote:
>> Attached to this email I am sending the first version of the patch but I have a problem with this patch. I can't make the test work. By some reason it can't find the .so needed by the test:
>>
>> FAIL:
>> ./setjmp-bug21895.so: cannot open shared object file: No such file or directory
>>
>> Notice that if I manually execute the command on make check stdout inside the folder it works fine. I'd appreciate any help on that.
>>
>> Regards
>>
>> Em 15-05-2018 17:48, Tulio Magno Quites Machado Filho escreveu:
>>> Florian Weimer<fw@deneb.enyo.de>  writes:
>>>
>>>> * Rogerio Alves:
>>>>
>>>>> One simple solution would be always restore the TOC pointer by uncomment
>>>>> the line bellow:
>>>>>
>>>>> /*     std r2,FRAME_TOC_SAVE(r1)       Restore the TOC save area.  */
>>>>>
>>>>> Or maybe we can check if we have a valid TOC pointer before restore it,
>>>>> instead #if defined SHARED.
>>>> Is the register reserved for the TOC pointer in static builds, too?
>>>> Then I suggest to unconditionally save nad restore it; not doing so
>>>> looks like a pointless micro-optimization.
>>>>
>>>> Another problem with sharing jump buffers across static dlopen is that
>>>> you might not have identical pointer guard values.
>>>>
>>>>> I would like to request for comments on this matter: Should we fix/work
>>>>> this? Is feasible to change longjmp to always restore TOC pointer?
>>>> Does setjmp already save it unonditionally?
>>>>
>>>> Removal of static dlopen is still some time away; it's likely not
>>>> going to happen in this cycle, and the fix looks simple enough.
>>> If static dlopen is still going to be supported for some cycles, I also agree
>>> it should be saved and restored unconditionally.
>>>
>> 0001-PATCH-v1-powerpc-Always-restore-TOC-on-longjmp.patch
>>
>>
>>  From eb9895f1548c8f6e1826095aee3221eaf9ce84c9 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] [PATCH v1] 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
> Please indicate where you actually tested this patch (since the make check
> is broken).
> 

I've fixed the test thanks to your suggestion.

>> 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.
>> 	*setjmp/Makefile: Include test build directives.
> You need to add each rules you might add, the ones you are changing, and
> removing (check other ChangeLog for example).
> 

Done.
>> 	*setjmp/setjmp-bug21895.c: new test file.
>> 	*setjmp/tst-setjmp-bug21895.c: new test file.
> Capitalize the new.
> 

Done.

>> ---
>>   setjmp/Makefile                              | 18 ++++++--
>>   setjmp/setjmp-bug21895.c                     | 42 ++++++++++++++++++
>>   setjmp/tst-setjmp-bug21895.c                 | 65 ++++++++++++++++++++++++++++
>>   sysdeps/powerpc/powerpc64/__longjmp-common.S |  5 +--
>>   4 files changed, 123 insertions(+), 7 deletions(-)
>>   create mode 100644 setjmp/setjmp-bug21895.c
>>   create mode 100644 setjmp/tst-setjmp-bug21895.c
>>
>> diff --git a/setjmp/Makefile b/setjmp/Makefile
>> index dc2fcc6..e715ee6 100644
>> --- a/setjmp/Makefile
>> +++ b/setjmp/Makefile
>> @@ -22,16 +22,28 @@ subdir	:= setjmp
>>   
>>   include ../Makeconfig
>>   
>> -headers	:= setjmp.h bits/setjmp.h bits/setjmp2.h
>> +headers	:= setjmp.h bits/setjmp.h bits/setjmp2.h bits/dlfcn.h dlfcn/dlfcn.h
> Why does it require adding more headers here? This is meant be installed afaik
> and I am not following the requirement here.
> 
>>   
>>   routines	:= setjmp sigjmp bsd-setjmp bsd-_setjmp \
>>   		   longjmp __longjmp jmp-unwind
>>   
>>   tests		:= tst-setjmp jmpbug bug269-setjmp tst-setjmp-fp \
>> -		   tst-sigsetjmp tst-setjmp-static
>> +		   tst-sigsetjmp tst-setjmp-static tst-setjmp-bug21895
>> +
> Since the idea is test for static build I would recommend to follow the other
> static naming, i.e, tst-setjmp-bug21895-static.
> 

Done. Added static suffix.

>>   tests-static	:= tst-setjmp-static
>>   
>> +modules-names = setjmp-bug21895
>>   
>>   include ../Rules
>>   
>> -$(objpfx)tst-setjmp-fp: $(libm)
> I do not think you meant to remove this rule.
> 

It was not removed I've write the new rules before this rule. Anyway 
I've fixed it on v2.

>> +test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
>> +
>> +ifeq ($(build-shared),yes)
>> +tests: $(test-modules)
>> +endif
>> +
>> +$(objpfx)setjmp-bug21895.so: $(libdl)
>> +$(objpfx)tst-setjmp-bug21895: $(libdl)
>> +$(objpfx)tst-setjmp-bug21895.out: $(objpfx)setjmp-bug21895.so
>> +
>> +$(objpfx)ts-tsetjmp-fp: $(libm)
> I think the idea is to build the test even if build-shared is set to no (since
> you are testing static linking).  I think what you want is:
> 
> ---
> 
> diff --git a/setjmp/Makefile b/setjmp/Makefile
> index dc2fcc6..231474b 100644
> --- a/setjmp/Makefile
> +++ b/setjmp/Makefile
> @@ -28,10 +28,17 @@ routines    := setjmp sigjmp bsd-setjmp bsd-_setjmp \
>                     longjmp __longjmp jmp-unwind
> 
>   tests          := tst-setjmp jmpbug bug269-setjmp tst-setjmp-fp \
> -                  tst-sigsetjmp tst-setjmp-static
> -tests-static   := tst-setjmp-static
> +                  tst-sigsetjmp tst-setjmp-static tst-setjmp-bug21895-static
> 
> +tests-static   := tst-setjmp-static tst-setjmp-bug21895-static
> +
> +modules-names = setjmp-bug21895
> 
>   include ../Rules
> 
>   $(objpfx)tst-setjmp-fp: $(libm)
> +$(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
> 
> ---
> 
> At least with qemu-static it seems to work with your patch.
> 
> 

Thanks to your help it's working now. That's exactly what I wanted.

>> diff --git a/setjmp/setjmp-bug21895.c b/setjmp/setjmp-bug21895.c
>> new file mode 100644
>> index 0000000..d6f5516
>> --- /dev/null
>> +++ b/setjmp/setjmp-bug21895.c
>> @@ -0,0 +1,42 @@
>> +/* Copyright (C) 2013-2018 Free Software Foundation, Inc.
> Wrong Copyright date span and missing one line description.
> 
Fixed.
>> +   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/>.  */
>> +
>> +/* Test that setjmp/longjmp interoperability with static dlopen.
>> +   Bugzila #21895.  */
>> +#include <alloca.h>
>> +#include <string.h>
>> +#include <setjmp.h>
>> +
>> +jmp_buf jb;
>> +void (*bar)(jmp_buf);
>> +
>> +void
>> +lbar (int i, ...)
>> +{
>> +  bar(jb);
>> +  for(;;);
>> +}
>> +
>> +void
>> +foo (void)
>> +{
>> +  int i = setjmp(jb);
>> +  char *c = alloca(256);
>> +  memset(c, 0, 256);
>> +  lbar(i);
>> +  for(;;);
>> +}
>> diff --git a/setjmp/tst-setjmp-bug21895.c b/setjmp/tst-setjmp-bug21895.c
>> new file mode 100644
>> index 0000000..5333494
>> --- /dev/null
>> +++ b/setjmp/tst-setjmp-bug21895.c
>> @@ -0,0 +1,65 @@
>> +/* Copyright (C) 2013-2018 Free Software Foundation, Inc.
> Wrong Copyright date span and missing one line description.
> 
Fixed.
>> +   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/>.  */
>> +
>> +/* Test that setjmp/longjmp interoperability with static dlopen.
>> +   Bugzila #21895.  */
>> +
>> +#include <setjmp.h>
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <dlfcn.h>
>> +
>> +static void
>> +bar (jmp_buf jb)
>> +{
>> +  static int i;
>> +  if (i++==1) exit(0);
>> +  longjmp(jb, i);
>> +}
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  void *h = dlopen("./setjmp-bug21895.so", RTLD_NOW);
> Better to just use name, since on test environment variable in Makefile
> change I proposed the path to build setjmp folder is already on the
> LD_LIBRARY_PATH list.
> 
>> +  if (!h) {
>> +    puts ("FAIL: ");
>> +    puts (dlerror());
>> +    return 1;
>> +  }
> Wrong indentation and please use libsupport macros.
> 

Fixed. Using libsupport now.
>> +
>> +  void (*pfoo)(void) = dlsym(h, "foo");
>> +  if (!pfoo) {
>> +    puts ("FAIL: ");
>> +    puts (dlerror());
>> +    return 1;
>> +  }
>> +
>> +  void (**ppbar)(jmp_buf) = dlsym(h, "bar");
>> +  if (!ppbar) {
>> +    puts ("FAIL: ");
>> +    puts (dlerror());
>> +    return 1;
>> +  }
>> +
>> +  *ppbar = bar;
>> +  pfoo();
>> +
>> +  for(;;);
>> +}
>> +
>> +#define TEST_FUNCTION do_test ()
>> +#include "../test-skeleton.c"
> We do not use test-skeleton.c inclusion any longer, use libsupport
> (support/test-driver.c).
> 

Done.

>> diff --git a/sysdeps/powerpc/powerpc64/__longjmp-common.S b/sysdeps/powerpc/powerpc64/__longjmp-common.S
>> index 0e10b8d..a5973c9 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.  */
> Space before tab in indent.
>

Done.

>>   	ld r21,((JB_GPRS+7)*8)(r3)
>>   	lfd fp21,((JB_FPRS+7)*8)(r3)
>>   	ld r22,((JB_GPRS+8)*8)(r3)
>> -- 2.7.4
>>


[-- Attachment #2: 0001-powerpc-Always-restore-TOC-on-longjmp.patch --]
[-- Type: text/x-patch, Size: 6632 bytes --]

From 989f90132a77d8f1199881d88f51384f25d452fb 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 v2] 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.
	*setjmp/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.
	*setjmp/setjmp-bug21895.c: New test file.
	*setjmp/tst-setjmp-bug21895-static.c: New test file.
---
 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.

 setjmp/Makefile                              | 16 ++++++-
 setjmp/setjmp-bug21895.c                     | 44 ++++++++++++++++++
 setjmp/tst-setjmp-bug21895-static.c          | 67 ++++++++++++++++++++++++++++
 sysdeps/powerpc/powerpc64/__longjmp-common.S |  5 +--
 4 files changed, 127 insertions(+), 5 deletions(-)
 create mode 100644 setjmp/setjmp-bug21895.c
 create mode 100644 setjmp/tst-setjmp-bug21895-static.c

diff --git a/setjmp/Makefile b/setjmp/Makefile
index dc2fcc6..d70daa2 100644
--- a/setjmp/Makefile
+++ b/setjmp/Makefile
@@ -29,9 +29,23 @@ routines	:= setjmp sigjmp bsd-setjmp bsd-_setjmp \
 
 tests		:= tst-setjmp jmpbug bug269-setjmp tst-setjmp-fp \
 		   tst-sigsetjmp tst-setjmp-static
-tests-static	:= tst-setjmp-static
 
+tests-static	:= tst-setjmp-static tst-setjmp-bug21895-static
+
+modules-names = setjmp-bug21895
 
 include ../Rules
 
 $(objpfx)tst-setjmp-fp: $(libm)
+
+test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
+
+ifeq ($(build-shared),yes)
+tests: $(test-modules)
+endif
+
+$(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
diff --git a/setjmp/setjmp-bug21895.c b/setjmp/setjmp-bug21895.c
new file mode 100644
index 0000000..f8f3016
--- /dev/null
+++ b/setjmp/setjmp-bug21895.c
@@ -0,0 +1,44 @@
+/* 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 <alloca.h>
+#include <string.h>
+#include <setjmp.h>
+
+jmp_buf jb;
+void (*bar)(jmp_buf);
+
+void
+lbar (int i, ...)
+{
+  bar(jb);
+  for(;;);
+}
+
+void
+foo (void)
+{
+  int i = setjmp(jb);
+  char *c = alloca(256);
+  memset(c, 0, 256);
+
+  lbar(i);
+
+  for(;;);
+}
diff --git a/setjmp/tst-setjmp-bug21895-static.c b/setjmp/tst-setjmp-bug21895-static.c
new file mode 100644
index 0000000..5338499
--- /dev/null
+++ b/setjmp/tst-setjmp-bug21895-static.c
@@ -0,0 +1,67 @@
+/* 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>
+
+static void
+bar (jmp_buf jb)
+{
+  static int i;
+  if (i++==1)
+    exit(0);	/* Success.  */
+
+  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) = 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>
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)
-- 
2.7.4

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

[Alexander Monakov]

On Tue, 22 May 2018, Rogerio Alves wrote:

> I've attached patch v2 to this email with the fixes suggested by Adhemerval.

Why was it necessary to change the prototype of lbar(), and does the testcase
still demonstrate the issue with that change (does it fail without the patch)?

Note that as soon as gcc is capable of optimizing out the alloca-memset code
in the testcase, it will no longer serve the purpose (maybe it's already
capable, when preparing the testcase I only checked -O0 IIRC). Surely there
are more direct and robust ways of checking that TOC is correctly restored?

Alexander

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

[Rogerio Alves]

Em 23-05-2018 06:26, Alexander Monakov escreveu:
> On Tue, 22 May 2018, Rogerio Alves wrote:
> 
>> I've attached patch v2 to this email with the fixes suggested by Adhemerval.
> 
> Why was it necessary to change the prototype of lbar(), and does the testcase
> still demonstrate the issue with that change (does it fail without the patch)?
> 

Yes it fails without the patch.

The change on lbar() prototype was made only because glibc is being 
compiled with and it give me an error due a  -Werror=strict-prototypes. 
But lbar(void) works fine but, you don't have a segmentation fault.

The problem is that if you have a parameter area on the stack 
alloca/memset will set TOC to 0x0 and lbar() prototype doesn't mean "no 
parameters" in C it means "any arguments" - C Standard, subclause 
6.7.6.3, paragraph 14 [ISO/IEC 9899:2011] - so the compiler ends up 
creating a parameter area for the function.

This code works if you use lbar(void) or if you don't have a parameter 
save area on stack:

lbar() - Segmentation fault
lbar(void) - Works (no arguments).
lbar(int a, int b, int c, int d, int e, int f, int g) - Works (less than 
8 parameters, use registers, no parameter saving area)
lbar(int a, int b, int c, int d, int e, int f, int g, int h, int i) - 
Segmentation fault


> Note that as soon as gcc is capable of optimizing out the alloca-memset code
> in the testcase, it will no longer serve the purpose (maybe it's alread > capable, when preparing the testcase I only checked -O0 IIRC). Surely 
there
> are more direct and robust ways of checking that TOC is correctly restored?
> 
I've tested with -O3 here and get the same problem here. If 
alloca/memset is optimized out or removed from the code the problem 
doesn't happens but, that's only if the caller stack frame doesn't 
change if don't, when longjmp execute you still have a valid TOC on the 
stack.

But described in https://sourceware.org/bugzilla/show_bug.cgi?id=269

"In this (simple) case the appl calls [bsd]_setjmp() which transfers to 
__setjmp.  Both are in libc.so.  So by the time __setjmp() gets control 
libc.so's TOC is already loaded.  In this case we need to reach back 
into the  callers frame and retrieve the callers TOC from the TOC save 
area. But in the static case the TOC is not saved and has not changed.

In GLIBC sysdeps/powerpc/powerpc64/setjmp.S is built 4 times (static, 
shared, profiled, and a special version (rtld-setjmp) for the dynamic 
linker.  In the  SHARED case (libc.so), external calls to setjmp will 
save the TOC on the stack, but internal libc calls will not.  So the 
trick is to do the correct thing for each case."

So your test has a example of a code that use a static function to call 
a longjmp that was set on a shared object called via (static) dlopen. In 
that case we should have to restore it to caller frame as the jump area 
is in shared object but, since longjmp is inside static compiled unit 
TOC is not restored.

I don't think that alloca/memset should destroy the caller stack like is 
happening but, also I think we have to restore TOC to the caller frame 
after the longjmp in that case also. I don't know if there's any other 
more direct and robust ways to checking if TOC is correctly restored. I 
can't think in anything easier than always restore.

Rogerio


> Alexander
> 

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

[Tulio Magno Quites Machado Filho]

Rogerio Alves <rcardoso@linux.vnet.ibm.com> writes:

> I don't think that alloca/memset should destroy the caller stack like is 
> happening but, also I think we have to restore TOC to the caller frame 
> after the longjmp in that case also.

Do we have a bug report for this?
If not, we need one.

-- 
Tulio Magno

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

[Alexander Monakov]

On Wed, 23 May 2018, Rogerio Alves wrote:
> I don't think that alloca/memset should destroy the caller stack like is
> happening but, also I think we have to restore TOC to the caller frame after
> the longjmp in that case also. I don't know if there's any other more direct
> and robust ways to checking if TOC is correctly restored. I can't think in
> anything easier than always restore.

I'm not challenging the idea that "always restoring" is an appropriate fix.
My question was about *verifying* that TOC register is restored as it ought
to, i.e. how the testcase needs to work. Right now, the test uses my code
from Bugzilla that worked okay for demonstration purposes, but has issues as
a long-term testsuite addition:

* it demonstrates the issue in a very intransparent fashion, relying on
  non-obvious interaction with alloca-memset part of the test;

* when GCC manages to optimize out the alloca-memset part, the test
  will cease to work for the intended purpose.

Alexander

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

[Rogerio Alves]

Em 23-05-2018 11:24, Alexander Monakov escreveu:
> On Wed, 23 May 2018, Rogerio Alves wrote:
>> I don't think that alloca/memset should destroy the caller stack like is
>> happening but, also I think we have to restore TOC to the caller frame after
>> the longjmp in that case also. I don't know if there's any other more direct
>> and robust ways to checking if TOC is correctly restored. I can't think in
>> anything easier than always restore.
> 
> I'm not challenging the idea that "always restoring" is an appropriate fix.
> My question was about *verifying* that TOC register is restored as it ought
> to, i.e. how the testcase needs to work. Right now, the test uses my code
> from Bugzilla that worked okay for demonstration purposes, but has issues as
> a long-term testsuite addition:
>
> * it demonstrates the issue in a very intransparent fashion, relying on
>    non-obvious interaction with alloca-memset part of the test;
> 
> * when GCC manages to optimize out the alloca-memset part, the test
>    will cease to work for the intended purpose.
> 
> Alexander
> 

Ok. I understand your concern. Let me see if I can change this test to 
check if the TOC was been restored.

Rogerio

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

[Rogerio Alves]

[-- Attachment #1: Type: text/plain, Size: 1755 bytes --]

This is v3 of patch.  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 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 and them I removed alloca/memset since it's no longer necessary for 
the test itself.

Em 25-05-2018 16:39, Rogerio Alves escreveu:
> 
> 
> Em 23-05-2018 11:24, Alexander Monakov escreveu:
>> On Wed, 23 May 2018, Rogerio Alves wrote:
>>> I don't think that alloca/memset should destroy the caller stack like is
>>> happening but, also I think we have to restore TOC to the caller 
>>> frame after
>>> the longjmp in that case also. I don't know if there's any other more 
>>> direct
>>> and robust ways to checking if TOC is correctly restored. I can't 
>>> think in
>>> anything easier than always restore.
>>
>> I'm not challenging the idea that "always restoring" is an appropriate 
>> fix.
>> My question was about *verifying* that TOC register is restored as it 
>> ought
>> to, i.e. how the testcase needs to work. Right now, the test uses my code
>> from Bugzilla that worked okay for demonstration purposes, but has 
>> issues as
>> a long-term testsuite addition:
>>
>> * it demonstrates the issue in a very intransparent fashion, relying on
>>    non-obvious interaction with alloca-memset part of the test;
>>
>> * when GCC manages to optimize out the alloca-memset part, the test
>>    will cease to work for the intended purpose.
>>
>> Alexander
>>
> 
> Ok. I understand your concern. Let me see if I can change this test to 
> check if the TOC was been restored.
> 
> Rogerio

[-- Attachment #2: 0001-powerpc-Always-restore-TOC-on-longjmp.patch --]
[-- Type: text/x-patch, Size: 7054 bytes --]

From 0b59a0e92f56901a3512069441d46778fc97c719 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 v3] 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.
	*setjmp/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.
	*setjmp/setjmp-bug21895.c: New test file.
	*setjmp/tst-setjmp-bug21895-static.c: New test file.
---
 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.

 setjmp/Makefile                              | 16 +++++-
 setjmp/setjmp-bug21895.c                     | 41 +++++++++++++++
 setjmp/tst-setjmp-bug21895-static.c          | 75 ++++++++++++++++++++++++++++
 sysdeps/powerpc/powerpc64/__longjmp-common.S |  5 +-
 4 files changed, 132 insertions(+), 5 deletions(-)
 create mode 100644 setjmp/setjmp-bug21895.c
 create mode 100644 setjmp/tst-setjmp-bug21895-static.c

diff --git a/setjmp/Makefile b/setjmp/Makefile
index dc2fcc6..d70daa2 100644
--- a/setjmp/Makefile
+++ b/setjmp/Makefile
@@ -29,9 +29,23 @@ routines	:= setjmp sigjmp bsd-setjmp bsd-_setjmp \
 
 tests		:= tst-setjmp jmpbug bug269-setjmp tst-setjmp-fp \
 		   tst-sigsetjmp tst-setjmp-static
-tests-static	:= tst-setjmp-static
 
+tests-static	:= tst-setjmp-static tst-setjmp-bug21895-static
+
+modules-names = setjmp-bug21895
 
 include ../Rules
 
 $(objpfx)tst-setjmp-fp: $(libm)
+
+test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
+
+ifeq ($(build-shared),yes)
+tests: $(test-modules)
+endif
+
+$(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
diff --git a/setjmp/setjmp-bug21895.c b/setjmp/setjmp-bug21895.c
new file mode 100644
index 0000000..d7c0133
--- /dev/null
+++ b/setjmp/setjmp-bug21895.c
@@ -0,0 +1,41 @@
+/* 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 <alloca.h>
+#include <string.h>
+#include <setjmp.h>
+
+jmp_buf jb;
+void (*bar)(jmp_buf);
+
+void
+lbar (void)
+{
+  bar(jb);
+  for(;;);
+}
+
+void
+foo (void)
+{
+  setjmp(jb);
+  lbar();
+
+  for(;;);
+}
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
@@ -0,0 +1,75 @@
+/* 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>
+
+static void
+bar (jmp_buf jb)
+{
+  static int i;
+  if (i++==1)
+    exit(0);	/* Success.  */
+
+  /* Destroy TOC area on caller frame.  */
+  asm volatile(
+    "li 14, 0\n\t"
+    "std 14, 24(%0)"
+    :
+    : "r" (jb->__jmpbuf[20])
+  );
+
+  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) = 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>
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)
-- 
2.7.4

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

[Florian Weimer]

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

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

[Rogerio Alves]

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

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
> 

[-- Attachment #2: 0001-powerpc-Always-restore-TOC-on-longjmp.patch --]
[-- Type: text/x-patch, Size: 7994 bytes --]

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

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

[Tulio Magno Quites Machado Filho]

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!

-- 
Tulio Magno

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

[Rogerio Alves]

Hi, everyone

I was trying to fix a issue related to setjmp/longjmp on powerpc64 
"ppc64 setjmp/longjmp not fully interoperable with static dlopen":

https://sourceware.org/bugzilla/show_bug.cgi?id=21895

The code sent by the person who reports this problem on bugzilla #21895 
uses a static dlopen to a function declared in a shared lib, this 
function foo() calls setjmp and a static function inside a static 
compiled file calls the longjmp. So the problem is in that case it's not 
restoring the r2 (TOC) to the frame as the code who calls the longjmp is 
compiled as static but, as setjmp is called inside a shared object file 
it will fails once it tries to retrieve the TOC pointer from the memory:

0x3fffb7fb0954 <foo+96>         ld      r2,24(r1)
Segmentation fault

The problem is that TOC is not being restored to the frame after the 
longjmp.  I find out those lines at 
sysdeps/powerpc/powerpc64/__longjmp-common.S:

#if defined SHARED && !IS_IN (rtld)
        std r2,FRAME_TOC_SAVE(r1)       /* Restore the callers TOC save 
area.  */
#endif
/*     std r2,FRAME_TOC_SAVE(r1)       Restore the TOC save area.  */


As described on bugzilla #269:

"In this (simple) case the appl calls [bsd]_setjmp() which transfers to 
__setjmp.  Both are in libc.so.  So by the time __setjmp() gets control 
libc.so's TOC is already loaded.  In this case we need to reach back 
into the  callers frame and retrieve the callers TOC from the TOC save 
area. But in the static case the TOC is not saved and has not changed.

In GLIBC sysdeps/powerpc/powerpc64/setjmp.S is built 4 times (static, 
shared, profiled, and a special version (rtld-setjmp) for the dynamic 
linker.  In the  SHARED case (libc.so), external calls to setjmp will 
save the TOC on the stack, but internal libc calls will not.  So the 
trick is to do the correct thing for each case."

Since the function who calls the longjmp is a static function in a 
static compiled file it will not restore the TOC. But, the function that 
setjmp is a function inside a share object called via dlopen.

One simple solution would be always restore the TOC pointer by uncomment 
the line bellow:

/*     std r2,FRAME_TOC_SAVE(r1)       Restore the TOC save area.  */

Or maybe we can check if we have a valid TOC pointer before restore it, 
instead #if defined SHARED.

However, I am not sure if this is something that we should work on. 
First of all, as far I known, glibc is deprecating the static dlopen. 
Also, on the code attached on bug #21895, after call setjmp function the 
code calls a alloca and a memset function and those function are 
destroying the caller stack somehow. Then when longjmp executes it gets 
a segmentation fault since alloca/memset destroyed the caller stack.

I would like to request for comments on this matter: Should we fix/work 
this? Is feasible to change longjmp to always restore TOC pointer? Any 
suggestions?

Regards

--
Rogerio Alves
Software Engineer - IBM

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

[Florian Weimer]

* Rogerio Alves:

> One simple solution would be always restore the TOC pointer by uncomment 
> the line bellow:
>
> /*     std r2,FRAME_TOC_SAVE(r1)       Restore the TOC save area.  */
>
> Or maybe we can check if we have a valid TOC pointer before restore it, 
> instead #if defined SHARED.

Is the register reserved for the TOC pointer in static builds, too?
Then I suggest to unconditionally save nad restore it; not doing so
looks like a pointless micro-optimization.

Another problem with sharing jump buffers across static dlopen is that
you might not have identical pointer guard values.

> I would like to request for comments on this matter: Should we fix/work 
> this? Is feasible to change longjmp to always restore TOC pointer?

Does setjmp already save it unonditionally?

Removal of static dlopen is still some time away; it's likely not
going to happen in this cycle, and the fix looks simple enough.

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

[Rogerio Alves]

Em 15-05-2018 15:50, Florian Weimer escreveu:
> * Rogerio Alves:
> 
>> One simple solution would be always restore the TOC pointer by uncomment
>> the line bellow:
>>
>> /*     std r2,FRAME_TOC_SAVE(r1)       Restore the TOC save area.  */
>>
>> Or maybe we can check if we have a valid TOC pointer before restore it,
>> instead #if defined SHARED.
> 
> Is the register reserved for the TOC pointer in static builds, too?
> Then I suggest to unconditionally save nad restore it; not doing so
> looks like a pointless micro-optimization.
> 

Yes. And, I agree with you: it's kinda pointless micro-optimization.

> Another problem with sharing jump buffers across static dlopen is that
> you might not have identical pointer guard values.
> 
>> I would like to request for comments on this matter: Should we fix/work
>> this? Is feasible to change longjmp to always restore TOC pointer?
> 
> Does setjmp already save it unonditionally?

Yes it saves. But the only difference it retrieves from the caller when 
shared:

#if defined SHARED && !IS_IN (rtld) 

     ld   r5,FRAME_TOC_SAVE(r1)  /* Retrieve the callers TOC.  */ 

     std  r5,(JB_GPR2*8)(3) 

#else 

     std  r2,(JB_GPR2*8)(3)

> 
> Removal of static dlopen is still some time away; it's likely not
> going to happen in this cycle, and the fix looks simple enough.
> 

Yes. In fact I am already working on a patch here just in case.

Thank you

Regards

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

[Tulio Magno Quites Machado Filho]

Florian Weimer <fw@deneb.enyo.de> writes:

> * Rogerio Alves:
>
>> One simple solution would be always restore the TOC pointer by uncomment 
>> the line bellow:
>>
>> /*     std r2,FRAME_TOC_SAVE(r1)       Restore the TOC save area.  */
>>
>> Or maybe we can check if we have a valid TOC pointer before restore it, 
>> instead #if defined SHARED.
>
> Is the register reserved for the TOC pointer in static builds, too?
> Then I suggest to unconditionally save nad restore it; not doing so
> looks like a pointless micro-optimization.
>
> Another problem with sharing jump buffers across static dlopen is that
> you might not have identical pointer guard values.
>
>> I would like to request for comments on this matter: Should we fix/work 
>> this? Is feasible to change longjmp to always restore TOC pointer?
>
> Does setjmp already save it unonditionally?
>
> Removal of static dlopen is still some time away; it's likely not
> going to happen in this cycle, and the fix looks simple enough.

If static dlopen is still going to be supported for some cycles, I also agree
it should be saved and restored unconditionally.

-- 
Tulio Magno

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

[Rogerio Alves]

[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]

Attached to this email I am sending the first version of the patch but I 
have a problem with this patch. I can't make the test work. By some 
reason it can't find the .so needed by the test:

FAIL:
./setjmp-bug21895.so: cannot open shared object file: No such file or 
directory

Notice that if I manually execute the command on make check stdout 
inside the folder it works fine. I'd appreciate any help on that.

Regards

Em 15-05-2018 17:48, Tulio Magno Quites Machado Filho escreveu:
> Florian Weimer <fw@deneb.enyo.de> writes:
> 
>> * Rogerio Alves:
>>
>>> One simple solution would be always restore the TOC pointer by uncomment
>>> the line bellow:
>>>
>>> /*     std r2,FRAME_TOC_SAVE(r1)       Restore the TOC save area.  */
>>>
>>> Or maybe we can check if we have a valid TOC pointer before restore it,
>>> instead #if defined SHARED.
>>
>> Is the register reserved for the TOC pointer in static builds, too?
>> Then I suggest to unconditionally save nad restore it; not doing so
>> looks like a pointless micro-optimization.
>>
>> Another problem with sharing jump buffers across static dlopen is that
>> you might not have identical pointer guard values.
>>
>>> I would like to request for comments on this matter: Should we fix/work
>>> this? Is feasible to change longjmp to always restore TOC pointer?
>>
>> Does setjmp already save it unonditionally?
>>
>> Removal of static dlopen is still some time away; it's likely not
>> going to happen in this cycle, and the fix looks simple enough.
> 
> If static dlopen is still going to be supported for some cycles, I also agree
> it should be saved and restored unconditionally.
> 

[-- Attachment #2: 0001-PATCH-v1-powerpc-Always-restore-TOC-on-longjmp.patch --]
[-- Type: text/x-patch, Size: 6295 bytes --]

From eb9895f1548c8f6e1826095aee3221eaf9ce84c9 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] [PATCH v1] 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.
	*setjmp/Makefile: Include test build directives.
	*setjmp/setjmp-bug21895.c: new test file.
	*setjmp/tst-setjmp-bug21895.c: new test file.
---
 setjmp/Makefile                              | 18 ++++++--
 setjmp/setjmp-bug21895.c                     | 42 ++++++++++++++++++
 setjmp/tst-setjmp-bug21895.c                 | 65 ++++++++++++++++++++++++++++
 sysdeps/powerpc/powerpc64/__longjmp-common.S |  5 +--
 4 files changed, 123 insertions(+), 7 deletions(-)
 create mode 100644 setjmp/setjmp-bug21895.c
 create mode 100644 setjmp/tst-setjmp-bug21895.c

diff --git a/setjmp/Makefile b/setjmp/Makefile
index dc2fcc6..e715ee6 100644
--- a/setjmp/Makefile
+++ b/setjmp/Makefile
@@ -22,16 +22,28 @@ subdir	:= setjmp
 
 include ../Makeconfig
 
-headers	:= setjmp.h bits/setjmp.h bits/setjmp2.h
+headers	:= setjmp.h bits/setjmp.h bits/setjmp2.h bits/dlfcn.h dlfcn/dlfcn.h
 
 routines	:= setjmp sigjmp bsd-setjmp bsd-_setjmp \
 		   longjmp __longjmp jmp-unwind
 
 tests		:= tst-setjmp jmpbug bug269-setjmp tst-setjmp-fp \
-		   tst-sigsetjmp tst-setjmp-static
+		   tst-sigsetjmp tst-setjmp-static tst-setjmp-bug21895
+
 tests-static	:= tst-setjmp-static
 
+modules-names = setjmp-bug21895
 
 include ../Rules
 
-$(objpfx)tst-setjmp-fp: $(libm)
+test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
+
+ifeq ($(build-shared),yes)
+tests: $(test-modules)
+endif
+
+$(objpfx)setjmp-bug21895.so: $(libdl)
+$(objpfx)tst-setjmp-bug21895: $(libdl)
+$(objpfx)tst-setjmp-bug21895.out: $(objpfx)setjmp-bug21895.so
+
+$(objpfx)ts-tsetjmp-fp: $(libm)
diff --git a/setjmp/setjmp-bug21895.c b/setjmp/setjmp-bug21895.c
new file mode 100644
index 0000000..d6f5516
--- /dev/null
+++ b/setjmp/setjmp-bug21895.c
@@ -0,0 +1,42 @@
+/* Copyright (C) 2013-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/>.  */
+
+/* Test that setjmp/longjmp interoperability with static dlopen.
+   Bugzila #21895.  */
+#include <alloca.h>
+#include <string.h>
+#include <setjmp.h>
+
+jmp_buf jb;
+void (*bar)(jmp_buf);
+
+void
+lbar (int i, ...)
+{
+  bar(jb);
+  for(;;);
+}
+
+void
+foo (void)
+{
+  int i = setjmp(jb);
+  char *c = alloca(256);
+  memset(c, 0, 256);
+  lbar(i);
+  for(;;);
+}
diff --git a/setjmp/tst-setjmp-bug21895.c b/setjmp/tst-setjmp-bug21895.c
new file mode 100644
index 0000000..5333494
--- /dev/null
+++ b/setjmp/tst-setjmp-bug21895.c
@@ -0,0 +1,65 @@
+/* Copyright (C) 2013-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/>.  */
+
+/* Test that setjmp/longjmp interoperability with static dlopen.
+   Bugzila #21895.  */
+
+#include <setjmp.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <dlfcn.h>
+
+static void
+bar (jmp_buf jb)
+{
+  static int i;
+  if (i++==1) exit(0);
+  longjmp(jb, i);
+}
+
+static int
+do_test (void)
+{
+  void *h = dlopen("./setjmp-bug21895.so", RTLD_NOW);
+  if (!h) {
+    puts ("FAIL: ");
+    puts (dlerror());
+    return 1;
+  }
+
+  void (*pfoo)(void) = dlsym(h, "foo");
+  if (!pfoo) {
+    puts ("FAIL: ");
+    puts (dlerror());
+    return 1;
+  }
+
+  void (**ppbar)(jmp_buf) = dlsym(h, "bar");
+  if (!ppbar) {
+    puts ("FAIL: ");
+    puts (dlerror());
+    return 1;
+  }
+
+  *ppbar = bar;
+  pfoo();
+
+  for(;;);
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/sysdeps/powerpc/powerpc64/__longjmp-common.S b/sysdeps/powerpc/powerpc64/__longjmp-common.S
index 0e10b8d..a5973c9 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)
-- 
2.7.4

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

[Adhemerval Zanella]

On 16/05/2018 16:36, Rogerio Alves wrote:
> Attached to this email I am sending the first version of the patch but I have a problem with this patch. I can't make the test work. By some reason it can't find the .so needed by the test:
> 
> FAIL:
> ./setjmp-bug21895.so: cannot open shared object file: No such file or directory
> 
> Notice that if I manually execute the command on make check stdout inside the folder it works fine. I'd appreciate any help on that.
> 
> Regards
> 
> Em 15-05-2018 17:48, Tulio Magno Quites Machado Filho escreveu:
>> Florian Weimer <fw@deneb.enyo.de> writes:
>>
>>> * Rogerio Alves:
>>>
>>>> One simple solution would be always restore the TOC pointer by uncomment
>>>> the line bellow:
>>>>
>>>> /*     std r2,FRAME_TOC_SAVE(r1)       Restore the TOC save area.  */
>>>>
>>>> Or maybe we can check if we have a valid TOC pointer before restore it,
>>>> instead #if defined SHARED.
>>>
>>> Is the register reserved for the TOC pointer in static builds, too?
>>> Then I suggest to unconditionally save nad restore it; not doing so
>>> looks like a pointless micro-optimization.
>>>
>>> Another problem with sharing jump buffers across static dlopen is that
>>> you might not have identical pointer guard values.
>>>
>>>> I would like to request for comments on this matter: Should we fix/work
>>>> this? Is feasible to change longjmp to always restore TOC pointer?
>>>
>>> Does setjmp already save it unonditionally?
>>>
>>> Removal of static dlopen is still some time away; it's likely not
>>> going to happen in this cycle, and the fix looks simple enough.
>>
>> If static dlopen is still going to be supported for some cycles, I also agree
>> it should be saved and restored unconditionally.
>>
> 
> 0001-PATCH-v1-powerpc-Always-restore-TOC-on-longjmp.patch
> 
> 
> From eb9895f1548c8f6e1826095aee3221eaf9ce84c9 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] [PATCH v1] 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

Please indicate where you actually tested this patch (since the make check
is broken).

> 
> 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.
> 	*setjmp/Makefile: Include test build directives.

You need to add each rules you might add, the ones you are changing, and
removing (check other ChangeLog for example).

> 	*setjmp/setjmp-bug21895.c: new test file.
> 	*setjmp/tst-setjmp-bug21895.c: new test file.

Capitalize the new.

> ---
>  setjmp/Makefile                              | 18 ++++++--
>  setjmp/setjmp-bug21895.c                     | 42 ++++++++++++++++++
>  setjmp/tst-setjmp-bug21895.c                 | 65 ++++++++++++++++++++++++++++
>  sysdeps/powerpc/powerpc64/__longjmp-common.S |  5 +--
>  4 files changed, 123 insertions(+), 7 deletions(-)
>  create mode 100644 setjmp/setjmp-bug21895.c
>  create mode 100644 setjmp/tst-setjmp-bug21895.c
> 
> diff --git a/setjmp/Makefile b/setjmp/Makefile
> index dc2fcc6..e715ee6 100644
> --- a/setjmp/Makefile
> +++ b/setjmp/Makefile
> @@ -22,16 +22,28 @@ subdir	:= setjmp
>  
>  include ../Makeconfig
>  
> -headers	:= setjmp.h bits/setjmp.h bits/setjmp2.h
> +headers	:= setjmp.h bits/setjmp.h bits/setjmp2.h bits/dlfcn.h dlfcn/dlfcn.h

Why does it require adding more headers here? This is meant be installed afaik
and I am not following the requirement here.

>  
>  routines	:= setjmp sigjmp bsd-setjmp bsd-_setjmp \
>  		   longjmp __longjmp jmp-unwind
>  
>  tests		:= tst-setjmp jmpbug bug269-setjmp tst-setjmp-fp \
> -		   tst-sigsetjmp tst-setjmp-static
> +		   tst-sigsetjmp tst-setjmp-static tst-setjmp-bug21895
> +

Since the idea is test for static build I would recommend to follow the other
static naming, i.e, tst-setjmp-bug21895-static.

>  tests-static	:= tst-setjmp-static
>  
> +modules-names = setjmp-bug21895
>  
>  include ../Rules
>  
> -$(objpfx)tst-setjmp-fp: $(libm)

I do not think you meant to remove this rule.

> +test-modules = $(addprefix $(objpfx),$(addsuffix .so,$(modules-names)))
> +
> +ifeq ($(build-shared),yes)
> +tests: $(test-modules)
> +endif
> +
> +$(objpfx)setjmp-bug21895.so: $(libdl)
> +$(objpfx)tst-setjmp-bug21895: $(libdl)
> +$(objpfx)tst-setjmp-bug21895.out: $(objpfx)setjmp-bug21895.so
> +
> +$(objpfx)ts-tsetjmp-fp: $(libm)

I think the idea is to build the test even if build-shared is set to no (since
you are testing static linking).  I think what you want is:

---

diff --git a/setjmp/Makefile b/setjmp/Makefile
index dc2fcc6..231474b 100644
--- a/setjmp/Makefile
+++ b/setjmp/Makefile
@@ -28,10 +28,17 @@ routines    := setjmp sigjmp bsd-setjmp bsd-_setjmp \
                   longjmp __longjmp jmp-unwind
 
 tests          := tst-setjmp jmpbug bug269-setjmp tst-setjmp-fp \
-                  tst-sigsetjmp tst-setjmp-static
-tests-static   := tst-setjmp-static
+                  tst-sigsetjmp tst-setjmp-static tst-setjmp-bug21895-static
 
+tests-static   := tst-setjmp-static tst-setjmp-bug21895-static
+
+modules-names = setjmp-bug21895
 
 include ../Rules
 
 $(objpfx)tst-setjmp-fp: $(libm)
+$(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

---

At least with qemu-static it seems to work with your patch.


> diff --git a/setjmp/setjmp-bug21895.c b/setjmp/setjmp-bug21895.c
> new file mode 100644
> index 0000000..d6f5516
> --- /dev/null
> +++ b/setjmp/setjmp-bug21895.c
> @@ -0,0 +1,42 @@
> +/* Copyright (C) 2013-2018 Free Software Foundation, Inc.

Wrong Copyright date span and missing one line description.

> +   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/>.  */
> +
> +/* Test that setjmp/longjmp interoperability with static dlopen.
> +   Bugzila #21895.  */
> +#include <alloca.h>
> +#include <string.h>
> +#include <setjmp.h>
> +
> +jmp_buf jb;
> +void (*bar)(jmp_buf);
> +
> +void
> +lbar (int i, ...)
> +{
> +  bar(jb);
> +  for(;;);
> +}
> +
> +void
> +foo (void)
> +{
> +  int i = setjmp(jb);
> +  char *c = alloca(256);
> +  memset(c, 0, 256);
> +  lbar(i);
> +  for(;;);
> +}
> diff --git a/setjmp/tst-setjmp-bug21895.c b/setjmp/tst-setjmp-bug21895.c
> new file mode 100644
> index 0000000..5333494
> --- /dev/null
> +++ b/setjmp/tst-setjmp-bug21895.c
> @@ -0,0 +1,65 @@
> +/* Copyright (C) 2013-2018 Free Software Foundation, Inc.

Wrong Copyright date span and missing one line description.

> +   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/>.  */
> +
> +/* Test that setjmp/longjmp interoperability with static dlopen.
> +   Bugzila #21895.  */
> +
> +#include <setjmp.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <dlfcn.h>
> +
> +static void
> +bar (jmp_buf jb)
> +{
> +  static int i;
> +  if (i++==1) exit(0);
> +  longjmp(jb, i);
> +}
> +
> +static int
> +do_test (void)
> +{
> +  void *h = dlopen("./setjmp-bug21895.so", RTLD_NOW);

Better to just use name, since on test environment variable in Makefile
change I proposed the path to build setjmp folder is already on the
LD_LIBRARY_PATH list.

> +  if (!h) {
> +    puts ("FAIL: ");
> +    puts (dlerror());
> +    return 1;
> +  }

Wrong indentation and please use libsupport macros. 

> +
> +  void (*pfoo)(void) = dlsym(h, "foo");
> +  if (!pfoo) {
> +    puts ("FAIL: ");
> +    puts (dlerror());
> +    return 1;
> +  }
> +
> +  void (**ppbar)(jmp_buf) = dlsym(h, "bar");
> +  if (!ppbar) {
> +    puts ("FAIL: ");
> +    puts (dlerror());
> +    return 1;
> +  }
> +
> +  *ppbar = bar;
> +  pfoo();
> +
> +  for(;;);
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"

We do not use test-skeleton.c inclusion any longer, use libsupport 
(support/test-driver.c).

> diff --git a/sysdeps/powerpc/powerpc64/__longjmp-common.S b/sysdeps/powerpc/powerpc64/__longjmp-common.S
> index 0e10b8d..a5973c9 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.  */

Space before tab in indent.

>  	ld r21,((JB_GPRS+7)*8)(r3)
>  	lfd fp21,((JB_FPRS+7)*8)(r3)
>  	ld r22,((JB_GPRS+8)*8)(r3)
> -- 2.7.4
>