[Mesa-dev] [PATCH v2 0/8] glsl/linker: fix location aliasing checks

[Timothy Arceri]

1 & 8:

Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>

Thanks!

On 24/10/17 20:28, Iago Toral Quiroga wrote:
> This v2 series is in fact an update and merge of the these two series:
> "glsl/linker: fix location aliasing checks" and "Implement location aliasing
> checks for SSO programs"
> 
> This v2 series introduces 3 changes:
> 
> 1. Use MAX_VARYINGS instead of MAX_VARYINGS_INCL_PATCH to define the size
>     of the explicit_locations array. This change, however, made a pre-existing
>     bug  show up: we do not check that explicit locations on inputs are valid
>     (< MAX_VARYINGS) before indexing the explicit_locations array. That can lead
>     to out of bounds accesses. There are some tests in dEQP and CTS that check this
>     however, because we were using MAX_VARYINGS_INCL_PATCH as the size of the
>     array, we were sill indexing inside the bounds of the array (even if the
>     location index was not correct) and we would still produce linker errors
>     when we didn't found matching outputs, however that was just luck, as we
>     could still be indexing out of bounds if the invalid location provided for
>     shader inputs were large enough. The first patch in this v2 series fixes
>     this by adding a trivial check for the input location index.
> 
> 2. For SSO, we only need to validate location aliasing for the inputs to the
>     first stage and outputs of the last stage, as pointed out by Timothy. This
>     is addressed in the last patch in the series.
> 
> 3. When we were validating interface block locations, we were not computing
>     correctly the location slot index for patch variables (these start at
>     VARYING_SLOT_PATCH0 not VARYING_SLOT_VAR0).
> 
> Illia: I also checked your original patch and I think this series includes
> all functional changes you had.
> 
> Patches 2-7 are already reviewed, changes 1) and 3) affect some of them but
> trivially so. Patches 1 (new in this series) and 8 still need the Rb.
> 
> Iago Toral Quiroga (8):
>    glsl/linker: report linker errors for invalid explicit locations on
>      inputs
>    glsl/linker: refactor link-time validation of output locations
>    glsl/linker: fix location aliasing checks for interface variables
>    glsl/linker: outputs in the same location must share interpolation
>    glsl/linker: outputs in the same location must share auxiliary storage
>    glsl/linker: create a helper function to validate explicit locations
>    glsl/linker: generalize validate_explicit_variable_location for SSO
>    glsl/linker: validate explicit locations for SSO programs
> 
>   src/compiler/glsl/link_varyings.cpp | 334 ++++++++++++++++++++++++++++--------
>   src/compiler/glsl/link_varyings.h   |   6 +
>   src/compiler/glsl/linker.cpp        |  10 ++
>   3 files changed, 279 insertions(+), 71 deletions(-)
>