[Ltrace-devel] FC5 ltrace sigsegvs traced process on ppc

[Petr Machata]

Petr Machata wrote:
> Basically what I did:
>  * secure PLT branch (now trunk) contains code to cope with secure PLTs,
> with few issues to fix
>  * call breakpoints_init only when the traced binary was started, you
> can't ptrace it earlier

This part of the fix breaks ltrace on fc4.  Didn't look at it yet.

>  * make sure you won't place breakpoint over another breakpoint,
> otherwise you end up with breakpoint insn in orig_value, and the binary
> will stuck on the first breakpoint, reinserting in endlessly.
> 
> Any advice on how to do this cleanly is welcome.
> 
> PM

Also the testsuite doesn't pass.

I've been looking at it today, and at least with some tests, the problem
is that ltrace isn't able to extract return address from PT_LNK register
in sysdeps/linux-gnu/ppc/regs.c.  It gets NULL, to be correct, the call
won't end with an error.  But because of that NULL, ltrace doesn't
notice when library call ends, displays <unfinished...> message, and
won't include the call in final statistics (in case of -c option).

Attaching the current patch.

PM
-------------- next part --------------
Index: elf.c
===================================================================
--- elf.c	(revision 45)
+++ elf.c	(working copy)
@@ -464,7 +464,7 @@
 			if (strcmp(xptr->name, PLTs_initialized_by_here) == 0) {
 				if (lte->ehdr.e_entry) {
 					add_library_symbol (
-						elf_plt2addr (lte, (void*)(long)
+						opd2addr (lte, (void*)(long)
 							lte->ehdr.e_entry),
 						PLTs_initialized_by_here,
 						lib_tail, 1, 0);
Index: sysdeps/linux-gnu/breakpoint.c
===================================================================
--- sysdeps/linux-gnu/breakpoint.c	(revision 45)
+++ sysdeps/linux-gnu/breakpoint.c	(working copy)
@@ -9,6 +9,8 @@
 #include "output.h"
 #include "debug.h"
 
+#include <errno.h>
+
 static unsigned char break_insn[] = BREAKPOINT_VALUE;
 
 #ifdef ARCH_HAVE_ENABLE_BREAKPOINT
@@ -24,20 +26,27 @@
 
 	debug(1, "enable_breakpoint(%d,%p)", pid, sbp->addr);
 
+	unsigned char orig[BREAKPOINT_LENGTH];
 	for (i = 0; i < 1 + ((BREAKPOINT_LENGTH - 1) / sizeof(long)); i++) {
 		long a =
 		    ptrace(PTRACE_PEEKTEXT, pid, sbp->addr + i * sizeof(long),
 			   0);
+		if (a == -1 && errno) {
+			perror ("ptrace");
+			return;
+		}
 		for (j = 0;
 		     j < sizeof(long)
 		     && i * sizeof(long) + j < BREAKPOINT_LENGTH; j++) {
 			unsigned char *bytes = (unsigned char *)&a;
 
-			sbp->orig_value[i * sizeof(long) + j] = bytes[j];
+			orig[i * sizeof(long) + j] = bytes[j];
 			bytes[j] = break_insn[i * sizeof(long) + j];
 		}
 		ptrace(PTRACE_POKETEXT, pid, sbp->addr + i * sizeof(long), a);
 	}
+	if (memcmp (orig, break_insn, BREAKPOINT_LENGTH) != 0)
+		memcpy (sbp->orig_value, orig, BREAKPOINT_LENGTH);
 }
 #endif				/* ARCH_HAVE_ENABLE_BREAKPOINT */
 
Index: sysdeps/linux-gnu/ppc/plt.c
===================================================================
--- sysdeps/linux-gnu/ppc/plt.c	(revision 45)
+++ sysdeps/linux-gnu/ppc/plt.c	(working copy)
@@ -1,4 +1,5 @@
 #include <gelf.h>
+#include <errno.h>
 #include "ltrace.h"
 #include "elf.h"
 #include "debug.h"
@@ -17,6 +18,7 @@
 
 	debug(3, 0);
 
+	debug(1, "sym2addr: sym=%s, pid=%d", sym->name, proc->pid);
 	if (sym->plt_type != LS_TOPLT_POINT) {
 		return addr;
 	}
@@ -46,6 +48,11 @@
 	// break-point right in the PLT.
 
 	pt_ret = ptrace(PTRACE_PEEKTEXT, proc->pid, addr, 0);
+	if (pt_ret == -1 && errno) {
+		debug(1, "ptrace: addr=%08x pt_ret=%08x", addr, pt_ret);
+		perror ("ptrace");
+		return 0;
+	}
 
 	if (proc->mask_32bit) {
 		// Assume big-endian.
Index: sysdeps/linux-gnu/ppc/arch.h
===================================================================
--- sysdeps/linux-gnu/ppc/arch.h	(revision 45)
+++ sysdeps/linux-gnu/ppc/arch.h	(working copy)
@@ -7,8 +7,11 @@
 #ifdef __powerpc64__ // Says 'ltrace' is 64 bits, says nothing about target.
 #define LT_ELFCLASS2	ELFCLASS64
 #define LT_ELF_MACHINE2	EM_PPC64
+#endif
 
+#ifdef __powerpc__
 #define PLT_REINITALISATION_BP    "_start"
+#endif
 
 #define PPC_NOP { 0x60, 0x00, 0x00, 0x00 }
 #define PPC_NOP_LENGTH 4
@@ -16,6 +19,3 @@
 #if (PPC_NOP_LENGTH != BREAKPOINT_LENGTH)
 #error "Length of the breakpoint value not equal to the length of a nop instruction"
 #endif
-
-
-#endif
Index: breakpoints.c
===================================================================
--- breakpoints.c	(revision 45)
+++ breakpoints.c	(working copy)
@@ -28,6 +28,7 @@
 		  struct library_symbol *libsym)
 {
 	struct breakpoint *sbp;
+	debug(1, "insert_breakpoint(symbol=%s, addr=%p)", libsym?libsym->name:"(nil)", addr);
 
 	if (!proc->breakpoints) {
 		proc->breakpoints =
@@ -165,11 +166,9 @@
 	} else {
 		proc->list_of_symbols = NULL;
 	}
-	sym = proc->list_of_symbols;
-	while (sym) {
+	for (sym = proc->list_of_symbols; sym; sym = sym->next) {
 		/* proc->pid==0 delays enabling. */
 		insert_breakpoint(proc, sym2addr(proc, sym), sym);
-		sym = sym->next;
 	}
 	proc->callstack_depth = 0;
 	proc->breakpoints_enabled = -1;
Index: wait_for_something.c
===================================================================
--- wait_for_something.c	(revision 45)
+++ wait_for_something.c	(working copy)
@@ -25,6 +25,7 @@
 	pid_t pid;
 	int status;
 	int tmp;
+	static int bp_initialized = 0;
 
 	if (!list_of_processes) {
 		debug(1, "No more children");
@@ -48,6 +49,12 @@
 		fprintf(stderr, "signal from wrong pid %u ?!?\n", pid);
 		exit(1);
 	}
+	if (!bp_initialized)
+	  {
+	    /* hack... */
+	    breakpoints_init (event.proc);
+	    bp_initialized = 1;
+	  }
 	get_arch_dep(event.proc);
 	event.proc->instruction_pointer = NULL;
 	debug(3, "signal from pid %u", pid);