From c116c31bfdaec3c9d5adbe892d8a1d145b6fcfc2 Mon Sep 17 00:00:00 2001 From: Stan Cox Date: Sun, 10 Jan 2010 21:43:17 -0500 Subject: [PATCH] Pull set of semaphore into its own function. * tapsets.cxx (uprobe_derived_probe_group::emit_module_decls): Move sdt_sem_address from standalone to stap_uprobes. Remove sdt_sem_tid. In emitted function stap_uprobe_change_plus distinguish VM_EXEC/VM_WRITE cases, use sdt_sem_address member, move setting of semaphores to new emitted function stap_uprobe_change_semaphore_plus. (stap_uprobe_process_found): Call stap_uprobe_change_semaphore_plus. (stap_uprobe_mmap_found): Likewise. * stap-postgres.stp (postgresrelease): New. Sync to current upstream version. * stap-tcl.sh: Check if wget failed. * stap-tcl.stp: Check for skipped probes. * xulrunner.exp: Check if wget failed. --- tapsets.cxx | 138 +++++++++++++++---------- testsuite/systemtap.apps/postgres.exp | 32 +++--- testsuite/systemtap.apps/stap-tcl.sh | 4 + testsuite/systemtap.apps/stap-tcl.stp | 4 +- testsuite/systemtap.apps/tcl.exp | 5 +- testsuite/systemtap.apps/xulrunner.exp | 8 +- 6 files changed, 119 insertions(+), 72 deletions(-) diff --git a/tapsets.cxx b/tapsets.cxx index a5717d16c..4b6305b24 100644 --- a/tapsets.cxx +++ b/tapsets.cxx @@ -4593,6 +4593,7 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->newline() << "static struct stap_uprobe {"; s.op->newline(1) << "union { struct uprobe up; struct uretprobe urp; };"; s.op->newline() << "int spec_index;"; // index into stap_uprobe_specs; <0 == free && unregistered + s.op->newline() << "unsigned long sdt_sem_address;"; s.op->newline(-1) << "} stap_uprobes [MAXUPROBES];"; // XXX: consider a slab cache or somesuch s.op->newline() << "DEFINE_MUTEX(stap_uprobes_lock);"; // protects against concurrent registration/unregistration @@ -4651,14 +4652,6 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->assert_0_indent(); - s.op->newline() << "unsigned long sdt_sem_address [] = {"; - for (unsigned i =0; iline() << "0,"; - s.op->line() << "0};"; - s.op->newline() << "unsigned long sdt_sem_tid [] = {"; - for (unsigned i =0; iline() << "0,"; - s.op->line() << "0};"; s.op->newline() << "static const struct stap_uprobe_spec {"; // NB: read-only structure s.op->newline(1) << "unsigned tfi;"; // index into stap_uprobe_finders[] s.op->newline() << "unsigned return_p:1;"; @@ -4761,13 +4754,15 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->newline(1) << "int handled_p = 0;"; s.op->newline() << "int slotted_p = 0;"; s.op->newline() << "const struct stap_uprobe_spec *sups = &stap_uprobe_specs [spec_index];"; + s.op->newline() << "struct stap_uprobe *sup;"; + s.op->newline() << "pid_t sdt_sem_pid;"; s.op->newline() << "int rc = 0;"; s.op->newline() << "int i;"; s.op->newline() << "if (likely(sups->tfi != tfi)) continue;"; // skip probes with an address beyond this map event; should not // happen unless a shlib/exec got mmapped in weirdly piecemeal - s.op->newline() << "if (likely(sups->address >= length)) continue;"; + s.op->newline() << "if (likely((vm_flags & VM_EXEC) && ((sups->address >= length) || (sups->sdt_sem_offset >= length)))) continue;"; // Found a uprobe_spec for this stap_uprobe_tf. Need to lock the // stap_uprobes[] array to allocate a free spot, but then we can @@ -4775,10 +4770,11 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->newline() << "mutex_lock (& stap_uprobes_lock);"; s.op->newline() << "for (i=0; inewline(1) << "struct stap_uprobe *sup = & stap_uprobes[i];"; + s.op->newline(1) << "sup = & stap_uprobes[i];"; // register new uprobe - s.op->newline() << "if (sup->spec_index < 0) {"; + // We make two passes for semaphores; see _stap_uprobe_change_semaphore_plus + s.op->newline() << "if (sup->spec_index < 0 || (sups->sdt_sem_offset && vm_flags & VM_WRITE && sup->spec_index == spec_index)) {"; s.op->newline(1) << "#if (UPROBES_API_VERSION < 2)"; // See PR6829 comment. s.op->newline() << "if (sup->spec_index == -1 && sup->up.kdata != NULL) continue;"; @@ -4803,28 +4799,18 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) // was full (registration; MAXUPROBES) or that no matching entry was // found (unregistration; should not happen). - s.op->newline() << "if (sups->sdt_sem_offset && (sdt_sem_tid[spec_index] != tsk->tgid || sdt_sem_address[spec_index] == 0)) {"; + s.op->newline() << "sdt_sem_pid = (sups->return_p ? sup->urp.u.pid : sup->up.pid);"; + s.op->newline() << "if (sups->sdt_sem_offset && (sdt_sem_pid != tsk->tgid || sup->sdt_sem_address == 0)) {"; s.op->indent(1); // If the probe is in the executable itself, the offset *is* the address. s.op->newline() << "if (vm_flags & VM_EXECUTABLE) {"; s.op->indent(1); - s.op->newline() << "sdt_sem_address[spec_index] = relocation + sups->sdt_sem_offset;"; - s.op->newline() << "sdt_sem_tid[spec_index] = tsk->tgid;"; + s.op->newline() << "sup->sdt_sem_address = relocation + sups->sdt_sem_offset;"; s.op->newline(-1) << "}"; - // If the probe is in a .so, we have to calculate the address - // when the initial mmap maps the entire solib, e.g. - // 7f089885a000-7f089885b000 rw-p- libtcl.so - // A subsequent mmap maps in the writeable segment where the - // semaphore control variable lives, which is when the variable is set. - // 7f089850d000-7f0898647000 r-xp- libtcl.so - // 7f0898647000-7f0898846000 ---p libtcl.so - // 7f0898846000-7f089885b000 rw-p- libtcl.so - // If the task changes, then recalculate the address. s.op->newline() << "else {"; s.op->indent(1); - s.op->newline() << "sdt_sem_address[spec_index] = (relocation - offset) + sups->sdt_sem_offset;"; - s.op->newline() << "sdt_sem_tid[spec_index] = tsk->tgid;"; + s.op->newline() << "sup->sdt_sem_address = (relocation - offset) + sups->sdt_sem_offset;"; s.op->newline(-1) << "}"; s.op->newline(-1) << "}"; // sdt_sem_offset @@ -4841,23 +4827,9 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->newline() << "sup->up.handler = &enter_uprobe_probe;"; s.op->newline() << "rc = register_uprobe (& sup->up);"; - s.op->newline() << "if (sdt_sem_address[spec_index]) {"; - s.op->newline(1) << "unsigned short sdt_semaphore = 0;"; // NB: fixed size - s.op->newline() << "if (get_user (sdt_semaphore, (unsigned short __user*) sdt_sem_address[spec_index]) == 0) {"; - // We have an executable or a writeable segment of a .so - s.op->newline(1) << "if (vm_flags == 0 || vm_flags & VM_WRITE) {"; - s.op->newline(1) << "sdt_semaphore ++;"; - s.op->newline() << "put_user (sdt_semaphore, (unsigned short __user*) sdt_sem_address[spec_index]);"; - s.op->newline(-1) << "}"; - s.op->newline(-1) << "}"; // sdt_sem_address - // XXX: error handling in __access_process_vm! - // XXX: need to analyze possibility of race condition - s.op->newline(-1) << "}"; - s.op->newline(-1) << "}"; - - s.op->newline() << "if (rc && !(vm_flags & VM_WRITE)) {"; // failed to register + s.op->newline() << "if (rc) {"; // failed to register s.op->newline(1) << "_stp_warn (\"u*probe failed %s[%d] '%s' addr %p rc %d\\n\", tsk->comm, tsk->tgid, sups->pp, (void*)(relocation + sups->address), rc);"; // NB: we need to release this slot, so we need to borrow the mutex temporarily. s.op->newline() << "mutex_lock (& stap_uprobes_lock);"; @@ -4890,6 +4862,59 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->assert_0_indent(); + s.op->newline(); + s.op->newline() << "static int stap_uprobe_change_semaphore_plus (struct task_struct *tsk, unsigned long relocation, unsigned long length, const struct stap_uprobe_tf *stf) {"; + s.op->newline(1) << "int tfi = (stf - stap_uprobe_finders);"; + s.op->newline() << "int spec_index;"; + s.op->newline() << "int rc = 0;"; + s.op->newline() << "struct stap_uprobe *sup;"; + s.op->newline() << "int i;"; + + // We make two passes for semaphores. + // The first pass, stap_uprobe_change_plus, calculates the address of the + // semaphore. If the probe is in a .so, we calculate the + // address when the initial mmap maps the entire solib, e.g. + // 7f089885a000-7f089885b000 rw-p- libtcl.so + // A subsequent mmap maps in the writeable segment where the + // semaphore control variable lives, e.g. + // 7f089850d000-7f0898647000 r-xp- libtcl.so + // 7f0898647000-7f0898846000 ---p libtcl.so + // 7f0898846000-7f089885b000 rw-p- libtcl.so + // The second pass, stap_uprobe_change_semaphore_plus, sets the semaphore. + // If the probe is in a .so this will be when the writeable segment of the .so + // is mapped in. If the task changes, then recalculate the address. + + s.op->newline() << "for (i=0; inewline(1) << "sup = & stap_uprobes[i];"; + s.op->newline() << "if (sup->spec_index == -1) continue;"; + s.op->newline() << "if (sup->sdt_sem_address != 0 && !(sup->up.pid == tsk->tgid " + << "&& sup->sdt_sem_address >= relocation && sup->sdt_sem_address < relocation+length)) continue;"; + + s.op->newline() << "if (sup->sdt_sem_address) {"; + s.op->newline(1) << "unsigned short sdt_semaphore = 0;"; // NB: fixed size + s.op->newline() << "if ((rc = get_user (sdt_semaphore, (unsigned short __user*) sup->sdt_sem_address)) == 0) {"; + s.op->newline(1) << "sdt_semaphore ++;"; + + s.op->newline() << "#ifdef DEBUG_UPROBES"; + s.op->newline() << "{"; + s.op->newline(1) << "const struct stap_uprobe_spec *sups = &stap_uprobe_specs [sup->spec_index];"; + s.op->newline() << "_stp_dbug(__FUNCTION__,__LINE__, \"+semaphore %#x @ %#lx spec %d idx %d task %d\\n\", " + << "sdt_semaphore, sup->sdt_sem_address, sup->spec_index, i, tsk->tgid);"; + s.op->newline(-1) << "}"; + s.op->newline() << "#endif"; + + s.op->newline() << "rc = put_user (sdt_semaphore, (unsigned short __user*) sup->sdt_sem_address);"; + s.op->newline(-1) << "}"; + // XXX: need to analyze possibility of race condition + + s.op->newline(-1) << "}"; + s.op->newline(-1) << "}"; + s.op->newline() << "return rc;"; + s.op->newline(-1) << "}"; + + s.op->assert_0_indent(); + + // Removing/unmapping a uprobe is simpler than adding one (in the _plus function above). // We need not care about stap_uprobe_finders or anything, we just scan through stap_uprobes[] // for a live probe within the given address range, and kill it. @@ -4989,9 +5014,12 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) s.op->newline() << "#endif"; // ET_EXEC events are modeled as if shlib events, but with 0 relocation bases - s.op->newline() << "if (register_p)"; - s.op->newline(1) << "return stap_uprobe_change_plus (tsk, 0, TASK_SIZE, stf, 0, 0);"; - s.op->newline(-1) << "else"; + s.op->newline() << "if (register_p) {"; + // s.op->newline(1) << "return stap_uprobe_change_plus (tsk, 0, TASK_SIZE, stf, 0, 0);"; + s.op->newline(1) << "int rc = stap_uprobe_change_plus (tsk, 0, TASK_SIZE, stf, 0, 0);"; + s.op->newline() << "stap_uprobe_change_semaphore_plus (tsk, 0, TASK_SIZE, stf);"; + s.op->newline() << "return rc;"; + s.op->newline(-1) << "} else"; s.op->newline(1) << "return stap_uprobe_change_minus (tsk, 0, TASK_SIZE, stf);"; s.op->indent(-1); s.op->newline(-1) << "}"; @@ -5008,14 +5036,19 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s) // 2 - the shared library we're interested in s.op->newline() << "if (path == NULL || strcmp (path, stf->pathname)) return 0;"; // 3 - mapping should be executable or writeable (for semaphore in .so) - s.op->newline() << "if (!((vm_flags & VM_EXEC) || (vm_flags & VM_WRITE))) return 0;"; - - s.op->newline() << "#ifdef DEBUG_TASK_FINDER_VMA"; - s.op->newline() << "_stp_dbug (__FUNCTION__,__LINE__, \"+mmap pid %d path %s addr %p length %u offset %p stf %p %p path %s\\n\", " + s.op->newline() << "if (vm_flags & VM_EXEC) {"; + s.op->newline(1) << "#ifdef DEBUG_TASK_FINDER_VMA"; + s.op->newline() << "_stp_dbug (__FUNCTION__,__LINE__, \"+mmap R-X pid %d path %s addr %p length %u offset %p stf %p %p path %s\\n\", " << "tsk->tgid, path, (void *) addr, (unsigned)length, (void*) offset, tgt, stf, stf->pathname);"; s.op->newline() << "#endif"; - s.op->newline() << "return stap_uprobe_change_plus (tsk, addr, length, stf, offset, vm_flags);"; + s.op->newline(-1) << "} else if (vm_flags & VM_WRITE) {"; + s.op->newline(1) << "#ifdef DEBUG_TASK_FINDER_VMA"; + s.op->newline() << "_stp_dbug (__FUNCTION__,__LINE__, \"+mmap RW- pid %d path %s addr %p length %u offset %p stf %p %p path %s\\n\", " + << "tsk->tgid, path, (void *) addr, (unsigned)length, (void*) offset, tgt, stf, stf->pathname);"; + s.op->newline() << "#endif"; + s.op->newline() << "return stap_uprobe_change_semaphore_plus (tsk, addr, length, stf);"; + s.op->newline(-1) << "} else return 0;"; s.op->newline(-1) << "}"; s.op->assert_0_indent(); @@ -5093,7 +5126,7 @@ uprobe_derived_probe_group::emit_module_exit (systemtap_session& s) s.op->newline() << "if (sup->spec_index < 0) continue;"; // free slot // PR10655: decrement that ENABLED semaphore - s.op->newline() << "if (sdt_sem_address[sup->spec_index]) {"; + s.op->newline() << "if (sup->sdt_sem_address) {"; s.op->newline(1) << "unsigned short sdt_semaphore;"; // NB: fixed size s.op->newline() << "pid_t pid = (sups->return_p ? sup->urp.u.pid : sup->up.pid);"; s.op->newline() << "struct task_struct *tsk;"; @@ -5113,14 +5146,13 @@ uprobe_derived_probe_group::emit_module_exit (systemtap_session& s) s.op->newline() << "#endif /* 2.6.31 */"; s.op->newline() << "if (tsk) {"; // just in case the thing exited while we weren't watching - s.op->newline(1) << "if (get_user (sdt_semaphore, (unsigned short __user*) sdt_sem_address[sup->spec_index]) == 0) {"; + s.op->newline(1) << "if (get_user (sdt_semaphore, (unsigned short __user*) sup->sdt_sem_address) == 0) {"; s.op->newline(1) << "sdt_semaphore --;"; s.op->newline() << "#ifdef DEBUG_UPROBES"; - s.op->newline() << "_stp_dbug (__FUNCTION__,__LINE__, \"-semaphore %#x @ %#lx\\n\", sdt_semaphore, sdt_sem_address[sup->spec_index]);"; + s.op->newline() << "_stp_dbug (__FUNCTION__,__LINE__, \"-semaphore %#x @ %#lx\\n\", sdt_semaphore, sup->sdt_sem_address);"; s.op->newline() << "#endif"; - s.op->newline() << "put_user (sdt_semaphore, (unsigned short __user*) sdt_sem_address[sup->spec_index]);"; + s.op->newline() << "put_user (sdt_semaphore, (unsigned short __user*) sup->sdt_sem_address);"; s.op->newline(-1) << "}"; - // XXX: error handling in __access_process_vm! // XXX: need to analyze possibility of race condition s.op->newline(-1) << "}"; s.op->newline() << "rcu_read_unlock();"; diff --git a/testsuite/systemtap.apps/postgres.exp b/testsuite/systemtap.apps/postgres.exp index 9d3c18c33..34d903e1a 100644 --- a/testsuite/systemtap.apps/postgres.exp +++ b/testsuite/systemtap.apps/postgres.exp @@ -12,8 +12,9 @@ if {! [info exists env(SYSTEMTAP_TESTAPPS)] || ( } ########## Create /tmp/stap-postgres.stp ########## -set postgresbuild "[pwd]/postgresql-8.3.6/bld" -set postgresdir "[pwd]/postgresql-8.3.6/install/" +set postgresrelease "8.3.9" +set postgresbuild "[pwd]/postgresql-$postgresrelease/bld" +set postgresdir "[pwd]/postgresql-$postgresrelease/install/" set pgdata "/tmp/stap-postgres" @@ -110,26 +111,29 @@ fi kill \$STAPPID \} -if \[ ! -r postgresql-8.3.6.tar.bz2 \] ; then -wget http://wwwmaster.postgresql.org/redir/198/h/source/v8.3.6/postgresql-8.3.6.tar.bz2 +if \[ ! -r postgresql-$postgresrelease.tar.bz2 \] ; then +wget http://wwwmaster.postgresql.org/redir/198/h/source/v${postgresrelease}/postgresql-$postgresrelease.tar.bz2 +fi +if \[ ! -r postgresql-$postgresrelease.tar.bz2 \] ; then + echo FAIL: wget $postgresrelease.tar.gz + exit fi if \[ ! -d $postgresbuild/src/backend \] ; then -tar -x -f postgresql-8.3.6.tar.bz2 +tar -x -f postgresql-${postgresrelease}.tar.bz2 fi -cd postgresql-8.3.6/ +cd postgresql-${postgresrelease}/ mkdir bld;cd bld ../configure --enable-dtrace --prefix=$postgresdir -# sed -i -e 's/ifeq (\$(PORTNAME), solaris)/ifeq (\$(enable_dtrace), yes)/' src/backend/Makefile sed -i -e 's/^CFLAGS = -O2.*\$/& -g -DEXPERIMENTAL_KPROBE_SDT/' src/Makefile.global -make -make install -run_tests kprobe +# make +# make install +# run_tests kprobe sed -i -e 's/-DEXPERIMENTAL_KPROBE_SDT//' src/Makefile.global -(cd src/backend/utils/ - make clean) +# (cd src/backend/utils/ +# make clean) make make install run_tests uprobe @@ -156,6 +160,6 @@ expect { if { $verbose == 0 } { catch {exec rm -rf $pgdata} catch {exec rm -rf $pgdata.stp $pgdata.log \ - $pgdata-markers.log $pgdata.sh postgresql-8.3.6.tar.bz2} -catch {exec rm -rf postgresql-8.3.6} + $pgdata-markers.log $pgdata.sh postgresql-${postgresrelease}.tar.bz2} +catch {exec rm -rf postgresql-${postgresrelease}} } diff --git a/testsuite/systemtap.apps/stap-tcl.sh b/testsuite/systemtap.apps/stap-tcl.sh index c3a7f6b70..2a0d05485 100644 --- a/testsuite/systemtap.apps/stap-tcl.sh +++ b/testsuite/systemtap.apps/stap-tcl.sh @@ -11,6 +11,10 @@ mkdir -p tcl if [ ! -r tcl$tclrelease-src.tar.gz ] ; then wget http://sourceforge.net/projects/tcl/files/Tcl/$tclrelease/tcl$tclrelease-src.tar.gz/download fi +if [ ! -r tcl$tclrelease-src.tar.gz ] ; then + echo FAIL: wget tcl$tclrelease-src.tar.gz + exit +fi if [ ! -d tcl/src ] ; then tar -x -z -f tcl$tclrelease-src.tar.gz diff --git a/testsuite/systemtap.apps/stap-tcl.stp b/testsuite/systemtap.apps/stap-tcl.stp index db3e36900..628d801f2 100644 --- a/testsuite/systemtap.apps/stap-tcl.stp +++ b/testsuite/systemtap.apps/stap-tcl.stp @@ -6,7 +6,7 @@ probe process(@1).library(@2).mark("*") { function judge(name, minvalue) { value = @count(counts[name]) - printf("%s %s %d %d\n", ((value>=minvalue)?"OK":"KO"), name, value, minvalue) + printf("%s %s Got: %d Expected Minimum: %d\n", ((value>=minvalue)?"OK":"KO"), name, value, minvalue) } probe end,error { @@ -21,7 +21,7 @@ probe end,error { judge("cmd__entry", 37000) judge("cmd__return", 37000) judge("cmd__result", 37000) - judge("cmd__args", 3700 /* not 37000? */) + judge("cmd__args", 37000) judge("cmd__info", 37000) judge("inst__start", 542000) judge("inst__done", 542000) diff --git a/testsuite/systemtap.apps/tcl.exp b/testsuite/systemtap.apps/tcl.exp index c95fa5e08..4c4a43428 100644 --- a/testsuite/systemtap.apps/tcl.exp +++ b/testsuite/systemtap.apps/tcl.exp @@ -54,6 +54,7 @@ expect { -timeout 1000 -re {^OK [^\r\n]*[\r\n]} { incr ok; exp_continue } -re {^KO [^\r\n]*[\r\n]} { incr ko; exp_continue } + -re {^ERROR: Skipped[^\r\n]*[\r\n]} { incr xok; exp_continue } -re {^ERROR[^\r\n]*[\r\n]} { incr ko; exp_continue } -re {^[^\r\n]*[\r\n]} { incr lines; exp_continue } timeout { fail "$test (timeout)" } @@ -66,4 +67,6 @@ if {$ok == 14 && $ko == 0} { } else { fail "$test ($ok $ko $lines)" } - +if {$xok == 1} { + xfail "$test (skipped probes)" +} diff --git a/testsuite/systemtap.apps/xulrunner.exp b/testsuite/systemtap.apps/xulrunner.exp index 7a6b934a5..2bcc6a209 100644 --- a/testsuite/systemtap.apps/xulrunner.exp +++ b/testsuite/systemtap.apps/xulrunner.exp @@ -81,6 +81,10 @@ fi if \[ ! -r xulrunner-$xulrelease.source.tar.bz2 \] ; then wget ftp://ftp.mozilla.org/pub/mozilla.org/xulrunner/releases/$xulrelease/source/xulrunner-$xulrelease.source.tar.bz2 fi +if \[ ! -r xulrunner-$xulrelease.source.tar.bz2 \] ; then + echo FAIL: wget xulrunner-$xulrelease.source.tar.bz2 + exit +fi if \[ ! -d xul/src \] ; then bunzip2 xulrunner-$xulrelease.source.tar.bz2 @@ -104,9 +108,9 @@ CFLAGS='-g -I$env(SYSTEMTAP_INCLUDES)' \ PATH=$env(SYSTEMTAP_PATH)/:\$PATH \ ../src/configure --prefix=$xuldir --enable-dtrace --enable-application=xulrunner sed -i '/include.*rules.mk/a\ -PROGOBJS+=./mozjs-dtrace.o' xul/bld/js/src/Makefile +PROGOBJS+=mozjs-dtrace.o' js/src/Makefile J=\$(getconf _NPROCESSORS_CONF) -make -j \$J +make fi run_tests uprobe -- 2.43.5