[PATCH] RISC-V: Fix elfutils testsuite unwind failures.

[Palmer Dabbelt]

On Sun, 13 Jan 2019 15:48:09 PST (-0800), Jim Wilson wrote:
> The clone.S patch fixes 2 elfutils testsuite unwind failures, where the
> backtrace gets stuck repeating __thread_start until we hit the backtrace
> limit.  This was confirmed by building and installing a patched glibc and
> then building elfutils and running its testsuite.
>
> Unfortunately, the testcase isn't working as expected and I don't know why.
> The testcase passes even when my clone.S patch is not installed.  The testcase
> looks logically similarly to the elfutils testcases that are failing.  Maybe
> there is a subtle difference in how the glibc unwinding works versus the
> elfutils unwinding?  I don't have good gdb pthread support yet, so I haven't
> found a way to debug this.  Anyways, I don't know if the testcase is useful or
> not.  If the testcase isn't useful then maybe the clone.S patch is OK without
> a testcase?
>
> Jim
>
> 	[BZ #24040]
> 	* elf/Makefile (CFLAGS-tst-unwind-main.c): Add -DUSE_PTHREADS=0.
> 	* elf/tst-unwind-main.c: If USE_PTHEADS, include pthread.h and error.h
> 	(func): New.
> 	(main): If USE_PTHREADS, call pthread_create to run func.  Otherwise
> 	call func directly.
> 	* nptl/Makefile (tests): Add tst-unwind-thread.
> 	(CFLAGS-tst-unwind-thread.c): Define.
> 	* nptl/tst-unwind-thread.c: New file.
> 	* sysdeps/unix/sysv/linux/riscv/clone.S (__thread_start): Mark ra
> 	as undefined.

I'm happy with the fix and can merge that.  I'm not sure if I can actually 
merge the test suite changes, though.

Does anyone oppose me merging the whole patch?

> ---
>  elf/Makefile                          |  2 +-
>  elf/tst-unwind-main.c                 | 28 ++++++++++++++++++++++++---
>  nptl/Makefile                         |  4 +++-
>  nptl/tst-unwind-thread.c              |  2 ++
>  sysdeps/unix/sysv/linux/riscv/clone.S |  5 +++++
>  5 files changed, 36 insertions(+), 5 deletions(-)
>  create mode 100644 nptl/tst-unwind-thread.c
>
> diff --git a/elf/Makefile b/elf/Makefile
> index 9cf5cd8dfd..815bbd2041 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1497,4 +1497,4 @@ $(objpfx)tst-big-note: $(objpfx)tst-big-note-lib.so
>
>  $(objpfx)tst-unwind-ctor: $(objpfx)tst-unwind-ctor-lib.so
>
> -CFLAGS-tst-unwind-main.c += -funwind-tables
> +CFLAGS-tst-unwind-main.c += -funwind-tables -DUSE_PTHREADS=0
> diff --git a/elf/tst-unwind-main.c b/elf/tst-unwind-main.c
> index 4089e7e907..0c345dc31f 100644
> --- a/elf/tst-unwind-main.c
> +++ b/elf/tst-unwind-main.c
> @@ -20,19 +20,41 @@
>  #include <unistd.h>
>  #include <support/test-driver.h>
>
> +#if USE_PTHREADS
> +# include <pthread.h>
> +# include <error.h>
> +#endif
> +
>  static _Unwind_Reason_Code
>  callback (struct _Unwind_Context *ctx, void *arg)
>  {
>    return _URC_NO_REASON;
>  }
>
> -int
> -main (void)
> +static void *
> +func (void *a)
>  {
>    /* Arrange for this test to be killed if _Unwind_Backtrace runs into an
>       endless loop.  We cannot use the test driver because the complete
>       call chain needs to be compiled with -funwind-tables so that
> -     _Unwind_Backtrace is able to reach _start.  */
> +     _Unwind_Backtrace is able to reach the start routine.  */
>    alarm (DEFAULT_TIMEOUT);
>    _Unwind_Backtrace (callback, 0);
> +  return a;
> +}
> +
> +int
> +main (void)
> +{
> +#if USE_PTHREADS
> +  pthread_t thr;
> +  int rc = pthread_create (&thr, NULL, &func, NULL);
> +  if (rc)
> +    error (1, rc, "pthread_create");
> +  rc = pthread_join (thr, NULL);
> +  if (rc)
> +    error (1, rc, "pthread_join");
> +#else
> +  func (NULL);
> +#endif
>  }
> diff --git a/nptl/Makefile b/nptl/Makefile
> index 340282c6cb..07c2f49235 100644
> --- a/nptl/Makefile
> +++ b/nptl/Makefile
> @@ -319,7 +319,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
>  	tst-cnd-basic tst-mtx-trylock tst-cnd-broadcast \
>  	tst-cnd-timedwait tst-thrd-detach tst-mtx-basic tst-thrd-sleep \
>  	tst-mtx-recursive tst-tss-basic tst-call-once tst-mtx-timedlock \
> -	tst-rwlock-pwn
> +	tst-rwlock-pwn tst-unwind-thread
>
>  tests-internal := tst-rwlock19 tst-rwlock20 \
>  		  tst-sem11 tst-sem12 tst-sem13 \
> @@ -709,6 +709,8 @@ $(objpfx)tst-audit-threads: $(objpfx)tst-audit-threads-mod2.so
>  $(objpfx)tst-audit-threads.out: $(objpfx)tst-audit-threads-mod1.so
>  tst-audit-threads-ENV = LD_AUDIT=$(objpfx)tst-audit-threads-mod1.so
>
> +CFLAGS-tst-unwind-thread.c += -funwind-tables
> +
>  # The tests here better do not run in parallel
>  ifneq ($(filter %tests,$(MAKECMDGOALS)),)
>  .NOTPARALLEL:
> diff --git a/nptl/tst-unwind-thread.c b/nptl/tst-unwind-thread.c
> new file mode 100644
> index 0000000000..d5c38e3709
> --- /dev/null
> +++ b/nptl/tst-unwind-thread.c
> @@ -0,0 +1,2 @@
> +#define USE_PTHREADS 1
> +#include "../elf/tst-unwind-main.c"
> diff --git a/sysdeps/unix/sysv/linux/riscv/clone.S b/sysdeps/unix/sysv/linux/riscv/clone.S
> index c079c1fb9f..0ff9ab3fd9 100644
> --- a/sysdeps/unix/sysv/linux/riscv/clone.S
> +++ b/sysdeps/unix/sysv/linux/riscv/clone.S
> @@ -69,6 +69,11 @@ L (error):
>
>  ENTRY (__thread_start)
>  L (thread_start):
> +	/* Terminate call stack by noting ra is undefined.  Use a dummy
> +	   .cfi_label to force starting the FDE.  */
> +	.cfi_label .Ldummy
> +	cfi_undefined (ra)
> +
>  	/* Restore the arg for user's function.  */
>  	REG_L		a1,0(sp)	/* Function pointer.  */
>  	REG_L		a0,SZREG(sp)	/* Argument pointer.  */