Re: Nasty typecasting / alignment issues in libdvbpsi

On Mon, Oct 15, 2012 at 12:30 PM, Martin Jackson
<[email protected]> wrote:
> Dear livdvbpsi developers,
>
> When compiling for host=mipsel-linux-uclibc (mipsel-linux-uclibc-gcc
> (GCC) 4.2.0), I ran into some rather horrible alignment issues related
> to typecasting of the various table structures.

Thanks for reporting this issue, I am looking into it.

> I got the following warnings (which become errors due to -Werror)
> using the latest git as of today
> (398ff6bac45aeda35320ff1b7535e726290af821)
>
> libtool: compile:  mipsel-linux-uclibc-gcc -DHAVE_CONFIG_H -I. -I.. -g
> -O2 -Wall -Werror --std=gnu99 -D_GNU_SOURCE -Wpointer-arith
> -Wcast-align -Wcast-qual -Wstrict-prototypes -Wshadow
> -Waggregate-return -Wmissing-prototypes -Wnested-externs
> -Wsign-compare -DDVBPSI_DIST -MT tot.lo -MD -MP -MF .deps/tot.Tpo -c
> tables/tot.c  -fPIC -DPIC -o .libs/tot.o
> cc1: warnings being treated as errors
> tables/tot.c: In function 'dvbpsi_tot_attach':
> tables/tot.c:78: warning: cast increases required alignment of target type
> tables/tot.c: In function 'dvbpsi_tot_detach':
> tables/tot.c:131: warning: cast increases required alignment of target type
> tables/tot.c: In function 'dvbpsi_tot_sections_gather':
> tables/tot.c:326: warning: cast increases required alignment of target type
>
>
> For example, in src/tables/tot.c:78
>     dvbpsi_tot_decoder_t *p_tot_decoder;
>     p_tot_decoder = (dvbpsi_tot_decoder_t *) dvbpsi_decoder_new(NULL,
>                                                 0, true,
> sizeof(dvbpsi_tot_decoder_t));
>
> So apparently, p_tot_decoder potentially has a different alignment to
> the result of dvbpsi_decoder_new. I imagine that this could lead to a
> crash in libdvbpsi if the warning is ignored. Also, a separate issue:

The DVBPSI_DECODER_COMMON members of dvbpsi_decoder_t are not alligned, which
is probably the cause of the compiler complaining.

> the libdvbpsi library appears not to compile without -Werror either
> for my mips toolchain or the regular centOS x86_64 toolchain, though I
> didn't have time to investigate.

-Werror is forced in configure.ac

Kind regards,

Jean-Paul Saman.

> Best regards,
>
> Martin Jackson
>
>
> I got it compiling with the following hacks: (no suggestion that this
> patch should be merged, it's nonsense but it got the job done for now)
>
> diff --git a/src/demux.h b/src/demux.h
> index 73b4ae0..c348153 100644
> --- a/src/demux.h
> +++ b/src/demux.h
> @@ -126,7 +126,7 @@ struct dvbpsi_demux_s
>      dvbpsi_demux_new_cb_t     pf_new_callback;    /*!< New subtable callback */
>      void *                    p_new_cb_data;      /*!< Data provided to the
>                                                       previous callback */
> -};
> +} __attribute__((__aligned__(128)));
>
>  /*****************************************************************************
>   * dvbpsi_AttachDemux
> @@ -211,7 +211,7 @@ typedef void (* dvbpsi_callback_gather_t)(dvbpsi_t
> *p_dvbpsi,  /*!< pointer to d
>  struct dvbpsi_decoder_s
>  {
>      DVBPSI_DECODER_COMMON
> -};
> +} __attribute__((__aligned__(128)));
>
>  /*****************************************************************************
>   * dvbpsi_decoder_new
> diff --git a/src/tables/atsc_eit.h b/src/tables/atsc_eit.h
> index 82f72d6..94bb5da 100644
> --- a/src/tables/atsc_eit.h
> +++ b/src/tables/atsc_eit.h
> @@ -83,7 +83,7 @@ typedef struct dvbpsi_atsc_eit_s
>      dvbpsi_atsc_eit_event_t   *p_first_event;   /*!< First event
> information structure. */
>
>      dvbpsi_descriptor_t     *p_first_descriptor; /*!< First
> descriptor structure. */
> -} dvbpsi_atsc_eit_t;
> +} __attribute__((__aligned__(128))) dvbpsi_atsc_eit_t;
>
>  /*****************************************************************************
>   * dvbpsi_eit_callback
> diff --git a/src/tables/atsc_ett.h b/src/tables/atsc_ett.h
> index 6baaf61..4f6eb76 100644
> --- a/src/tables/atsc_ett.h
> +++ b/src/tables/atsc_ett.h
> @@ -70,7 +70,7 @@ typedef struct dvbpsi_atsc_ett_s
>                                                   multiple string structure */
>
>      dvbpsi_descriptor_t    *p_first_descriptor; /*!< First descriptor. */
> -} dvbpsi_atsc_ett_t;
> +} __attribute__((__aligned__(128)))  dvbpsi_atsc_ett_t;
>
>  /*****************************************************************************
>   * dvbpsi_atsc_ett_callback
> diff --git a/src/tables/atsc_mgt.h b/src/tables/atsc_mgt.h
> index d55fdad..004c44a 100644
> --- a/src/tables/atsc_mgt.h
> +++ b/src/tables/atsc_mgt.h
> @@ -83,7 +83,7 @@ typedef struct dvbpsi_atsc_mgt_s
>      dvbpsi_atsc_mgt_table_t   *p_first_table;   /*!< First table
> information structure. */
>
>      dvbpsi_descriptor_t    *p_first_descriptor; /*!< First descriptor. */
> -} dvbpsi_atsc_mgt_t;
> +} __attribute__((__aligned__(128))) dvbpsi_atsc_mgt_t;
>
>  /*****************************************************************************
>   * dvbpsi_mgt_callback
> diff --git a/src/tables/atsc_stt.h b/src/tables/atsc_stt.h
> index eca622c..de620c4 100644
> --- a/src/tables/atsc_stt.h
> +++ b/src/tables/atsc_stt.h
> @@ -55,7 +55,7 @@ typedef struct dvbpsi_atsc_stt_s
>      uint16_t                i_daylight_savings; /*!< Daylight savings
> control bytes. */
>
>      dvbpsi_descriptor_t    *p_first_descriptor; /*!< First descriptor. */
> -} dvbpsi_atsc_stt_t;
> +} __attribute__((__aligned__(128)))  dvbpsi_atsc_stt_t;
>
>  /*****************************************************************************
>   * dvbpsi_atsc_stt_callback
> diff --git a/src/tables/atsc_vct.h b/src/tables/atsc_vct.h
> index cfe7e9e..7c395c8 100644
> --- a/src/tables/atsc_vct.h
> +++ b/src/tables/atsc_vct.h
> @@ -97,7 +97,7 @@ typedef struct dvbpsi_atsc_vct_s
>      dvbpsi_descriptor_t         *p_first_descriptor; /*!< First descriptor. */
>      dvbpsi_atsc_vct_channel_t   *p_first_channel;    /*!< First
> channel information structure. */
>
> -} dvbpsi_atsc_vct_t;
> +} __attribute__((__aligned__(128))) dvbpsi_atsc_vct_t;
>
>  /*****************************************************************************
>   * dvbpsi_vct_callback
> diff --git a/src/tables/bat_private.h b/src/tables/bat_private.h
> index ac6f726..5414902 100644
> --- a/src/tables/bat_private.h
> +++ b/src/tables/bat_private.h
> @@ -44,7 +44,7 @@ typedef struct dvbpsi_bat_decoder_s
>      dvbpsi_bat_t                  current_bat;
>      dvbpsi_bat_t *                p_building_bat;
>
> -} dvbpsi_bat_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_bat_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_bat_sections_gather
> diff --git a/src/tables/cat_private.h b/src/tables/cat_private.h
> index 93b862c..3269e49 100644
> --- a/src/tables/cat_private.h
> +++ b/src/tables/cat_private.h
> @@ -45,7 +45,7 @@ typedef struct dvbpsi_cat_decoder_s
>      dvbpsi_cat_t                  current_cat;
>      dvbpsi_cat_t *                p_building_cat;
>
> -} dvbpsi_cat_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_cat_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_cat_sections_gather
> diff --git a/src/tables/eit_private.h b/src/tables/eit_private.h
> index 1bf9108..8af52ea 100644
> --- a/src/tables/eit_private.h
> +++ b/src/tables/eit_private.h
> @@ -45,7 +45,7 @@ typedef struct dvbpsi_eit_decoder_s
>
>      uint8_t                       i_first_received_section_number;
>
> -} dvbpsi_eit_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_eit_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_eit_sections_gather
> diff --git a/src/tables/nit_private.h b/src/tables/nit_private.h
> index 8eadce7..d1ccc8c 100644
> --- a/src/tables/nit_private.h
> +++ b/src/tables/nit_private.h
> @@ -47,7 +47,7 @@ typedef struct dvbpsi_nit_decoder_s
>      dvbpsi_nit_t                  current_nit;
>      dvbpsi_nit_t *                p_building_nit;
>
> -} dvbpsi_nit_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_nit_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_nit_sections_gather
> diff --git a/src/tables/pat_private.h b/src/tables/pat_private.h
> index 8801b0d..188e38b 100644
> --- a/src/tables/pat_private.h
> +++ b/src/tables/pat_private.h
> @@ -43,7 +43,7 @@ typedef struct dvbpsi_pat_decoder_s
>      dvbpsi_pat_t                  current_pat;
>      dvbpsi_pat_t *                p_building_pat;
>
> -} dvbpsi_pat_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_pat_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_pat_sections_gather
> diff --git a/src/tables/pmt_private.h b/src/tables/pmt_private.h
> index ed75a8b..2b936bd 100644
> --- a/src/tables/pmt_private.h
> +++ b/src/tables/pmt_private.h
> @@ -45,7 +45,7 @@ typedef struct dvbpsi_pmt_decoder_s
>      dvbpsi_pmt_t                  current_pmt;
>      dvbpsi_pmt_t *                p_building_pmt;
>
> -} dvbpsi_pmt_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_pmt_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_pmt_sections_gather
> diff --git a/src/tables/rst_private.h b/src/tables/rst_private.h
> index 2f1664e..c391f76 100644
> --- a/src/tables/rst_private.h
> +++ b/src/tables/rst_private.h
> @@ -42,7 +42,7 @@ typedef struct dvbpsi_rst_decoder_s
>      dvbpsi_rst_t                  current_rst;
>      dvbpsi_rst_t *                p_building_rst;
>
> -} dvbpsi_rst_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_rst_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_GatherRSTSections
> diff --git a/src/tables/sdt_private.h b/src/tables/sdt_private.h
> index 1a84dba..b1219fe 100644
> --- a/src/tables/sdt_private.h
> +++ b/src/tables/sdt_private.h
> @@ -43,7 +43,7 @@ typedef struct dvbpsi_sdt_decoder_s
>      dvbpsi_sdt_t                  current_sdt;
>      dvbpsi_sdt_t *                p_building_sdt;
>
> -} dvbpsi_sdt_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_sdt_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_sdt_sections_gather
> diff --git a/src/tables/sis_private.h b/src/tables/sis_private.h
> index d539b00..fc69451 100644
> --- a/src/tables/sis_private.h
> +++ b/src/tables/sis_private.h
> @@ -43,7 +43,7 @@ typedef struct dvbpsi_sis_decoder_s
>      dvbpsi_sis_t                  current_sis;
>      dvbpsi_sis_t                  *p_building_sis;
>
> -} dvbpsi_sis_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_sis_decoder_t;
>
>  /*****************************************************************************
>   * dvbpsi_sis_sections_gather
> diff --git a/src/tables/tot_private.h b/src/tables/tot_private.h
> index 86ac502..9a43a8f 100644
> --- a/src/tables/tot_private.h
> +++ b/src/tables/tot_private.h
> @@ -46,7 +46,7 @@ typedef struct dvbpsi_tot_decoder_s
>      dvbpsi_tot_t                  current_tot;
>      dvbpsi_tot_t                  *p_building_tot;
>
> -} dvbpsi_tot_decoder_t;
> +} __attribute__((__aligned__(128))) dvbpsi_tot_decoder_t;
>
>
>  /*****************************************************************************
> _______________________________________________
> libdvbpsi-devel mailing list
> [email protected]
> http://mailman.videolan.org/listinfo/libdvbpsi-devel
_______________________________________________
libdvbpsi-devel mailing list
[email protected]
http://mailman.videolan.org/listinfo/libdvbpsi-devel