u_dynarray: turn util_dynarray_{grow, resize} into element-oriented macros

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

v3:
- fix iris, lima, panfrost

Reviewed-by: Marek Olšák <marek.olsak@amd.com>

diff --git a/src/gallium/drivers/iris/iris_fence.c b/src/gallium/drivers/iris/iris_fence.c
index 06452f709669d905e77daeffe402682e6d3f4fc9..145b3e48dd174ba58e61a06320335811894cb204 100644 (file)
--- a/src/gallium/drivers/iris/iris_fence.c
+++ b/src/gallium/drivers/iris/iris_fence.c
@@ -93,7 +93,7 @@ iris_batch_add_syncpt(struct iris_batch *batch,
                       unsigned flags)
 {
    struct drm_i915_gem_exec_fence *fence =
-      util_dynarray_grow(&batch->exec_fences, sizeof(*fence));
+      util_dynarray_grow(&batch->exec_fences, struct drm_i915_gem_exec_fence, 1);
 
    *fence = (struct drm_i915_gem_exec_fence) {
       .handle = syncpt->handle,
@@ -101,7 +101,7 @@ iris_batch_add_syncpt(struct iris_batch *batch,
    };
 
    struct iris_syncpt **store =
-      util_dynarray_grow(&batch->syncpts, sizeof(*store));
+      util_dynarray_grow(&batch->syncpts, struct iris_syncpt *, 1);
 
    *store = NULL;
    iris_syncpt_reference(batch->screen, store, syncpt);

diff --git a/src/gallium/drivers/lima/lima_draw.c b/src/gallium/drivers/lima/lima_draw.c
index c97258c592f2194edc1c943986c08f6278b70852..c87542d24bb462e16e55640338d81e6cf4539743 100644 (file)
--- a/src/gallium/drivers/lima/lima_draw.c
+++ b/src/gallium/drivers/lima/lima_draw.c
@@ -123,7 +123,7 @@ struct lima_render_state {
 /* plbu commands */
 #define PLBU_CMD_BEGIN(max) { \
    int i = 0, max_n = max; \
-   uint32_t *plbu_cmd = util_dynarray_grow_cap(&ctx->plbu_cmd_array, max_n * 4);
+   uint32_t *plbu_cmd = util_dynarray_ensure_cap(&ctx->plbu_cmd_array, ctx->plbu_cmd_array.size + max_n * 4);
 
 #define PLBU_CMD_END() \
    assert(i <= max_n); \
@@ -172,7 +172,7 @@ struct lima_render_state {
 /* vs commands */
 #define VS_CMD_BEGIN(max) { \
    int i = 0, max_n = max; \
-   uint32_t *vs_cmd = util_dynarray_grow_cap(&ctx->vs_cmd_array, max_n * 4);
+   uint32_t *vs_cmd = util_dynarray_ensure_cap(&ctx->vs_cmd_array, ctx->vs_cmd_array.size + max_n * 4);
 
 #define VS_CMD_END() \
    assert(i <= max_n); \
@@ -1425,7 +1425,7 @@ static void
 lima_finish_plbu_cmd(struct lima_context *ctx)
 {
    int i = 0;
-   uint32_t *plbu_cmd = util_dynarray_grow_cap(&ctx->plbu_cmd_array, 2 * 4);
+   uint32_t *plbu_cmd = util_dynarray_ensure_cap(&ctx->plbu_cmd_array, ctx->plbu_cmd_array.size + 2 * 4);
 
    plbu_cmd[i++] = 0x00000000;
    plbu_cmd[i++] = 0x50000000; /* END */

diff --git a/src/gallium/drivers/lima/lima_submit.c b/src/gallium/drivers/lima/lima_submit.c
index 83c78bf82dafdd3e256515e681840ad8e22d5c74..c54289d1470d2b9c8547174fa50ecee64665d615 100644 (file)
--- a/src/gallium/drivers/lima/lima_submit.c
+++ b/src/gallium/drivers/lima/lima_submit.c
@@ -106,11 +106,11 @@ bool lima_submit_add_bo(struct lima_submit *submit, struct lima_bo *bo, uint32_t
    }
 
    struct drm_lima_gem_submit_bo *submit_bo =
-      util_dynarray_grow(&submit->gem_bos, sizeof(*submit_bo));
+      util_dynarray_grow(&submit->gem_bos, struct drm_lima_gem_submit_bo, 1);
    submit_bo->handle = bo->handle;
    submit_bo->flags = flags;
 
-   struct lima_bo **jbo = util_dynarray_grow(&submit->bos, sizeof(*jbo));
+   struct lima_bo **jbo = util_dynarray_grow(&submit->bos, struct lima_bo, 1);
    *jbo = bo;
 
    /* prevent bo from being freed when submit start */

diff --git a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c
index 86e3599325e65eb81de0909b9bafe8863c2da56a..2bcb62b97d8c7086b909e9c5b59c7ac11d766f37 100644 (file)
--- a/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c
+++ b/src/gallium/drivers/nouveau/nv30/nvfx_fragprog.c
@@ -73,7 +73,7 @@ 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);
 }
 

diff --git a/src/gallium/drivers/nouveau/nv50/nv50_state.c b/src/gallium/drivers/nouveau/nv50/nv50_state.c
index 55167a27c095acac5be6238302e85fe1848d19a0..228feced5d1785a1074e0d22f77ab48af1c7c0a5 100644 (file)
--- a/src/gallium/drivers/nouveau/nv50/nv50_state.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_state.c
@@ -1263,10 +1263,9 @@ nv50_set_global_bindings(struct pipe_context *pipe,
 
    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) {

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
index 12e21862ee028939aa400fd49b3b3e3f6f87edcd..2ab51c8529e303c5437ed32040e4d9c45d5544aa 100644 (file)
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state.c
@@ -1370,10 +1370,9 @@ nvc0_set_global_bindings(struct pipe_context *pipe,
 
    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) {

diff --git a/src/gallium/drivers/panfrost/midgard/midgard_emit.c b/src/gallium/drivers/panfrost/midgard/midgard_emit.c
index bf6ee4508f59ed8d4beae7056b1bec19408e44ce..3c331551dd8db981baa0afc73d30a0d9ee40c19d 100644 (file)
--- a/src/gallium/drivers/panfrost/midgard/midgard_emit.c
+++ b/src/gallium/drivers/panfrost/midgard/midgard_emit.c
@@ -147,11 +147,11 @@ emit_alu_bundle(compiler_context *ctx,
                         source = &scalarized;
                 }
 
-                memcpy(util_dynarray_grow(emission, size), source, size);
+                memcpy(util_dynarray_grow_bytes(emission, 1, size), source, size);
         }
 
         /* Emit padding (all zero) */
-        memset(util_dynarray_grow(emission, bundle->padding), 0, bundle->padding);
+        memset(util_dynarray_grow_bytes(emission, 1, bundle->padding), 0, bundle->padding);
 
         /* Tack on constants */
 

diff --git a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
index ab7a2705c9a5d2c472c5e948b9a772b1931e6a56..4c5e03380e1feef75032bae5d8c736343b8e0697 100644 (file)
--- a/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
+++ b/src/intel/compiler/brw_nir_analyze_ubo_ranges.c
@@ -281,7 +281,7 @@ brw_nir_analyze_ubo_ranges(const struct brw_compiler *compiler,
          }
 
          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;

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index b0e5198aad15fa9c3bb420f4cee9d06d80aa6ff7..a7c6840631580710af565febbedfb96c6f4bf99f 100644 (file)
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -294,7 +294,7 @@ bucket_vma_alloc(struct brw_bufmgr *bufmgr,
        * 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;
@@ -351,7 +351,7 @@ bucket_vma_free(struct bo_cache_bucket *bucket, uint64_t address)
 
    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... */

diff --git a/src/util/u_dynarray.h b/src/util/u_dynarray.h
index 769c38205460374216397cc6f0fb599ed58eb19f..000feaa834939e9baa958dc4240d2d67c28f4fcd 100644 (file)
--- a/src/util/u_dynarray.h
+++ b/src/util/u_dynarray.h
@@ -29,6 +29,7 @@
 
 #include <stdlib.h>
 #include <string.h>
+#include <limits.h>
 #include "ralloc.h"
 
 #ifdef __cplusplus
@@ -99,17 +100,18 @@ util_dynarray_ensure_cap(struct util_dynarray *buf, unsigned newcap)
    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;
@@ -120,14 +122,27 @@ 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
@@ -153,7 +168,10 @@ util_dynarray_trim(struct util_dynarray *buf)
    }
 }
 
-#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)))