diff mbox series

net/cxgbe: fix dangling pointer for gcc12

Message ID 20220117143630.3044764-1-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/cxgbe: fix dangling pointer for gcc12 |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Ferruh Yigit Jan. 17, 2022, 2:36 p.m. UTC 
  Observed with: gcc (GCC) 12.0.0 20220116 (experimental)

In file included from ../lib/eal/linux/include/rte_os.h:14,
                 from ../lib/eal/include/rte_common.h:28,
                 from ../lib/eal/include/rte_log.h:25,
                 from ../lib/ethdev/rte_ethdev.h:164,
                 from ../lib/ethdev/ethdev_driver.h:18,
                 from ../drivers/net/cxgbe/base/t4vf_hw.c:6:
In function ‘t4_os_atomic_add_tail’,
    inlined from ‘t4vf_wr_mbox_core’ at ../drivers/net/cxgbe/base/t4vf_hw.c:115:2:
../drivers/net/cxgbe/base/adapter.h:742:9: error: storing the address of local variable ‘entry’ in ‘((struct mbox_list *)adapter)[96].tqh_last’ [-Werror=dangling-pointer=]
  742 |         TAILQ_INSERT_TAIL(head, entry, next);
      |         ^~~~~~~~~~~~~~~~~
../drivers/net/cxgbe/base/t4vf_hw.c: In function ‘t4vf_wr_mbox_core’:
../drivers/net/cxgbe/base/t4vf_hw.c:86:27: note: ‘entry’ declared here
   86 |         struct mbox_entry entry;
      |                           ^~~~~
../drivers/net/cxgbe/base/t4vf_hw.c:86:27: note: ‘adapter’ declared here

It is a valid compiler warning, make local variable a global one.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

---

Fix is done in a quickest way, mainly to report the issue,
please feel free to suggest another solution for the build error.
---
 drivers/net/cxgbe/base/t4vf_hw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rahul Lakkireddy Jan. 19, 2022, 10:01 p.m. UTC | #1 
Hi Ferruh,

On Monday, January 01/17/22, 2022 at 14:36:30 +0000, Ferruh Yigit wrote:
> Observed with: gcc (GCC) 12.0.0 20220116 (experimental)
> 
> In file included from ../lib/eal/linux/include/rte_os.h:14,
>                  from ../lib/eal/include/rte_common.h:28,
>                  from ../lib/eal/include/rte_log.h:25,
>                  from ../lib/ethdev/rte_ethdev.h:164,
>                  from ../lib/ethdev/ethdev_driver.h:18,
>                  from ../drivers/net/cxgbe/base/t4vf_hw.c:6:
> In function ‘t4_os_atomic_add_tail’,
>     inlined from ‘t4vf_wr_mbox_core’ at ../drivers/net/cxgbe/base/t4vf_hw.c:115:2:
> ../drivers/net/cxgbe/base/adapter.h:742:9: error: storing the address of local variable ‘entry’ in ‘((struct mbox_list *)adapter)[96].tqh_last’ [-Werror=dangling-pointer=]
>   742 |         TAILQ_INSERT_TAIL(head, entry, next);
>       |         ^~~~~~~~~~~~~~~~~
> ../drivers/net/cxgbe/base/t4vf_hw.c: In function ‘t4vf_wr_mbox_core’:
> ../drivers/net/cxgbe/base/t4vf_hw.c:86:27: note: ‘entry’ declared here
>    86 |         struct mbox_entry entry;
>       |                           ^~~~~
> ../drivers/net/cxgbe/base/t4vf_hw.c:86:27: note: ‘adapter’ declared here
> 
> It is a valid compiler warning, make local variable a global one.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> ---
> 
> Fix is done in a quickest way, mainly to report the issue,
> please feel free to suggest another solution for the build error.
> ---

Thanks for reporting this issue.

The intention of this code is to use the address of the local variable
stored on stack to serialize access from multiple threads. This
address is used only within the scope of this function and is not
accessed from outside. I'm also stumped on why this warning is not
showing up inside t4_wr_mbox_meat_timeout() in t4_hw.c, which also
has similar code.

Nevertheless, I've sent a patch at [1] to dynamically allocate/free
the memory instead and fix the warning.

[1] https://mails.dpdk.org/archives/dev/2022-January/232816.html

Thanks,
Rahul

>  drivers/net/cxgbe/base/t4vf_hw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/cxgbe/base/t4vf_hw.c b/drivers/net/cxgbe/base/t4vf_hw.c
> index 561d759dbc0d..b42c4e78eba9 100644
> --- a/drivers/net/cxgbe/base/t4vf_hw.c
> +++ b/drivers/net/cxgbe/base/t4vf_hw.c
> @@ -83,7 +83,7 @@ int t4vf_wr_mbox_core(struct adapter *adapter,
>  
>  	u32 mbox_ctl = T4VF_CIM_BASE_ADDR + A_CIM_VF_EXT_MAILBOX_CTRL;
>  	__be64 cmd_rpl[MBOX_LEN / 8];
> -	struct mbox_entry entry;
> +	static struct mbox_entry entry;
>  	unsigned int delay_idx;
>  	u32 v, mbox_data;
>  	const __be64 *p;
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/drivers/net/cxgbe/base/t4vf_hw.c b/drivers/net/cxgbe/base/t4vf_hw.c
index 561d759dbc0d..b42c4e78eba9 100644
--- a/drivers/net/cxgbe/base/t4vf_hw.c
+++ b/drivers/net/cxgbe/base/t4vf_hw.c
@@ -83,7 +83,7 @@  int t4vf_wr_mbox_core(struct adapter *adapter,
 
 	u32 mbox_ctl = T4VF_CIM_BASE_ADDR + A_CIM_VF_EXT_MAILBOX_CTRL;
 	__be64 cmd_rpl[MBOX_LEN / 8];
-	struct mbox_entry entry;
+	static struct mbox_entry entry;
 	unsigned int delay_idx;
 	u32 v, mbox_data;
 	const __be64 *p;