Message ID | 20230309221302.642e82d9@gandalf.local.home |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:5915:0:0:0:0:0 with SMTP id v21csp661409wrd; Thu, 9 Mar 2023 19:25:22 -0800 (PST) X-Google-Smtp-Source: AK7set+asFfhuqCnReDNaoOckMfsLrfjIfR0b85+xseDrjnWTjr7VgGy7Gu4hjQ1kLi42/hZGhW4 X-Received: by 2002:a17:90a:cb88:b0:234:10c:a0d0 with SMTP id a8-20020a17090acb8800b00234010ca0d0mr936142pju.6.1678418722405; Thu, 09 Mar 2023 19:25:22 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1678418722; cv=none; d=google.com; s=arc-20160816; b=bWGxaA1lEU4q3cWX/TOtmxBZr4ou3QmjLogwApnv/MLx8TrbR4J4PdF04mgKqybchs OMCb13iARYINvSreEKBFxmf1Marwugmzvkx9LA4BjWzWfv3maKqxHwdmfircb2F9RuLB DwEAS6vVs/CqX8J0dp35Z/+6oY51P8th2wP2Mg9fKbp0orXVv+z/Ql9ZewlwBEhXNZxL YDM9gGQ2ToWsKZXVKWQhN5h+VY/NBl5mA9jFKNfqRZsPaDBheo3krWjsP9E9wRnjam4a 8lOs+TigL3CqG1yj4n6BDRdHwQ8aIuhjYupkwOzAT2FBZMt8YaZnXkGpxyTmyWSpUW4G X2vQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:subject:cc:to:from:date; bh=zgORdnrVMo5fK8I8mUklFuyi0itDQI50lf3b3UnGB0s=; b=SkAP4LYYBr8dUmPCMLZ/QDiHHU+gRMDzLih2IC3n0M3/nbLKNWu2CAVxLh+i/1n+z8 7Ze8XNU2UIra3nwReXcSceBoFt+luhvv2cKY7uImywbyXhNOKD7v9t0ah+vUUufqsdcE ep2dZ8a9YDHsH74CruFQgQyNs5bPgE8S6FGeYLjBqXCTulSxMV1q8X9Rr4TFwaYipVWG RxykOnTHmAuCd7p17mvmNcu3Yt/ISTh7Q9KaoIEmf0lEvQdivxW6epMm7OYta3zP6nmw y6PMULhNVu901RimcIZAPjIkfX28ooRus0SKD36zOWrglnDomvkl72QmrLoEKxLjaRrE fYjA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id ca2-20020a17090af30200b00233773a522fsi1406056pjb.141.2023.03.09.19.25.07; Thu, 09 Mar 2023 19:25:22 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229603AbjCJDNJ (ORCPT <rfc822;carlos.wei.hk@gmail.com> + 99 others); Thu, 9 Mar 2023 22:13:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35790 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229445AbjCJDNH (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 9 Mar 2023 22:13:07 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 776312BEFC; Thu, 9 Mar 2023 19:13:05 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 199B36069D; Fri, 10 Mar 2023 03:13:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0CFE1C433D2; Fri, 10 Mar 2023 03:13:03 +0000 (UTC) Date: Thu, 9 Mar 2023 22:13:02 -0500 From: Steven Rostedt <rostedt@goodmis.org> To: LKML <linux-kernel@vger.kernel.org>, Linux Trace Kernel <linux-trace-kernel@vger.kernel.org> Cc: Masami Hiramatsu <mhiramat@kernel.org>, Douglas RAILLARD <douglas.raillard@arm.com> Subject: [PATCH] tracing: Error if a trace event has an array for a __field() Message-ID: <20230309221302.642e82d9@gandalf.local.home> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.7 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1759949590460002764?= X-GMAIL-MSGID: =?utf-8?q?1759949590460002764?= |
Series |
tracing: Error if a trace event has an array for a __field()
|
|
Commit Message
Steven Rostedt
March 10, 2023, 3:13 a.m. UTC
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/ 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(-)
Comments
Hi Steven, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v6.3-rc1] [cannot apply to next-20230310] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Steven-Rostedt/tracing-Error-if-a-trace-event-has-an-array-for-a-__field/20230310-111456 patch link: https://lore.kernel.org/r/20230309221302.642e82d9%40gandalf.local.home patch subject: [PATCH] tracing: Error if a trace event has an array for a __field() config: x86_64-randconfig-a006 (https://download.01.org/0day-ci/archive/20230310/202303101645.28bnQoH2-lkp@intel.com/config) compiler: gcc-11 (Debian 11.3.0-8) 11.3.0 reproduce (this is a W=1 build): # https://github.com/intel-lab-lkp/linux/commit/4828e98216144ead91bbb26298aae865dac9f837 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Steven-Rostedt/tracing-Error-if-a-trace-event-has-an-array-for-a-__field/20230310-111456 git checkout 4828e98216144ead91bbb26298aae865dac9f837 # save the config file mkdir build_dir && cp config build_dir/.config make W=1 O=build_dir ARCH=x86_64 olddefconfig make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash kernel/rcu/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202303101645.28bnQoH2-lkp@intel.com/ All errors (new ones prefixed by >>): In file included from include/trace/define_trace.h:102, from include/trace/events/rcu.h:839, from kernel/rcu/rcu.h:13, from kernel/rcu/update.c:49: include/trace/events/rcu.h: In function 'trace_event_get_offsets_rcu_torture_read': >> include/trace/stages/stage5_get_offsets.h:23:53: error: expected ')' before '*' token 23 | { (void)sizeof(struct _test_no_array_##item *); } | ~ ^ include/trace/trace_events.h:263:9: note: in definition of macro 'DECLARE_EVENT_CLASS' 263 | tstruct; \ | ^~~~~~~ include/trace/trace_events.h:43:30: note: in expansion of macro 'PARAMS' 43 | PARAMS(tstruct), \ | ^~~~~~ include/trace/events/rcu.h:11:25: note: in expansion of macro 'TRACE_EVENT' 11 | #define TRACE_EVENT_RCU TRACE_EVENT | ^~~~~~~~~~~ include/trace/events/rcu.h:770:9: note: in expansion of macro 'TP_STRUCT__entry' 770 | TP_STRUCT__entry( | ^~~~~~~~~~~~~~~~ include/trace/events/rcu.h:771:17: note: in expansion of macro '__field' 771 | __field(char, rcutorturename[RCUTORTURENAME_LEN]) | ^~~~~~~ >> include/trace/events/rcu.h:771:45: error: array type has incomplete element type 'struct _test_no_array_rcutorturename' 771 | __field(char, rcutorturename[RCUTORTURENAME_LEN]) | ^ include/trace/trace_events.h:263:9: note: in definition of macro 'DECLARE_EVENT_CLASS' 263 | tstruct; \ | ^~~~~~~ include/trace/trace_events.h:43:30: note: in expansion of macro 'PARAMS' 43 | PARAMS(tstruct), \ | ^~~~~~ include/trace/events/rcu.h:11:25: note: in expansion of macro 'TRACE_EVENT' 11 | #define TRACE_EVENT_RCU TRACE_EVENT | ^~~~~~~~~~~ include/trace/events/rcu.h:770:9: note: in expansion of macro 'TP_STRUCT__entry' 770 | TP_STRUCT__entry( | ^~~~~~~~~~~~~~~~ include/trace/events/rcu.h:771:17: note: in expansion of macro '__field' 771 | __field(char, rcutorturename[RCUTORTURENAME_LEN]) | ^~~~~~~ vim +23 include/trace/stages/stage5_get_offsets.h 11 12 /* 13 * Fields should never declare an array: i.e. __field(int, arr[5]) 14 * If they do, it will cause issues in parsing and possibly corrupt the 15 * events. To prevent that from happening, test the sizeof() a fictitious 16 * type called "struct _test_no_array_##item" which will fail if "item" 17 * contains array elements (like "arr[5]"). 18 * 19 * If you hit this, use __array(int, arr, 5) instead. 20 */ 21 #undef __field 22 #define __field(type, item) \ > 23 { (void)sizeof(struct _test_no_array_##item *); } 24
On Fri, 10 Mar 2023 16:37:09 +0800 kernel test robot <lkp@intel.com> wrote: > include/trace/events/rcu.h:771:17: note: in expansion of macro '__field' > 771 | __field(char, rcutorturename[RCUTORTURENAME_LEN]) > | ^~~~~~~ Awesome, it found the bug that this patch is suppose to find! :-) The above needs to be changed to: __array(char, rcutorturename, RCUTORTURENAME_LEN) And my patch will fail builds that have arrays in __field() macros. Yes, I'm not going to apply this patch until the current bugs in the kernel are fixed, because this patch will cause the kernel not to build if it has this type of bug. -- Steve
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 >
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)