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

[Brian Paul]

On 11/01/2018 09:29 AM, Eric Engestrom wrote:
> 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 :)

But there should probably be a comment explaining what's going on to 
avoid future confusion. :)

-Brian