[v6,2/2] lib/pipeline: Fix gcc compilation error using ASan

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

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed

Commit Message

Peng, ZhihongX Sept. 30, 2021, 5:27 a.m. UTC
  From: Zhihong Peng <zhihongx.peng@intel.com>

After adding ASan, the gcc compilation check will be stricter.
"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>
---
 lib/pipeline/rte_swx_pipeline.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
  

Comments

David Marchand Sept. 30, 2021, 8:29 a.m. UTC | #1
On Thu, Sep 30, 2021 at 7:37 AM <zhihongx.peng@intel.com> wrote:
>
> From: Zhihong Peng <zhihongx.peng@intel.com>

Commit titles don't start with lib/.


>
> After adding ASan, the gcc compilation check will be stricter.
> "Control reaches end of non-void function" error occurs here.

Fwiw, I could not pinpoint the right version where this warning appears.
I can't see it with gcc v4.8.5 (rhel7), but I get it with gcc 11.2.1 (fc34).
Do you know which versions are affected? Just asking for info.


>
> Fixes: f38913b7fb8e (pipeline: add meter array to SWX)

Should be formatted as:
Fixes: f38913b7fb8e ("pipeline: add meter array to SWX")

Please use a git alias as suggested in the contribution guide.


> Cc: stable@dpdk.org
>
> Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> ---
>  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 *

There are two other functions (instr_table_translate, and
instr_extern_translate) which have the same pattern in this file.
Odd that the compiler is not reporting them.
  
Peng, ZhihongX Oct. 12, 2021, 2:41 a.m. UTC | #2
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, September 30, 2021 4:30 PM
> To: Peng, ZhihongX <zhihongx.peng@intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>; dev <dev@dpdk.org>; Lin, Xueqin
> <xueqin.lin@intel.com>; dpdk stable <stable@dpdk.org>
> Subject: Re: [dpdk-stable] [PATCH v6 2/2] lib/pipeline: Fix gcc compilation
> error using ASan
> 
> On Thu, Sep 30, 2021 at 7:37 AM <zhihongx.peng@intel.com> wrote:
> >
> > From: Zhihong Peng <zhihongx.peng@intel.com>
> 
> Commit titles don't start with lib/.

The v9 version will be fixed.
> 
> >
> > After adding ASan, the gcc compilation check will be stricter.
> > "Control reaches end of non-void function" error occurs here.
> 
> Fwiw, I could not pinpoint the right version where this warning appears.
> I can't see it with gcc v4.8.5 (rhel7), but I get it with gcc 11.2.1 (fc34).
> Do you know which versions are affected? Just asking for info.
> 
> 
> >
> > Fixes: f38913b7fb8e (pipeline: add meter array to SWX)
> 
> Should be formatted as:
> Fixes: f38913b7fb8e ("pipeline: add meter array to SWX")
>
> Please use a git alias as suggested in the contribution guide.

The v9 version will be fixed.
 
> 
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> > Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> > ---
> >  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 *
> 
> There are two other functions (instr_table_translate, and
> instr_extern_translate) which have the same pattern in this file.
> Odd that the compiler is not reporting them.

The lowest version we tried is gcc version 7.4.0, the highest gcc version
9.3.0, these versions will report errors. As for why some do not report
errors, it may be gcc's own problem, we just let the compilation pass.

> --
> David Marchand
  

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 *