[PATCH v2] perf probe: Clear all structure fields in clear_perf_{probe, trace}_event()

[PATCH v2] perf probe: Clear all structure fields in clear_perf_{probe, trace}_event()

[Li Huafei]

I added two probe events:

  # perf probe -f -a schedule+8
  Added new event:
    probe:schedule       (on schedule+8)

  You can now use it in all perf tools, such as:

          perf record -e probe:schedule -aR sleep 1

  # perf probe -f -a schedule+20
  Added new event:
    probe:schedule_1     (on schedule+20)

  You can now use it in all perf tools, such as:

          perf record -e probe:schedule_1 -aR sleep 1

However, 'perf probe -l' shows the same offset:

  # perf probe -l
    probe:schedule       (on schedule+8@kernel/sched/core.c)
    probe:schedule_1     (on schedule+8@kernel/sched/core.c)

__show_perf_probe_events() does not clean up the 'pev' content when
parsing the rawlist. If the 'pev->offset' is not set while processing
the next probe event string, the offset value of the previous event will
be used.  After adding debug information, it was found that indeed there
was line number information when processing 'probe:schedule_1', so the
offset was not set and used the offset from 'probe:schedule'.

To fix this, clear all the fields of the structures in
clear_perf_{probe, trace}_event(). not just the allocated fields.

Fixes: d8f9da240495 ("perf tools: Use zfree() where applicable")
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
---
Changes in v2:
 - Do the cleanup in clear_perf_{probe, trace}_event().
 - Refine the subject and the commit log.

v1: https://lore.kernel.org/lkml/20241108181909.3515716-1-lihuafei1@huawei.com/
---
 tools/perf/util/probe-event.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a17c9b8a7a79..47903bb56fc6 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2376,8 +2376,8 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
 			field = next;
 		}
 	}
-	pev->nargs = 0;
 	zfree(&pev->args);
+	memset(pev, 0, sizeof(*pev));
 }
 
 #define strdup_or_goto(str, label)	\
@@ -2475,7 +2475,7 @@ void clear_probe_trace_event(struct probe_trace_event *tev)
 		}
 	}
 	zfree(&tev->args);
-	tev->nargs = 0;
+	memset(tev, 0, sizeof(*tev));
 }
 
 struct kprobe_blacklist_node {
-- 
2.25.1

Re: [PATCH v2] perf probe: Clear all structure fields in clear_perf_{probe, trace}_event()

[Masami Hiramatsu]

On Thu, 14 Nov 2024 18:54:51 +0800
Li Huafei <lihuafei1@huawei.com> wrote:

> I added two probe events:
> 
>   # perf probe -f -a schedule+8
>   Added new event:
>     probe:schedule       (on schedule+8)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe:schedule -aR sleep 1
> 
>   # perf probe -f -a schedule+20
>   Added new event:
>     probe:schedule_1     (on schedule+20)
> 
>   You can now use it in all perf tools, such as:
> 
>           perf record -e probe:schedule_1 -aR sleep 1
> 
> However, 'perf probe -l' shows the same offset:
> 
>   # perf probe -l
>     probe:schedule       (on schedule+8@kernel/sched/core.c)
>     probe:schedule_1     (on schedule+8@kernel/sched/core.c)
> 
> __show_perf_probe_events() does not clean up the 'pev' content when
> parsing the rawlist. If the 'pev->offset' is not set while processing
> the next probe event string, the offset value of the previous event will
> be used.  After adding debug information, it was found that indeed there
> was line number information when processing 'probe:schedule_1', so the
> offset was not set and used the offset from 'probe:schedule'.
> 
> To fix this, clear all the fields of the structures in
> clear_perf_{probe, trace}_event(). not just the allocated fields.
> 

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks!

> Fixes: d8f9da240495 ("perf tools: Use zfree() where applicable")
> Signed-off-by: Li Huafei <lihuafei1@huawei.com>
> ---
> Changes in v2:
>  - Do the cleanup in clear_perf_{probe, trace}_event().
>  - Refine the subject and the commit log.
> 
> v1: https://lore.kernel.org/lkml/20241108181909.3515716-1-lihuafei1@huawei.com/
> ---
>  tools/perf/util/probe-event.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index a17c9b8a7a79..47903bb56fc6 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2376,8 +2376,8 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
>  			field = next;
>  		}
>  	}
> -	pev->nargs = 0;
>  	zfree(&pev->args);
> +	memset(pev, 0, sizeof(*pev));
>  }
>  
>  #define strdup_or_goto(str, label)	\
> @@ -2475,7 +2475,7 @@ void clear_probe_trace_event(struct probe_trace_event *tev)
>  		}
>  	}
>  	zfree(&tev->args);
> -	tev->nargs = 0;
> +	memset(tev, 0, sizeof(*tev));
>  }
>  
>  struct kprobe_blacklist_node {
> -- 
> 2.25.1
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>