[PATCH] tests: Simplify backtrace-native tests. Drop raise jmp patching even for x86_64.

[Mark Wielaard <mjw at redhat dot com>]

Hopefully this is the last patch before the elfutils 0.167 release.
Please let me know if you find issue with this patch or have anything
else pending for the release. I'll try to finally push out the release
tommorrow otherwise.

Thanks,

Mark

The backtrace-native[-biarch] testcase was a little too clever in places making
it unreliable.

On x86_64 we tried to make an interesting backtrace by catching the first signal
and then replacing the pc with the address of the first instruction of a function.
Then we would raise a new signal, through ptrace, to create a backtrace that went
from a signal frame into a frame at the start of a function. That way we could
check that we were trying to fetch the correct CFI for the (jmp) function even at
the first instruction (normally we would substract one from the return address to
get at the call address).

This works as long as the CFI for the jmp() function is identical to the CFI for
the raise() function that we "patched away". Unfortunately on Fedora rawhide glibc
has a rewritten raise() implementation that has different CFI, in particular the
CFA is calculated differently. Making the testcase fail because we cannot properly
unwind from jmp(). So this special x86_64 case has been disabled (the code is still
there in case we find another way to test this in a more reliable way).

On Ubuntu there have been spurious testcase failures because see_exec_module
found two Dwfl_Modules with the same path. This would trigger an assert. Although
this might indicate some issue (maybe we are not parsing the proc/pid/map correctly?)
it isn't clear that it really is a bug. Since the assert is not very helpful
finding any actual bug and for the testcase it is only necessary that the
first Dwfl_Module that represents the executable is found we just pick that
Dwfl_Module and don't iterate through any of the others.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 tests/ChangeLog         | 16 ++++++++++++++++
 tests/backtrace-child.c | 18 ++++++++++++++----
 tests/backtrace.c       | 39 +++++++++++++++++++++------------------
 3 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index 1d8c5b0..2e9aa0d 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,19 @@
+2016-08-25  Mark Wielaard  <mjw@redhat.com>
+
+	* backtrace-child.c: Disable and add documentation about why we disable
+	RAISE_JMP_PATCHING even on x86_64.
+	* backtrace.c (is_x86_64_native): Rename to...
+	(use_raise_jmp_patching): ... this.
+	(callback_verify): Use use_raise_jmp_patching instead of
+	is_x86_64_native.
+	(see_exec_module): Return DWARF_CB_ABORT after finding the correct exe
+	path.
+	(prepare_thread): Use RAISE_JMP_PATCHING instead of __x86_64__
+	conditional.
+	(exec_dump): Only assert on data.mod != NULL. Drop ptrdiff. Use
+	RAISE_JMP_PATCHING instead of __x86_64__ conditional. Use
+	use_raise_jmp_patching instead of is_x86_64_native.
+
 2016-08-24  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (EXTRA_DIST): Add testfilesparc64attrs.o.bz2.
diff --git a/tests/backtrace-child.c b/tests/backtrace-child.c
index 40e7b32..cf4547c 100644
--- a/tests/backtrace-child.c
+++ b/tests/backtrace-child.c
@@ -1,5 +1,5 @@
 /* Test child for parent backtrace test.
-   Copyright (C) 2013 Red Hat, Inc.
+   Copyright (C) 2013, 2016 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -19,7 +19,8 @@
    --ptraceme will call ptrace (PTRACE_TRACEME) in the two threads.
    --gencore will call abort () at its end.
    Main thread will signal SIGUSR2.  Other thread will signal SIGUSR1.
-   On x86_64 only:
+   There used to be a difference between x86_64 and other architectures.
+   To test getting a signal at the very first instruction of a function:
      PC will get changed to function 'jmp' by backtrace.c function
      prepare_thread.  Then SIGUSR2 will be signalled to backtrace-child
      which will invoke function sigusr2.
@@ -66,8 +67,17 @@
    # 5 0xf77c1a48 - 1      start
    # 6 0xf77699da - 1      start_thread
    # 7 0xf769bbfe - 1      __clone
+
+   But the raise jmp patching was unreliable. It depends on the CFI for the raise()
+   function in glibc to be the same as for the jmp() function. This is not always
+   the case. Some newer glibc versions rewrote raise() and now the CFA is calculated
+   differently. So we disable raise jmp patching everywhere.
    */
 
+#ifdef __x86_64__
+/* #define RAISE_JMP_PATCHING 1 */
+#endif
+
 #include <config.h>
 #include <assert.h>
 #include <stdlib.h>
@@ -130,7 +140,7 @@ dummy1 (void)
   asm volatile ("");
 }
 
-#ifdef __x86_64__
+#ifdef RAISE_JMP_PATCHING
 static NOINLINE_NOCLONE USED void
 jmp (void)
 {
@@ -157,7 +167,7 @@ stdarg (int f UNUSED, ...)
       assert (errno == 0);
       assert (l == 0);
     }
-#ifdef __x86_64__
+#ifdef RAISE_JMP_PATCHING
   if (! gencore)
     {
       /* Execution will get PC patched into function jmp.  */
diff --git a/tests/backtrace.c b/tests/backtrace.c
index 1247643..2440ab3 100644
--- a/tests/backtrace.c
+++ b/tests/backtrace.c
@@ -1,5 +1,5 @@
 /* Test program for unwinding of frames.
-   Copyright (C) 2013, 2014 Red Hat, Inc.
+   Copyright (C) 2013, 2014, 2016 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -64,7 +64,7 @@ dump_modules (Dwfl_Module *mod, void **userdata __attribute__ ((unused)),
   return DWARF_CB_OK;
 }
 
-static bool is_x86_64_native;
+static bool use_raise_jmp_patching;
 static pid_t check_tid;
 
 static void
@@ -93,7 +93,7 @@ callback_verify (pid_t tid, unsigned frameno, Dwarf_Addr pc,
   static bool reduce_frameno = false;
   if (reduce_frameno)
     frameno--;
-  if (! is_x86_64_native && frameno >= 2)
+  if (! use_raise_jmp_patching && frameno >= 2)
     frameno += 2;
   const char *symname2 = NULL;
   switch (frameno)
@@ -112,8 +112,8 @@ callback_verify (pid_t tid, unsigned frameno, Dwarf_Addr pc,
     case 2: // x86_64 only
       /* __restore_rt - glibc maybe does not have to have this symbol.  */
       break;
-    case 3: // x86_64 only
-      if (is_x86_64_native)
+    case 3: // use_raise_jmp_patching
+      if (use_raise_jmp_patching)
 	{
 	  /* Verify we trapped on the very first instruction of jmp.  */
 	  assert (symname != NULL && strcmp (symname, "jmp") == 0);
@@ -138,7 +138,7 @@ callback_verify (pid_t tid, unsigned frameno, Dwarf_Addr pc,
       // there is no guarantee that the compiler doesn't reorder the
       // instructions or even inserts some padding instructions at the end
       // (which apparently happens on ppc64).
-      if (is_x86_64_native)
+      if (use_raise_jmp_patching)
         assert (symname2 == NULL || strcmp (symname2, "backtracegen") != 0);
       break;
   }
@@ -243,10 +243,10 @@ see_exec_module (Dwfl_Module *mod, void **userdata __attribute__ ((unused)),
     return DWARF_CB_OK;
   assert (data->mod == NULL);
   data->mod = mod;
-  return DWARF_CB_OK;
+  return DWARF_CB_ABORT;
 }
 
-/* On x86_64 only:
+/* We used to do this on x86_64 only (see backtrace-child why we now don't):
      PC will get changed to function 'jmp' by backtrace.c function
      prepare_thread.  Then SIGUSR2 will be signalled to backtrace-child
      which will invoke function sigusr2.
@@ -254,13 +254,17 @@ see_exec_module (Dwfl_Module *mod, void **userdata __attribute__ ((unused)),
      instruction of a function.  Properly handled unwind should not slip into
      the previous unrelated function.  */
 
+#ifdef __x86_64__
+/* #define RAISE_JMP_PATCHING 1 */
+#endif
+
 static void
 prepare_thread (pid_t pid2 __attribute__ ((unused)),
 		void (*jmp) (void) __attribute__ ((unused)))
 {
-#ifndef __x86_64__
+#ifndef RAISE_JMP_PATCHING
   abort ();
-#else /* x86_64 */
+#else /* RAISE_JMP_PATCHING */
   long l;
   struct user_regs_struct user_regs;
   errno = 0;
@@ -278,7 +282,7 @@ prepare_thread (pid_t pid2 __attribute__ ((unused)),
   assert (got == pid2);
   assert (WIFSTOPPED (status));
   assert (WSTOPSIG (status) == SIGUSR1);
-#endif /* __x86_64__ */
+#endif /* RAISE_JMP_PATCHING */
 }
 
 #include <asm/unistd.h>
@@ -370,21 +374,20 @@ exec_dump (const char *exec)
   assert (ssize > 0 && ssize < (ssize_t) sizeof (data.selfpath));
   data.selfpath[ssize] = '\0';
   data.mod = NULL;
-  ptrdiff_t ptrdiff = dwfl_getmodules (dwfl, see_exec_module, &data, 0);
-  assert (ptrdiff == 0);
+  dwfl_getmodules (dwfl, see_exec_module, &data, 0);
   assert (data.mod != NULL);
   GElf_Addr loadbase;
   Elf *elf = dwfl_module_getelf (data.mod, &loadbase);
   GElf_Ehdr ehdr_mem, *ehdr = gelf_getehdr (elf, &ehdr_mem);
   assert (ehdr != NULL);
   /* It is false also on x86_64 with i386 inferior.  */
-#ifndef __x86_64__
-  is_x86_64_native = false;
-#else /* __x86_64__ */
-  is_x86_64_native = ehdr->e_machine == EM_X86_64;
+#ifndef RAISE_JMP_PATCHING
+  use_raise_jmp_patching = false;
+#else /* RAISE_JMP_PATCHING_ */
+  use_raise_jmp_patching = ehdr->e_machine == EM_X86_64;
 #endif /* __x86_64__ */
   void (*jmp) (void) = 0;
-  if (is_x86_64_native)
+  if (use_raise_jmp_patching)
     {
       // Find inferior symbol named "jmp".
       int nsym = dwfl_module_getsymtab (data.mod);
-- 
2.7.4