[v2,3/3] u_dynarray: turn util_dynarray_{grow, resize} into element-oriented macros

Submitted by Nicolai Hähnle on May 13, 2019, 2:58 p.m.

Details

Message ID 20190513145808.11024-4-nhaehnle@gmail.com
State New
Headers show
Series "u_dynarray: minor API cleanups" ( rev: 2 ) in Mesa

Not browsing as part of any series.

Commit Message

Nicolai Hähnle May 13, 2019, 2:58 p.m.
From: Nicolai Hähnle <nicolai.haehnle@amd.com>

The main motivation for this change is API ergonomics: most operations
on dynarrays are really on elements, not on bytes, so it's weird to have
grow and resize as the odd operations out.

The secondary motivation is memory safety. Users of the old byte-oriented
functions would often multiply a number of elements with the element size,
which could overflow, and checking for overflow is tedious.

With this change, we only need to implement the overflow checks once.
The checks are cheap: since eltsize is a compile-time constant and the
functions should be inlined, they only add a single comparison and an
unlikely branch.

v2:
- ensure operations are no-op when allocation fails
- in util_dynarray_clone, call resize_bytes with a compile-time constant element size
---
 .../drivers/nouveau/nv30/nvfx_fragprog.c      |  2 +-
 src/gallium/drivers/nouveau/nv50/nv50_state.c |  5 +-
 src/gallium/drivers/nouveau/nvc0/nvc0_state.c |  5 +-
 .../compiler/brw_nir_analyze_ubo_ranges.c     |  2 +-
 src/mesa/drivers/dri/i965/brw_bufmgr.c        |  4 +-
 src/util/u_dynarray.h                         | 46 +++++++++++++------
 6 files changed, 40 insertions(+), 24 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c
index 86e3599325e..2bcb62b97d8 100644
--- a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c
+++ b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c
@@ -66,21 +66,21 @@  release_temps(struct nvfx_fpc *fpc)
    fpc->r_temps &= ~fpc->r_temps_discard;
    fpc->r_temps_discard = 0ULL;
 }
 
 static inline struct nvfx_reg
 nvfx_fp_imm(struct nvfx_fpc *fpc, float a, float b, float c, float d)
 {
    float v[4] = {a, b, c, d};
    int idx = fpc->imm_data.size >> 4;
 
-   memcpy(util_dynarray_grow(&fpc->imm_data, sizeof(float) * 4), v, 4 * sizeof(float));
+   memcpy(util_dynarray_grow(&fpc->imm_data, float, 4), v, 4 * sizeof(float));
    return nvfx_reg(NVFXSR_IMM, idx);
 }
 
 static void
 grow_insns(struct nvfx_fpc *fpc, int size)
 {
    struct nv30_fragprog *fp = fpc->fp;
 
    fp->insn_len += size;
    fp->insn = realloc(fp->insn, sizeof(uint32_t) * fp->insn_len);
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c b/src/gallium/drivers/nouveau/nv50/nv50_state.c
index 55167a27c09..228feced5d1 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
@@ -1256,24 +1256,23 @@  nv50_set_global_bindings(struct pipe_context *pipe,
                          struct pipe_resource **resources,
                          uint32_t **handles)
 {
    struct nv50_context *nv50 = nv50_context(pipe);
    struct pipe_resource **ptr;
    unsigned i;
    const unsigned end = start + nr;
 
    if (nv50->global_residents.size <= (end * sizeof(struct pipe_resource *))) {
       const unsigned old_size = nv50->global_residents.size;
-      const unsigned req_size = end * sizeof(struct pipe_resource *);
-      util_dynarray_resize(&nv50->global_residents, req_size);
+      util_dynarray_resize(&nv50->global_residents, struct pipe_resource *, end);
       memset((uint8_t *)nv50->global_residents.data + old_size, 0,
-             req_size - old_size);
+             nv50->global_residents.size - old_size);
    }
 
    if (resources) {
       ptr = util_dynarray_element(
          &nv50->global_residents, struct pipe_resource *, start);
       for (i = 0; i < nr; ++i) {
          pipe_resource_reference(&ptr[i], resources[i]);
          nv50_set_global_handle(handles[i], resources[i]);
       }
    } else {
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
index 12e21862ee0..2ab51c8529e 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
@@ -1363,24 +1363,23 @@  nvc0_set_global_bindings(struct pipe_context *pipe,
                          struct pipe_resource **resources,
                          uint32_t **handles)
 {
    struct nvc0_context *nvc0 = nvc0_context(pipe);
    struct pipe_resource **ptr;
    unsigned i;
    const unsigned end = start + nr;
 
    if (nvc0->global_residents.size <= (end * sizeof(struct pipe_resource *))) {
       const unsigned old_size = nvc0->global_residents.size;
-      const unsigned req_size = end * sizeof(struct pipe_resource *);
-      util_dynarray_resize(&nvc0->global_residents, req_size);
+      util_dynarray_resize(&nvc0->global_residents, struct pipe_resource *, end);
       memset((uint8_t *)nvc0->global_residents.data + old_size, 0,
-             req_size - old_size);
+             nvc0->global_residents.size - old_size);
    }
 
    if (resources) {
       ptr = util_dynarray_element(
          &nvc0->global_residents, struct pipe_resource *, start);
       for (i = 0; i < nr; ++i) {
          pipe_resource_reference(&ptr[i], resources[i]);
          nvc0_set_global_handle(handles[i], resources[i]);
       }
    } else {
diff --git a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
index ab7a2705c9a..4c5e03380e1 100644
--- a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
+++ b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
@@ -274,21 +274,21 @@  brw_nir_analyze_ubo_ranges(const struct brw_compiler *compiler,
              * bitfield.  There are no more ranges to process.
              */
             first_hole = 64;
             offsets = 0;
          } else {
             /* We've processed all bits before first_hole.  Mask them off. */
             offsets &= ~((1ull << first_hole) - 1);
          }
 
          struct ubo_range_entry *entry =
-            util_dynarray_grow(&ranges, sizeof(struct ubo_range_entry));
+            util_dynarray_grow(&ranges, struct ubo_range_entry, 1);
 
          entry->range.block = b;
          entry->range.start = first_bit;
          /* first_hole is one beyond the end, so we don't need to add 1 */
          entry->range.length = first_hole - first_bit;
          entry->benefit = 0;
 
          for (int i = 0; i < entry->range.length; i++)
             entry->benefit += info->uses[first_bit + i];
       }
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index 7b0ddfb64dd..a89db7b8e40 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -287,21 +287,21 @@  bucket_vma_alloc(struct brw_bufmgr *bufmgr,
        * memory for 64 blocks from a larger allocator (either a larger
        * bucket or util_vma).
        *
        * We align the address to the node size (64 blocks) so that
        * bucket_vma_free can easily compute the starting address of this
        * block by rounding any address we return down to the node size.
        *
        * Set the first bit used, and return the start address.
        */
       uint64_t node_size = 64ull * bucket->size;
-      node = util_dynarray_grow(vma_list, sizeof(struct vma_bucket_node));
+      node = util_dynarray_grow(vma_list, struct vma_bucket_node, 1);
 
       if (unlikely(!node))
          return 0ull;
 
       uint64_t addr = vma_alloc(bufmgr, memzone, node_size, node_size);
       node->start_address = gen_48b_address(addr);
       node->bitmap = ~1ull;
       return node->start_address;
    }
 
@@ -344,21 +344,21 @@  bucket_vma_free(struct bo_cache_bucket *bucket, uint64_t address)
 
    util_dynarray_foreach(vma_list, struct vma_bucket_node, cur) {
       if (cur->start_address == start) {
          node = cur;
          break;
       }
    }
 
    if (!node) {
       /* No node - the whole group of 64 blocks must have been in-use. */
-      node = util_dynarray_grow(vma_list, sizeof(struct vma_bucket_node));
+      node = util_dynarray_grow(vma_list, struct vma_bucket_node, 1);
 
       if (unlikely(!node))
          return; /* bogus, leaks some GPU VMA, but nothing we can do... */
 
       node->start_address = start;
       node->bitmap = 0ull;
    }
 
    /* Set the bit to return the memory. */
    assert((node->bitmap & (1ull << bit)) == 0ull);
diff --git a/src/util/u_dynarray.h b/src/util/u_dynarray.h
index 769c3820546..000feaa8349 100644
--- a/src/util/u_dynarray.h
+++ b/src/util/u_dynarray.h
@@ -22,20 +22,21 @@ 
  * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
  * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  *
  **************************************************************************/
 
 #ifndef U_DYNARRAY_H
 #define U_DYNARRAY_H
 
 #include <stdlib.h>
 #include <string.h>
+#include <limits.h>
 #include "ralloc.h"
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 /* A zero-initialized version of this is guaranteed to represent an
  * empty array.
  *
  * Also, size <= capacity and data != 0 if and only if capacity != 0
@@ -92,49 +93,63 @@  util_dynarray_ensure_cap(struct util_dynarray *buf, unsigned newcap)
       if (!data)
          return 0;
 
       buf->data = data;
       buf->capacity = capacity;
    }
 
    return (void *)((char *)buf->data + buf->size);
 }
 
-static inline void *
-util_dynarray_grow_cap(struct util_dynarray *buf, int diff)
-{
-   return util_dynarray_ensure_cap(buf, buf->size + diff);
-}
-
 /* use util_dynarray_trim to reduce the allocated storage */
-static inline void *
-util_dynarray_resize(struct util_dynarray *buf, unsigned newsize)
+MUST_CHECK static inline void *
+util_dynarray_resize_bytes(struct util_dynarray *buf, unsigned nelts, size_t eltsize)
 {
+   if (unlikely(nelts > UINT_MAX / eltsize))
+      return 0;
+
+   unsigned newsize = nelts * eltsize;
    void *p = util_dynarray_ensure_cap(buf, newsize);
+   if (!p)
+      return 0;
+
    buf->size = newsize;
 
    return p;
 }
 
 static inline void
 util_dynarray_clone(struct util_dynarray *buf, void *mem_ctx,
                     struct util_dynarray *from_buf)
 {
    util_dynarray_init(buf, mem_ctx);
-   util_dynarray_resize(buf, from_buf->size);
-   memcpy(buf->data, from_buf->data, from_buf->size);
+   if (util_dynarray_resize_bytes(buf, from_buf->size, 1))
+      memcpy(buf->data, from_buf->data, from_buf->size);
 }
 
-static inline void *
-util_dynarray_grow(struct util_dynarray *buf, int diff)
+MUST_CHECK static inline void *
+util_dynarray_grow_bytes(struct util_dynarray *buf, unsigned ngrow, size_t eltsize)
 {
-   return util_dynarray_resize(buf, buf->size + diff);
+   unsigned growbytes = ngrow * eltsize;
+
+   if (unlikely(ngrow > (UINT_MAX / eltsize) ||
+                growbytes > UINT_MAX - buf->size))
+      return 0;
+
+   unsigned newsize = buf->size + growbytes;
+   void *p = util_dynarray_ensure_cap(buf, newsize);
+   if (!p)
+      return 0;
+
+   buf->size = newsize;
+
+   return p;
 }
 
 static inline void
 util_dynarray_trim(struct util_dynarray *buf)
 {
    if (buf->size != buf->capacity) {
       if (buf->size) {
          if (buf->mem_ctx) {
             buf->data = reralloc_size(buf->mem_ctx, buf->data, buf->size);
          } else {
@@ -146,21 +161,24 @@  util_dynarray_trim(struct util_dynarray *buf)
             ralloc_free(buf->data);
          } else {
             free(buf->data);
          }
          buf->data = NULL;
          buf->capacity = 0;
       }
    }
 }
 
-#define util_dynarray_append(buf, type, v) do {type __v = (v); memcpy(util_dynarray_grow((buf), sizeof(type)), &__v, sizeof(type));} while(0)
+#define util_dynarray_append(buf, type, v) do {type __v = (v); memcpy(util_dynarray_grow_bytes((buf), 1, sizeof(type)), &__v, sizeof(type));} while(0)
+/* Returns a pointer to the space of the first new element (in case of growth) or NULL on failure. */
+#define util_dynarray_resize(buf, type, nelts) util_dynarray_resize_bytes(buf, (nelts), sizeof(type))
+#define util_dynarray_grow(buf, type, ngrow) util_dynarray_grow_bytes(buf, (ngrow), sizeof(type))
 #define util_dynarray_top_ptr(buf, type) (type*)((char*)(buf)->data + (buf)->size - sizeof(type))
 #define util_dynarray_top(buf, type) *util_dynarray_top_ptr(buf, type)
 #define util_dynarray_pop_ptr(buf, type) (type*)((char*)(buf)->data + ((buf)->size -= sizeof(type)))
 #define util_dynarray_pop(buf, type) *util_dynarray_pop_ptr(buf, type)
 #define util_dynarray_contains(buf, type) ((buf)->size >= sizeof(type))
 #define util_dynarray_element(buf, type, idx) ((type*)(buf)->data + (idx))
 #define util_dynarray_begin(buf) ((buf)->data)
 #define util_dynarray_end(buf) ((void*)util_dynarray_element((buf), char, (buf)->size))
 #define util_dynarray_num_elements(buf, type) ((buf)->size / sizeof(type))