[PATCH] Add clone-f test

[Fei, Jie/费 杰]

Hi,

On 01/15/2016 11:33 PM, Dmitry V. Levin wrote:
> Hi,
>
> On Fri, Jan 15, 2016 at 05:06:13PM +0800, Fei, Jie/费 杰 wrote:
>> Hi!
>>
>> I'm still not sure what kind of tests are needed so I continued writing
>> patches.
>> Could you give me some advice on these patches?
> First of all, there has to be clear what is being tested.
> Taking your clone-f.test as an example, are you testing just the fact
> of following both threads after clone syscall, or maybe something beyond
> that?
Yes, this patch is testing whether strace can trace both threads after
clone syscall.
>> On 01/13/2016 04:43 PM, Fei Jie wrote:
>>> tests: add clone-f.test.
>>>
>>> Check how strace -f follows clone syscall.
>>>
>>> * tests/clone-f.c: New file.
>>> * tests/clone-f.test: New test.
>>> * tests/Makefile.am: (check_PROGRAMS): Add clone-f.
>>> (TESTS): Add clone-f.test.
>>> * tests/.gitignore: Add clone-f.
>>> ---
>>>    tests/.gitignore   |  1 +
>>>    tests/Makefile.am  |  2 ++
>>>    tests/clone-f.c    | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>>    tests/clone-f.test | 11 +++++++++++
>>>    4 files changed, 58 insertions(+)
>>>    create mode 100644 tests/clone-f.c
>>>    create mode 100755 tests/clone-f.test
>>>
>>> diff --git a/tests/.gitignore b/tests/.gitignore
>>> index cfe1e9f..7aa2449 100644
>>> --- a/tests/.gitignore
>>> +++ b/tests/.gitignore
>>> @@ -12,6 +12,7 @@ bpf
>>>    caps
>>>    clock_nanosleep
>>>    clock_xettime
>>> +clone-f
>>>    epoll_create1
>>>    eventfd
>>>    execve
>>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>>> index 33f76cb..80292c4 100644
>>> --- a/tests/Makefile.am
>>> +++ b/tests/Makefile.am
>>> @@ -58,6 +58,7 @@ check_PROGRAMS = \
>>>    	caps \
>>>    	clock_nanosleep \
>>>    	clock_xettime \
>>> +	clone-f \
>>>    	epoll_create1 \
>>>    	eventfd \
>>>    	execve \
>>> @@ -194,6 +195,7 @@ TESTS = \
>>>    	caps.test \
>>>    	clock_nanosleep.test \
>>>    	clock_xettime.test \
>>> +	clone-f.test \
>>>    	dumpio.test \
>>>    	epoll_create1.test \
>>>    	eventfd.test \
>>> diff --git a/tests/clone-f.c b/tests/clone-f.c
>>> new file mode 100644
>>> index 0000000..df97b54
>>> --- /dev/null
>>> +++ b/tests/clone-f.c
>>> @@ -0,0 +1,44 @@
>>> +#include "tests.h"
>>> +#include <stdio.h>
>>> +#include <string.h>
>>> +#include <unistd.h>
>>> +#include <stdlib.h>
>>> +#include <sched.h>
>>> +#include <signal.h>
>>> +
>>> +#define CHILD_STACK_SIZE 16384
>>> +
>>> +static int
>>> +logit(const char *const str)
>>> +{
>>> +	return pwrite(-1, str, strlen(str), 0) >= 0;
> My original idea of using pwrite for text marks in the log haven't passed
> the test of real world: I stumbled upon a system where pwrite is not
> implemented using pwrite64 syscall.  I had to replace pwrite with chdir
> which is portable (see commit v4.11-125-g6833d61).
Thanks, I've replaced pwrite with chdir in patch-v2.
>
>>> +}
>>> +
>>> +void
>>> +child(void)
>>> +{
>>> +	logit("child");
>>> +}
>>> +
>>> +int main()
>>> +{
>>> +	logit("parent");
>>> +
>>> +	void *child_stack;
>>> +	if ((child_stack = (void *) malloc(CHILD_STACK_SIZE)) == NULL) {
>>> +		printf("cannot allocate stack for child!\n");
>>> +	}
>>> +
>>> +	pid_t child_pid = clone(&child, child_stack + CHILD_STACK_SIZE, CLONE_VM, NULL);
> On some architectures supported by strace, stack grows upwards.
>
> clone behavior noticeably depends on the clone flags.  Is there any
> particular reason to use these flags in the test?
I added some codes to determine which way of clone is used in patch-v2.
>
>>> +	if (child_pid < 0) {
>>> +		perror_msg_and_fail("clone");
>>> +	}
>>> +
>>> +	free(child_stack);
>>> +
>>> +	pid_t pid = getpid();
>>> +	printf("%-5d pwrite64(-1, \"parent\", 6, 0) = -1 EBADF (%m)\n"
>>> +	       "%-5d pwrite64(-1, \"child\", 5, 0) = -1 EBADF (%m)\n",
>>> +	       pid, child_pid);
> There is no guarantee of the parent being first, or visa versa.
> I explicitly added synchronization into fork-f.test and vfork-f.test
> to make the output predictable.
Thanks, I set the clone flag SIGCHLD, and let parent do chdir() after
child finished in patch-v2.
>
>>> +	return 0;
>>> +}
>>> diff --git a/tests/clone-f.test b/tests/clone-f.test
>>> new file mode 100755
>>> index 0000000..439426d
>>> --- /dev/null
>>> +++ b/tests/clone-f.test
>>> @@ -0,0 +1,11 @@
>>> +#!/bin/sh
>>> +
>>> +. "${srcdir=.}/init.sh"
>>> +
>>> +OUT="$LOG.out"
>>> +run_prog > /dev/null
>>> +run_strace -a32 -epwrite64 -esignal=none -f -qq $args >"$OUT"
>>> +match_diff "$LOG" "$OUT"
>>> +rm -f "$OUT"
>>> +
>>> +exit 0
>
>
> ------------------------------------------------------------------------------
> Site24x7 APM Insight: Get Deep Visibility into Application Performance
> APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
> Monitor end-to-end web transactions and take corrective actions now
> Troubleshoot faster and improve end-user experience. Signup Now!
> http://pubads.g.doubleclick.net/gampad/clk?id=267308311&iu=/4140
>
>
> _______________________________________________
> Strace-devel mailing list
> Strace-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel

-- 
Thanks!
Fei Jie



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160126/e399957b/attachment.html>