Re: [PATCH] libtracefs: Have tracefs_dynevent_get_all() find kprobes and uprobes properly

On 16/10/2024 2:46 pm, Steven Rostedt wrote:
> On Wed, 16 Oct 2024 09:33:00 +0100
> Metin Kaya <[email protected]> wrote:
> 
> 
>>>
>>> I could successfully apply the patch on 4cbebed79b1f ("libtracefs:
>>> Documentation: Add missing documentation to meson.build").
>>> The unit tests at [1] passes with this patch.
>>>
>>> Just for the records, there are some unit test failures in libtracefs
>>> (they were there even before the patch):
>>>
>>> $ sudo ./utest/trace-utest
>>>        CUnit - A unit testing framework for C - Version 2.1-3
>>>        http://cunit.sourceforge.net/
>>>
>>> Memory mapped buffers not supported
>>>
>>> Suite: tracefs library
>>>     Test: Test tracefs/debugfs mounting ...FAILED
>>>       1. tracefs-utest.c:1806  - ret == 0
> 
> Note, if you have anything in the /sys/kernel/tracing directory (including
> gdb instances of trace-cmd), it will fail to unmount. I tripped over this
> too.

I ran "trace-cmd reset && umount /sys/kernel/tracing" before the unit 
test. And this error is gone.

> 
>>>     Test: trace cpu read ...passed
>>>     Test: trace cpu read_buf_percent ...passed
>>>     Test: trace cpu pipe ...passed
>>>     Test: trace pid events filter ...passed
>>>     Test: trace pid function filter ...passed
>>>     Test: trace sql ...passed
>>>     Test: trace sql trace onmax ...passed
>>>     Test: trace sql trace onchange ...passed
>>>     Test: trace sql snapshot onmax ...passed
>>>     Test: trace sql snapshot onchange ...passed
>>>     Test: trace sql save onmax ...passed
>>>     Test: trace sql save onchange ...passed
>>>     Test: trace sql trace and snapshot onmax ...passed
>>>     Test: trace sql trace and snapshot onchange ...passed
>>>     Test: tracing file / directory APIs ...passed
>>>     Test: instance file / directory APIs ...passed
>>>     Test: instance file descriptor ...passed
>>>     Test: instance reset ...passed
>>>     Test: systems and events APIs ...passed
>>>     Test: tracefs_iterate_snapshot_events API ...passed
>>>     Test: tracefs_iterate_raw_events API ...FAILED
>>>       1. tracefs-utest.c:235  - ret == sizeof(struct test_sample)
>>>       2. tracefs-utest.c:235  - ret == sizeof(struct test_sample)
> 
> Did you do a trace-cmd reset before running the tests?

Yes. And one of these failures still persists:
   Test: tracefs_iterate_snapshot_events API ...FAILED
     1. tracefs-utest.c:235  - ret == sizeof(struct test_sample)

> 
>>>     Test: Follow events ...passed
>>>     Test: Follow events clear ...passed
>>>     Test: tracefs_tracers API ...passed
>>>     Test: tracefs_local events API ...passed
>>>     Test: tracefs_instances_walk API ...passed
>>>     Test: tracefs_get_clock API ...passed
>>>     Test: tracing on / off ...passed
>>>     Test: tracing options ...passed
>>>     Test: custom system directory ...passed
>>>     Test: ftrace marker ...passed
>>>     Test: kprobes ...passed
>>>     Test: synthetic events ...passed
>>>     Test: eprobes ...passed
>>>     Test: uprobes ...FAILED
>>>       1. tracefs-utest.c:2222  - ret == 0
>>>       2. tracefs-utest.c:2222  - ret == 0
> 
> Did you have left over uprobes?

Nope. I ran "trace-cmd reset" before the test and confirmed no uprobes 
are left. But still getting these failures.

> 
>>>
>>> Run Summary:    Type    Total      Ran   Passed Failed Inactive
>>>                 suites        1        1      n/a      0        0
>>>                  tests       36       36       33      3        0
>>>                asserts 30341119 30341119 30341114      5      n/a
>>>
>>> Elapsed time =  124.937 seconds
>>>
>>> With that,
>>>
>>> Reviewed-by: Metin Kaya <[email protected]>
>>>
>>>
>>> Thanks a lot for fixing it promptly!
>>>
>>> [1]
>>> https://lore.kernel.org/all/[email protected]/
>>>
>>>    
>>
>> Sorry, I need to revoke my Reviewed-by. Because, "trace-cmd reset"
>> cannot destroy uretprobes after this patch.
>>
>> # cd /sys/kernel/tracing/
>> /sys/kernel/tracing # cat uprobe_events
>> /sys/kernel/tracing # echo 'r /bin/bash:0x4245c0' > uprobe_events
>> /sys/kernel/tracing # cat uprobe_events
>> r:uprobes/p_bash_0x4245c0 /bin/bash:0x00000000004245c0
>> /sys/kernel/tracing # trace-cmd reset
>> /sys/kernel/tracing # cat uprobe_events
>> r:uprobes/p_bash_0x4245c0 /bin/bash:0x00000000004245c0
>>
>> OTOH, "trace-cmd reset" is able to destroy uretprobes if there is also a
>> kprobe in addition to a uretprobe:
> 
> Oops, I know why. I had a bug in the code that collects all the probes and
> looks at different files. It assumed that if the return of
> tracefs_instance_file_read() returns NULL from kprobe_events, it is an
> error. But that function also returns NULL if the file is empty. I changed
> the code from:
> 
> 	content = tracefs_instance_file_read(NULL, desc->file, NULL);
> 	if (!content)
> 		return -1;
> 
> to:
> 
> 	if (!tracefs_file_exists(NULL, desc->file))
> 		return -1;
> 
> 	content = tracefs_instance_file_read(NULL, desc->file, NULL);
> 	/* File exists, but may be empty */
> 	if (!content)
> 		return 0;
> 
> I'll send out a v2. Thanks for testing.
> 
> I'll also add more tests to the libtracefs utest to check for this too.
> 
> -- Steve
> 
> 
>>
>> /sys/kernel/tracing # cat dynamic_events
>> /sys/kernel/tracing # cat uprobe_events
>> /sys/kernel/tracing # echo 'p do_sys_open' > kprobe_events
>> /sys/kernel/tracing # echo 'r /bin/bash:0x4245c0' > uprobe_events
>> /sys/kernel/tracing # cat dynamic_events
>> p:kprobes/p_do_sys_open_0 do_sys_open
>> r:uprobes/p_bash_0x4245c0 /bin/bash:0x00000000004245c0
>> /sys/kernel/tracing # cat uprobe_events
>> r:uprobes/p_bash_0x4245c0 /bin/bash:0x00000000004245c0
>> /sys/kernel/tracing # trace-cmd reset
>> /sys/kernel/tracing # cat dynamic_events
>> /sys/kernel/tracing # cat uprobe_events
>> /sys/kernel/tracing #
>