Date: Mon, 7 Nov 2011 22:16:02 +0800 From: Paul Ambrose <ambrosehua@gmail.com> To: Ryan Stone <rysto32@gmail.com> Cc: FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: [PATCH] Fix types of arguments to dtrace syscall return probes Message-ID: <CAMwoQQ4c0XHf1kX9uXk5KOD-nRZezOW%2BB2cB03gUO9g2j32Hfg@mail.gmail.com> In-Reply-To: <CAFMmRNzpj8hGG5YWsKTmcRuphw37eUr6bcDTn6-bBWKOOcQGmQ@mail.gmail.com> References: <CAFMmRNzpj8hGG5YWsKTmcRuphw37eUr6bcDTn6-bBWKOOcQGmQ@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Thank you for your work. I will give it a try in stable/9. I have
another two PR:
kern/160307 and bin/160275 that maybe you can help;
the reason of bin/160275 is complicated, but the fix for kernel crash
caused by module without ctf section(for example, nvidia.ko) missed
somthing
(my mistake)
in kern/kern_ctf.c
---------------------------------------------------------------------------=
-----
int link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
....
struct nameidata nd;
struct thread *td =3D curthread;
uint8_t ctf_hdr[CTF_HDR_SIZE];
#endif
int error =3D 0;
if (lf =3D=3D NULL || lc =3D=3D NULL)
return (EINVAL);
/* Set the defaults for no CTF present. That's not a crime! */
bzero(lc, sizeof(*lc));
#ifdef DDB_CTF
/*
* First check if we've tried to load CTF data previously and the
* CTF ELF section wasn't found. We flag that condition by setting
* ctfcnt to -1. See below.
*/
if (ef->ctfcnt < 0) //second
time with same module,
return (0);
/* Now check if we've already loaded the CTF data.. */
if (ef->ctfcnt > 0) {
/* We only need to load once. */
lc->ctftab =3D ef->ctftab;
lc->ctfcnt =3D ef->ctfcnt;
lc->symtab =3D ef->ddbsymtab;
lc->strtab =3D ef->ddbstrtab;
lc->strcnt =3D ef->ddbstrcnt;
lc->nsym =3D ef->ddbsymcnt;
lc->ctfoffp =3D (uint32_t **) &ef->ctfoff;
lc->typoffp =3D (uint32_t **) &ef->typoff;
lc->typlenp =3D &ef->typlen;
return (0);
}
/*
* We need to try reading the CTF data. Flag no CTF data present
* by default and if we actually succeed in reading it, we'll
* update ctfcnt to the number of bytes read.
*/
ef->ctfcnt =3D -1;
---------------------------------------------------------------------
fisrt time, set ctfcnt to -1, if module does not has ctf section, and
ef->ctfcnt does NOT update with a valid value(>0) later,
then second time enter this function with same module(ef), previous
if (ef->ctfcnt < 0)
return (0); // should not be 0 here
pass over first time ctf section check, I think we need another fix .
---------------------------------------------------------------------------=
---------
diff --git a/sys/kern/kern_ctf.c b/sys/kern/kern_ctf.c
index bdff96e..2737860 100644
--- a/sys/kern/kern_ctf.c
+++ b/sys/kern/kern_ctf.c
@@ -90,7 +90,7 @@ link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
* ctfcnt to -1. See below.
*/
if (ef->ctfcnt < 0)
- return (0);
+ return (EFTYPE);
/* Now check if we've already loaded the CTF data.. */
if (ef->ctfcnt > 0) {
---------------------------------------------------------------------------=
------------
And I post another fix for the ${NORMAL_CTFCONVERT} issue, could you
check it for me?
kern/160307, I check the /boot/kernel/kernel with ctfdump, and found
the kernel image has right ctf information, do you
have any idea?
And thank you again for your work!
---------------------------------------------------------------------------=
-----------------------
[PATCH] Fix kernel panics when using dtrace fbt return probes on i386
[PATCH] Fix types of arguments to dtrace syscall return probes
walltimestamp curpsinfo->pr_psargs has no args from "Shawn Webb
lattera@gmail.com".
commit ec734d4fb07fec8d1b5fb8d1d4c8caa0fada4eea
Author: rstone <rstone@FreeBSD.org>
Replace fasttrap_copyout() with uwrite(). FreeBSD copyout() is not abl=
e to
write to the .text section of a process.
commit 83b04575eb8f73e557f245bb2814ac6db0a89696
Author: rstone <rstone@FreeBSD.org>
Fix the DTrace pid return trap interrupt vector. Previously we were us=
ing
31, but that vector is reserved.
2011/11/6 Ryan Stone <rysto32@gmail.com>:
> Currently if you try to use the args[] array passed to a syscall
> return probe, you get variables with the wrong type. =A0This is because
> the systrace implementation is currently using the same function to
> provide the same argument types for both the entry and return probes,
> which is completely wrong. =A0For example:
>
> # dtrace -v -l -n syscall::mmap:return
> =A0 ID =A0 PROVIDER =A0 =A0 =A0 =A0 =A0 =A0MODULE =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 =A0 =A0 =A0 =A0FUNCTION NAME
> 32159 =A0 =A0syscall =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mmap return
>
> =A0 =A0 =A0 =A0Probe Description Attributes
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Identifier Names: Private
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Data Semantics: =A0 Private
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Dependency Class: ISA
>
> =A0 =A0 =A0 =A0Argument Attributes
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Identifier Names: Private
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Data Semantics: =A0 Private
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Dependency Class: ISA
>
> =A0 =A0 =A0 =A0Argument Types
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args[0]: caddr_t
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args[1]: size_t
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args[2]: int
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args[3]: int
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args[4]: int
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args[5]: off_t
>
> The following patch[1] fixes this and provides the correct type to all
> return probes. =A0For example,
>
> # dtrace -l -v -n syscall::mmap:return
> =A0 ID =A0 PROVIDER =A0 =A0 =A0 =A0 =A0 =A0MODULE =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 =A0 =A0 =A0 =A0FUNCTION NAME
> =A02000 =A0 =A0syscall =A0 =A0 =A0 =A0 =A0 freebsd =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mmap return
>
> =A0 =A0 =A0 =A0Probe Description Attributes
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Identifier Names: Private
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Data Semantics: =A0 Private
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Dependency Class: Unknown
>
> =A0 =A0 =A0 =A0Argument Attributes
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Identifier Names: Private
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Data Semantics: =A0 Private
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0Dependency Class: ISA
>
> =A0 =A0 =A0 =A0Argument Types
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args[0]: caddr_t
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0args[1]: caddr_t
>
>
> The patch:
> diff --git a/sys/cddl/dev/systrace/systrace.c b/sys/cddl/dev/systrace/sys=
trace.c
> index cc48747..31c11c2 100644
> --- a/sys/cddl/dev/systrace/systrace.c
> +++ b/sys/cddl/dev/systrace/systrace.c
> @@ -220,8 +220,12 @@ systrace_getargdesc(void *arg, dtrace_id_t id,
> void *parg, dtrace_argdesc_t *des
> =A0{
> =A0 =A0 =A0 =A0int sysnum =3D SYSTRACE_SYSNUM((uintptr_t)parg);
>
> - =A0 =A0 =A0 systrace_setargdesc(sysnum, desc->dtargd_ndx, desc->dtargd_=
native,
> - =A0 =A0 =A0 =A0 =A0 sizeof(desc->dtargd_native));
> + =A0 =A0 =A0 if (SYSTRACE_ISENTRY((uintptr_t)parg))
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 systrace_entry_setargdesc(sysnum, desc->dta=
rgd_ndx,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 desc->dtargd_native, sizeof(desc->d=
targd_native));
> + =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 systrace_return_setargdesc(sysnum, desc->dt=
argd_ndx,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 desc->dtargd_native, sizeof(desc->d=
targd_native));
>
> =A0 =A0 =A0 =A0if (desc->dtargd_native[0] =3D=3D '\0')
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0desc->dtargd_ndx =3D DTRACE_ARGNONE;
> diff --git a/sys/kern/makesyscalls.sh b/sys/kern/makesyscalls.sh
> index d1162b5..1f081ce 100644
> --- a/sys/kern/makesyscalls.sh
> +++ b/sys/kern/makesyscalls.sh
> @@ -38,6 +38,7 @@ sysinc=3D"sysinc.switch.$$"
> =A0sysarg=3D"sysarg.switch.$$"
> =A0sysprotoend=3D"sysprotoend.$$"
> =A0systracetmp=3D"systrace.$$"
> +systraceret=3D"systraceret.$$"
>
> =A0if [ -r capabilities.conf ]; then
> =A0 =A0 =A0 =A0capenabled=3D`cat capabilities.conf | grep -v "^#" | grep =
-v "^$"`
> @@ -46,9 +47,9 @@ else
> =A0 =A0 =A0 =A0capenabled=3D""
> =A0fi
>
> -trap "rm $sysaue $sysdcl $syscompat $syscompatdcl $syscompat4
> $syscompat4dcl $syscompat6 $syscompat6dcl $syscompat7 $syscompat7dcl
> $sysent $sysinc $sysarg $sysprotoend $systracetmp" 0
> +trap "rm $sysaue $sysdcl $syscompat $syscompatdcl $syscompat4
> $syscompat4dcl $syscompat6 $syscompat6dcl $syscompat7 $syscompat7dcl
> $sysent $sysinc $sysarg $sysprotoend $systracetmp $systraceret" 0
>
> -touch $sysaue $sysdcl $syscompat $syscompatdcl $syscompat4
> $syscompat4dcl $syscompat6 $syscompat6dcl $syscompat7 $syscompat7dcl
> $sysent $sysinc $sysarg $sysprotoend $systracetmp
> +touch $sysaue $sysdcl $syscompat $syscompatdcl $syscompat4
> $syscompat4dcl $syscompat6 $syscompat6dcl $syscompat7 $syscompat7dcl
> $sysent $sysinc $sysarg $sysprotoend $systracetmp $systraceret
>
> =A0case $# in
> =A0 =A0 0) echo "usage: $0 input-file <config-file>" 1>&2
> @@ -96,6 +97,7 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0sysmk =3D \"$sysmk\"
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0systrace =3D \"$systrace\"
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0systracetmp =3D \"$systracetmp\"
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 systraceret =3D \"$systraceret\"
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compat =3D \"$compat\"
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compat4 =3D \"$compat4\"
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compat6 =3D \"$compat6\"
> @@ -179,9 +181,12 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf "\tint64_t *iarg =A0=3D (int64_t *)=
uarg;\n" > systrace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf "\tswitch (sysnum) {\n" > systrace
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf "static void\nsystrace_setargdesc(in=
t sysnum,
> int ndx, char *desc, size_t descsz)\n{\n\tconst char *p =3D NULL;\n" >
> systracetmp
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf "static void\nsystrace_entry_setargd=
esc(int
> sysnum, int ndx, char *desc, size_t descsz)\n{\n\tconst char *p =3D
> NULL;\n" > systracetmp
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf "\tswitch (sysnum) {\n" > systracet=
mp
>
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf "static void\nsystrace_return_setarg=
desc(int
> sysnum, int ndx, char *desc, size_t descsz)\n{\n\tconst char *p =3D
> NULL;\n" > systraceret
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf "\tswitch (sysnum) {\n" > systracere=
t
> +
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0next
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0NF =3D=3D 0 || $1 ~ /^;/ {
> @@ -202,6 +207,7 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > sysnames
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > systrace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > systracetmp
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 print > systraceret
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0savesyscall =3D syscall
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0next
> =A0 =A0 =A0 =A0}
> @@ -216,6 +222,7 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > sysnames
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > systrace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > systracetmp
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 print > systraceret
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0syscall =3D savesyscall
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0next
> =A0 =A0 =A0 =A0}
> @@ -230,6 +237,7 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > sysnames
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > systrace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0print > systracetmp
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 print > systraceret
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0next
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0syscall !=3D $1 {
> @@ -303,7 +311,8 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0parserr($end, ")")
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0end--
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 f++ =A0 =A0 #function return type
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 syscallret=3D$f
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 f++
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0funcname=3D$f
>
> @@ -387,6 +396,7 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0parseline()
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("\t/* %s */\n\tcase %d: {\n", funcn=
ame,
> syscall) > systrace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("\t/* %s */\n\tcase %d:\n", funcnam=
e, syscall)
>> systracetmp
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("\t/* %s */\n\tcase %d:\n", funcname=
, syscall)
>> systraceret
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (argc > 0) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("\t\tswitch(ndx) {\=
n") > systracetmp
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("\t\tstruct %s *p =
=3D params;\n",
> argalias) > systrace
> @@ -406,6 +416,10 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 argname[i], argtype[i]) > systrace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("\t\tdefault:\n\t\t=
\tbreak;\n\t\t};\n")
>> systracetmp
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("\t\tif (ndx =3D=3D =
0 || ndx =3D=3D 1)\n") > systraceret
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("\t\t\tp =3D \"%s\";=
\n", syscallret) > systraceret
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf("\t\tbreak;\n") > sy=
straceret
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("\t\t*n_args =3D %d;\n\t\tbreak;\n\=
t}\n", argc) > systrace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf("\t\tbreak;\n") > systracetmp
> @@ -623,6 +637,7 @@ s/\$//g
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0> syshdr
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf "\tdefault:\n\t\t*n_args =3D
> 0;\n\t\tbreak;\n\t};\n}\n" > systrace
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0printf "\tdefault:\n\t\tbreak;\n\t};\n\tif=
(p !=3D
> NULL)\n\t\tstrlcpy(desc, p, descsz);\n}\n" > systracetmp
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 printf "\tdefault:\n\t\tbreak;\n\t};\n\tif =
(p !=3D
> NULL)\n\t\tstrlcpy(desc, p, descsz);\n}\n" > systraceret
> =A0 =A0 =A0 =A0} '
>
> =A0cat $sysinc $sysent >> $syssw
> @@ -633,4 +648,5 @@ cat $sysarg $sysdcl \
> =A0 =A0 =A0 =A0$syscompat7 $syscompat7dcl \
> =A0 =A0 =A0 =A0$sysaue $sysprotoend > $sysproto
> =A0cat $systracetmp >> $systrace
> +cat $systraceret >> $systrace
>
>
> [1] Can also be found at
> http://people.freebsd.org/~rstone/patches/dtrace_syscall_ret.diff
> _______________________________________________
> freebsd-current@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org=
"
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAMwoQQ4c0XHf1kX9uXk5KOD-nRZezOW%2BB2cB03gUO9g2j32Hfg>