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

2018-06-04 Thread Martin Pelikán
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] < 0x1) {
+  vtn_err("words[1] was 0x%x, want >= 0x1", words[1]);
+  goto fail;
+   }
 
-   vtn_assert(words[0] == SpvMagicNumber);
-   vtn_assert(words[1] >= 0x1);
/* 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

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


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

2018-05-14 Thread Martin Pelikán
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 | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/compiler/spirv/spirv_to_nir.c 
b/src/compiler/spirv/spirv_to_nir.c
index 78437428aa..a05364ba2f 100644
--- a/src/compiler/spirv/spirv_to_nir.c
+++ b/src/compiler/spirv/spirv_to_nir.c
@@ -4011,19 +4011,35 @@ 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;
 
-   vtn_assert(words[0] == SpvMagicNumber);
-   vtn_assert(words[1] >= 0x1);
+   if (words[0] != SpvMagicNumber) {
+  vtn_warn("words[0] was 0x%x, want 0x%x", words[0], SpvMagicNumber);
+  goto fail;
+   }
+   if (words[1] < 0x1) {
+  vtn_warn("words[1] was 0x%x, want >= 0x1", words[1]);
+  goto fail;
+   }
/* words[2] == generator magic */
unsigned value_id_bound = words[3];
-   vtn_assert(words[4] == 0);
+   if (words[4] != 0) {
+  vtn_warn("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 *
-- 
2.17.0.441.gb46fe60e1d-goog

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