Re: [PATCH elfutils 2/2] [tests] parse inode in /proc/pid/maps correctly in run-backtrace-data.sh

[Mark Wielaard <mark@klomp.org>]

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

On Thu, Jan 31, 2019 at 10:14:32PM +0100, Mark Wielaard wrote:
> So your fix is correct.
> But the testcase is also slightly wrong.
> It really shouldn't check errno if the function didn't fail.
> There is no guarantee that it will be zero.

So, even though it found a bug, I am removing these asserts.
They really aren't correct. When a function fails, it will
set errno. But if a function succeeds, there is no guarantee
that it will set errno to zero.

Attached patch pushed to master.

Cheers,

Mark

[-- Attachment #2: 0001-tests-Remove-assert-errno-0-from-tests.patch --]
[-- Type: text/x-diff, Size: 5639 bytes --]

From fe7d3f3635e66fe8ec1fde91f886857b0dea7d22 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Thu, 31 Jan 2019 23:18:25 +0100
Subject: [PATCH] tests: Remove assert (errno == 0) from tests.

When a function fails it might set errno. But it isn't a guarantee
that if a function succeeds that it sets errno to zero.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 tests/ChangeLog         | 10 ++++++++++
 tests/backtrace-child.c |  2 --
 tests/backtrace-data.c  | 10 ----------
 tests/backtrace.c       |  5 -----
 4 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/tests/ChangeLog b/tests/ChangeLog
index c91764fb3..c2d868878 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,13 @@
+2019-01-31  Mark Wielaard  <mark@klomp.org>
+
+	* backtrace-child.c (stdarg): Remove assert (errno == 0).
+	(main): Likewise.
+	* backtrace-data.c (maps_lookup): Likewise.
+	(set_initial_registers): Likewise.
+	(main): Likewise.
+	* backtrace.c (prepare_thread): Likewise.
+	(exec_dump): Likewise.
+
 2019-01-29  Yonghong Song  <yhs@fb.com>
 
 	* backtrace-data.c (maps_lookup): Use %*u, not %*x, to parse
diff --git a/tests/backtrace-child.c b/tests/backtrace-child.c
index 9c6ba94f1..8bfed478c 100644
--- a/tests/backtrace-child.c
+++ b/tests/backtrace-child.c
@@ -164,7 +164,6 @@ stdarg (int f UNUSED, ...)
   if (ptraceme)
     {
       long l = ptrace (PTRACE_TRACEME, 0, NULL, NULL);
-      assert (errno == 0);
       assert (l == 0);
     }
 #ifdef RAISE_JMP_PATCHING
@@ -236,7 +235,6 @@ main (int argc UNUSED, char **argv)
     {
       errno = 0;
       long l = ptrace (PTRACE_TRACEME, 0, NULL, NULL);
-      assert (errno == 0);
       assert (l == 0);
     }
   if (gencore)
diff --git a/tests/backtrace-data.c b/tests/backtrace-data.c
index b389d6aff..907b47801 100644
--- a/tests/backtrace-data.c
+++ b/tests/backtrace-data.c
@@ -96,10 +96,8 @@ maps_lookup (pid_t pid, Dwarf_Addr addr, GElf_Addr *basep)
 {
   char *fname;
   int i = asprintf (&fname, "/proc/%ld/maps", (long) pid);
-  assert (errno == 0);
   assert (i > 0);
   FILE *f = fopen (fname, "r");
-  assert (errno == 0);
   assert (f);
   free (fname);
   for (;;)
@@ -107,7 +105,6 @@ maps_lookup (pid_t pid, Dwarf_Addr addr, GElf_Addr *basep)
       // 37e3c22000-37e3c23000 rw-p 00022000 00:11 49532 /lib64/ld-2.14.90.so */
       unsigned long start, end, offset;
       i = fscanf (f, "%lx-%lx %*s %lx %*x:%*x %*u", &start, &end, &offset);
-      assert (errno == 0);
       if (i != 3)
           break;
       char *filename = strdup ("");
@@ -129,7 +126,6 @@ maps_lookup (pid_t pid, Dwarf_Addr addr, GElf_Addr *basep)
       if (start <= addr && addr < end)
 	{
 	  i = fclose (f);
-	  assert (errno == 0);
 	  assert (i == 0);
 
 	  *basep = start - offset;
@@ -183,7 +179,6 @@ set_initial_registers (Dwfl_Thread *thread,
 
   struct user_regs_struct user_regs;
   long l = ptrace (PTRACE_GETREGS, child, NULL, &user_regs);
-  assert (errno == 0);
   assert (l == 0);
 
   Dwarf_Word dwarf_regs[17];
@@ -284,11 +279,9 @@ main (int argc __attribute__ ((unused)), char **argv __attribute__ ((unused)))
   switch (child)
   {
     case -1:
-      assert (errno == 0);
       assert (0);
     case 0:;
       long l = ptrace (PTRACE_TRACEME, 0, NULL, NULL);
-      assert (errno == 0);
       assert (l == 0);
       raise (SIGUSR1);
       return 0;
@@ -298,7 +291,6 @@ main (int argc __attribute__ ((unused)), char **argv __attribute__ ((unused)))
 
   int status;
   pid_t pid = waitpid (child, &status, 0);
-  assert (errno == 0);
   assert (pid == child);
   assert (WIFSTOPPED (status));
   assert (WSTOPSIG (status) == SIGUSR1);
@@ -316,7 +308,6 @@ main (int argc __attribute__ ((unused)), char **argv __attribute__ ((unused)))
 
   struct user_regs_struct user_regs;
   long l = ptrace (PTRACE_GETREGS, child, NULL, &user_regs);
-  assert (errno == 0);
   assert (l == 0);
   report_module (dwfl, child, user_regs.rip);
 
@@ -330,7 +321,6 @@ main (int argc __attribute__ ((unused)), char **argv __attribute__ ((unused)))
   dwfl_end (dwfl);
   kill (child, SIGKILL);
   pid = waitpid (child, &status, 0);
-  assert (errno == 0);
   assert (pid == child);
   assert (WIFSIGNALED (status));
   assert (WTERMSIG (status) == SIGKILL);
diff --git a/tests/backtrace.c b/tests/backtrace.c
index 24ab68dd4..05e8ef82e 100644
--- a/tests/backtrace.c
+++ b/tests/backtrace.c
@@ -281,16 +281,13 @@ prepare_thread (pid_t pid2 __attribute__ ((unused)),
   struct user_regs_struct user_regs;
   errno = 0;
   l = ptrace (PTRACE_GETREGS, pid2, 0, (intptr_t) &user_regs);
-  assert (errno == 0);
   assert (l == 0);
   user_regs.rip = (intptr_t) jmp;
   l = ptrace (PTRACE_SETREGS, pid2, 0, (intptr_t) &user_regs);
-  assert (errno == 0);
   assert (l == 0);
   l = ptrace (PTRACE_CONT, pid2, NULL, (void *) (intptr_t) SIGUSR2);
   int status;
   pid_t got = waitpid (pid2, &status, __WALL);
-  assert (errno == 0);
   assert (got == pid2);
   assert (WIFSTOPPED (status));
   assert (WSTOPSIG (status) == SIGUSR1);
@@ -358,7 +355,6 @@ exec_dump (const char *exec)
   errno = 0;
   int status;
   pid_t got = waitpid (pid, &status, 0);
-  assert (errno == 0);
   assert (got == pid);
   assert (WIFSTOPPED (status));
   // Main thread will signal SIGUSR2.  Other thread will signal SIGUSR1.
@@ -368,7 +364,6 @@ exec_dump (const char *exec)
      __WCLONE, probably despite pthread_create already had to be called the new
      task is not yet alive enough for waitpid.  */
   pid_t pid2 = waitpid (-1, &status, __WALL);
-  assert (errno == 0);
   assert (pid2 > 0);
   assert (pid2 != pid);
   assert (WIFSTOPPED (status));
-- 
2.20.1