[PATCH v3] Implement -e trace=/regex option

[Dmitry V. Levin]

On Wed, Apr 12, 2017 at 09:31:51PM +0800, JingPiao Chen wrote:
> * qualify.c (qualify_syscall_regex): New function.

It should bein with

* qualify.c: Include <regex.h>

> (qualify_syscall_name): Use qualify_syscall_regex function.

Use it?

> * strace.1: Document it.

What's "it"?

> * NEWS: Mention this.

Mention what?

> * tests/regex.test: Check this.

New file?

> * tests/Makefile.am (DECODER_TESTS): Add regex.test.

Add it?

> * tests/options-syntax.test:
> Add check for invaild regexp and the regexp never match a syscall.

grammar: and for regexp that doesn't match a syscall.

> --- a/NEWS
> +++ b/NEWS
> @@ -12,6 +12,7 @@ Noteworthy changes in release ?.?? (????-??-??)
>    * Added -e trace=%clock option for tracing clock_* syscalls.
>    * Added -e trace=%statfs option for tracing statfs, statfs64 and statvfs
>      syscalls.
> +  * Added -e trace=/regex option for tracing match regular expression syscalls.

grammar: for filtering syscalls using regular expressions.

> @@ -198,6 +199,48 @@ qualify_syscall_number(const char *s, struct number_set *set)
>  	return done;
>  }
>  
> +static bool
> +qualify_syscall_regex(const char *s, struct number_set *set)
> +{
> +	regex_t preg;
> +	int rc;
> +
> +	if ((rc = regcomp(&preg, s, REG_EXTENDED | REG_NOSUB)) != 0) {
> +		size_t len = regerror(rc, &preg, NULL, 0);
> +		char buf[len];

We don't use VLA on the stack, it is not portable.  In particular,
it won't pass travis-ci clang tests.  If you've pushed it to github
and enabled travis-ci, you'd have noticed.

Why do you need it here?

> +		regerror(rc, &preg, buf, len);
> +		error_msg_and_die("regcomp: '%s' (%s)", s, buf);

regcomp: %s: %s

> +	}
> +
> +	unsigned int p;
> +	bool found = false;
> +	for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> +		unsigned int i;
> +
> +		for (i = 0; i < nsyscall_vec[p]; ++i) {
> +			if (!sysent_vec[p][i].sys_name)
> +				continue;
> +			rc = regexec(&preg, sysent_vec[p][i].sys_name,
> +				     0, NULL, 0);
> +			if (rc == REG_NOMATCH)
> +				continue;
> +			else if (rc) {
> +				size_t len = regerror(rc, &preg, NULL, 0);
> +				char buf[len];
> +
> +				regerror(rc, &preg, buf, len);
> +				error_msg_and_die("regexec: '%s' (%s)", s, buf);

This error diagnostics looks very similar to the regcomp case.
Let's move it to a separate function.

> --- a/strace.1
> +++ b/strace.1
> @@ -383,6 +383,15 @@ about the user/kernel boundary if only a subset of system calls
>  are being monitored.  The default is
>  .BR trace = all .
>  .PP
> +.TP

Shouldn't it be just .TP without .PP?

> +.BR "\-e\ trace" = /regex

\fB\-e\ trace\fR=/\,\fIregex\fR

> +Trace only the system calls match the

grammar: Trace only those system calls that match the

> +.BR regex .

.IR regex .

> --- a/tests/options-syntax.test
> +++ b/tests/options-syntax.test
> @@ -72,6 +72,7 @@ check_e "invalid system call '-2'" -e -2
>  check_e "invalid system call '-3'" -etrace=-3
>  check_e "invalid system call '-4'" -e trace=-4
>  check_e "invalid system call '-5'" -e trace=1,-5
> +check_e "invalid system call '/non_syscall'" -e trace=/non_syscall
>  check_e "invalid system call '2147483647'" -e 2147483647
>  check_e "invalid system call '2147483648'" -e 2147483648
>  check_e "invalid system call '4294967295'" -e 4294967295
> @@ -144,3 +145,24 @@ done
>  [ -z "$args" ] ||
>  	dump_log_and_fail_with \
>  		"strace $args failed to print expected diagnostics"
> +
> +# Check regular expression
> +# ignore the error message print by library function
> +check_regex_e()
> +{
> +	local pattern="$1"; shift
> +	cat > "$EXP" << __EOF__
> +$strace_exp: $pattern
> +__EOF__
> +	$STRACE "$@" 2> "$LOG" &&
> +		dump_log_and_fail_with \
> +			"strace $* failed to handle the error properly"
> +	match_grep "$LOG" "$EXP" ||
> +		dump_log_and_fail_with \
> +			"strace $* failed to print expected diagnostics"
> +}

There is nothing about regex in this function besides its name,
the only difference from check_e is that it uses match_grep instead
of match_diff, so please give it a more appropriate name and description.

Also, please move it between check_e and check_h.

> +check_regex_e "regcomp: '\?id' \(.*\)$" -e trace='/?id'

What's the use of adding this funny tail \(.*\)$ ?


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20170413/7754671a/attachment.bin>