[Mesa-dev] [PATCH mesa] move variable-sized struct at the end of parent struct

[Eric Engestrom]

On Thursday, 2018-11-01 12:19:10 +0000, Lionel Landwerlin wrote:
> On 01/11/2018 11:59, andrey simiklit wrote:
> > Hello,
> > 
> > Please find my comments below:
> > 
> > On Thu, Nov 1, 2018 at 12:24 PM Eric Engestrom <eric.engestrom at intel.com
> > <mailto:eric.engestrom at intel.com>> wrote:
> > 
> >       warning: field 'base' with variable sized type 'struct
> >     drm_i915_query_topology_info'
> >       not at the end of a struct or class is a GNU extension
> >     [-Wgnu-variable-sized-type-not-at-end]
> > 
> >     Signed-off-by: Eric Engestrom <eric.engestrom at intel.com
> >     <mailto:eric.engestrom at intel.com>>
> >     ---
> >      src/intel/dev/gen_device_info.c | 2 +-
> >      1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> >     diff --git a/src/intel/dev/gen_device_info.c
> >     b/src/intel/dev/gen_device_info.c
> >     index 5dbd06075722f8cc644e..242fe163447a4265acfb 100644
> >     --- a/src/intel/dev/gen_device_info.c
> >     +++ b/src/intel/dev/gen_device_info.c
> >     @@ -991,8 +991,8 @@ gen_device_info_update_from_masks(struct
> >     gen_device_info *devinfo,
> >                                        uint32_t n_eus)
> >      {
> >         struct {
> >     -      struct drm_i915_query_topology_info base;
> >            uint8_t data[100];
> >     +      struct drm_i915_query_topology_info base;
> >         } topology;
> > 
> > 
> > I can be wrong, but it seems like here the 'data[100]' field should
> > rather be placed after the 'base' field
> > because it was done as far as I understood to allocate memory on the stack
> > for the 'drm_i915_query_topology_info::data[]' field. I guess that this
> > patch
> > may introduce the stack corruption on the following line:
> >    for (int b = 0; b < topology.base.subslice_offset; b++)
> >       topology.base.data[b] = (slice_mask >> (b * 8)) & 0xff;
> 
> 
> Andrey's right, the base field needs to be first and data following.
> 
> nack :(

Right, I misunderstood this code (I was confused how it was working in
the first place, but now I understand).
Thanks both; the warning is wrong, this code is exactly how it should be :)

> 
> 
> > 
> >         assert((slice_mask & 0xff) == slice_mask);
> >     --     Cheers,
> >       Eric
> > 
> >     _______________________________________________
> >     mesa-dev mailing list
> >     mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
> >     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > 
> > 
> > Regards,
> > Andrii.
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>