[PATCH vkd3d v4 2/8] vkd3d-shader/hlsl: Avoid leaks on memory allocation failures when parsing initializers.

[Francisco Casas]

Signed-off-by: Francisco Casas

---
v4:
- This patch is new in v4.

Signed-off-by: Francisco Casas 
---
 libs/vkd3d-shader/hlsl.y | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 92dcb3f2..8c66a33c 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -3211,7 +3211,10 @@ complex_initializer:
         {
             $$.args_count = 1;
             if (!($$.args = hlsl_alloc(ctx, sizeof(*$$.args))))
+            {
+                destroy_instr_list($1);
                 YYABORT;
+            }
             $$.args[0] = node_from_list($1);
             $$.instrs = $1;
         }
@@ -3240,7 +3243,11 @@ initializer_expr_list:
         {
             $$ = $1;
             if (!($$.args = hlsl_realloc(ctx, $$.args, ($$.args_count + 1) * sizeof(*$$.args))))
+            {
+                free_parse_initializer(&$$);
+                destroy_instr_list($3);
                 YYABORT;
+            }
             $$.args[$$.args_count++] = node_from_list($3);
             list_move_tail($$.instrs, $3);
             vkd3d_free($3);

[Zebediah Figura]

On 3/9/22 07:50, Francisco Casas wrote:
> Signed-off-by: Francisco Casas
>
> v4:
> - This patch is new in v4.
> Signed-off-by: Francisco Casas
>
> libs/vkd3d-shader/hlsl.y | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index 92dcb3f2..8c66a33c 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -3211,7 +3211,10 @@ complex_initializer:
>           {
>               $$.args_count = 1;
>               if (!($$.args = hlsl_alloc(ctx, sizeof(*$$.args))))
> -
>        {
> -
>            destroy_instr_list($1);
>             YYABORT;
> -
>        }
>         $$.args[0] = node_from_list($1);
>         $$.instrs = $1;
>     }
>
> @@ -3240,7 +3243,11 @@ initializer_expr_list:
>           {
>               $$ = $1;
>               if (!($$.args = hlsl_realloc(ctx, $$.args, ($$.args_count + 1) * sizeof(*$$.args))))
> -
>        {
> -
>            free_parse_initializer(&$$);
> -
>            destroy_instr_list($3);
>             YYABORT;
> -
>        }
>         $$.args[$$.args_count++] = node_from_list($3);
>         list_move_tail($$.instrs, $3);
>         vkd3d_free($3);

Unfortunately this still leaks the old $$.args (or $1.args). We should 
probably be using array_reserve() here. There's a similar problem with 3/8.

[Francisco Casas]

Hi,

March 9, 2022 3:51 PM, "Zebediah Figura"  wrote:
> > @@ -3240,7 +3243,11 @@ initializer_expr_list:
> > {
> > $$ = $1;
> > if (!($$.args = hlsl_realloc(ctx, $$.args, ($$.args_count + 1) * sizeof(*$$.args))))
> > - {
> > - free_parse_initializer(&$$);
> > - destroy_instr_list($3);
> >
> > YYABORT;
> > - }
> >
> > $$.args[$$.args_count++] = node_from_list($3);
> > list_move_tail($$.instrs, $3);
> > vkd3d_free($3);
>
> Unfortunately this still leaks the old $$.args (or $1.args). We should probably be using
> array_reserve() here. There's a similar problem with 3/8.

If I am not mistaken, free_parse_initializer(&$$) takes care of that.
Currently its definition is:
---
static void free_parse_initializer(struct parse_initializer *initializer)
{
    destroy_instr_list(initializer->instrs);
    vkd3d_free(initializer->args);
}
---

Same for 3/8. Unless I am missing something...

[Zebediah Figura]

On 3/9/22 14:10, Francisco Casas wrote:
> Hi,
>
> March 9, 2022 3:51 PM, "Zebediah Figura"  wrote:
> > > @@ -3240,7 +3243,11 @@ initializer_expr_list:
> > > {
> > > $$ = $1;
> > > if (!($$.args = hlsl_realloc(ctx, $$.args, ($$.args_count + 1) * sizeof(*$$.args))))
> > > - {
> > > - free_parse_initializer(&$$);
> > > - destroy_instr_list($3);
> > >
> > > YYABORT;
> > > - }
> > >
> > > $$.args[$$.args_count++] = node_from_list($3);
> > > list_move_tail($$.instrs, $3);
> > > vkd3d_free($3);
> >
> > Unfortunately this still leaks the old $$.args (or $1.args). We should probably be using
> > array_reserve() here. There's a similar problem with 3/8.
>
> If I am not mistaken, free_parse_initializer(&$$) takes care of that.
> Currently its definition is:
>
> static void free_parse_initializer(struct parse_initializer *initializer)
> {
>      destroy_instr_list(initializer->instrs);
>      vkd3d_free(initializer->args);
> }
>
> Same for 3/8. Unless I am missing something...

The problem is that you're assigning $$.args to NULL, so nothing is 
actually freed.

[Francisco Casas]

March 9, 2022 5:17 PM, "Zebediah Figura"  wrote:
> The problem is that you're assigning $$.args to NULL, so nothing is actually freed.

Sweet mother of memory! 
Thanks, I see it now.