Re: [PATCH] drm/radeon/atombios: fix variable sized arrays for newer gccs (v2)

From: "Michel Dänzer" <michel@daenzer.net>
To: Alex Deucher <alexdeucher@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/radeon/atombios: fix variable sized arrays for newer gccs (v2)
Date: Wed, 21 Aug 2013 09:50:45 +0200	[thread overview]
Message-ID: <1377071445.27829.65.camel@thor.local> (raw)
In-Reply-To: <1377019008-10398-1-git-send-email-alexander.deucher@amd.com>

On Die, 2013-08-20 at 13:16 -0400, Alex Deucher wrote:
> Newer versions of gcc seem to wander off into no-man's land
> when using variably sized arrays.  Atombios tends to do things
> like:
> 
> struct object {
>     u8 version;
>     u8 num_elements;
>     u32 elements[1]; /* num_elements entries */
> };
> 
> We then do things like the following in the driver code:
> 
> for (i = 0; i < atom_object->num_elements; i++) {
>     driver_object[i] = atom_object->elements[i];
> }
> 
> With previous versions of gcc this used to work fine, but with
> 4.7 and 4.8, it seems to generate code that wanders off into the
> weeds.
> 
> According to:
> http://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> The traditional way of handling variably sized arrays (ISO C90)
> is to use an array length of 1.  Gcc allows you to use an array
> length of 0 and newer versions of gcc only seem to do the
> right thing when 0 is used.

[...]

> diff --git a/drivers/gpu/drm/radeon/atombios.h b/drivers/gpu/drm/radeon/atombios.h
> index 16b120c..3f1f011 100644
> --- a/drivers/gpu/drm/radeon/atombios.h
> +++ b/drivers/gpu/drm/radeon/atombios.h
[...]
> @@ -4028,15 +4028,15 @@ typedef struct _ATOM_OBJECT_TABLE                         //Above 4 object table
[...]
>  typedef struct _ATOM_SRC_DST_TABLE_FOR_ONE_OBJECT         //usSrcDstTableOffset pointing to this structure
>  {
>    UCHAR               ucNumberOfSrc;
> -  USHORT              usSrcObjectID[1];
> +  USHORT              usSrcObjectID[0];
>    UCHAR               ucNumberOfDst;
> -  USHORT              usDstObjectID[1];
> +  USHORT              usDstObjectID[0];
>  }ATOM_SRC_DST_TABLE_FOR_ONE_OBJECT;

[...]

> @@ -6194,8 +6194,8 @@ typedef struct _ATOM_INIT_REG_INDEX_FORMAT{
>  typedef struct _ATOM_INIT_REG_BLOCK{
>  	USHORT													usRegIndexTblSize;													//size of asRegIndexBuf
>  	USHORT													usRegDataBlkSize;														//size of ATOM_MEMORY_SETTING_DATA_BLOCK
> -	ATOM_INIT_REG_INDEX_FORMAT			asRegIndexBuf[1];
> -	ATOM_MEMORY_SETTING_DATA_BLOCK	asRegDataBuf[1];
> +	ATOM_INIT_REG_INDEX_FORMAT			asRegIndexBuf[0];
> +	ATOM_MEMORY_SETTING_DATA_BLOCK	asRegDataBuf[0];
>  }ATOM_INIT_REG_BLOCK;
>  
>  #define END_OF_REG_INDEX_BLOCK  0x0ffff
> @@ -6938,8 +6938,8 @@ typedef struct _ATOM_DISP_OUT_INFO
>    ATOM_COMMON_TABLE_HEADER sHeader;  
>  	USHORT ptrTransmitterInfo;
>  	USHORT ptrEncoderInfo;
> -	ASIC_TRANSMITTER_INFO  asTransmitterInfo[1];
> -	ASIC_ENCODER_INFO      asEncoderInfo[1];
> +	ASIC_TRANSMITTER_INFO  asTransmitterInfo[0];
> +	ASIC_ENCODER_INFO      asEncoderInfo[0];
>  }ATOM_DISP_OUT_INFO;
>  
>  typedef struct _ATOM_DISP_OUT_INFO_V2
> @@ -6948,8 +6948,8 @@ typedef struct _ATOM_DISP_OUT_INFO_V2
>  	USHORT ptrTransmitterInfo;
>  	USHORT ptrEncoderInfo;
>    USHORT ptrMainCallParserFar;                  // direct address of main parser call in VBIOS binary. 
> -	ASIC_TRANSMITTER_INFO  asTransmitterInfo[1];
> -	ASIC_ENCODER_INFO      asEncoderInfo[1];
> +	ASIC_TRANSMITTER_INFO  asTransmitterInfo[0];
> +	ASIC_ENCODER_INFO      asEncoderInfo[0];
>  }ATOM_DISP_OUT_INFO_V2;

I'm not sure how these structs are supposed to work, before or after
this change... The compiler can't know how to access the fields after
the first variably-sized array?

-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel