[DTrace-devel] [PATCH v2 2/2] tests, actions: fix tst.symmod.sh

[Kris Van Hees]

I am renewing my question from Feb 20...  It seems that a v3 of this patch is
needed to address issues as discussed in the exchange below.  For now, I am
deferring this patch pending such a v3.

On Tue, Feb 20, 2024 at 02:31:42PM -0500, Eugene Loh via DTrace-devel wrote:
> One question is what need there is for this test.  I think the "need" was to
> wean the test suite off of /proc/kallmodsyms.  It's unclear to me what
> urgency we feel on that.
> 
> Generalizing to any symbol opens up various cans of worms.  E.g., DTrace
> won't handle every kall[mod]syms symbol, and the test needs to be tweaked to
> strip [] off mod names.
> 
> Maybe for now, there is a simple solution.  I think the test name and
> location make it clear that it's doing a sanity ("unit") test on the sym()
> and mod() actions.  Nick points out that mod() is not robustly tested if we
> only pick one symbol that we know is in vmlinux.  But still, maybe a "good
> enough" solution for now is something like:
> 
> - # pick a test symbol from /proc/kallmodsyms
> + # pick a test symbol from /proc/kallsyms
> - read ADD NAM MOD <<< `awk '/ksys_write/ {print $1, $4, $5}'
> /proc/kallmodsyms`
> + read ADD NAM <<< `awk '/ksys_write/ {print $1, $3}' /proc/kallsyms`
> - # a blank module means the module is vmlinux
> + # the module is vmlinux
>  - if [ x$MOD == x ]; then
> -     MOD=vmlinux
> - fi
> + MOD=vmlinux
> 
> On 2/20/24 11:43, Kris Van Hees via DTrace-devel wrote:
> > We still need a v3 posted, I think?
> > 
> > On Tue, Feb 13, 2024 at 04:02:25PM +0000, Nick Alcock wrote:
> > > On 13 Feb 2024, Eugene Loh via DTrace-devel told this:
> > > 
> > > > The comment about never using shuf does not belong in the commit message.
> > > > 
> > > > The indentation in the file is with tabs, so that style should be preserved.
> > > ... it was? Whoops :)
> > > 
> > > > Instead of inflooping (in flooping?), how about "infinitely looping".? (I do find some occurrences of "inflooping" on the internet,
> > > > but might as well be less slangy?)
> > > I find 'infinitely looping' clunky. "Looping infinitely" is even
> > > clunkier. Inflooping is an established term of art, frankly.
> > > (Irrelevant given the below anyway.)
> > > 
> > > > Anyhow, instead of the shuffle approach, how about "awk 'NF == 4' /proc/kallsyms" to a temp file and use that?
> > > Oh yeah that would work, though I don't see what's wrong with using
> > > shuf...
> > > 
> > > > But even that won't be enough.? Once you allow "any" symbol from
> > > > /proc/kallsyms, you risk encountering symbols that DTrace doesn't know
> > > > about.? Stuff like __ksymtab* and __kstrtab* and others.? I think we
> > > > filter that stuff out in dt_modsym_addsym().? The same filtering is
> > > > done in test/unittest/consumer/tst.symbols.c.? It's the headache one
> > > > saves by going after a specific symbol like ksys_write.
> > > ... but doing *that* seems like it would render the test nearly useless.
> > > 
> > > the fact that tst.symmod.sh as I found it was broken for *all modules*
> > > even though much of its code was meant to handle them suggests that
> > > whatever it was intended to test, it wasn't actually testing it. (There
> > > are no comments describing what it's for, which makes it hard to figure
> > > out how to preserve that property when changing it.)
> > > 
> > > (I mean among other things it was carefully selecting a guaranteed
> > > non-modular symbol, so what all this code to figure out what module it
> > > was in was for, even if it had worked, I have no idea...)
> > > 
> > _______________________________________________
> > DTrace-devel mailing list
> > DTrace-devel at oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/dtrace-devel
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel