[RFC v1] [GSoC] Add test for -l/--syscall-limit option

[Sahil Siddiq]

Hi,

Thank you for the detailed feedback.

On Friday, 24 February 2023 23:45:58 IST Dmitry V. Levin wrote:
> > diff --git a/tests/gen_tests.in b/tests/gen_tests.in
> > index eceac8d60..c11c49329 100644
> > --- a/tests/gen_tests.in
> > +++ b/tests/gen_tests.in
> > @@ -995,6 +995,7 @@ strace--absolute-timestamps-format-unix-precision-ns
> > +strace-ttt.test 9 --absolu> 
> > strace--absolute-timestamps-format-unix-precision-s +strace-ttt.test 0
> > --absolute-timestamps=format:unix --absolute-timestamps=precision:s
> > strace--absolute-timestamps-format-unix-precision-us +strace-ttt.test 6
> > --absolute-timestamps=precision:us --absolute-timestamps=format:unix
> > strace--decode-pids-comm        --decode-pids=comm --trace=getppid,tgkill
> > --signal='!SIGCHLD,SIGCONT' -q -f -a 18> 
> > +strace--syscall-limit  --syscall-limit=4 --trace=getpid,getppid -q -f -a 9
> >
> > strace--follow-forks-output-separately +strace-ff.test --follow-forks
> > --output-separately
> Please keep this list sorted, too.

Sorry, I must have missed this.

> > +       pid_t child = fork();
> > +       if (child < 0)
> > +               perror_msg_and_fail("fork");
> > +       else if (child == 0) {
> > +               pid = getpid();
> > +               ppid = getppid();
> > +               printf("%u getpid() = %d\n", pid, pid);
> > +               printf("%u getppid() = %d\n", pid, ppid);
> > +               fflush(stdout);
> > +               return 0;
> > +       }
> > +       int status;
> > +       while ((waitpid(child, &status, 0)) != child) {
> > +               if (errno == EINTR)
> > +                       continue;
> > +               perror_msg_and_fail("waitpid: %d", child);
> > +       }
>
> Note that your --syscall-limit implementation detaches from the parent
> process in a way that causes it to be killed by strace with SIGTERM,
> so the remaining code is not reached.  I'm not sure you planned to
> implement --syscall-limit this way, and I'm doubt that users would
> expect such behavior.  This is also in contrast with --detach-on=execve
> which also detaches its tracee but doesn't kill it, see detach_on_execve
> handling in maybe_allocate_tcb().

No, it wasn't my intention to kill the tracee with SIGTERM on detaching. I'll take
a look at --detach-on=execve to understand the detaching process better.

> > +       printf("+++ exited with 0 +++\n");
> > +
> > +       ppid = getpid();
> > +       ppid = getppid();
> > +       printf("%u getpid() = %d\n", pid, pid);
> > +       printf("%u getppid() = %d\n", pid, ppid);
> > +       printf("+++ exited with 0 +++\n");
> > +       return WEXITSTATUS(status);
> > +}
>
> So this code works unless executed under strace --syscall-limit.

Yes, this is what I expect when a process is started under strace with the
--syscall-limit option (strace --syscall-limit=NUMBER process). But if the
process is already running as a stand-alone process, and strace is attached
to the process via the -p option, then I would expect the process to run to
completion normally even after getting detached from strace.

> > +
> > +int
> > +main(int argc, char **argv)
> > +{
> > +       return syscall_limit_detach();
> > +}
>
> It would be better to declare main without arguments if they are not going
> to be used anyway.

I was thinking of making use of arguments when adding the tests for the --trace-path
and --status options. I was thinking of having different bash scripts for each option.
But each script will make use of the same .c file. I'll use "QUIRK:PROG-ARGS" to pass an
argument to the main function. Based on that argument, the relevant helper function
will be called.

Thanks,
Sahil