On Wed, 24 Feb 2021 20:37:08 -0500
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> > + TP_printk("call_site=%pS ptr=%p name=%s",
> > + (void *)__entry->call_site, __entry->ptr, __entry->name)
>
> You must use __get_str(name) here!
>
> (see other users of this logic in include/trace/events/*.h)
>
> What is happening is that TP_fast_assign() is called by the tracepoint
> logic (at the time of the event), then much later (seconds, minutes,
> hours, days!), when the user does a "cat trace" of the file, the
> __entry->name is read and the printf logic is called against it. Well,
> the s->name that __entry->name points to, can be long gone by then!
>
> Instead, using __string() tells the TRACE_EVENT() macro that this is a
> dynamic string. The __assign_str() records the string into the ring
> buffer. The __get_str() retrieves the string from the ring buffer as
> part of the event, so it stays around as long as the event being read
> by the trace file is around.
>
> Please do not apply this patch as is, it is very buggy!
I wonder if we can add something to checkpatch that can check if
TP_printk() has a call to "%s" where it references a __entry->xxx and
not a __get_str(), and will warn about it.
There a a few cases where its OK. Like RCU uses a TPS() macro around
strings it passes into the tracepoint, which is used for strings that
never are freed, and maps the string pointer to the string for user
space. But RCU is the only user of that I believe.
-- Steve
