On Thu, Aug 17, 2017 at 7:22 PM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
> These helpers are much nicer than just using assert because they don't
> kill your process. Instead, it longjmps back to spirv_to_nir(), cleans
> up all the temporary memory, and nicely returns NULL. While crashing is
> completely OK in the Vulkan world, it's not considered to be quite so
> nice in GL. This should help us to make SPIR-V parsing much more
> robust. The one downside here is that vtn_assert is not compiled out in
> release builds like assert() is so it isn't free.
> ---
> src/compiler/spirv/spirv_to_nir.c | 20 ++++++++++++++++++++
> src/compiler/spirv/vtn_private.h | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 51 insertions(+)
>
> diff --git a/src/compiler/spirv/spirv_to_nir.c
> b/src/compiler/spirv/spirv_to_nir.c
> index e59f2b2..af542e8 100644
> --- a/src/compiler/spirv/spirv_to_nir.c
> +++ b/src/compiler/spirv/spirv_to_nir.c
> @@ -104,6 +104,20 @@ _vtn_warn(struct vtn_builder *b, const char *file,
> unsigned line,
> va_end(args);
> }
>
> +void
> +_vtn_fail(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 parsing FAILED:\n",
> + file, line, fmt, args);
> + va_end(args);
> +
> + longjmp(b->fail_jump, 1);
> +}
> +
> struct spec_constant_value {
> bool is_double;
> union {
> @@ -3420,6 +3434,12 @@ spirv_to_nir(const uint32_t *words, size_t word_count,
> b->entry_point_name = entry_point_name;
> b->ext = ext;
>
> + /* See also _vtn_fail() */
> + if (setjmp(b->fail_jump)) {
> + ralloc_free(b);
> + return NULL;
> + }
> +
> const uint32_t *word_end = words + word_count;
>
> /* Handle the SPIR-V header (first 4 dwords) */
> diff --git a/src/compiler/spirv/vtn_private.h
> b/src/compiler/spirv/vtn_private.h
> index 3eb601d..f640289 100644
> --- a/src/compiler/spirv/vtn_private.h
> +++ b/src/compiler/spirv/vtn_private.h
> @@ -28,6 +28,8 @@
> #ifndef _VTN_PRIVATE_H_
> #define _VTN_PRIVATE_H_
>
> +#include <setjmp.h>
> +
> #include "nir/nir.h"
> #include "nir/nir_builder.h"
> #include "util/u_dynarray.h"
> @@ -49,6 +51,32 @@ 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__)
>
> +/** Fail SPIR-V parsing
> + *
> + * This function logs an error and then bails out of the shader compile using
> + * longjmp. This being safe relies on two things:
> + *
> + * 1) We must guarantee that setjmp is called after allocating the builder
> + * and setting up b->debug (so that logging works) but before before any
> + * errors have a chance to occur.
> + *
> + * 2) While doing the SPIR-V -> NIR conversion, we need to be careful to
> + * ensure that all heap allocations happen through ralloc and are
> parented
> + * to the builder.
> + *
> + * So long as these two things continue to hold, we can easily longjmp back
> to
> + * spirv_to_nir(), clean up the builder, and return NULL.
> + */
Technically speaking, there's probably some more requirements, but
they might not be relevant here, for instance that no failures can
happen while mutex locks are being held.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
