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

[Eugene Loh]

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