[PATCH] remove dead "ifndef CLONE_PTRACE" branch, make setbpt easier to read
Denys Vlasenko
dvlasenk at redhat.com
Wed Jun 22 11:48:19 UTC 2011
Hi,
process.c #defines CLONE_PTRACE for Linux, so it can't be undefined.
Therefore #ifndef CLONE_PTRACE code is dead (since at least 2004).
This patch removes it.
While at it, it adds a comment about setbpt on Linux
and untangles a particularly badly obfuscated bit of code
in setbpt:
- (tcp->u_arg[arg0_index] | CLONE_PTRACE)
- & ~(tcp->u_arg[arg0_index] & CLONE_VFORK
- ? CLONE_VFORK | CLONE_VM : 0)
becomes:
+ long new_arg0 = (tcp->u_arg[arg0_index] | CLONE_PTRACE);
+ if (new_arg0 & CLONE_VFORK)
+ new_arg0 &= ~(unsigned long)(CLONE_VFORK | CLONE_VM);
Please review.
--
vda
diff -d -urpN strace.6/defs.h strace.7/defs.h
--- strace.6/defs.h 2011-06-22 13:31:45.323884488 +0200
+++ strace.7/defs.h 2011-06-22 00:38:57.646897421 +0200
@@ -566,6 +566,12 @@ extern void print_sock_optmgmt(struct tc
extern void printrusage(struct tcb *, long);
extern void printuid(const char *, unsigned long);
extern int clearbpt(struct tcb *);
+/*
+ * On Linux, "setbpt" is a misnomer: we don't set a breakpoint
+ * (IOW: no poking in user's text segment),
+ * instead we change fork/vfork/clone into clone(CLONE_PTRACE).
+ * On newer kernels, we use PTRACE_O_TRACECLONE/TRACE[V]FORK instead.
+ */
extern int setbpt(struct tcb *);
extern int sigishandled(struct tcb *, int);
extern void printcall(struct tcb *);
diff -d -urpN strace.6/process.c strace.7/process.c
--- strace.6/process.c 2011-06-22 13:37:06.210864935 +0200
+++ strace.7/process.c 2011-06-22 13:36:57.602929960 +0200
@@ -780,7 +780,6 @@ handle_new_child(struct tcb *tcp, int pi
{
struct tcb *tcpchild;
-#ifdef CLONE_PTRACE /* See new setbpt code. */
tcpchild = pid2tcb(pid);
if (tcpchild != NULL) {
/* The child already reported its startup trap
@@ -792,30 +791,15 @@ handle_new_child(struct tcb *tcp, int pi
[preattached child %d of %d in weird state!]\n",
pid, tcp->pid);
}
- else
-#endif /* CLONE_PTRACE */
- {
+ else {
tcpchild = alloctcb(pid);
}
-#ifndef CLONE_PTRACE
- /* Attach to the new child */
- if (ptrace(PTRACE_ATTACH, pid, (char *) 1, 0) < 0) {
- if (bpt)
- clearbpt(tcp);
- perror("PTRACE_ATTACH");
- fprintf(stderr, "Too late?\n");
- droptcb(tcpchild);
- return 0;
- }
-#endif /* !CLONE_PTRACE */
-
- if (bpt)
- clearbpt(tcp);
-
tcpchild->flags |= TCB_ATTACHED;
- /* Child has BPT too, must be removed on first occasion. */
+
if (bpt) {
+ clearbpt(tcp);
+ /* Child has BPT too, must be removed on first occasion. */
tcpchild->flags |= TCB_BPTSET;
tcpchild->baddr = tcp->baddr;
memcpy(tcpchild->inst, tcp->inst,
diff -d -urpN strace.6/util.c strace.7/util.c
--- strace.6/util.c 2011-06-22 00:19:30.268714125 +0200
+++ strace.7/util.c 2011-06-22 13:32:30.653923439 +0200
@@ -1494,9 +1494,9 @@ setbpt(struct tcb *tcp)
return 0;
# endif
- case SYS_clone:
+ case SYS_clone: ;
# ifdef SYS_clone2
- case SYS_clone2:
+ case SYS_clone2: ;
# endif
/* ia64 calls directly `clone (CLONE_VFORK | CLONE_VM)'
contrary to x86 SYS_vfork above. Even on x86 we turn the
@@ -1506,12 +1506,12 @@ setbpt(struct tcb *tcp)
clear also CLONE_VM but only in the CLONE_VFORK case as
otherwise we would break pthread_create. */
- if ((arg_setup(tcp, &state) < 0
- || set_arg0(tcp, &state,
- (tcp->u_arg[arg0_index] | CLONE_PTRACE)
- & ~(tcp->u_arg[arg0_index] & CLONE_VFORK
- ? CLONE_VFORK | CLONE_VM : 0)) < 0
- || arg_finish_change(tcp, &state) < 0))
+ long new_arg0 = (tcp->u_arg[arg0_index] | CLONE_PTRACE);
+ if (new_arg0 & CLONE_VFORK)
+ new_arg0 &= ~(unsigned long)(CLONE_VFORK | CLONE_VM);
+ if (arg_setup(tcp, &state) < 0
+ || set_arg0(tcp, &state, new_arg0) < 0
+ || arg_finish_change(tcp, &state) < 0)
return -1;
tcp->flags |= TCB_BPTSET;
tcp->inst[0] = tcp->u_arg[arg0_index];
@@ -1535,7 +1535,8 @@ clearbpt(struct tcb *tcp)
|| restore_arg0(tcp, &state, tcp->inst[0]) < 0
|| restore_arg1(tcp, &state, tcp->inst[1]) < 0
|| arg_finish_change(tcp, &state))
- if (errno != ESRCH) return -1;
+ if (errno != ESRCH)
+ return -1;
tcp->flags &= ~TCB_BPTSET;
return 0;
}
More information about the Strace-devel
mailing list