On Wed, Apr 5, 2017 at 1:12 PM, Gregory Hainaut
<gregory.hain...@gmail.com> wrote:
> Context:
> Nouveau uses NULL strings for unnamed parameter of texture gather
> offsets opcode.
To be clear, this isn't a "nouveau" thing, as it is well downstream of
all this GLSL stuff. And FWIW llvmpipe/softpipe also support the
4-offset TG4 (I think). It's a GLSL IR thing. If it needs changing,
you can change how GLSL IR represents this stuff, instead of
necessarily supporting this wart.
Unfortunately I don't understand the problem clearly enough to tell
what's going on and what the proper solution is.
>
> 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.hain...@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 = ¶ms->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
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
