On Tue, 2011-06-21 at 01:25 +0400, Dmitry V. Levin wrote:
> On Mon, Jun 20, 2011 at 09:24:15PM +0200, Denys Vlasenko wrote:
> [...]
> > int options = ptrace_setoptions_for_all;
> > if (followfork) // && (tcp->parent == NULL))
> > <=== HERE
> > options |= ptrace_setoptions_followfork;
> > if (options) {
> > if (debug)
> > fprintf(stderr, "setting opts %x on
> > pid %d\n", options, tcp->pid);
> > if (ptrace(PTRACE_SETOPTIONS, tcp->pid,
> > NULL, options) < 0) {
> > if (errno != ESRCH) {
> > /* Should never happen,
> > really */
> >
> > error_msg_and_die("PTRACE_SETOPTIONS");
> > }
> > }
> > }
> >
> > The check (tcp->parent == NULL) in old code was meant to check
> > "if we are not a child created by auto-attach" - in this case,
> > options need to be set on the child; otherwise they are inherited
> > and do not need to be set.
> >
> > I misunderstood the check and if tcp->parent is not NULL, I was
> > setting only ptrace_setoptions_for_all bits!
> >
> > Simple solution is depicted above - just comment out "&& (tcp->parent ==
> > NULL))".
> >
> > I will test the "more optimal" solution which does the check, but sets
> > correct opts.
> >
> > Meanwhile, can you confirm that commenting-out works for you too?
>
> Yes, removing this "tcp->parent == NULL" check fixes the issue, thanks.
I committed the fix. Please take a look:
--
vda
commit f44cce48bbbd573cc5ae801f69f857433160b03a
Author: Denys Vlasenko <[email protected]>
Date: Tue Jun 21 14:34:10 2011 +0200
Fix regression introduced by "Properly handle real SIGTRAPs" change
Commit 3454e4b463e6c22c7ea8c5461ef5a077f4650a54
introduced a bug: sometimes, TRACECLONE/TRACE[V]FORK opts were not set.
The check (tcp->parent == NULL) in old code was meant to check
"if we are not a child created by auto-attach" - in this case,
options need to be set on the child; otherwise they are inherited
and do not need to be set.
I misunderstood the check and if tcp->parent is not NULL, I was
setting only ptrace_setoptions_for_all bits.
This change fixes the problem. Since the fixed logic makes it
unnecessary to keep two sets of options in separate variables,
I merge them back into one variable, ptrace_setoptions.
* defs.h: Merge ptrace_setoptions_followfork and ptrace_setoptions_for_all
into one variable, ptrace_setoptions.
* strace.c: Likewise.
(test_ptrace_setoptions_followfork): Use ptrace_setoptions variable.
(test_ptrace_setoptions_for_all): Likewise.
(main): Likewise.
* process.c (internal_fork): Likewise.
(internal_exec): Likewise.
* strace.c (trace): Fix the bug where different options were set
depending on "tcp->parent == NULL" condition. Add a comment
which makes it more clear why this condition is checked.
Signed-off-by: Denys Vlasenko <[email protected]>
diff --git a/defs.h b/defs.h
index b2ca923..75b002d 100644
--- a/defs.h
+++ b/defs.h
@@ -508,8 +508,7 @@ typedef enum {
extern struct tcb **tcbtab;
extern int *qual_flags;
extern int debug, followfork;
-extern unsigned int ptrace_setoptions_followfork;
-extern unsigned int ptrace_setoptions_for_all;
+extern unsigned int ptrace_setoptions;
extern int dtime, xflag, qflag;
extern cflag_t cflag;
extern int acolumn;
diff --git a/process.c b/process.c
index fc5fa90..0584f54 100644
--- a/process.c
+++ b/process.c
@@ -904,7 +904,7 @@ Process %u resumed (parent %d ready)\n",
int
internal_fork(struct tcb *tcp)
{
- if ((ptrace_setoptions_followfork
+ if ((ptrace_setoptions
& (PTRACE_O_TRACECLONE | PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK))
== (PTRACE_O_TRACECLONE | PTRACE_O_TRACEFORK | PTRACE_O_TRACEVFORK))
return 0;
@@ -1751,7 +1751,7 @@ internal_exec(struct tcb *tcp)
tcp->flags &= ~TCB_WAITEXECVE;
else {
/* Maybe we have post-execve SIGTRAP suppressed? */
- if (!(ptrace_setoptions_for_all & PTRACE_O_TRACEEXEC))
+ if (!(ptrace_setoptions & PTRACE_O_TRACEEXEC))
tcp->flags |= TCB_WAITEXECVE; /* no */
}
#endif /* LINUX && TCB_WAITEXECVE */
diff --git a/strace.c b/strace.c
index 3298795..93e6d29 100644
--- a/strace.c
+++ b/strace.c
@@ -84,8 +84,7 @@ extern char *optarg;
int debug = 0, followfork = 0;
-unsigned int ptrace_setoptions_followfork = 0;
-unsigned int ptrace_setoptions_for_all = 0;
+unsigned int ptrace_setoptions = 0;
/* Which WSTOPSIG(status) value marks syscall traps? */
static unsigned int syscall_trap_sig = SIGTRAP;
int dtime = 0, xflag = 0, qflag = 0;
@@ -826,7 +825,7 @@ test_ptrace_setoptions_followfork(void)
}
}
if (expected_grandchild && expected_grandchild == found_grandchild)
- ptrace_setoptions_followfork |= test_options;
+ ptrace_setoptions |= test_options;
return 0;
}
@@ -907,10 +906,10 @@ test_ptrace_setoptions_for_all(void)
if (it_worked) {
syscall_trap_sig = (SIGTRAP | 0x80);
- ptrace_setoptions_for_all = test_options;
+ ptrace_setoptions |= test_options;
if (debug)
- fprintf(stderr, "ptrace_setoptions_for_all = %#x\n",
- ptrace_setoptions_for_all);
+ fprintf(stderr, "ptrace_setoptions = %#x\n",
+ ptrace_setoptions);
return;
}
@@ -1136,11 +1135,11 @@ main(int argc, char *argv[])
fprintf(stderr,
"Test for options supported by
PTRACE_SETOPTIONS "
"failed, giving up using this feature.\n");
- ptrace_setoptions_followfork = 0;
+ ptrace_setoptions = 0;
}
if (debug)
- fprintf(stderr, "ptrace_setoptions_followfork = %#x\n",
- ptrace_setoptions_followfork);
+ fprintf(stderr, "ptrace_setoptions = %#x\n",
+ ptrace_setoptions);
}
test_ptrace_setoptions_for_all();
#endif
@@ -2679,16 +2678,16 @@ Process %d attached (waiting for parent)\n",
}
}
#ifdef LINUX
- int options = ptrace_setoptions_for_all;
- if (followfork && (tcp->parent == NULL))
- options |= ptrace_setoptions_followfork;
- if (options) {
- if (debug)
- fprintf(stderr, "setting opts %x on pid
%d\n", options, tcp->pid);
- if (ptrace(PTRACE_SETOPTIONS, tcp->pid, NULL,
options) < 0) {
- if (errno != ESRCH) {
- /* Should never happen, really
*/
-
perror_msg_and_die("PTRACE_SETOPTIONS");
+ /* If options were not set for this tracee yet */
+ if (tcp->parent == NULL) {
+ if (ptrace_setoptions) {
+ if (debug)
+ fprintf(stderr, "setting opts
%x on pid %d\n", ptrace_setoptions, tcp->pid);
+ if (ptrace(PTRACE_SETOPTIONS, tcp->pid,
NULL, ptrace_setoptions) < 0) {
+ if (errno != ESRCH) {
+ /* Should never happen,
really */
+
perror_msg_and_die("PTRACE_SETOPTIONS");
+ }
}
}
}
------------------------------------------------------------------------------
EditLive Enterprise is the world's most technically advanced content
authoring tool. Experience the power of Track Changes, Inline Image
Editing and ensure content is compliant with Accessibility Checking.
http://p.sf.net/sfu/ephox-dev2dev
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel
