From de8a919702a377207e83434ab40d91c3013ed96c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Nicolai=20H=C3=A4hnle?= Date: Mon, 13 May 2019 16:58:08 +0200 Subject: [PATCH] u_dynarray: turn util_dynarray_{grow, resize} into element-oriented macros MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- src/gallium/drivers/iris/iris_fence.c | 4 +- src/gallium/drivers/lima/lima_draw.c | 6 +-- src/gallium/drivers/lima/lima_submit.c | 4 +- .../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 +- .../drivers/panfrost/midgard/midgard_emit.c | 4 +- .../compiler/brw_nir_analyze_ubo_ranges.c | 2 +- src/mesa/drivers/dri/i965/brw_bufmgr.c | 4 +- src/util/u_dynarray.h | 46 +++++++++++++------ 10 files changed, 49 insertions(+), 33 deletions(-) diff --git a/src/gallium/drivers/iris/iris_fence.c b/src/gallium/drivers/iris/iris_fence.c index 06452f70966..145b3e48dd1 100644 --- 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 c97258c592f..c87542d24bb 100644 --- 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 83c78bf82da..c54289d1470 100644 --- 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 86e3599325e..2bcb62b97d8 100644 --- 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 55167a27c09..228feced5d1 100644 --- 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 12e21862ee0..2ab51c8529e 100644 --- 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 bf6ee4508f5..3c331551dd8 100644 --- 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 ab7a2705c9a..4c5e03380e1 100644 --- 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 b0e5198aad1..a7c68406315 100644 --- 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 769c3820546..000feaa8349 100644 --- a/src/util/u_dynarray.h +++ b/src/util/u_dynarray.h @@ -29,6 +29,7 @@ #include #include +#include #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))) -- 2.30.2