[FFmpeg-devel] lavc/vaapi_encode_h265: Suppress the "above array bounds" warning.

From f3678e0ceb691b6df5957a2c3d26d4f0d91d4ff5 Mon Sep 17 00:00:00 2001
From: Jun Zhao <jun.zhao@intel.com>
Date: Wed, 26 Apr 2017 14:00:56 +0800
Subject: [PATCH] lavc/vaapi_encode_h265: Suppress the "above array bounds"
 warning.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

setting the layer_id_included_flag lead to a potential array out of bounds,
build warning message as follow:
libavcodec/vaapi_encode_h265.c: In function
‘vaapi_encode_h265_write_sequence_header’:
libavcodec/vaapi_encode_h265.c:305:49: warning: array subscript is above
array bounds [-Warray-bounds]

Signed-off-by: Jun Zhao <jun.zhao@intel.com>
---
 libavcodec/vaapi_encode_h265.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 26/04/17 07:44, Jun Zhao wrote:
> From f3678e0ceb691b6df5957a2c3d26d4f0d91d4ff5 Mon Sep 17 00:00:00 2001
> From: Jun Zhao <jun.zhao@intel.com>
> Date: Wed, 26 Apr 2017 14:00:56 +0800
> Subject: [PATCH] lavc/vaapi_encode_h265: Suppress the "above array bounds"
>  warning.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> setting the layer_id_included_flag lead to a potential array out of bounds,
> build warning message as follow:
> libavcodec/vaapi_encode_h265.c: In function
> ‘vaapi_encode_h265_write_sequence_header’:
> libavcodec/vaapi_encode_h265.c:305:49: warning: array subscript is above
> array bounds [-Warray-bounds]

Can you explain what the aim of this is?  You haven't actually set any of the layer_id_included_flag in this patch, so is there more to follow dealing with the layers / layer sets?

In any case, the dimensions of the array should probably be [MAX_LAYER_SETS][MAX_LAYERS] rather than the current values.  (With MAX_LAYER_SETS having value 1 currently, but more if you intend to add more.)

> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
> ---
>  libavcodec/vaapi_encode_h265.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
> index 6e008b7b9c..1b2a49806b 100644
> --- a/libavcodec/vaapi_encode_h265.c
> +++ b/libavcodec/vaapi_encode_h265.c
> @@ -66,7 +66,7 @@ typedef struct VAAPIEncodeH265MiscSequenceParams {
>      unsigned int vps_num_layer_sets_minus1;
>      unsigned int sps_max_sub_layers_minus1;
>      char sps_temporal_id_nesting_flag;
> -    char layer_id_included_flag[MAX_LAYERS][64];
> +    char layer_id_included_flag[MAX_LAYERS+1][64];
>  
>      // Profile/tier/level parameters.
>      char general_profile_compatibility_flag[32];
> -- 
> 2.11.0
>

On 2017/4/30 20:47, Mark Thompson wrote:
> On 26/04/17 07:44, Jun Zhao wrote:
>> From f3678e0ceb691b6df5957a2c3d26d4f0d91d4ff5 Mon Sep 17 00:00:00 2001
>> From: Jun Zhao <jun.zhao@intel.com>
>> Date: Wed, 26 Apr 2017 14:00:56 +0800
>> Subject: [PATCH] lavc/vaapi_encode_h265: Suppress the "above array bounds"
>>  warning.
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> setting the layer_id_included_flag lead to a potential array out of bounds,
>> build warning message as follow:
>> libavcodec/vaapi_encode_h265.c: In function
>> ‘vaapi_encode_h265_write_sequence_header’:
>> libavcodec/vaapi_encode_h265.c:305:49: warning: array subscript is above
>> array bounds [-Warray-bounds]
> 
> Can you explain what the aim of this is?  You haven't actually set any of the layer_id_included_flag in this patch, so is there more to follow dealing with the layers / layer sets?
> 
> In any case, the dimensions of the array should probably be [MAX_LAYER_SETS][MAX_LAYERS] rather than the current values.  (With MAX_LAYER_SETS having value 1 currently, but more if you intend to add more.)

Because in vaapi_encode_h265_write_vps(), when setting the layer_id_included_flag, the index i start from 1, it's a potential above array bounds issue when MAX_LAYERS == 1. 

> 
>> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
>> ---
>>  libavcodec/vaapi_encode_h265.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>> index 6e008b7b9c..1b2a49806b 100644
>> --- a/libavcodec/vaapi_encode_h265.c
>> +++ b/libavcodec/vaapi_encode_h265.c
>> @@ -66,7 +66,7 @@ typedef struct VAAPIEncodeH265MiscSequenceParams {
>>      unsigned int vps_num_layer_sets_minus1;
>>      unsigned int sps_max_sub_layers_minus1;
>>      char sps_temporal_id_nesting_flag;
>> -    char layer_id_included_flag[MAX_LAYERS][64];
>> +    char layer_id_included_flag[MAX_LAYERS+1][64];
>>  
>>      // Profile/tier/level parameters.
>>      char general_profile_compatibility_flag[32];
>> -- 
>> 2.11.0
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

On 02/05/17 01:10, Jun Zhao wrote:
> On 2017/4/30 20:47, Mark Thompson wrote:
>> On 26/04/17 07:44, Jun Zhao wrote:
>>> From f3678e0ceb691b6df5957a2c3d26d4f0d91d4ff5 Mon Sep 17 00:00:00 2001
>>> From: Jun Zhao <jun.zhao@intel.com>
>>> Date: Wed, 26 Apr 2017 14:00:56 +0800
>>> Subject: [PATCH] lavc/vaapi_encode_h265: Suppress the "above array bounds"
>>>  warning.
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> setting the layer_id_included_flag lead to a potential array out of bounds,
>>> build warning message as follow:
>>> libavcodec/vaapi_encode_h265.c: In function
>>> ‘vaapi_encode_h265_write_sequence_header’:
>>> libavcodec/vaapi_encode_h265.c:305:49: warning: array subscript is above
>>> array bounds [-Warray-bounds]
>>
>> Can you explain what the aim of this is?  You haven't actually set any of the layer_id_included_flag in this patch, so is there more to follow dealing with the layers / layer sets?
>>
>> In any case, the dimensions of the array should probably be [MAX_LAYER_SETS][MAX_LAYERS] rather than the current values.  (With MAX_LAYER_SETS having value 1 currently, but more if you intend to add more.)
> 
> Because in vaapi_encode_h265_write_vps(), when setting the layer_id_included_flag, the index i start from 1, it's a potential above array bounds issue when MAX_LAYERS == 1. 

With current settings (i.e. MAX_LAYERS is 1 and vps_num_layer_sets_minus1 is 0) none of the layer_id_included_flag are written - that code is only present now so that the function matches the standard.

If you want to add more layers / layer sets, then indeed the constants would need to be changed to reflect that.

>>> Signed-off-by: Jun Zhao <jun.zhao@intel.com>
>>> ---
>>>  libavcodec/vaapi_encode_h265.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/vaapi_encode_h265.c b/libavcodec/vaapi_encode_h265.c
>>> index 6e008b7b9c..1b2a49806b 100644
>>> --- a/libavcodec/vaapi_encode_h265.c
>>> +++ b/libavcodec/vaapi_encode_h265.c
>>> @@ -66,7 +66,7 @@ typedef struct VAAPIEncodeH265MiscSequenceParams {
>>>      unsigned int vps_num_layer_sets_minus1;
>>>      unsigned int sps_max_sub_layers_minus1;
>>>      char sps_temporal_id_nesting_flag;
>>> -    char layer_id_included_flag[MAX_LAYERS][64];
>>> +    char layer_id_included_flag[MAX_LAYERS+1][64];
>>>  
>>>      // Profile/tier/level parameters.
>>>      char general_profile_compatibility_flag[32];
>>> -- 
>>> 2.11.0
>>>