Looks reasonable to me.
Reviewed-by: Roland Scheidegger <srol...@vmware.com>


Am 16.10.18 um 10:07 schrieb Gert Wollny:
> From: Gert Wollny <gert.wol...@collabora.com>
> 
> The number of immediate constants was fixed and the size check was
> only done by means of an assertion. Given this a shader that emits
> more immediate constants would result in a memory corruption when
> mesa is build in release mode.
> 
> Instead of using this fixed limit allocate the space dynamically, let it 
> grow as needed, and also remove the unused ImmArray.
> 
> Fixes: dEQP-GLES31.functional.ssbo.layout.random.arrays_of_arrays.1
> 
> Signed-off-by: Gert Wollny <gert.wol...@collabora.com>
> ---
>  src/gallium/auxiliary/tgsi/tgsi_exec.c | 13 ++++++++++++-
>  src/gallium/auxiliary/tgsi/tgsi_exec.h |  7 +++----
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.c 
> b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> index 59194ebe31..5db515a075 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.c
> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.c
> @@ -1223,7 +1223,17 @@ tgsi_exec_machine_bind_shader(
>           {
>              uint size = parse.FullToken.FullImmediate.Immediate.NrTokens - 1;
>              assert( size <= 4 );
> -            assert( mach->ImmLimit + 1 <= TGSI_EXEC_NUM_IMMEDIATES );
> +            if (mach->ImmLimit >= mach->ImmsReserved) {
> +               unsigned newReserved = mach->ImmsReserved ? 2 * 
> mach->ImmsReserved : 128;
> +               float4 *imms = REALLOC(mach->Imms, mach->ImmsReserved, 
> newReserved * sizeof(float4));
> +               if (imms) {
> +                  mach->ImmsReserved = newReserved;
> +                  mach->Imms = imms;
> +               } else {
> +                  debug_printf("Unable to (re)allocate space for immidiate 
> constants\n");
> +                  break;
> +               }
> +            }
>  
>              for( i = 0; i < size; i++ ) {
>                 mach->Imms[mach->ImmLimit][i] = 
> @@ -1337,6 +1347,7 @@ tgsi_exec_machine_destroy(struct tgsi_exec_machine 
> *mach)
>     if (mach) {
>        FREE(mach->Instructions);
>        FREE(mach->Declarations);
> +      FREE(mach->Imms);
>  
>        align_free(mach->Inputs);
>        align_free(mach->Outputs);
> diff --git a/src/gallium/auxiliary/tgsi/tgsi_exec.h 
> b/src/gallium/auxiliary/tgsi/tgsi_exec.h
> index ed8b9e8869..6d4ac38142 100644
> --- a/src/gallium/auxiliary/tgsi/tgsi_exec.h
> +++ b/src/gallium/auxiliary/tgsi/tgsi_exec.h
> @@ -231,7 +231,6 @@ struct tgsi_sampler
>  };
>  
>  #define TGSI_EXEC_NUM_TEMPS       4096
> -#define TGSI_EXEC_NUM_IMMEDIATES  256
>  
>  /*
>   * Locations of various utility registers (_I = Index, _C = Channel)
> @@ -341,6 +340,7 @@ enum tgsi_break_type {
>  
>  #define TGSI_EXEC_MAX_BREAK_STACK (TGSI_EXEC_MAX_LOOP_NESTING + 
> TGSI_EXEC_MAX_SWITCH_NESTING)
>  
> +typedef float float4[4];
>  
>  /**
>   * Run-time virtual machine state for executing TGSI shader.
> @@ -352,9 +352,8 @@ struct tgsi_exec_machine
>     struct tgsi_exec_vector       Temps[TGSI_EXEC_NUM_TEMPS +
>                                         TGSI_EXEC_NUM_TEMP_EXTRAS];
>  
> -   float                         Imms[TGSI_EXEC_NUM_IMMEDIATES][4];
> -
> -   float                         ImmArray[TGSI_EXEC_NUM_IMMEDIATES][4];
> +   unsigned                       ImmsReserved;
> +   float4                         *Imms;
>  
>     struct tgsi_exec_vector       *Inputs;
>     struct tgsi_exec_vector       *Outputs;
> 

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to