On 2017-10-11 13:38:46, Jason Ekstrand wrote:
> From: Connor Abbott <cwabbo...@gmail.com>
>
> Despite the name, it could only be used if you immediately wrote to the
> pointer. Noboby was using it outside of one test, so clearly this
> behavior wasn't that useful. Instead, make it return an offset into the
> data buffer so that the result isn't invalidated if you later write to
> the blob. In conjunction with blob_overwrite_bytes(), this will be
> useful for leaving a placeholder and then filling it in later, which
> we'll need to do for handling phi nodes when serializing NIR.
>
> v2 (Jason Ekstrand):
> - Improve the blob_overwrite_uint32 documentation
> - Detect overflow in the offset + to_write computation
> - Add a blob_reserve_uint32 helper
> - Add a blob_overwrite_intptr helper
Why not add blob_reserve_uint32 and blob_overwrite_intptr in a
separate patch? I think fixing the alignment issue is worth
highlighting in its own patch.
-Jordan
> ---
> src/compiler/blob.c | 37 +++++++++++++++++++++----
> src/compiler/blob.h | 54
> ++++++++++++++++++++++++++-----------
> src/compiler/glsl/tests/blob_test.c | 4 +--
> 3 files changed, 73 insertions(+), 22 deletions(-)
>
> diff --git a/src/compiler/blob.c b/src/compiler/blob.c
> index 59ad8a3..c5ed9f5 100644
> --- a/src/compiler/blob.c
> +++ b/src/compiler/blob.c
> @@ -130,7 +130,7 @@ blob_overwrite_bytes(struct blob *blob,
> size_t to_write)
> {
> /* Detect an attempt to overwrite data out of bounds. */
> - if (blob->size < offset + to_write)
> + if (offset + to_write < offset || blob->size < offset + to_write)
> return false;
>
> VG(VALGRIND_CHECK_MEM_IS_DEFINED(bytes, to_write));
> @@ -156,20 +156,34 @@ blob_write_bytes(struct blob *blob, const void *bytes,
> size_t to_write)
> return true;
> }
>
> -uint8_t *
> +ssize_t
> blob_reserve_bytes(struct blob *blob, size_t to_write)
> {
> - uint8_t *ret;
> + ssize_t ret;
>
> if (! grow_to_fit (blob, to_write))
> - return NULL;
> + return -1;
>
> - ret = blob->data + blob->size;
> + ret = blob->size;
> blob->size += to_write;
>
> return ret;
> }
>
> +ssize_t
> +blob_reserve_uint32(struct blob *blob)
> +{
> + align_blob(blob, sizeof(uint32_t));
> + return blob_reserve_bytes(blob, sizeof(uint32_t));
> +}
> +
> +ssize_t
> +blob_reserve_intptr(struct blob *blob)
> +{
> + align_blob(blob, sizeof(intptr_t));
> + return blob_reserve_bytes(blob, sizeof(intptr_t));
> +}
> +
> bool
> blob_write_uint32(struct blob *blob, uint32_t value)
> {
> @@ -178,11 +192,15 @@ blob_write_uint32(struct blob *blob, uint32_t value)
> return blob_write_bytes(blob, &value, sizeof(value));
> }
>
> +#define ASSERT_ALIGNED(_offset, _align) \
> + assert(ALIGN((_offset), (_align)) == (_offset))
> +
> bool
> blob_overwrite_uint32 (struct blob *blob,
> size_t offset,
> uint32_t value)
> {
> + ASSERT_ALIGNED(offset, sizeof(value));
> return blob_overwrite_bytes(blob, offset, &value, sizeof(value));
> }
>
> @@ -203,6 +221,15 @@ blob_write_intptr(struct blob *blob, intptr_t value)
> }
>
> bool
> +blob_overwrite_intptr (struct blob *blob,
> + size_t offset,
> + intptr_t value)
> +{
> + ASSERT_ALIGNED(offset, sizeof(value));
> + return blob_overwrite_bytes(blob, offset, &value, sizeof(value));
> +}
> +
> +bool
> blob_write_string(struct blob *blob, const char *str)
> {
> return blob_write_bytes(blob, str, strlen(str) + 1);
> diff --git a/src/compiler/blob.h b/src/compiler/blob.h
> index 1ef6d99..ad4b6fa 100644
> --- a/src/compiler/blob.h
> +++ b/src/compiler/blob.h
> @@ -126,24 +126,32 @@ blob_write_bytes(struct blob *blob, const void *bytes,
> size_t to_write);
> * Reserve space in \blob for a number of bytes.
> *
> * Space will be allocated within the blob for these byes, but the bytes will
> - * be left uninitialized. The caller is expected to use the return value to
> - * write directly (and immediately) to these bytes.
> + * be left uninitialized. The caller is expected to use \sa
> + * blob_overwrite_bytes to write to these bytes.
> *
> - * \note The return value is valid immediately upon return, but can be
> - * invalidated by any other call to a blob function. So the caller should
> call
> - * blob_reserve_byes immediately before writing through the returned pointer.
> - *
> - * This function is intended to be used when interfacing with an existing API
> - * that is not aware of the blob API, (so that blob_write_bytes cannot be
> - * called).
> - *
> - * \return A pointer to space allocated within \blob to which \to_write bytes
> - * can be written, (or NULL in case of any allocation error).
> + * \return An offset to space allocated within \blob to which \to_write bytes
> + * can be written, (or -1 in case of any allocation error).
> */
> -uint8_t *
> +ssize_t
> blob_reserve_bytes(struct blob *blob, size_t to_write);
>
> /**
> + * Similar to \sa blob_reserve_bytes, but only reserves an uint32_t worth of
> + * space. Note that this must be used if later reading with \sa
> + * blob_read_uint32, since it aligns the offset correctly.
> + */
> +ssize_t
> +blob_reserve_uint32(struct blob *blob);
> +
> +/**
> + * Similar to \sa blob_reserve_bytes, but only reserves an intptr_t worth of
> + * space. Note that this must be used if later reading with \sa
> + * blob_read_intptr, since it aligns the offset correctly.
> + */
> +ssize_t
> +blob_reserve_intptr(struct blob *blob);
> +
> +/**
> * Overwrite some data previously written to the blob.
> *
> * Writes data to an existing portion of the blob at an offset of \offset.
> @@ -186,8 +194,7 @@ blob_write_uint32(struct blob *blob, uint32_t value);
> *
> * size_t offset;
> *
> - * offset = blob->size;
> - * blob_write_uint32 (blob, 0); // placeholder
> + * offset = blob_reserve_uint32(blob);
> * ... various blob write calls, writing N items ...
> * blob_overwrite_uint32 (blob, offset, N);
> *
> @@ -226,6 +233,23 @@ bool
> blob_write_intptr(struct blob *blob, intptr_t value);
>
> /**
> + * Overwrite a uint32_t previously written to the blob.
> + *
> + * Writes a uint32_t value to an existing portion of the blob at an offset of
> + * \offset. This data range must have previously been written to the blob by
> + * one of the blob_write_* calls.
> + *
> + * For example usage, see blob_overwrite_uint32
> + *
> + * \return True unless the requested position or position+to_write lie
> outside
> + * the current blob's size.
> + */
> +bool
> +blob_overwrite_intptr(struct blob *blob,
> + size_t offset,
> + intptr_t value);
> +
> +/**
> * Add a NULL-terminated string to a blob, (including the NULL terminator).
> *
> * \return True unless allocation failed.
> diff --git a/src/compiler/glsl/tests/blob_test.c
> b/src/compiler/glsl/tests/blob_test.c
> index df0e91a..1b3ab6e 100644
> --- a/src/compiler/glsl/tests/blob_test.c
> +++ b/src/compiler/glsl/tests/blob_test.c
> @@ -120,7 +120,7 @@ test_write_and_read_functions (void)
> {
> struct blob *blob;
> struct blob_reader reader;
> - uint8_t *reserved;
> + ssize_t reserved;
> size_t str_offset, uint_offset;
> uint8_t reserve_buf[sizeof(reserve_test_str)];
>
> @@ -131,7 +131,7 @@ test_write_and_read_functions (void)
> blob_write_bytes(blob, bytes_test_str, sizeof(bytes_test_str));
>
> reserved = blob_reserve_bytes(blob, sizeof(reserve_test_str));
> - memcpy(reserved, reserve_test_str, sizeof(reserve_test_str));
> + blob_overwrite_bytes(blob, reserved, reserve_test_str,
> sizeof(reserve_test_str));
>
> /* Write a placeholder, (to be replaced later via overwrite_bytes) */
> str_offset = blob->size;
> --
> 2.5.0.400.gff86faf
>
> _______________________________________________
> 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
