[Bug testsuite/14574] Add test coverage for stapdyn's runtime

["dsmith at redhat dot com" <sourceware-bugzilla at sourceware dot org>]

http://sourceware.org/bugzilla/show_bug.cgi?id=14574

--- Comment #5 from David Smith <dsmith at redhat dot com> 2012-09-21 16:06:35 UTC ---
(In reply to comment #4)
> (In reply to comment #2)
> > Created attachment 6640 [details]
> > Alpha (non-working) patch.
> > 
> > Here's a patch that implements a 'dyninst_p' function and a function that
> > returns the list of supported runtimes.
> > 
> > However, the testcase that has been modified to use the new functions doesn't
> > work, for several reasons (some listed in comments in the patch):
> > 
> > - The dyninst runtime requires a target executable. Finding an appropriate one
> > is tricky (on 32-bit x86 at least), and I'm having trouble with redirection. 
> > The latter is probably a tcl problem.
> 
> From your patch:
> +# FIXME: For dyninst, we currently have to have a target executable
> +# (even though the script doesn't actually need one). A first attempt
> +# was something simple like '/bin/true'. However, on x86 that gets
> +# you:
> +#
> +#   arch-x86.C[4377]:  invalid immediate size 0 in insn
> +#   symtab.C[1848]:  failed to getAnnotations here [main]
> +#   symtab.C[1881]:  failed to getAnnotations here [main]
> 
> These sorts of errors should be fixed now in dyninst.git, and I'll be updating
> Fedora packages with a new snapshot soon.  (both in official F18/rawhide and in
> http://repos.fedorapeople.org/repos/jistone/dyninst/ for F16/17)


Good deal, I'll check out the new packages.


> +# Sigh. So, we'll use 'stap' instead. Notice we've got to get rid of
> +# stap's output, to avoid confusing expect.
> +#
> +#   stap_runtimes_run2 $srcdir/$subdir/$test -c "stap -V > /dev/null 2>&1"
> +#
> +# Sigh again. The above doesn't work, I get "ambiguous redirect".
> +#
> +# Here's another stab:
> +stap_runtimes_run2 $srcdir/$subdir/$test -c "/bin/sh < /dev/null"
> +# Sigh, sigh, sigh.  the above doesn't work either - FAIL.
> 
> Curious.  Do these loads work for normal kernel-mode stap?  I wonder if your
> space before /dev/null is confusing something?
> 
> Another problem is that because of the redirects, stapio would usually back off
> and run these indirectly as sh -c '...'.  Even the second sh load would do this
> to get the redirect.  I tried to do this for stapdyn too, not sure how well,
> but since we aren't doing multiprocess yet, we'll *only* attach to that outer
> "sh -c" process.  Maybe that's ok for what you're doing, but worth noting.


I'm sure the errors I was getting here with my redirection efforts are tcl
quoting problems, since redirections like that work fine from the command line.


> > - Since end probes aren't currently working with the dyninst runtime, the test
> > scripts themselves are failing (since the map_hash.exp testcase uses the auto
> > end variable printing feature).
> 
> Ok, well let's just focus on test that don't need "end" for now. :/
> 
> > Comments on the general approach would still be appreciated.
> 
> I worry a little about how often "stap -V" will run for every dyninst_p call -
> can that memoize itself in a global?  Or maybe be set at configure time through
> a HAVE_DYNINST replacement?


Running 'stap -V' too often could slow things down a bit.  We could cache the
fact whether dyninst is enabled in setup_systemtap_environment() (which is run
once I believe) in an environment variable, then check that environment
variable in dyninst_p().


> (In reply to comment #3)
> > I've also been looking at modifying testsuite/systemtap.pass1-4/buildok.exp. 
> > We've got 2 main problems here:
> > 
> > - Several of the test files buildok.exp operates on in testsuite/buildok/
> > aren't really systemtap scripts but shell scripts (i.e. they start with "#!
> > /bin/sh").  I don't see any way of passing in '--runtime=dyninst' to those
> > testcases.  The affected test files are:
> > 
> >   testsuite/buildok/cmdline01.stp:#!/bin/sh
> >   testsuite/buildok/fortytwo.stp:#! /bin/sh
> >   testsuite/buildok/oldlocals01.stp:#! /bin/sh
> >   testsuite/buildok/scsi-detailed.stp:#! /bin/sh
> >   testsuite/buildok/thirtythree.stp:#! /bin/sh
> >   testsuite/buildok/thirtytwo.stp:#! /bin/sh
> >   testsuite/buildok/two.stp:#! /bin/sh
> 
> They'd have to be modified appropriately, but can't they just use their
> parameters as $1 or even "$@" in the script?  


Hmm, I hadn't thought of that I'll give it a shot.


> Also, how do you propose filtering kernel-specific buildok tests out?


I'm about to post a new version of buildok.exp.


> > - The builok.exp testcase uses 'stap_run_batch' (from
> > testsuite/lib/systemtap.exp).  You can pass as many arguments to that function
> > as you like, but it only looks at the first arg (as a filename).  So, passing
> > in the extra "--runtime=dyninst" argument will mean some changes to
> > 'stap_run_batch'.  (This might be a problem for systemtap.server/client.exp,
> > since it tries to pass extra args to 'stap_run_batch'.)
> 
> Do you mean that client.exp is broken now, since its extra args are ignored? 
> Sounds like fixing stap_run_batch is a good idea though.


It is possible that client.exp is broken.  I think I've fixed stap_run_batch,
and I'll see what that does to client.exp.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.