* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
[not found] <f88767a0-1a38-564d-7bd8-0b5db5bc04df@linux.vnet.ibm.com>
@ 2018-05-22 18:41 ` Rogerio Alves
2018-05-23 9:27 ` Alexander Monakov
0 siblings, 1 reply; 16+ messages in thread
From: Rogerio Alves @ 2018-05-22 18:41 UTC (permalink / raw)
To: libc-alpha
[-- 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-22 18:41 ` [RFC] powerpc: restore TOC when static longjmp to shared object Rogerio Alves
@ 2018-05-23 9:27 ` Alexander Monakov
2018-05-23 12:33 ` Rogerio Alves
0 siblings, 1 reply; 16+ messages in thread
From: Alexander Monakov @ 2018-05-23 9:27 UTC (permalink / raw)
To: Rogerio Alves; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-23 9:27 ` Alexander Monakov
@ 2018-05-23 12:33 ` Rogerio Alves
2018-05-23 13:45 ` Tulio Magno Quites Machado Filho
2018-05-23 14:25 ` Alexander Monakov
0 siblings, 2 replies; 16+ messages in thread
From: Rogerio Alves @ 2018-05-23 12:33 UTC (permalink / raw)
To: Alexander Monakov; +Cc: libc-alpha
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-23 12:33 ` Rogerio Alves
@ 2018-05-23 13:45 ` Tulio Magno Quites Machado Filho
2018-05-23 14:25 ` Alexander Monakov
1 sibling, 0 replies; 16+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-05-23 13:45 UTC (permalink / raw)
To: Rogerio Alves, Alexander Monakov; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-23 12:33 ` Rogerio Alves
2018-05-23 13:45 ` Tulio Magno Quites Machado Filho
@ 2018-05-23 14:25 ` Alexander Monakov
2018-05-25 19:39 ` Rogerio Alves
1 sibling, 1 reply; 16+ messages in thread
From: Alexander Monakov @ 2018-05-23 14:25 UTC (permalink / raw)
To: Rogerio Alves; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-23 14:25 ` Alexander Monakov
@ 2018-05-25 19:39 ` Rogerio Alves
2018-06-04 14:34 ` Rogerio Alves
0 siblings, 1 reply; 16+ messages in thread
From: Rogerio Alves @ 2018-05-25 19:39 UTC (permalink / raw)
To: Alexander Monakov; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-25 19:39 ` Rogerio Alves
@ 2018-06-04 14:34 ` Rogerio Alves
2018-06-04 14:38 ` Florian Weimer
0 siblings, 1 reply; 16+ messages in thread
From: Rogerio Alves @ 2018-06-04 14:34 UTC (permalink / raw)
To: Alexander Monakov; +Cc: libc-alpha
[-- 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-06-04 14:34 ` Rogerio Alves
@ 2018-06-04 14:38 ` Florian Weimer
2018-06-21 19:14 ` Rogerio Alves
0 siblings, 1 reply; 16+ messages in thread
From: Florian Weimer @ 2018-06-04 14:38 UTC (permalink / raw)
To: Rogerio Alves, Alexander Monakov; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-06-04 14:38 ` Florian Weimer
@ 2018-06-21 19:14 ` Rogerio Alves
2018-07-16 19:11 ` Tulio Magno Quites Machado Filho
0 siblings, 1 reply; 16+ messages in thread
From: Rogerio Alves @ 2018-06-21 19:14 UTC (permalink / raw)
To: Florian Weimer, Alexander Monakov; +Cc: libc-alpha
[-- 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-06-21 19:14 ` Rogerio Alves
@ 2018-07-16 19:11 ` Tulio Magno Quites Machado Filho
0 siblings, 0 replies; 16+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-07-16 19:11 UTC (permalink / raw)
To: Rogerio Alves, libc-alpha; +Cc: Florian Weimer, Alexander Monakov
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-16 19:36 ` Rogerio Alves
@ 2018-05-16 20:56 ` Adhemerval Zanella
0 siblings, 0 replies; 16+ messages in thread
From: Adhemerval Zanella @ 2018-05-16 20:56 UTC (permalink / raw)
To: libc-alpha
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
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-15 20:49 ` Tulio Magno Quites Machado Filho
@ 2018-05-16 19:36 ` Rogerio Alves
2018-05-16 20:56 ` Adhemerval Zanella
0 siblings, 1 reply; 16+ messages in thread
From: Rogerio Alves @ 2018-05-16 19:36 UTC (permalink / raw)
To: Tulio Magno Quites Machado Filho, Florian Weimer; +Cc: libc-alpha
[-- 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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-15 18:53 ` Florian Weimer
2018-05-15 19:15 ` Rogerio Alves
@ 2018-05-15 20:49 ` Tulio Magno Quites Machado Filho
2018-05-16 19:36 ` Rogerio Alves
1 sibling, 1 reply; 16+ messages in thread
From: Tulio Magno Quites Machado Filho @ 2018-05-15 20:49 UTC (permalink / raw)
To: Florian Weimer, Rogerio Alves; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-15 18:53 ` Florian Weimer
@ 2018-05-15 19:15 ` Rogerio Alves
2018-05-15 20:49 ` Tulio Magno Quites Machado Filho
1 sibling, 0 replies; 16+ messages in thread
From: Rogerio Alves @ 2018-05-15 19:15 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC] powerpc: restore TOC when static longjmp to shared object
2018-05-15 17:29 Rogerio Alves
@ 2018-05-15 18:53 ` Florian Weimer
2018-05-15 19:15 ` Rogerio Alves
2018-05-15 20:49 ` Tulio Magno Quites Machado Filho
0 siblings, 2 replies; 16+ messages in thread
From: Florian Weimer @ 2018-05-15 18:53 UTC (permalink / raw)
To: Rogerio Alves; +Cc: libc-alpha
* 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC] powerpc: restore TOC when static longjmp to shared object
@ 2018-05-15 17:29 Rogerio Alves
2018-05-15 18:53 ` Florian Weimer
0 siblings, 1 reply; 16+ messages in thread
From: Rogerio Alves @ 2018-05-15 17:29 UTC (permalink / raw)
To: libc-alpha
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
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-07-16 19:11 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <f88767a0-1a38-564d-7bd8-0b5db5bc04df@linux.vnet.ibm.com>
2018-05-22 18:41 ` [RFC] powerpc: restore TOC when static longjmp to shared object Rogerio Alves
2018-05-23 9:27 ` Alexander Monakov
2018-05-23 12:33 ` Rogerio Alves
2018-05-23 13:45 ` Tulio Magno Quites Machado Filho
2018-05-23 14:25 ` Alexander Monakov
2018-05-25 19:39 ` Rogerio Alves
2018-06-04 14:34 ` Rogerio Alves
2018-06-04 14:38 ` Florian Weimer
2018-06-21 19:14 ` Rogerio Alves
2018-07-16 19:11 ` Tulio Magno Quites Machado Filho
2018-05-15 17:29 Rogerio Alves
2018-05-15 18:53 ` Florian Weimer
2018-05-15 19:15 ` Rogerio Alves
2018-05-15 20:49 ` Tulio Magno Quites Machado Filho
2018-05-16 19:36 ` Rogerio Alves
2018-05-16 20:56 ` Adhemerval Zanella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).