[Mesa-dev] [PATCH] compiler/spirv: reject invalid shader code properly

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

On Fri, Jun 1, 2018 at 5:27 AM, Martin Pelikán <mpel at google.com> wrote:

> After bebe3d626e5, b->fail_jump is prepared after vtn_create_builder
> which can longjmp(3) to it through its vtx_assert()s.  This corrupts
> the stack and creates confusing core dumps, so we need to avoid it.
>
> While there, I decided to print the offending values for debugability.
> ---
>  src/compiler/spirv/spirv_to_nir.c | 39 +++++++++++++++++++++++++++----
>  src/compiler/spirv/vtn_private.h  |  4 ++++
>  2 files changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index 78437428aa..1685ddc34b 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -129,6 +129,18 @@ _vtn_warn(struct vtn_builder *b, const char *file,
> unsigned line,
>     va_end(args);
>  }
>
> +void
> +_vtn_err(struct vtn_builder *b, const char *file, unsigned line,
> +          const char *fmt, ...)
> +{
> +   va_list args;
> +
> +   va_start(args, fmt);
> +   vtn_log_err(b, NIR_SPIRV_DEBUG_LEVEL_ERROR, "SPIR-V ERROR:\n",
> +               file, line, fmt, args);
> +   va_end(args);
> +}
> +
>  void
>  _vtn_fail(struct vtn_builder *b, const char *file, unsigned line,
>            const char *fmt, ...)
> @@ -4011,19 +4023,36 @@ vtn_create_builder(const uint32_t *words, size_t
> word_count,
>     b->entry_point_name = entry_point_name;
>     b->options = options;
>
> -   /* Handle the SPIR-V header (first 4 dwords)  */
> -   vtn_assert(word_count > 5);
> +   /*
> +    * Handle the SPIR-V header (first 4 dwords).
> +    * Can't use vtx_assert() as the setjmp(3) target isn't initialized
> yet.
> +    */
> +   if (word_count <= 5)
> +      goto fail;
> +
> +   if (words[0] != SpvMagicNumber) {
> +      vtn_err("words[0] was 0x%x, want 0x%x", words[0], SpvMagicNumber);
> +      goto fail;
> +   }
> +   if (words[1] < 0x10000) {
> +      vtn_err("words[1] was 0x%x, want >= 0x10000", words[1]);
> +      goto fail;
> +   }
>
> -   vtn_assert(words[0] == SpvMagicNumber);
> -   vtn_assert(words[1] >= 0x10000);
>     /* words[2] == generator magic */
>     unsigned value_id_bound = words[3];
> -   vtn_assert(words[4] == 0);
> +   if (words[4] != 0) {
> +      vtn_err("words[4] was %u, want 0", words[4]);
> +      goto fail;
> +   }
>
>     b->value_id_bound = value_id_bound;
>     b->values = rzalloc_array(b, struct vtn_value, value_id_bound);
>
>     return b;
> + fail:
> +   ralloc_free(b);
> +   return NULL;
>  }
>
>  nir_function *
> diff --git a/src/compiler/spirv/vtn_private.h b/src/compiler/spirv/vtn_
> private.h
> index b501bbf9b4..3146d8eeb5 100644
> --- a/src/compiler/spirv/vtn_private.h
> +++ b/src/compiler/spirv/vtn_private.h
> @@ -51,6 +51,10 @@ void _vtn_warn(struct vtn_builder *b, const char *file,
> unsigned line,
>                 const char *fmt, ...) PRINTFLIKE(4, 5);
>  #define vtn_warn(...) _vtn_warn(b, __FILE__, __LINE__, __VA_ARGS__)
>
> +void _vtn_err(struct vtn_builder *b, const char *file, unsigned line,
> +               const char *fmt, ...) PRINTFLIKE(4, 5);
> +#define vtn_err(...) _vtn_err(b, __FILE__, __LINE__, __VA_ARGS__)
> +
>  /** Fail SPIR-V parsing
>   *
>   * This function logs an error and then bails out of the shader compile
> using
> --
> 2.17.0.921.gf22659ad46-goog
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180601/43f4eec3/attachment.html>