On Mon, 2008-12-08 at 10:40 +0800, Wenji Huang wrote:
> Denys Vlasenko wrote:
> > On Fri, 2008-12-05 at 11:10 +0800, Wenji Huang wrote:
> >> There are two test cases I collected/updated those work fine
> >> on mainline 2.6.28-rc7. But fail on 2.6.28-rc7+latest utrace patch.
sigstep.c has "This testcase is part of GDB, the GNU debugger."
comment. And gdb indeed has sigstep.c file. But it does not
seem to have much in common with your sigstep.c.
multi-step-same-time.c does not seem to be from GDB.
Therefore they are not an established, known-to-work testcases
(at least I can't find where they come from), they are just some
C programs which work on vanilla kernel and fail on utrace.
This means that they need to pass through a review before
we can assume the problem is in utrace and not in the tests.
And this means that you need to explain unclear moments,
and tweak parts of tests where they do not seem to be doing
the right thing(s), or are simply superfluous.
At least that's how I understand the situation.
Do you see it the same? I'm asking because you did not provide
a tweaked (shortened?) version of sigstep.c in your reply,
which I sort-of-expected.
Do you want someone else to do it?
> Fail on 2.6.28-rc7 + utrace patch (Dec 2nd), x86 machine(SMP).But works
> fine on upstream 2.6.28-rc7 kernel.
> >
> >> child = fork ();
> >> assert (child >= 0);
> >> if (child == 0) {
> >> l = ptrace (PTRACE_TRACEME, 0, NULL, NULL);
> >> assert(l==0);
> >
> > I think it's customary to put "raise(SIGSTOP)" *immediately*
> > after PTRACE_TRACEME. Then waitpid() for SIGSTOP in parent.
> > You may achieve this by simply moving PTRACE_TRACEME down,
> > to raise(SIGSTOP).
>
> I think it's harmless.
I think it makes sense to do it, and retest. Why take chances?
> >> /* Set up the signal handler. */
> >> memset (&action, 0, sizeof (action));
> >> action.sa_handler = handler;
> >> sigaction(SIGUSR1, &action, NULL);
> >>
> >> raise(SIGSTOP);
> >> pid = waitpid(child, &status, 0); /* stop at handler */
> >> assert(pid==child);
> >> assert(WIFSTOPPED(status));
> >> assert(WSTOPSIG(status) == SIGUSR2);
> >>
> >> l = ptrace(PTRACE_SINGLESTEP, child, 0, 0);/* step in handler */
> >> assert(l==0);
> >
> > We are stepping over what instruction here? "done = 1"?
> > 1. What if returning from raise(SIGUSR2) needs
> > some instructions too in this arch and/or libc?
> > 2. Does "done = 1;" compile into single instruction?
> >
> >> /* update the value */
> >> pid = waitpid(child, &status, 0);
> >> assert(pid==child);
> >> assert(WIFSTOPPED(status));
> >> assert(WSTOPSIG(status) == SIGTRAP);
> >
> > Above line is line 86. You have assertion failure here. So, what signal
> > do you get instead of SIGTRAP?
>
> In fact, the real reason is "assert(0) reached".
Sorry. I am still interested what value of WSTOPSIG(status)
you are getting here. It's a useful bit of information.
> We expected it should
> be single stepped in handler, but seem not like that. It continues
> running and returns from handler till to assert(0) in child program.
> You can add some useless instructions to handler function to know about it.
> >> /* return from handler */
> >
> > Comment is wrong. How do you know that return from handler is a single
> > instruction? Why do you even want to return from handler if you
> > plan to SIGKILL it immediately? Will it still work in mainline
> > and fail on patched kernel if you skip this PTRACE_SINGLESTEP?
> > If yes, will it still work/fail if you also replace
> > ptrace(PTRACE_CONT, ... SIGKILL) with plain kill(child, SIGKILL);?
> >
> Yes, something is not very accurate. But it's not much important.
It is important to have the test as simple as possible.
In this case, we do not precisely know what makes traced child
to be released. Removing "unneeded" steps from the test will help
to narrow it down.
--
vda
