From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 093E6A0A0E; Tue, 11 May 2021 11:33:58 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C001640140; Tue, 11 May 2021 11:33:57 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mails.dpdk.org (Postfix) with ESMTP id 6AC964003E; Tue, 11 May 2021 11:33:55 +0200 (CEST) IronPort-SDR: sSTygPmYUPGfV3WhoU0UzM26t2NgiYyHFiBXhMrkuRWI7a7UqL3Wsq83XwOBpI0B/TU9f5AxAI D51erXYjRJ1Q== X-IronPort-AV: E=McAfee;i="6200,9189,9980"; a="179662547" X-IronPort-AV: E=Sophos;i="5.82,290,1613462400"; d="scan'208";a="179662547" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 May 2021 02:33:54 -0700 IronPort-SDR: wDE4PSgpna6xxVPHcOLDdNVROEJztSXBwq1QjN3RTOWPuZtUvYf2LYNrMn7nQLLziPMI8WSvbP kKtktxTTb2gg== X-IronPort-AV: E=Sophos;i="5.82,290,1613462400"; d="scan'208";a="536949872" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.224.45]) ([10.213.224.45]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 May 2021 02:33:51 -0700 To: "Wang, Haiyue" , "Yang, Qiming" , "Zhang, Qi Z" , "Stillwell Jr, Paul M" , "Lu, Wenzhuo" , "Rong, Leyi" , "Shukla, Shivanshu" Cc: "dev@dpdk.org" , "stable@dpdk.org" , Kevin Traynor , Ajit Khaparde References: <20210510150319.1496105-1-ferruh.yigit@intel.com> <20210510150319.1496105-3-ferruh.yigit@intel.com> From: Ferruh Yigit X-User: ferruhy Message-ID: Date: Tue, 11 May 2021 10:33:48 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 5/11/2021 2:59 AM, Wang, Haiyue wrote: >> -----Original Message----- >> From: Yigit, Ferruh >> Sent: Tuesday, May 11, 2021 01:28 >> To: Wang, Haiyue ; Yang, Qiming ; Zhang, Qi Z >> ; Stillwell Jr, Paul M ; Lu, Wenzhuo >> ; Rong, Leyi ; Shukla, Shivanshu >> >> Cc: dev@dpdk.org; stable@dpdk.org; Kevin Traynor ; Ajit Khaparde >> >> Subject: Re: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11 >> >> On 5/10/2021 6:04 PM, Wang, Haiyue wrote: >>>> -----Original Message----- >>>> From: dev On Behalf Of Ferruh Yigit >>>> Sent: Monday, May 10, 2021 23:03 >>>> To: Yang, Qiming ; Zhang, Qi Z ; Stillwell Jr, Paul M >>>> ; Lu, Wenzhuo ; Rong, Leyi >> ; >>>> Shukla, Shivanshu >>>> Cc: Yigit, Ferruh ; dev@dpdk.org; stable@dpdk.org; Kevin Traynor >>>> ; Ajit Khaparde >>>> Subject: [dpdk-dev] [PATCH 3/4] net/ice/base: fix build with gcc11 >>>> >>>> Reproduced with '--buildtype=debugoptimized' config, >>>> compiler version: gcc (GCC) 12.0.0 20210509 (experimental) >>>> >>>> There are multiple build errors, like: >>>> ../drivers/net/ice/base/ice_switch.c: In function ‘ice_add_marker_act’: >>>> ../drivers/net/ice/base/ice_switch.c:3727:15: >>>> warning: array subscript ‘struct ice_aqc_sw_rules_elem[0]’ >>>> is partly outside array bounds of ‘unsigned char[52]’ >>>> [-Warray-bounds] >>>> 3727 | lg_act->type = CPU_TO_LE16(ICE_AQC_SW_RULES_T_LG_ACT); >>>> | ^~ >>>> In file included from ../drivers/net/ice/base/ice_type.h:52, >>>> from ../drivers/net/ice/base/ice_common.h:8, >>>> from ../drivers/net/ice/base/ice_switch.h:8, >>>> from ../drivers/net/ice/base/ice_switch.c:5: >>>> ../drivers/net/ice/base/ice_osdep.h:209:29: >>>> note: referencing an object of size 52 allocated by ‘rte_zmalloc’ >>>> 209 | #define ice_malloc(h, s) rte_zmalloc(NULL, s, 0) >>>> | ^~~~~~~~~~~~~~~~~~~~~~~ >>>> ../drivers/net/ice/base/ice_switch.c:3720:50: >>>> note: in expansion of macro ‘ice_malloc’ >>>> lg_act = (struct ice_aqc_sw_rules_elem *)ice_malloc(hw, rules_size); >>>> >>>> These errors are mainly because allocated memory is cast to >>>> "struct ice_aqc_sw_rules_elem *" but allocated size is less than the size >>>> of "struct ice_aqc_sw_rules_elem". >>>> >>>> "struct ice_aqc_sw_rules_elem" has multiple other structs has unions, >>>> based on which one is used allocated memory being less than the size of >>>> "struct ice_aqc_sw_rules_elem" is logically correct but compiler is >>>> complaining about it. >>>> >>>> As a solution making sure allocated memory size is at least size of >>>> "struct ice_aqc_sw_rules_elem". >>>> The function to use the struct is 'ice_aq_sw_rules()', and it already has >>>> parameter for size of the rule, allocating more than needed shouldn't >>>> cause any problem. >>>> >>>> Fixes: c7dd15931183 ("net/ice/base: add virtual switch code") >>>> Fixes: 02acdce2f553 ("net/ice/base: add MAC filter with marker and counter") >>>> Fixes: f89aa3affa9e ("net/ice/base: support removing advanced rule") >>>> Cc: stable@dpdk.org >>>> >>>> Signed-off-by: Ferruh Yigit >>>> --- >>>> Cc: paul.m.stillwell.jr@intel.com >>>> Cc: qi.z.zhang@intel.com >>>> Cc: leyi.rong@intel.com >>>> Cc: Kevin Traynor >>>> Cc: Ajit Khaparde >>>> --- >>>> drivers/net/ice/base/ice_switch.c | 30 +++++++++++++++++++++++------- >>>> 1 file changed, 23 insertions(+), 7 deletions(-) >>> >>> GCC bug ? >>> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98266 >>> >>> Bug 98266 - [11 Regression] bogus array subscript is partly outside array bounds on virtual >> inheritance >>> >> >> I am not sure if this is a gcc defect. >> >> Here there is a memory allocated and assigned to "struct ice_aqc_sw_rules_elem >> *", but allocated memory size is less than the struct size. As far as I >> understand this is the reason of compiler warning. >> >> For this case it may not be problem logically since both who allocates memory >> and who uses the memory follows a contract, but there is a mismatch between >> pointer type and object. If some other function wants to access all fields of >> the struct, it will be out of bound access. >> > > That's why they are static function, not API. These functions are designed for > internally using to handle the compact memory layout with different types. > > These XXX_SIZE is a helper to get the different memory type size. Of course, > this should be very careful, in other words, as you said: follows a contract. ;-) > As said above the code is logically correct (at least as far as I can see), but it can be dangerous from programming perspective. > > #define DUMMY_ETH_HDR_LEN 16 > #define ICE_SW_RULE_RX_TX_ETH_HDR_SIZE \ > (offsetof(struct ice_aqc_sw_rules_elem, pdata.lkup_tx_rx.hdr) + \ > (DUMMY_ETH_HDR_LEN * \ > sizeof(((struct ice_sw_rule_lkup_rx_tx *)0)->hdr[0]))) > #define ICE_SW_RULE_RX_TX_NO_HDR_SIZE \ > (offsetof(struct ice_aqc_sw_rules_elem, pdata.lkup_tx_rx.hdr)) > #define ICE_SW_RULE_LG_ACT_SIZE(n) \ > (offsetof(struct ice_aqc_sw_rules_elem, pdata.lg_act.act) + \ > ((n) * sizeof(((struct ice_sw_rule_lg_act *)0)->act[0]))) > #define ICE_SW_RULE_VSI_LIST_SIZE(n) \ > (offsetof(struct ice_aqc_sw_rules_elem, pdata.vsi_list.vsi) + \ > ((n) * sizeof(((struct ice_sw_rule_vsi_list *)0)->vsi[0]))) > > > >> >> >