On Thu, Aug 17, 2017 at 10:29 AM, Erik Faye-Lund <kusmab...@gmail.com> wrote:
> 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. > Right. That wouldn't be bad to state. Fortunately, all uses of mutexes from within spirv_to_nir are encapsulated in glsl_type functions so we have no way to get back into spirv_to_nir code while a mutex is held. I'll add that one to the list. --Jason
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev