From owner-freebsd-current@FreeBSD.ORG Mon Nov 7 14:16:04 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 25BE9106566C for ; Mon, 7 Nov 2011 14:16:04 +0000 (UTC) (envelope-from ambrosehua@gmail.com) Received: from mail-fx0-f54.google.com (mail-fx0-f54.google.com [209.85.161.54]) by mx1.freebsd.org (Postfix) with ESMTP id A0B278FC0A for ; Mon, 7 Nov 2011 14:16:03 +0000 (UTC) Received: by faar19 with SMTP id r19so7321707faa.13 for ; Mon, 07 Nov 2011 06:16:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=lJMRZbcbpfyCDLR3SgEZpDuJfLdJagGTg0UEdcmPRS0=; b=AuMz4o6ytN07mw+/PxH7V2RPRByNzO1H91SQygpcCiIkqydieYDMM5R5qPyD2skieB eBN/h3ZPMvIf59anIV8Arwjaq3UzoRCRlk4rXaXi4Zy+fgfr92dNfrjrj5vMQjeV8LuY n9sYEdyXBa1YMiyRay97pTVghNggxkuZdVm8Y= MIME-Version: 1.0 Received: by 10.223.77.71 with SMTP id f7mr47149900fak.33.1320675362485; Mon, 07 Nov 2011 06:16:02 -0800 (PST) Received: by 10.223.63.71 with HTTP; Mon, 7 Nov 2011 06:16:02 -0800 (PST) In-Reply-To: References: Date: Mon, 7 Nov 2011 22:16:02 +0800 Message-ID: From: Paul Ambrose To: Ryan Stone Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: FreeBSD Current Subject: Re: [PATCH] Fix types of arguments to dtrace syscall return probes X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Nov 2011 14:16:04 -0000 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 Replace fasttrap_copyout() with uwrite(). FreeBSD copyout() is not abl= e to write to the .text section of a process. commit 83b04575eb8f73e557f245bb2814ac6db0a89696 Author: rstone Fix the DTrace pid return trap interrupt vector. Previously we were us= ing 31, but that vector is reserved. 2011/11/6 Ryan Stone : > 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 " 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= " >