[PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

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

Hi,

On s390/s390x backtrace(buffer, size) returns the series of called 
functions until "makecontext_ret" and additional entries (up to "size") 
with "makecontext_ret".
GDB-backtrace is also warning: "Backtrace stopped: previous frame 
identical to this frame (corrupt stack?)"

To reproduce this scenario you have to setup a new context with 
makecontext() and activate it with setcontext(). See e.g. cf() function 
in testcase stdlib/tst-makecontext.c.
Or see bug in libgo "Bug 66303 - runtime.Caller() returns infinitely 
deep stack frames on s390x " 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66303).

This patch omits the cfi_startproc/cfi_endproc directives in ENTRY/END 
macro of __makecontext_ret. Thus no frame information is generated in 
.eh_frame and backtrace stops after __makecontext_ret. There is also no 
.eh_frame info for _start or thread_start functions.

Ok to commit?

Bye
Stefan

---
2015-06-09  Stefan Liebler  <stli@linux.vnet.ibm.com>

	[BZ #18508]
	* sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S
	(__makecontext_ret): Omit cfi_startproc and cfi_endproc.
	* sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S:
	Likewise.

[-- Attachment #2: 20150609_backtrace_makecontext.patch --]
[-- Type: text/x-patch, Size: 1452 bytes --]

diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S b/sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S
index e1f9347..8d36ea2 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S
@@ -17,6 +17,14 @@
 
 #include <sysdep.h>
 
+/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
+   if backtrace() was called within a context created by makecontext.
+   (there is also no .eh_frame info for _start or thread_start)  */
+#undef cfi_startproc
+#define cfi_startproc
+#undef cfi_endproc
+#define cfi_endproc
+
 ENTRY(__makecontext_ret)
 	basr  %r14,%r7
 	ltr   %r8,%r8			/* Check whether uc_link is 0.  */
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S b/sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S
index 11a3cd3..64619f1 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S
@@ -17,6 +17,14 @@
 
 #include <sysdep.h>
 
+/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
+   if backtrace() was called within a context created by makecontext.
+   (there is also no .eh_frame info for _start or thread_start)  */
+#undef cfi_startproc
+#define cfi_startproc
+#undef cfi_endproc
+#define cfi_endproc
+
 ENTRY(__makecontext_ret)
 	basr	%r14,%r7
 	ltgr	%r8,%r8			/* Check whether uc_link is 0.  */

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Florian Weimer]

On 06/09/2015 05:42 PM, Stefan Liebler wrote:

> On s390/s390x backtrace(buffer, size) returns the series of called
> functions until "makecontext_ret" and additional entries (up to "size")
> with "makecontext_ret".
> GDB-backtrace is also warning: "Backtrace stopped: previous frame
> identical to this frame (corrupt stack?)"

Is it possible to extract a test case from the Go program, by calling
makecontext and backtrace in the same way to Go run time does?

I think this would be a useful addition to the glibc test suite.

-- 
Florian Weimer / Red Hat Product Security

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

On 06/12/2015 11:14 AM, Florian Weimer wrote:
> On 06/09/2015 05:42 PM, Stefan Liebler wrote:
>
>> On s390/s390x backtrace(buffer, size) returns the series of called
>> functions until "makecontext_ret" and additional entries (up to "size")
>> with "makecontext_ret".
>> GDB-backtrace is also warning: "Backtrace stopped: previous frame
>> identical to this frame (corrupt stack?)"
>
> Is it possible to extract a test case from the Go program, by calling
> makecontext and backtrace in the same way to Go run time does?
>
> I think this would be a useful addition to the glibc test suite.
>
Yes, i can make a testcase like stdlib/tst-makecontext.c or extend it - 
see below.

Can i call _Unwind_Backtrace directly on all architectures? Or does it 
exist on all architectures?
The diff below uses dlopen to load libgcc, but e.g. s390 needs 
libgcc_s.so.1 and others like m68k needs libgcc_s.so.2.
Is there a way to load the correct libgcc_s.so on all architectures?

As an alternative i could use the glibc backtrace() method. But at least 
for the x86_64 version and all others who simply includes it, this 
testcase would not fail, because the backtrace_helper function has a 
check, if the address has changed in two consecutively calls.
The go runtime hasn't such a check and would fail like reported by the 
mentioned go-bug.

Any ideas?

Bye Stefan

diff --git a/stdlib/tst-makecontext.c b/stdlib/tst-makecontext.c
index 29a588e..33f9d9e 100644
--- a/stdlib/tst-makecontext.c
+++ b/stdlib/tst-makecontext.c
@@ -19,23 +19,62 @@
  #include <stdlib.h>
  #include <stdio.h>
  #include <ucontext.h>
+#include <assert.h>
+#include <unwind.h>
+#include <dlfcn.h>

  ucontext_t ucp;
-char st1[8192];
+char st1[16384];
  __thread int thr;

+struct trace_arg
+{
+  int cnt, size;
+};
+
  int somevar = -76;
  long othervar = -78L;

+static _Unwind_Reason_Code
+backtrace_helper (struct _Unwind_Context *ctx, void *a)
+{
+  struct trace_arg *arg = a;
+
+  if (++arg->cnt == arg->size)
+    return _URC_END_OF_STACK;
+  return _URC_NO_REASON;
+}
+
  void
  cf (int i)
  {
+  struct trace_arg arg = { .size = 100, .cnt = -1 };
+  void *handle;
+  _Unwind_Reason_Code (*unwind_backtrace) (_Unwind_Trace_Fn, void *);
+
    if (i != othervar || thr != 94)
      {
        printf ("i %d thr %d\n", i, thr);
        exit (1);
      }

+  /* Test if callback function of _Unwind_Backtrace is not called 
infinitely
+     times. See Bug 18508 or gcc bug "Bug 66303 - runtime.Caller() returns
+     infinitely deep stack frames on s390x.".
+     The go runtime calls backtrace_full() in
+     <gcc-src>/libbacktrace/backtrace.c, which uses 
_Unwind_Backtrace().  */
+  handle = __libc_dlopen ("libgcc_s.so.1");
+  if (handle != NULL)
+    {
+      unwind_backtrace = __libc_dlsym (handle, "_Unwind_Backtrace");
+      if (unwind_backtrace != NULL)
+       {
+         unwind_backtrace (backtrace_helper, &arg);
+         assert (arg.cnt != -1 && arg.cnt < 100);
+       }
+      __libc_dlclose (handle);
+    }
+
    /* Since uc_link below has been set to NULL, setcontext is supposed to
       terminate the process normally after this function returns.  */
  }

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Andreas Schwab]

Stefan Liebler <stli@linux.vnet.ibm.com> writes:

> Is there a way to load the correct libgcc_s.so on all architectures?

#include <gnu/lib-names.h>
dlopen(LIBGCC_S_SO, ...)

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

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

On 06/12/2015 05:40 PM, Andreas Schwab wrote:
> Stefan Liebler <stli@linux.vnet.ibm.com> writes:
>
>> Is there a way to load the correct libgcc_s.so on all architectures?
>
> #include <gnu/lib-names.h>
> dlopen(LIBGCC_S_SO, ...)
>
> Andreas.
>
Thanks.
Here is the patch with the enhanced tst-makecontext testcase.
It calls _Unwind_Backtrace within cf() function, which is called in a 
makecontext context and check if the callback function is not called 
infinitely times.

Tested on s390/s390x before and after this patch.
Before: Test fails; After: Test succeeds.

Ok to commit?

Bye Stefan

---
2015-06-15  Stefan Liebler  <stli@linux.vnet.ibm.com>

	[BZ #18508]
	* stdlib/Makefile ($(objpfx)tst-makecontext3):
	Depend on $(libdl).
	* stdlib/tst-makecontext.c (cf): Test if _Unwind_Backtrace
	is not called infinitely times.
	(backtrace_helper): New function.
	(trace_arg): New struct.
	(st1): Enlarge stack size.
	* sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S:
	(__makecontext_ret): Omit cfi_startproc and cfi_endproc.
	* sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S:
	Likewise.

[-- Attachment #2: 20150615_makecontext_backtrace --]
[-- Type: text/plain, Size: 3581 bytes --]

diff --git a/stdlib/Makefile b/stdlib/Makefile
index 3300dd2..7fc5a80 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -164,3 +164,5 @@ $(objpfx)tst-setcontext3.out: tst-setcontext3.sh $(objpfx)tst-setcontext3
 		 '$(run-program-env)' '$(test-program-prefix-after-env)' \
 		 $(common-objpfx)stdlib/; \
 	$(evaluate-test)
+
+$(objpfx)tst-makecontext: $(libdl)
diff --git a/stdlib/tst-makecontext.c b/stdlib/tst-makecontext.c
index 29a588e..8170e8a 100644
--- a/stdlib/tst-makecontext.c
+++ b/stdlib/tst-makecontext.c
@@ -19,23 +19,62 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <ucontext.h>
+#include <assert.h>
+#include <unwind.h>
+#include <dlfcn.h>
+#include <gnu/lib-names.h>
 
 ucontext_t ucp;
-char st1[8192];
+char st1[16384];
 __thread int thr;
 
 int somevar = -76;
 long othervar = -78L;
 
+struct trace_arg
+{
+  int cnt, size;
+};
+
+static _Unwind_Reason_Code
+backtrace_helper (struct _Unwind_Context *ctx, void *a)
+{
+  struct trace_arg *arg = a;
+  if (++arg->cnt == arg->size)
+    return _URC_END_OF_STACK;
+  return _URC_NO_REASON;
+}
+
 void
 cf (int i)
 {
+  struct trace_arg arg = { .size = 100, .cnt = -1 };
+  void *handle;
+  _Unwind_Reason_Code (*unwind_backtrace) (_Unwind_Trace_Fn, void *);
+
   if (i != othervar || thr != 94)
     {
       printf ("i %d thr %d\n", i, thr);
       exit (1);
     }
 
+  /* Test if callback function of _Unwind_Backtrace is not called infinitely
+     times. See Bug 18508 or gcc bug "Bug 66303 - runtime.Caller() returns
+     infinitely deep stack frames on s390x.".
+     The go runtime calls backtrace_full() in
+     <gcc-src>/libbacktrace/backtrace.c, which uses _Unwind_Backtrace().  */
+  handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);
+  if (handle != NULL)
+    {
+      unwind_backtrace = dlsym (handle, "_Unwind_Backtrace");
+      if (unwind_backtrace != NULL)
+	{
+	  unwind_backtrace (backtrace_helper, &arg);
+	  assert (arg.cnt != -1 && arg.cnt < 100);
+	}
+      dlclose (handle);
+    }
+
   /* Since uc_link below has been set to NULL, setcontext is supposed to
      terminate the process normally after this function returns.  */
 }
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S b/sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S
index e1f9347..8d36ea2 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S
@@ -17,6 +17,14 @@
 
 #include <sysdep.h>
 
+/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
+   if backtrace() was called within a context created by makecontext.
+   (there is also no .eh_frame info for _start or thread_start)  */
+#undef cfi_startproc
+#define cfi_startproc
+#undef cfi_endproc
+#define cfi_endproc
+
 ENTRY(__makecontext_ret)
 	basr  %r14,%r7
 	ltr   %r8,%r8			/* Check whether uc_link is 0.  */
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S b/sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S
index 11a3cd3..64619f1 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S
@@ -17,6 +17,14 @@
 
 #include <sysdep.h>
 
+/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
+   if backtrace() was called within a context created by makecontext.
+   (there is also no .eh_frame info for _start or thread_start)  */
+#undef cfi_startproc
+#define cfi_startproc
+#undef cfi_endproc
+#define cfi_endproc
+
 ENTRY(__makecontext_ret)
 	basr	%r14,%r7
 	ltgr	%r8,%r8			/* Check whether uc_link is 0.  */

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

PING

On 06/15/2015 10:47 AM, Stefan Liebler wrote:
> On 06/12/2015 05:40 PM, Andreas Schwab wrote:
>> Stefan Liebler <stli@linux.vnet.ibm.com> writes:
>>
>>> Is there a way to load the correct libgcc_s.so on all architectures?
>>
>> #include <gnu/lib-names.h>
>> dlopen(LIBGCC_S_SO, ...)
>>
>> Andreas.
>>
> Thanks.
> Here is the patch with the enhanced tst-makecontext testcase.
> It calls _Unwind_Backtrace within cf() function, which is called in a
> makecontext context and check if the callback function is not called
> infinitely times.
>
> Tested on s390/s390x before and after this patch.
> Before: Test fails; After: Test succeeds.
>
> Ok to commit?
>
> Bye Stefan
>
> ---
> 2015-06-15  Stefan Liebler  <stli@linux.vnet.ibm.com>
>
>      [BZ #18508]
>      * stdlib/Makefile ($(objpfx)tst-makecontext3):
>      Depend on $(libdl).
>      * stdlib/tst-makecontext.c (cf): Test if _Unwind_Backtrace
>      is not called infinitely times.
>      (backtrace_helper): New function.
>      (trace_arg): New struct.
>      (st1): Enlarge stack size.
>      * sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S:
>      (__makecontext_ret): Omit cfi_startproc and cfi_endproc.
>      * sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S:
>      Likewise.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

ping

On 06/19/2015 12:44 PM, Stefan Liebler wrote:
> PING
>
> On 06/15/2015 10:47 AM, Stefan Liebler wrote:
>> On 06/12/2015 05:40 PM, Andreas Schwab wrote:
>>> Stefan Liebler <stli@linux.vnet.ibm.com> writes:
>>>
>>>> Is there a way to load the correct libgcc_s.so on all architectures?
>>>
>>> #include <gnu/lib-names.h>
>>> dlopen(LIBGCC_S_SO, ...)
>>>
>>> Andreas.
>>>
>> Thanks.
>> Here is the patch with the enhanced tst-makecontext testcase.
>> It calls _Unwind_Backtrace within cf() function, which is called in a
>> makecontext context and check if the callback function is not called
>> infinitely times.
>>
>> Tested on s390/s390x before and after this patch.
>> Before: Test fails; After: Test succeeds.
>>
>> Ok to commit?
>>
>> Bye Stefan
>>
>> ---
>> 2015-06-15  Stefan Liebler  <stli@linux.vnet.ibm.com>
>>
>>      [BZ #18508]
>>      * stdlib/Makefile ($(objpfx)tst-makecontext3):
>>      Depend on $(libdl).
>>      * stdlib/tst-makecontext.c (cf): Test if _Unwind_Backtrace
>>      is not called infinitely times.
>>      (backtrace_helper): New function.
>>      (trace_arg): New struct.
>>      (st1): Enlarge stack size.
>>      * sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S:
>>      (__makecontext_ret): Omit cfi_startproc and cfi_endproc.
>>      * sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S:
>>      Likewise.
>
>

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

ping

On 06/26/2015 01:11 PM, Stefan Liebler wrote:
> ping
>
> On 06/19/2015 12:44 PM, Stefan Liebler wrote:
>> PING
>>
>> On 06/15/2015 10:47 AM, Stefan Liebler wrote:
>>> On 06/12/2015 05:40 PM, Andreas Schwab wrote:
>>>> Stefan Liebler <stli@linux.vnet.ibm.com> writes:
>>>>
>>>>> Is there a way to load the correct libgcc_s.so on all architectures?
>>>>
>>>> #include <gnu/lib-names.h>
>>>> dlopen(LIBGCC_S_SO, ...)
>>>>
>>>> Andreas.
>>>>
>>> Thanks.
>>> Here is the patch with the enhanced tst-makecontext testcase.
>>> It calls _Unwind_Backtrace within cf() function, which is called in a
>>> makecontext context and check if the callback function is not called
>>> infinitely times.
>>>
>>> Tested on s390/s390x before and after this patch.
>>> Before: Test fails; After: Test succeeds.
>>>
>>> Ok to commit?
>>>
>>> Bye Stefan
>>>
>>> ---
>>> 2015-06-15  Stefan Liebler  <stli@linux.vnet.ibm.com>
>>>
>>>      [BZ #18508]
>>>      * stdlib/Makefile ($(objpfx)tst-makecontext3):
>>>      Depend on $(libdl).
>>>      * stdlib/tst-makecontext.c (cf): Test if _Unwind_Backtrace
>>>      is not called infinitely times.
>>>      (backtrace_helper): New function.
>>>      (trace_arg): New struct.
>>>      (st1): Enlarge stack size.
>>>      * sysdeps/unix/sysv/linux/s390/s390-32/__makecontext_ret.S:
>>>      (__makecontext_ret): Omit cfi_startproc and cfi_endproc.
>>>      * sysdeps/unix/sysv/linux/s390/s390-64/__makecontext_ret.S:
>>>      Likewise.
>>
>>
>
>

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Florian Weimer]

On 06/15/2015 10:47 AM, Stefan Liebler wrote:
> +/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
> +   if backtrace() was called within a context created by makecontext.
> +   (there is also no .eh_frame info for _start or thread_start)  */

> +/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
> +   if backtrace() was called within a context created by makecontext.
> +   (there is also no .eh_frame info for _start or thread_start)  */

I think this should read:

“We do not want .eh_frame ifno so that __makecontext_ret stops unwinding
if backtrace was called within a context created by makecontext.  (There
is also no .eh_frame info for _start or thread_start.)”

Could a native speaker review this, please?

Rest of the patch seems okay to me.

-- 
Florian Weimer / Red Hat Product Security

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[H.J. Lu]

On Fri, Jul 3, 2015 at 2:11 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 06/15/2015 10:47 AM, Stefan Liebler wrote:
>> +/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
>> +   if backtrace() was called within a context created by makecontext.
>> +   (there is also no .eh_frame info for _start or thread_start)  */
>
>> +/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
>> +   if backtrace() was called within a context created by makecontext.
>> +   (there is also no .eh_frame info for _start or thread_start)  */
>
> I think this should read:
>
> “We do not want .eh_frame ifno so that __makecontext_ret stops unwinding
> if backtrace was called within a context created by makecontext.  (There
> is also no .eh_frame info for _start or thread_start.)”
>
> Could a native speaker review this, please?
>
> Rest of the patch seems okay to me.
>

The new test failed on Fedora/22/ i686:

cf (i=-78) at tst-makecontext.c:50
50 {
(gdb)
55  if (i != othervar || thr != 94)
(gdb)
51  struct trace_arg arg = { .size = 100, .cnt = -1 };
(gdb)
55  if (i != othervar || thr != 94)
(gdb)
66  handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);
(gdb)
67  if (handle != NULL)
(gdb)
66  handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);
(gdb)
67  if (handle != NULL)
(gdb)
69      unwind_backtrace = dlsym (handle, "_Unwind_Backtrace");
(gdb)
70      if (unwind_backtrace != NULL)
(gdb)
72  unwind_backtrace (backtrace_helper, &arg);
(gdb)

Program received signal SIGSEGV, Segmentation fault.
0xf7de14a1 in ?? () from /lib/libgcc_s.so.1

-- 
H.J.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[H.J. Lu]

On Tue, Jul 7, 2015 at 10:31 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jul 3, 2015 at 2:11 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 06/15/2015 10:47 AM, Stefan Liebler wrote:
>>> +/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
>>> +   if backtrace() was called within a context created by makecontext.
>>> +   (there is also no .eh_frame info for _start or thread_start)  */
>>
>>> +/* We do not want .eh_frame info for __makecontext_ret to stop unwinding
>>> +   if backtrace() was called within a context created by makecontext.
>>> +   (there is also no .eh_frame info for _start or thread_start)  */
>>
>> I think this should read:
>>
>> “We do not want .eh_frame ifno so that __makecontext_ret stops unwinding
>> if backtrace was called within a context created by makecontext.  (There
>> is also no .eh_frame info for _start or thread_start.)”
>>
>> Could a native speaker review this, please?
>>
>> Rest of the patch seems okay to me.
>>
>
> The new test failed on Fedora/22/ i686:
>
> cf (i=-78) at tst-makecontext.c:50
> 50 {
> (gdb)
> 55  if (i != othervar || thr != 94)
> (gdb)
> 51  struct trace_arg arg = { .size = 100, .cnt = -1 };
> (gdb)
> 55  if (i != othervar || thr != 94)
> (gdb)
> 66  handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);
> (gdb)
> 67  if (handle != NULL)
> (gdb)
> 66  handle = dlopen (LIBGCC_S_SO, RTLD_LAZY);
> (gdb)
> 67  if (handle != NULL)
> (gdb)
> 69      unwind_backtrace = dlsym (handle, "_Unwind_Backtrace");
> (gdb)
> 70      if (unwind_backtrace != NULL)
> (gdb)
> 72  unwind_backtrace (backtrace_helper, &arg);
> (gdb)
>
> Program received signal SIGSEGV, Segmentation fault.
> 0xf7de14a1 in ?? () from /lib/libgcc_s.so.1
>

I opened:

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

-- 
H.J.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Carlos O'Donell]

On 07/07/2015 01:47 PM, H.J. Lu wrote:
> I opened:
> https://sourceware.org/bugzilla/show_bug.cgi?id=18635

Stefan,

Please fix this promptly as glibc 2.22 will be released
shortly and the test results should be clean. If we can't
fix it promptly, then we should revert the test changes.

Cheers,
Carlos.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

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

On 07/07/2015 09:02 PM, Carlos O'Donell wrote:
> On 07/07/2015 01:47 PM, H.J. Lu wrote:
>> I opened:
>> https://sourceware.org/bugzilla/show_bug.cgi?id=18635
>
> Stefan,
>
> Please fix this promptly as glibc 2.22 will be released
> shortly and the test results should be clean. If we can't
> fix it promptly, then we should revert the test changes.
>
> Cheers,
> Carlos.
>
Hi,

the testcase seems to be okay, but there is a bug in i686 backtrace 
handling if the context was set via makecontext.
If we revert the test changes, the test case will pass, but the bug is 
only hidden.

If you call backtrace() function in such a context, you'll get a 
segmentation fault, too. backtrace () also uses _Unwind_Backtrace.

The "exitcode"-block within __makecontext in 
sysdeps/unix/sysv/linux/i386/makecontext.S is surrounded by cfi_endproc 
and cfi_startproc.
The start addresses of these cfi's seems okay, but the end addresses are 
strange - see readelf/objdump output below.

readelf --debug-dump=frames libc.so:
000043dc 00000014 000043e0 FDE cie=00000000 pc=0003f610..0e44f810
Augmentation data: 41 0e 04 10 00 00 00 f8

000043f4 00000010 000043f8 FDE cie=00000000 pc=0003f67e..0003f67e
Augmentation data: 00 00 00 0c 44 00 00 d8 43 ec ff a6 00 00 00 0

objdump -d libc.so:
/* ENTRY(__makecontext) */
0003f610 <makecontext>:
3f610: 8b 44 24 04 mov 0x4(%esp),%eax
...
/* cfi_endproc  in makecontext.S.  */
/* L(exitcode): */
3f65b: 8d 24 9c lea (%esp,%ebx,4),%esp
3f65e: e8 00 00 00 00 call 3f663 <makecontext+0x53>
3f663: 5b pop %ebx
3f664: 81 c3 91 39 16 00 add $0x163991,%ebx
3f66a: 83 3c 24 00 cmpl $0x0,(%esp)
3f66e: 74 08 je 3f678 <makecontext+0x68>
3f670: e8 2b ff ff ff call 3f5a0 <setcontext>
3f675: 89 04 24 mov %eax,(%esp)
3f678: e8 13 01 ff ff call 2f790 <exit>
3f67d: f4 hlt
/* cfi_startproc in makecontext.S.  */
/* END(__makecontext)  */
3f67e: 90 nop
3f67f: 90 nop


For a quick test, I extracted the exitcode-block to a new function with 
ENTRY/END-macros and undefined cfi_start/end_proc, like it is done in 
s390-makecontext_ret - see attached patch. Afterwards _Unwind_backtrace 
does not segfault anymore.

Please test/comment.

Bye Stefan

[-- Attachment #2: i386_makecontext.patch --]
[-- Type: text/x-patch, Size: 1174 bytes --]

diff --git a/sysdeps/unix/sysv/linux/i386/makecontext.S b/sysdeps/unix/sysv/linux/i386/makecontext.S
index 8364fb9..f88abf9 100644
--- a/sysdeps/unix/sysv/linux/i386/makecontext.S
+++ b/sysdeps/unix/sysv/linux/i386/makecontext.S
@@ -85,6 +85,7 @@ ENTRY(__makecontext)
 #endif
 	/* 'makecontext' returns no value.  */
 	ret
+END(__makecontext)
 
 	/* This is the helper code which gets called if a function which
 	   is registered with 'makecontext' returns.  In this case we
@@ -92,7 +93,11 @@ ENTRY(__makecontext)
 	   the context 'makecontext' manipulated at the time of the
 	   'makecontext' call.  If the pointer is NULL the process must
 	   terminate.  */
-	cfi_endproc
+#undef cfi_startproc
+#define cfi_startproc
+#undef cfi_endproc
+#define cfi_endproc
+ENTRY(__makecontext_ret)
 L(exitcode):
 	/* This removes the parameters passed to the function given to
 	   'makecontext' from the stack.  EBX contains the number of
@@ -116,7 +121,6 @@ L(exitcode):
 	/* The 'exit' call should never return.  In case it does cause
 	   the process to terminate.  */
 	hlt
-	cfi_startproc
-END(__makecontext)
+END(__makecontext_ret)
 
 weak_alias (__makecontext, makecontext)

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[H.J. Lu]

On Wed, Jul 8, 2015 at 5:00 AM, Stefan Liebler <stli@linux.vnet.ibm.com> wrote:
> On 07/07/2015 09:02 PM, Carlos O'Donell wrote:
>>
>> On 07/07/2015 01:47 PM, H.J. Lu wrote:
>>>
>>> I opened:
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=18635
>>
>>
>> Stefan,
>>
>> Please fix this promptly as glibc 2.22 will be released
>> shortly and the test results should be clean. If we can't
>> fix it promptly, then we should revert the test changes.
>>
>> Cheers,
>> Carlos.
>>
> Hi,
>
> the testcase seems to be okay, but there is a bug in i686 backtrace handling
> if the context was set via makecontext.
> If we revert the test changes, the test case will pass, but the bug is only
> hidden.
>
> If you call backtrace() function in such a context, you'll get a
> segmentation fault, too. backtrace () also uses _Unwind_Backtrace.
>
> The "exitcode"-block within __makecontext in
> sysdeps/unix/sysv/linux/i386/makecontext.S is surrounded by cfi_endproc and
> cfi_startproc.
> The start addresses of these cfi's seems okay, but the end addresses are
> strange - see readelf/objdump output below.
>
> readelf --debug-dump=frames libc.so:
> 000043dc 00000014 000043e0 FDE cie=00000000 pc=0003f610..0e44f810
> Augmentation data: 41 0e 04 10 00 00 00 f8
>
> 000043f4 00000010 000043f8 FDE cie=00000000 pc=0003f67e..0003f67e
> Augmentation data: 00 00 00 0c 44 00 00 d8 43 ec ff a6 00 00 00 0
>
> objdump -d libc.so:
> /* ENTRY(__makecontext) */
> 0003f610 <makecontext>:
> 3f610: 8b 44 24 04 mov 0x4(%esp),%eax
> ...
> /* cfi_endproc  in makecontext.S.  */
> /* L(exitcode): */
> 3f65b: 8d 24 9c lea (%esp,%ebx,4),%esp
> 3f65e: e8 00 00 00 00 call 3f663 <makecontext+0x53>
> 3f663: 5b pop %ebx
> 3f664: 81 c3 91 39 16 00 add $0x163991,%ebx
> 3f66a: 83 3c 24 00 cmpl $0x0,(%esp)
> 3f66e: 74 08 je 3f678 <makecontext+0x68>
> 3f670: e8 2b ff ff ff call 3f5a0 <setcontext>
> 3f675: 89 04 24 mov %eax,(%esp)
> 3f678: e8 13 01 ff ff call 2f790 <exit>
> 3f67d: f4 hlt
> /* cfi_startproc in makecontext.S.  */
> /* END(__makecontext)  */
> 3f67e: 90 nop
> 3f67f: 90 nop
>
>
> For a quick test, I extracted the exitcode-block to a new function with
> ENTRY/END-macros and undefined cfi_start/end_proc, like it is done in
> s390-makecontext_ret - see attached patch. Afterwards _Unwind_backtrace does
> not segfault anymore.
>
> Please test/comment.

It works.

Thanks.

-- 
H.J.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Carlos O'Donell]

On 07/08/2015 08:00 AM, Stefan Liebler wrote:
> On 07/07/2015 09:02 PM, Carlos O'Donell wrote:
>> On 07/07/2015 01:47 PM, H.J. Lu wrote:
>>> I opened:
>>> https://sourceware.org/bugzilla/show_bug.cgi?id=18635
>>
>> Stefan,
>>
>> Please fix this promptly as glibc 2.22 will be released
>> shortly and the test results should be clean. If we can't
>> fix it promptly, then we should revert the test changes.
>>
>> Cheers,
>> Carlos.
>>
> Hi,
> 
> the testcase seems to be okay, but there is a bug in i686 backtrace handling if the context was set via makecontext.
> If we revert the test changes, the test case will pass, but the bug is only hidden.

If that's the case then please file a bug on sourceware for
this particular failure, mark the test as XFAIL for i686 and
use a comment to reference the bug when you mark it XFAIL.

Cheers,
Carlos.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Carlos O'Donell]

On 07/08/2015 10:43 AM, Carlos O'Donell wrote:
> On 07/08/2015 08:00 AM, Stefan Liebler wrote:
>> On 07/07/2015 09:02 PM, Carlos O'Donell wrote:
>>> On 07/07/2015 01:47 PM, H.J. Lu wrote:
>>>> I opened:
>>>> https://sourceware.org/bugzilla/show_bug.cgi?id=18635
>>>
>>> Stefan,
>>>
>>> Please fix this promptly as glibc 2.22 will be released
>>> shortly and the test results should be clean. If we can't
>>> fix it promptly, then we should revert the test changes.
>>>
>>> Cheers,
>>> Carlos.
>>>
>> Hi,
>>
>> the testcase seems to be okay, but there is a bug in i686 backtrace handling if the context was set via makecontext.
>> If we revert the test changes, the test case will pass, but the bug is only hidden.
> 
> If that's the case then please file a bug on sourceware for
> this particular failure, mark the test as XFAIL for i686 and
> use a comment to reference the bug when you mark it XFAIL.

Bug 18635 was already filed by H.J, so use that.

c.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Carlos O'Donell]

On 07/08/2015 10:44 AM, H.J. Lu wrote:
>> For a quick test, I extracted the exitcode-block to a new function with
>> ENTRY/END-macros and undefined cfi_start/end_proc, like it is done in
>> s390-makecontext_ret - see attached patch. Afterwards _Unwind_backtrace does
>> not segfault anymore.
>>
>> Please test/comment.
> 
> It works.

But we don't really know why...

c.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[H.J. Lu]

On Wed, Jul 8, 2015 at 7:47 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 07/08/2015 10:44 AM, H.J. Lu wrote:
>>> For a quick test, I extracted the exitcode-block to a new function with
>>> ENTRY/END-macros and undefined cfi_start/end_proc, like it is done in
>>> s390-makecontext_ret - see attached patch. Afterwards _Unwind_backtrace does
>>> not segfault anymore.
>>>
>>> Please test/comment.
>>
>> It works.
>
> But we don't really know why...
>

The hand-written frame info in i386/makecontext.S is wrong as
shown in

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


Program received signal SIGSEGV, Segmentation fault.
uw_frame_state_for (context=context@entry=0x8050120 <st1+15968>,
    fs=fs@entry=0x80501a0 <st1+16096>)
    at /export/gnu/import/git/sources/gcc/libgcc/unwind-dw2.c:1249
1249      return MD_FALLBACK_FRAME_STATE_FOR (context, fs);
(gdb) bt
#0  uw_frame_state_for (context=context@entry=0x8050120 <st1+15968>,
    fs=fs@entry=0x80501a0 <st1+16096>)
    at /export/gnu/import/git/sources/gcc/libgcc/unwind-dw2.c:1249
#1  0xf7e102a8 in _Unwind_Backtrace (trace=0x8049420 <backtrace_helper>,
    trace_argument=0x8050298 <st1+16344>)
    at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:290
#2  0x080494a6 in cf (i=-78) at tst-makecontext.c:72
#3  0xf7e55d1b in makecontext ()
    at ../sysdeps/unix/sysv/linux/i386/makecontext.S:87
#4  0xffffffb2 in ?? () <<<<<<<< Bogus return address
#5  0x00000000 in ?? ()
(gdb)


-- 
H.J.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

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

On 07/08/2015 04:45 PM, Carlos O'Donell wrote:
> On 07/08/2015 10:43 AM, Carlos O'Donell wrote:
>> On 07/08/2015 08:00 AM, Stefan Liebler wrote:
>>> On 07/07/2015 09:02 PM, Carlos O'Donell wrote:
>>>> On 07/07/2015 01:47 PM, H.J. Lu wrote:
>>>>> I opened:
>>>>> https://sourceware.org/bugzilla/show_bug.cgi?id=18635
>>>>
>>>> Stefan,
>>>>
>>>> Please fix this promptly as glibc 2.22 will be released
>>>> shortly and the test results should be clean. If we can't
>>>> fix it promptly, then we should revert the test changes.
>>>>
>>>> Cheers,
>>>> Carlos.
>>>>
>>> Hi,
>>>
>>> the testcase seems to be okay, but there is a bug in i686 backtrace handling if the context was set via makecontext.
>>> If we revert the test changes, the test case will pass, but the bug is only hidden.
>>
>> If that's the case then please file a bug on sourceware for
>> this particular failure, mark the test as XFAIL for i686 and
>> use a comment to reference the bug when you mark it XFAIL.
>
> Bug 18635 was already filed by H.J, so use that.
>
> c.
>
>

This patch marks stdlib/tst-makecontext as XFAIL for i686.
Ok to commit?

Bye
Stefan

---
2015-07-09  Stefan Liebler  <stli@linux.vnet.ibm.com>

	* sysdeps/i386/i686/Makefile (test-xfail-tst-makecontext):
	New variable.

[-- Attachment #2: i686_tst_makecontext_xfail.patch --]
[-- Type: text/x-patch, Size: 492 bytes --]

diff --git a/sysdeps/i386/i686/Makefile b/sysdeps/i386/i686/Makefile
index 5ce9fc6..83517c4 100644
--- a/sysdeps/i386/i686/Makefile
+++ b/sysdeps/i386/i686/Makefile
@@ -18,3 +18,9 @@ ASFLAGS-.op += -Wa,-mtune=i686
 ASFLAGS-.og += -Wa,-mtune=i686
 ASFLAGS-.oS += -Wa,-mtune=i686
 endif
+
+ifeq ($(subdir),stdlib)
+# _Unwind_Backtrace from libgcc produces a segmentation fault if it was
+# called within a context created by makecontext. See Bug 18635.
+test-xfail-tst-makecontext = yes
+endif

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

Ping!
Is this patch okay for 2.22?

On 07/09/2015 10:23 AM, Stefan Liebler wrote:
> On 07/08/2015 04:45 PM, Carlos O'Donell wrote:
>> On 07/08/2015 10:43 AM, Carlos O'Donell wrote:
>>> On 07/08/2015 08:00 AM, Stefan Liebler wrote:
>>>> On 07/07/2015 09:02 PM, Carlos O'Donell wrote:
>>>>> On 07/07/2015 01:47 PM, H.J. Lu wrote:
>>>>>> I opened:
>>>>>> https://sourceware.org/bugzilla/show_bug.cgi?id=18635
>>>>>
>>>>> Stefan,
>>>>>
>>>>> Please fix this promptly as glibc 2.22 will be released
>>>>> shortly and the test results should be clean. If we can't
>>>>> fix it promptly, then we should revert the test changes.
>>>>>
>>>>> Cheers,
>>>>> Carlos.
>>>>>
>>>> Hi,
>>>>
>>>> the testcase seems to be okay, but there is a bug in i686 backtrace
>>>> handling if the context was set via makecontext.
>>>> If we revert the test changes, the test case will pass, but the bug
>>>> is only hidden.
>>>
>>> If that's the case then please file a bug on sourceware for
>>> this particular failure, mark the test as XFAIL for i686 and
>>> use a comment to reference the bug when you mark it XFAIL.
>>
>> Bug 18635 was already filed by H.J, so use that.
>>
>> c.
>>
>>
>
> This patch marks stdlib/tst-makecontext as XFAIL for i686.
> Ok to commit?
>
> Bye
> Stefan
>
> ---
> 2015-07-09  Stefan Liebler  <stli@linux.vnet.ibm.com>
>
>      * sysdeps/i386/i686/Makefile (test-xfail-tst-makecontext):
>      New variable.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Carlos O'Donell]

On 07/14/2015 02:11 AM, Stefan Liebler wrote:
> Ping!
> Is this patch okay for 2.22?

Yes, please check this in.

c.

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Andreas Schwab]

Stefan Liebler <stli@linux.vnet.ibm.com> writes:

> This patch marks stdlib/tst-makecontext as XFAIL for i686.

Why i686?  It fails on any ix86.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

[PATCH] Properly terminate FDE in makecontext for ix86 (bug 18635)

[Andreas Schwab]

	[BZ #18635]
	* sysdeps/unix/sysv/linux/i386/makecontext.S: Terminate FDE before
	ret.
	* sysdeps/i386/i686/Makefile (test-xfail-tst-makecontext): Remove.
---
 sysdeps/i386/i686/Makefile                 | 6 ------
 sysdeps/unix/sysv/linux/i386/makecontext.S | 5 ++++-
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/sysdeps/i386/i686/Makefile b/sysdeps/i386/i686/Makefile
index 83517c4..5ce9fc6 100644
--- a/sysdeps/i386/i686/Makefile
+++ b/sysdeps/i386/i686/Makefile
@@ -18,9 +18,3 @@ ASFLAGS-.op += -Wa,-mtune=i686
 ASFLAGS-.og += -Wa,-mtune=i686
 ASFLAGS-.oS += -Wa,-mtune=i686
 endif
-
-ifeq ($(subdir),stdlib)
-# _Unwind_Backtrace from libgcc produces a segmentation fault if it was
-# called within a context created by makecontext. See Bug 18635.
-test-xfail-tst-makecontext = yes
-endif
diff --git a/sysdeps/unix/sysv/linux/i386/makecontext.S b/sysdeps/unix/sysv/linux/i386/makecontext.S
index 8364fb9..bcf8de6 100644
--- a/sysdeps/unix/sysv/linux/i386/makecontext.S
+++ b/sysdeps/unix/sysv/linux/i386/makecontext.S
@@ -83,6 +83,10 @@ ENTRY(__makecontext)
 #else
 	movl	$L(exitcode), (%edx)
 #endif
+	/* We need to terminate the FDE here instead of after ret because
+	   the unwinder looks at ra-1 for unwind information.  */
+	cfi_endproc
+
 	/* 'makecontext' returns no value.  */
 	ret
 
@@ -92,7 +96,6 @@ ENTRY(__makecontext)
 	   the context 'makecontext' manipulated at the time of the
 	   'makecontext' call.  If the pointer is NULL the process must
 	   terminate.  */
-	cfi_endproc
 L(exitcode):
 	/* This removes the parameters passed to the function given to
 	   'makecontext' from the stack.  EBX contains the number of
-- 
2.5.0

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH][BZ #18508] S390: Fix "backtrace() returns infinitely deep stack frames with makecontext()".

[Stefan Liebler]

On 08/04/2015 12:13 PM, Andreas Schwab wrote:
> Stefan Liebler <stli@linux.vnet.ibm.com> writes:
>
>> This patch marks stdlib/tst-makecontext as XFAIL for i686.
>
> Why i686?  It fails on any ix86.
>
> Andreas.
>
There was a report, that tst-makecontext failes on i686.
Thus for i686 it was marked as XFAIL - no other reason.

Re: [PATCH] Properly terminate FDE in makecontext for ix86 (bug 18635)

[Roland McGrath]

This seems like it could have a test case.

Re: [PATCH] Properly terminate FDE in makecontext for ix86 (bug 18635)

[Roland McGrath]

HJ pointed out to me that there was a test case and you removed XFAIL for
it.
So that all looks good to me.

Re: [PATCH] Properly terminate FDE in makecontext for ix86 (bug 18635)

[Carlos O'Donell]

On 08/04/2015 06:40 AM, Andreas Schwab wrote:
> 	[BZ #18635]
> 	* sysdeps/unix/sysv/linux/i386/makecontext.S: Terminate FDE before
> 	ret.
> 	* sysdeps/i386/i686/Makefile (test-xfail-tst-makecontext): Remove.

Are you suggesting this for 2.22? I wanted this fixed for 2.22, but there
wasn't consensus as to what should be fixed.

How well tested is this?

c.

Re: [PATCH] Properly terminate FDE in makecontext for ix86 (bug 18635)

[Andreas Schwab]

"Carlos O'Donell" <carlos@redhat.com> writes:

> How well tested is this?

No testsuite regressions, and I also made sure that gdb always shows a
proper backtrace when stepping through makecontext and the new context.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."