diff mbox series

[v10,3/4] pipeline: Fix compilation error with gcc ASan

Message ID 20211015151110.1876850-3-zhihongx.peng@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series [v10,1/4] Enable ASan for memory detector on DPDK |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Peng, ZhihongX Oct. 15, 2021, 3:11 p.m. UTC 
  From: Zhihong Peng <zhihongx.peng@intel.com>

The gcc will check code more stricter when ASan enabled.
"Control reaches end of non-void function" error occurs here.

Fixes: f38913b7fb8e ("pipeline: add meter array to SWX")
Cc: stable@dpdk.org

Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
---
v7: no change
v8: no change
v9: Modify the submit log
v10:no change
---
 lib/pipeline/rte_swx_pipeline.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Cristian Dumitrescu Oct. 18, 2021, 12:21 p.m. UTC | #1 
> -----Original Message-----
> From: Peng, ZhihongX <zhihongx.peng@intel.com>
> Sent: Friday, October 15, 2021 4:11 PM
> To: david.marchand@redhat.com; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; Peng, ZhihongX
> <zhihongx.peng@intel.com>; stable@dpdk.org
> Subject: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> 
> From: Zhihong Peng <zhihongx.peng@intel.com>
> 
> The gcc will check code more stricter when ASan enabled.
> "Control reaches end of non-void function" error occurs here.
> 
> Fixes: f38913b7fb8e ("pipeline: add meter array to SWX")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> ---
> v7: no change
> v8: no change
> v9: Modify the submit log
> v10:no change
> ---
>  lib/pipeline/rte_swx_pipeline.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/pipeline/rte_swx_pipeline.c b/lib/pipeline/rte_swx_pipeline.c
> index 1cd09a4b44..0acd6c6752 100644
> --- a/lib/pipeline/rte_swx_pipeline.c
> +++ b/lib/pipeline/rte_swx_pipeline.c
> @@ -4642,7 +4642,7 @@ instr_meter_translate(struct rte_swx_pipeline *p,
>  		return 0;
>  	}
> 
> -	CHECK(0, EINVAL);
> +	return -EINVAL;
>  }
> 
>  static inline void
> @@ -5937,7 +5937,7 @@ instr_translate(struct rte_swx_pipeline *p,
>  					      instr,
>  					      data);
> 
> -	CHECK(0, EINVAL);
> +	return -EINVAL;
>  }
> 
>  static struct instruction_data *
> --
> 2.25.1

NACK.

This is a false issue, no bug is here. CHECK(0, EINVAL) translates to an unconditional return -EINVAL.

Does this tool work correctly when macros are present? Maybe the tool should parse the preprocessed C code as opposed to initial C code?

Regards,
Cristian
Peng, ZhihongX Oct. 18, 2021, 12:54 p.m. UTC | #2 
> -----Original Message-----
> From: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Sent: Monday, October 18, 2021 8:22 PM
> To: Peng, ZhihongX <zhihongx.peng@intel.com>;
> david.marchand@redhat.com; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; stable@dpdk.org
> Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> 
> 
> 
> > -----Original Message-----
> > From: Peng, ZhihongX <zhihongx.peng@intel.com>
> > Sent: Friday, October 15, 2021 4:11 PM
> > To: david.marchand@redhat.com; Burakov, Anatoly
> > <anatoly.burakov@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mcnamara, John
> > <john.mcnamara@intel.com>
> > Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; Peng, ZhihongX
> > <zhihongx.peng@intel.com>; stable@dpdk.org
> > Subject: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> >
> > From: Zhihong Peng <zhihongx.peng@intel.com>
> >
> > The gcc will check code more stricter when ASan enabled.
> > "Control reaches end of non-void function" error occurs here.
> >
> > Fixes: f38913b7fb8e ("pipeline: add meter array to SWX")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> > Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> > ---
> > v7: no change
> > v8: no change
> > v9: Modify the submit log
> > v10:no change
> > ---
> >  lib/pipeline/rte_swx_pipeline.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/pipeline/rte_swx_pipeline.c
> > b/lib/pipeline/rte_swx_pipeline.c index 1cd09a4b44..0acd6c6752 100644
> > --- a/lib/pipeline/rte_swx_pipeline.c
> > +++ b/lib/pipeline/rte_swx_pipeline.c
> > @@ -4642,7 +4642,7 @@ instr_meter_translate(struct rte_swx_pipeline *p,
> >  		return 0;
> >  	}
> >
> > -	CHECK(0, EINVAL);
> > +	return -EINVAL;
> >  }
> >
> >  static inline void
> > @@ -5937,7 +5937,7 @@ instr_translate(struct rte_swx_pipeline *p,
> >  					      instr,
> >  					      data);
> >
> > -	CHECK(0, EINVAL);
> > +	return -EINVAL;
> >  }
> >
> >  static struct instruction_data *
> > --
> > 2.25.1
> 
> NACK.
> 
> This is a false issue, no bug is here. CHECK(0, EINVAL) translates to an
> unconditional return -EINVAL.
> Does this tool work correctly when macros are present? Maybe the tool
> should parse the preprocessed C code as opposed to initial C code?

Yes, this is not a bug, it just solves the problem that cannot be passed after
adding the asan compiler option.
Only part of the macro reports errors, which may be caused by the tool itself,
but this tool is part of gcc and clang, so we still have to make the code not report errors.
> Regards,
> Cristian
Cristian Dumitrescu Oct. 19, 2021, 11:26 a.m. UTC | #3 
> -----Original Message-----
> From: Peng, ZhihongX <zhihongx.peng@intel.com>
> Sent: Monday, October 18, 2021 1:55 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> david.marchand@redhat.com; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; stable@dpdk.org
> Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> 
> > -----Original Message-----
> > From: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > Sent: Monday, October 18, 2021 8:22 PM
> > To: Peng, ZhihongX <zhihongx.peng@intel.com>;
> > david.marchand@redhat.com; Burakov, Anatoly
> > <anatoly.burakov@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> > Mcnamara, John <john.mcnamara@intel.com>
> > Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; stable@dpdk.org
> > Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> >
> >
> >
> > > -----Original Message-----
> > > From: Peng, ZhihongX <zhihongx.peng@intel.com>
> > > Sent: Friday, October 15, 2021 4:11 PM
> > > To: david.marchand@redhat.com; Burakov, Anatoly
> > > <anatoly.burakov@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mcnamara, John
> > > <john.mcnamara@intel.com>
> > > Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; Peng, ZhihongX
> > > <zhihongx.peng@intel.com>; stable@dpdk.org
> > > Subject: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> > >
> > > From: Zhihong Peng <zhihongx.peng@intel.com>
> > >
> > > The gcc will check code more stricter when ASan enabled.
> > > "Control reaches end of non-void function" error occurs here.
> > >
> > > Fixes: f38913b7fb8e ("pipeline: add meter array to SWX")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> > > Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> > > ---
> > > v7: no change
> > > v8: no change
> > > v9: Modify the submit log
> > > v10:no change
> > > ---
> > >  lib/pipeline/rte_swx_pipeline.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/pipeline/rte_swx_pipeline.c
> > > b/lib/pipeline/rte_swx_pipeline.c index 1cd09a4b44..0acd6c6752 100644
> > > --- a/lib/pipeline/rte_swx_pipeline.c
> > > +++ b/lib/pipeline/rte_swx_pipeline.c
> > > @@ -4642,7 +4642,7 @@ instr_meter_translate(struct rte_swx_pipeline
> *p,
> > >  		return 0;
> > >  	}
> > >
> > > -	CHECK(0, EINVAL);
> > > +	return -EINVAL;
> > >  }
> > >
> > >  static inline void
> > > @@ -5937,7 +5937,7 @@ instr_translate(struct rte_swx_pipeline *p,
> > >  					      instr,
> > >  					      data);
> > >
> > > -	CHECK(0, EINVAL);
> > > +	return -EINVAL;
> > >  }
> > >
> > >  static struct instruction_data *
> > > --
> > > 2.25.1
> >
> > NACK.
> >
> > This is a false issue, no bug is here. CHECK(0, EINVAL) translates to an
> > unconditional return -EINVAL.
> > Does this tool work correctly when macros are present? Maybe the tool
> > should parse the preprocessed C code as opposed to initial C code?
> 
> Yes, this is not a bug, it just solves the problem that cannot be passed after
> adding the asan compiler option.
> Only part of the macro reports errors, which may be caused by the tool itself,
> but this tool is part of gcc and clang, so we still have to make the code not
> report errors.
> > Regards,
> > Cristian

Hi Zhihong,

If this is not a bug in the pipeline library, why then does your patch has fix in the tile, has the Fixes label and CC-es stable@dpdk.org? Please remove and rephrase accordingly.

I agree this is not a bug, and based on your statements I understand this is a sort of issue or limitation with the tool . I would prefer we fix the tool rather than fixing correct code in order to please the tool. This is likely not going to be an isolated case, but a recurring issue.

Hence, I am reluctantly OK to ack this patch in order to allow the tool in (after the "fix" claim is removed), hopefully the tool will prove its benefit to DPDK.

Regards,
Cristian
Peng, ZhihongX Oct. 19, 2021, 12:11 p.m. UTC | #4 
> -----Original Message-----
> From: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Sent: Tuesday, October 19, 2021 7:26 PM
> To: Peng, ZhihongX <zhihongx.peng@intel.com>;
> david.marchand@redhat.com; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Mcnamara, John <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; stable@dpdk.org
> Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc ASan
> 
> 
> 
> > -----Original Message-----
> > From: Peng, ZhihongX <zhihongx.peng@intel.com>
> > Sent: Monday, October 18, 2021 1:55 PM
> > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>;
> > david.marchand@redhat.com; Burakov, Anatoly
> > <anatoly.burakov@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> Mcnamara,
> > John <john.mcnamara@intel.com>
> > Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; stable@dpdk.org
> > Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with gcc
> > ASan
> >
> > > -----Original Message-----
> > > From: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> > > Sent: Monday, October 18, 2021 8:22 PM
> > > To: Peng, ZhihongX <zhihongx.peng@intel.com>;
> > > david.marchand@redhat.com; Burakov, Anatoly
> > > <anatoly.burakov@intel.com>; Ananyev, Konstantin
> > > <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> > > Mcnamara, John <john.mcnamara@intel.com>
> > > Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>;
> > > stable@dpdk.org
> > > Subject: RE: [PATCH v10 3/4] pipeline: Fix compilation error with
> > > gcc ASan
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Peng, ZhihongX <zhihongx.peng@intel.com>
> > > > Sent: Friday, October 15, 2021 4:11 PM
> > > > To: david.marchand@redhat.com; Burakov, Anatoly
> > > > <anatoly.burakov@intel.com>; Ananyev, Konstantin
> > > > <konstantin.ananyev@intel.com>; stephen@networkplumber.org;
> > > > Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Mcnamara,
> > > > John <john.mcnamara@intel.com>
> > > > Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; Peng,
> > > > ZhihongX <zhihongx.peng@intel.com>; stable@dpdk.org
> > > > Subject: [PATCH v10 3/4] pipeline: Fix compilation error with gcc
> > > > ASan
> > > >
> > > > From: Zhihong Peng <zhihongx.peng@intel.com>
> > > >
> > > > The gcc will check code more stricter when ASan enabled.
> > > > "Control reaches end of non-void function" error occurs here.
> > > >
> > > > Fixes: f38913b7fb8e ("pipeline: add meter array to SWX")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> > > > Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> > > > ---
> > > > v7: no change
> > > > v8: no change
> > > > v9: Modify the submit log
> > > > v10:no change
> > > > ---
> > > >  lib/pipeline/rte_swx_pipeline.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/lib/pipeline/rte_swx_pipeline.c
> > > > b/lib/pipeline/rte_swx_pipeline.c index 1cd09a4b44..0acd6c6752
> > > > 100644
> > > > --- a/lib/pipeline/rte_swx_pipeline.c
> > > > +++ b/lib/pipeline/rte_swx_pipeline.c
> > > > @@ -4642,7 +4642,7 @@ instr_meter_translate(struct
> > > > rte_swx_pipeline
> > *p,
> > > >  		return 0;
> > > >  	}
> > > >
> > > > -	CHECK(0, EINVAL);
> > > > +	return -EINVAL;
> > > >  }
> > > >
> > > >  static inline void
> > > > @@ -5937,7 +5937,7 @@ instr_translate(struct rte_swx_pipeline *p,
> > > >  					      instr,
> > > >  					      data);
> > > >
> > > > -	CHECK(0, EINVAL);
> > > > +	return -EINVAL;
> > > >  }
> > > >
> > > >  static struct instruction_data *
> > > > --
> > > > 2.25.1
> > >
> > > NACK.
> > >
> > > This is a false issue, no bug is here. CHECK(0, EINVAL) translates
> > > to an unconditional return -EINVAL.
> > > Does this tool work correctly when macros are present? Maybe the
> > > tool should parse the preprocessed C code as opposed to initial C code?
> >
> > Yes, this is not a bug, it just solves the problem that cannot be
> > passed after adding the asan compiler option.
> > Only part of the macro reports errors, which may be caused by the tool
> > itself, but this tool is part of gcc and clang, so we still have to
> > make the code not report errors.
> > > Regards,
> > > Cristian
> 
> Hi Zhihong,
> 
> If this is not a bug in the pipeline library, why then does your patch has fix in
> the tile, has the Fixes label and CC-es stable@dpdk.org? Please remove and
> rephrase accordingly.
> 
> I agree this is not a bug, and based on your statements I understand this is a
> sort of issue or limitation with the tool . I would prefer we fix the tool rather
> than fixing correct code in order to please the tool. This is likely not going to
> be an isolated case, but a recurring issue.
> 
> Hence, I am reluctantly OK to ack this patch in order to allow the tool in (after
> the "fix" claim is removed), hopefully the tool will prove its benefit to DPDK.

ok, I will delete it.
> Regards,
> Cristian
diff mbox series

Patch

diff --git a/lib/pipeline/rte_swx_pipeline.c b/lib/pipeline/rte_swx_pipeline.c
index 1cd09a4b44..0acd6c6752 100644
--- a/lib/pipeline/rte_swx_pipeline.c
+++ b/lib/pipeline/rte_swx_pipeline.c
@@ -4642,7 +4642,7 @@  instr_meter_translate(struct rte_swx_pipeline *p,
 		return 0;
 	}
 
-	CHECK(0, EINVAL);
+	return -EINVAL;
 }
 
 static inline void
@@ -5937,7 +5937,7 @@  instr_translate(struct rte_swx_pipeline *p,
 					      instr,
 					      data);
 
-	CHECK(0, EINVAL);
+	return -EINVAL;
 }
 
 static struct instruction_data *