Messages in this thread | | | Date | Sat, 11 Mar 2023 00:19:14 +0900 | From | Masami Hiramatsu (Google) <> | Subject | Re: [PATCH] tracing: Error if a trace event has an array for a __field() |
| |
On Thu, 9 Mar 2023 22:13:02 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org> > > A __field() in the TRACE_EVENT() macro is used to set up the fields of the > trace event data. It is for single storage units (word, char, int, > pointer, etc) and not for complex structures or arrays. Unfortunately, > there's nothing preventing the build from accepting: > > __field(int, arr[5]); > > from building. It will turn into a array value. This use to work fine, as > the offset and size use to be determined by the macro using the field name, > but things have changed and the offset and size are now determined by the > type. So the above would only be size 4, and the next field will be > located 4 bytes from it (instead of 20). > > The proper way to declare static arrays is to use the __array() macro. > > Instead of __field(int, arr[5]) it should be __array(int, arr, 5). > > Add some macro tricks to the building of a trace event from the > TRACE_EVENT() macro such that __field(int, arr[5]) will fail to build. A > comment by the failure will explain why the build failed. > > Link: https://lore.kernel.org/lkml/20230306122549.236561-1-douglas.raillard@arm.com/ >
Looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thank you!
> Reported-by: Douglas RAILLARD <douglas.raillard@arm.com> > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org> > --- > include/trace/stages/stage5_get_offsets.h | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/include/trace/stages/stage5_get_offsets.h b/include/trace/stages/stage5_get_offsets.h > index ac5c24d3beeb..e30a13be46ba 100644 > --- a/include/trace/stages/stage5_get_offsets.h > +++ b/include/trace/stages/stage5_get_offsets.h > @@ -9,17 +9,30 @@ > #undef __entry > #define __entry entry > > +/* > + * Fields should never declare an array: i.e. __field(int, arr[5]) > + * If they do, it will cause issues in parsing and possibly corrupt the > + * events. To prevent that from happening, test the sizeof() a fictitious > + * type called "struct _test_no_array_##item" which will fail if "item" > + * contains array elements (like "arr[5]"). > + * > + * If you hit this, use __array(int, arr, 5) instead. > + */ > #undef __field > -#define __field(type, item) > +#define __field(type, item) \ > + { (void)sizeof(struct _test_no_array_##item *); } > > #undef __field_ext > -#define __field_ext(type, item, filter_type) > +#define __field_ext(type, item, filter_type) \ > + { (void)sizeof(struct _test_no_array_##item *); } > > #undef __field_struct > -#define __field_struct(type, item) > +#define __field_struct(type, item) \ > + { (void)sizeof(struct _test_no_array_##item *); } > > #undef __field_struct_ext > -#define __field_struct_ext(type, item, filter_type) > +#define __field_struct_ext(type, item, filter_type) \ > + { (void)sizeof(struct _test_no_array_##item *); } > > #undef __array > #define __array(type, item, len) > -- > 2.39.1 >
-- Masami Hiramatsu (Google) <mhiramat@kernel.org>
| |