Re: [dpdk-dev] [PATCH v3 00/40] Fix build on gcc8 and various bugs

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Thu, 10 May 2018 15:11:11 +0800
> From: Andy Green <andy@warmcat.com>
> To: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> CC: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 00/40] Fix build on gcc8 and various bugs
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
>  Thunderbird/52.7.0
> 
> 
> 
> On 05/10/2018 02:12 PM, Jerin Jacob wrote:
> > -----Original Message-----
> > > Date: Thu, 10 May 2018 10:46:18 +0800
> > > From: Andy Green <andy@warmcat.com>
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH v3 00/40] Fix build on gcc8 and various bugs
> > > User-Agent: StGit/unknown-version
> > > 
> > > The following series gets current master able to build
> > > itself, and allow lagopus to build against it, on Fedora 28 +
> > > x86_64 using gcc 8.0.1.
> > > 
> > > The first 17 patches have already been through two spins and
> > > this time are corrected for all the comment (thanks to
> > > everybody who commented) since v2, and have tested-by /
> > > acked-bys applied.  The first workaround patch for the hash
> > > function cast problem is dropped since something has already
> > > been applied in master since yesterday to address it.
> > > 
> > > The additional 23 patches are fixes for problems found
> > > actually trying to build lagopus using current master.
> > > These are almost entirely related to signed / unsigned
> > > or truncation without explicit casts inside dpdk
> > > headers.
> > 
> > 
> > Tested this series on gcc version 8.1.0
> > 
> > Found following error:
> 
> On gcc 8.0.1 on Fedora 28, where this doesn't appear using x86_64 defconfig.
> This is with the basis the current master HEAD
> 8ea41438832a360aed2b7ba49fb75e310a2ff1dc

I use the same change set. I think, In GCC 8.1, they have added 
strict function type casting check.

I see a lot failure in examples with GCC 8.1, Are you tested the examples directory?

> 
> I assume 8.1 just got better at spotting the problem.
> 
> > /export/dpdk.org/test/test/test_table_pipeline.c: In function
> > ‘test_table_pipeline’:
> > /export/dpdk.org/test/test/test_table_pipeline.c:521:3: error: cast
> > between incompatible function types from ‘int (* (*)(struct rte_pipeline
> > *, struct rte_mbuf **, uint64_t,  struct rte_pipeline_table_entry **,
> > void *))(struct rte_pipeline *, struct rte_mbuf **, uint64_t,  struct
> > rte_pipeline_table_entry *, void *)’ {aka ‘int (* (*)(struct
> > rte_pipeline *, struct rte_mbuf **, long unsigned int,  struct
> > rte_pipeline_table_entry **, void *))(struct rte_pipeline *, struct
> > rte_mbuf **, long unsigned int,  struct rte_pipeline_table_entry *, void
> > *)’} to ‘int (*)(struct rte_pipeline *, struct rte_mbuf **, uint64_t,
> > struct rte_pipeline_table_entry *, void *)’ {aka ‘int (*)(struct
> > rte_pipeline *, struct rte_mbuf **, long unsigned int,  struct
> > rte_pipeline_table_entry *, void *)’} [-Werror=cast-function-type]
> >     (rte_pipeline_table_action_handler_miss) table_action_stub_miss;
> >     ^
> > /export/dpdk.org/test/test/test_table_pipeline.c:557:3: error: cast
> > between incompatible function types from ‘int (* (*)(struct rte_pipeline
> > *, struct rte_mbuf **, uint64_t,  struct rte_pipeline_table_entry **,
> > void *))(struct rte_pipeline *, struct rte_mbuf **, uint64_t,  struct
> > rte_pipeline_table_entry *, void *)’ {aka ‘int (* (*)(struct
> > rte_pipeline *, struct rte_mbuf **, long unsigned int,  struct
> > rte_pipeline_table_entry **, void *))(struct rte_pipeline *, struct
> > rte_mbuf **, long unsigned int,  struct rte_pipeline_table_entry *, void
> > *)’} to ‘int (*)(struct rte_pipeline *, struct rte_mbuf **, uint64_t,
> > struct rte_pipeline_table_entry *, void *)’ {aka ‘int (*)(struct
> > rte_pipeline *, struct rte_mbuf **, long unsigned int,  struct
> > rte_pipeline_table_entry *, void *)’} [-Werror=cast-function-type]
> >     (rte_pipeline_table_action_handler_miss)table_action_stub_miss;
> 
> Hum...
> 
> That seems to be declared:
> 
> rte_pipeline_table_action_handler_miss action_handler_miss = NULL;
> 
> which is
> 
> typedef int (*rte_pipeline_table_action_handler_miss)(
>         struct rte_pipeline *p,
>         struct rte_mbuf **pkts,
>         uint64_t pkts_mask,
>         struct rte_pipeline_table_entry *entry,    <<<<=====
>         void *arg);
> 
> We're doing
> 
> action_handler_miss =
>    (rte_pipeline_table_action_handler_miss)table_action_stub_miss;
> 
> The prototype for that is
> 
> rte_pipeline_table_action_handler_miss
> table_action_stub_miss(struct rte_pipeline *p,
> 			struct rte_mbuf **pkts,
> 			uint64_t pkts_mask,
> 			struct rte_pipeline_table_entry **entry,  <<====
> 			void *arg);
> 
> It's true, the fourth arg has a different level of indirection... it seems
> another real pre-existing problem.
> 
> However the stub doesn't use the fourth arg "entry"
> 
> rte_pipeline_table_action_handler_miss
> table_action_stub_miss(struct rte_pipeline *p,
>         __attribute__((unused)) struct rte_mbuf **pkts,
>         uint64_t pkts_mask,
>         __attribute__((unused)) struct rte_pipeline_table_entry **entry,
>         __attribute__((unused)) void *arg)
> {
>         printf("STUB Table Action Miss - setting mask to 0x%"PRIx64"\n",
>                 override_miss_mask);
>         pkts_mask = (~override_miss_mask) & 0x3;
>         rte_pipeline_ah_packet_drop(p, pkts_mask);
>         return 0;
> }
> 
> Therefore, since I can't get the error here, can you check if this solves
> it?
> 
> diff --git a/test/test/test_table_pipeline.c
> b/test/test/test_table_pipeline.c
> index 055a1a4e7..d007d55ce 100644
> --- a/test/test/test_table_pipeline.c
> +++ b/test/test/test_table_pipeline.c
> @@ -71,7 +71,7 @@ table_action_stub_hit(struct rte_pipeline *p, struct
> rte_mbuf **pkts,
> 
>  rte_pipeline_table_action_handler_miss
>  table_action_stub_miss(struct rte_pipeline *p, struct rte_mbuf **pkts,
> -       uint64_t pkts_mask, struct rte_pipeline_table_entry **entry, void
> *arg);
> +       uint64_t pkts_mask, struct rte_pipeline_table_entry *entry, void
> *arg);
> 
>  rte_pipeline_table_action_handler_hit
>  table_action_0x00(__attribute__((unused)) struct rte_pipeline *p,
> @@ -105,7 +105,7 @@ rte_pipeline_table_action_handler_miss
>  table_action_stub_miss(struct rte_pipeline *p,
>         __attribute__((unused)) struct rte_mbuf **pkts,
>         uint64_t pkts_mask,
> -       __attribute__((unused)) struct rte_pipeline_table_entry **entry,
> +       __attribute__((unused)) struct rte_pipeline_table_entry *entry,
>         __attribute__((unused)) void *arg)
>  {
>         printf("STUB Table Action Miss - setting mask to 0x%"PRIx64"\n",
> 
> 
> At least I confirmed it still builds without error on 8.0.1 with my series
> and that patch.

OK.

The above patch does not help, GCC 8.1 simply don't like any from
function typecasting. see ^^^^^ in the quote below.


/export/dpdk.org/test/test/test_table_pipeline.c: In function
‘test_table_pipeline’:
/export/dpdk.org/test/test/test_table_pipeline.c:521:3: error: cast
between incompatible function types from ‘int (* (*)(struct rte_pipeline
*, struct rte_mbuf **, uint64_t,  struct rte_pipeline_table_entry *,
void *))(struct rte_pipeline *, struct rte_mbuf **, uint64_t,  struct
rte_pipeline_table_entry *, void *)’ {aka ‘int (* (*)(struct
                                             ^^^^^^^^

rte_pipeline *, struct rte_mbuf **, long unsigned int,  struct
rte_pipeline_table_entry *, void *))(struct rte_pipeline *, struct
rte_mbuf **, long unsigned int,  struct rte_pipeline_table_entry *, void
*)’} to ‘int (*)(struct rte_pipeline *, struct rte_mbuf **, uint64_t,
         ^^^^^^^^^


struct rte_pipeline_table_entry *, void *)’ {aka ‘int (*)(struct
rte_pipeline *, struct rte_mbuf **, long unsigned int,  struct
rte_pipeline_table_entry *, void *)’} [-Werror=cast-function-type]
   (rte_pipeline_table_action_handler_miss) table_action_stub_miss;
   ^
/export/dpdk.org/test/test/test_table_pipeline.c:557:3: error: cast
between incompatible function types from ‘int (* (*)(struct rte_pipeline
*, struct rte_mbuf **, uint64_t,  struct rte_pipeline_table_entry *,
void *))(struct rte_pipeline *, struct rte_mbuf **, uint64_t,  struct
rte_pipeline_table_entry *, void *)’ {aka ‘int (* (*)(struct
rte_pipeline *, struct rte_mbuf **, long unsigned int,  struct
rte_pipeline_table_entry *, void *))(struct rte_pipeline *, struct
rte_mbuf **, long unsigned int,  struct rte_pipeline_table_entry *, void
*)’} to ‘int (*)(struct rte_pipeline *, struct rte_mbuf **, uint64_t,
struct rte_pipeline_table_entry *, void *)’ {aka ‘int (*)(struct
rte_pipeline *, struct rte_mbuf **, long unsigned int,  struct
rte_pipeline_table_entry *, void *)’} [-Werror=cast-function-type]
   (rte_pipeline_table_action_handler_miss)table_action_stub_miss;
   ^
cc1: all warnings being treated as errors