[Mesa-dev] [PATCH v3] glsl/blob: avoid NULL ptr in blob_write_string/blob_read_string

[Gregory Hainaut]

Context:
Nouveau uses NULL strings for unnamed parameter of texture gather
offsets opcode.

Fix piglit crashes of the 'texturegatheroffsets' tests on Nouveau

v3: Redo patch based on Nicolai/Timothy feedback
Create dedicated blob_write_string_optional/blob_read_string_optional to
handle null pointer string.
Add an assert in blob_write_string to ease debug

Signed-off-by: Gregory Hainaut <gregory.hainaut at gmail.com>
---
 src/compiler/glsl/blob.c           | 33 +++++++++++++++++++++++++++++++++
 src/compiler/glsl/blob.h           | 27 +++++++++++++++++++++++++++
 src/compiler/glsl/shader_cache.cpp |  4 ++--
 3 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/src/compiler/glsl/blob.c b/src/compiler/glsl/blob.c
index 769ebf1..62d6bf5 100644
--- a/src/compiler/glsl/blob.c
+++ b/src/compiler/glsl/blob.c
@@ -176,9 +176,29 @@ blob_write_intptr(struct blob *blob, intptr_t value)
 bool
 blob_write_string(struct blob *blob, const char *str)
 {
+   // Please use blob_write_string_optional instead
+   assert(str != NULL);
+
    return blob_write_bytes(blob, str, strlen(str) + 1);
 }
 
+bool
+blob_write_string_optional(struct blob *blob, const char *str)
+{
+   bool ret;
+   const uint8_t flag = str != NULL ? 1 : 0;
+
+   ret = blob_write_bytes(blob, &flag, 1);
+
+   if (!ret)
+      return false;
+
+   if (str != NULL)
+      ret = blob_write_string(blob, str);
+
+   return ret;
+}
+
 void
 blob_reader_init(struct blob_reader *blob, uint8_t *data, size_t size)
 {
@@ -321,3 +341,16 @@ blob_read_string(struct blob_reader *blob)
 
    return ret;
 }
+
+char *
+blob_read_string_optional(struct blob_reader *blob)
+{
+   uint8_t *flag;
+   flag = (uint8_t *)blob_read_bytes(blob, 1);
+
+   if (flag == NULL || *flag == 0) {
+      return NULL;
+   }
+
+   return blob_read_string(blob);
+}
diff --git a/src/compiler/glsl/blob.h b/src/compiler/glsl/blob.h
index 940c81e..2f58cc3 100644
--- a/src/compiler/glsl/blob.h
+++ b/src/compiler/glsl/blob.h
@@ -203,6 +203,7 @@ blob_write_intptr(struct blob *blob, intptr_t value);
 
 /**
  * Add a NULL-terminated string to a blob, (including the NULL terminator).
+ * If str could be NULL, blob_write_string_optional must be used instead.
  *
  * \return True unless allocation failed.
  */
@@ -210,6 +211,15 @@ bool
 blob_write_string(struct blob *blob, const char *str);
 
 /**
+ * Add a flag + a NULL-terminated string to a blob, (including the NULL
+ * terminator). The flag will be zero if str is NULL.
+ *
+ * \return True unless allocation failed.
+ */
+bool
+blob_write_string_optional(struct blob *blob, const char *str);
+
+/**
  * Start reading a blob, (initializing the contents of \blob for reading).
  *
  * After this call, the caller can use the various blob_read_* functions to
@@ -294,6 +304,23 @@ blob_read_intptr(struct blob_reader *blob);
 char *
 blob_read_string(struct blob_reader *blob);
 
+/**
+ * Read a NULL-terminated string from the current location, (and update the
+ * current location to just past this string).
+ *
+ * \note Similar as blob_read_string but return NULL if written string was NULL.
+ *
+ * \note The memory returned belongs to the data underlying the blob reader. The
+ * caller must copy the string in order to use the string after the lifetime
+ * of the data underlying the blob reader.
+ *
+ * \return The string read (see note above about memory lifetime). However, if
+ * there is no NULL byte remaining within the blob, this function returns
+ * NULL.
+ */
+char *
+blob_read_string_optional(struct blob_reader *blob);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/compiler/glsl/shader_cache.cpp b/src/compiler/glsl/shader_cache.cpp
index f5e6a22..e07d2c4 100644
--- a/src/compiler/glsl/shader_cache.cpp
+++ b/src/compiler/glsl/shader_cache.cpp
@@ -1078,7 +1078,7 @@ write_shader_parameters(struct blob *metadata,
       struct gl_program_parameter *param = &params->Parameters[i];
 
       blob_write_uint32(metadata, param->Type);
-      blob_write_string(metadata, param->Name);
+      blob_write_string_optional(metadata, param->Name);
       blob_write_uint32(metadata, param->Size);
       blob_write_uint32(metadata, param->DataType);
       blob_write_bytes(metadata, param->StateIndexes,
@@ -1104,7 +1104,7 @@ read_shader_parameters(struct blob_reader *metadata,
    _mesa_reserve_parameter_storage(params, num_parameters);
    while (i < num_parameters) {
       gl_register_file type = (gl_register_file) blob_read_uint32(metadata);
-      const char *name = blob_read_string(metadata);
+      const char *name = blob_read_string_optional(metadata);
       unsigned size = blob_read_uint32(metadata);
       unsigned data_type = blob_read_uint32(metadata);
       blob_copy_bytes(metadata, (uint8_t *) state_indexes,
-- 
2.1.4