[PATCH 2/2] powerpc: Provide a fallback for old kernels without PTRACE_GETREGS

[Anton Blanchard]

Hi Denys,

> Applied both patches, thanks.

Thanks! FYI I just realised my git tree was pointing at an old URL:

git://strace.git.sourceforge.net/gitroot/strace/strace

Would it be worth shutting that tree down if it is no longer in use?

> Can you take a look at the silly dance we do with ppc_result variable?
> 
> get_syscall_result():
> 
>         ppc_result = ppc_regs.gpr[3];
>         if (ppc_regs.ccr & SO_MASK)
>                 ppc_result = -ppc_result;
> 
> 
> later, get_error():
> 
>         if (check_errno && is_negated_errno(ppc_result)) {
>                 tcp->u_rval = -1;
>                 u_error = -ppc_result;
>         }
>         else {
>                 tcp->u_rval = ppc_result;
>         }
> 
> From this, looks like PPC does NOT use "if it's negative
> and below max errno" logic - it has a dedicated error bit,
> SO_MASK. But our code shoehorn it into that logic.
> 
> Shouldn't get_syscall_result() code bit just nixed,
> and get_error() changed to this? -
> 
>         if (ppc_regs.ccr & SO_MASK) {
>                 tcp->u_rval = -1;
>                 u_error = ppc_regs.gpr[3];
>         }
>         else {
>                 tcp->u_rval = ppc_regs.gpr[3];
>         }

This looks good and is much simpler. I ran a few tests to verify we
get reasonable output for failing system calls.

> get_regs():
> 
>        get_regs_error = ptrace(PTRACE_GETREGS, pid, NULL, (long)
> &ppc_regs); if (get_regs_error && errno == EIO)
>                get_regs_error = powerpc_getregs_old(pid);
> 
> The fallback will be taken _every time_ on old kernels.
> Meaning, we will try PTRACE_GETREGS pointlessly twice per syscall.
> We should remember the first failure instead.
> 
> Care to fix and test these problems?

Thanks for doing this too. A simple build fix below.

Anton
--

powerpc: Fix iflag build issue 

Commit 6b3016e43 (POWERPC: read ppc_regs.nip if -i) cause a build
breakage. Fix it by making iflag global.

Signed-off-by: Anton Blanchard <anton at samba.org>
---

diff --git a/defs.h b/defs.h
index 64cfc8d..6e0bd88 100644
--- a/defs.h
+++ b/defs.h
@@ -550,6 +550,7 @@ typedef enum {
 extern cflag_t cflag;
 extern bool debug_flag;
 extern bool Tflag;
+extern bool iflag;
 extern unsigned int qflag;
 extern bool not_failing_only;
 extern bool show_fd_path;
diff --git a/strace.c b/strace.c
index 94706ad..9aa2b36 100644
--- a/strace.c
+++ b/strace.c
@@ -78,10 +78,10 @@ bool need_fork_exec_workarounds = 0;
 bool debug_flag = 0;
 bool Tflag = 0;
 unsigned int qflag = 0;
+bool iflag = 0;
 /* Which WSTOPSIG(status) value marks syscall traps? */
 static unsigned int syscall_trap_sig = SIGTRAP;
 static unsigned int tflag = 0;
-static bool iflag = 0;
 static bool rflag = 0;
 static bool print_pid_pfx = 0;