[Mesa-dev] [PATCH 06/11] compiler/blob: make blob_reserve_bytes() more useful

[Jason Ekstrand]

From: Connor Abbott <cwabbott0 at 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
---
 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