From 46531964d07473cf959a6f43490b4caef78b394c Mon Sep 17 00:00:00 2001 From: Alistair Leslie-Hughes Date: Fri, 17 Nov 2023 12:53:31 +1100 Subject: [PATCH] Improve DynamicBVH code to make it clearer how the stack/heap works. Inspired by a Coverity issue about possible memcpy usage and overlapping memory. --- core/math/dynamic_bvh.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/core/math/dynamic_bvh.h b/core/math/dynamic_bvh.h index dbc1cb31de5..9b49fcc3c80 100644 --- a/core/math/dynamic_bvh.h +++ b/core/math/dynamic_bvh.h @@ -328,7 +328,8 @@ void DynamicBVH::aabb_query(const AABB &p_box, QueryResult &r_result) { volume.min = p_box.position; volume.max = p_box.position + p_box.size; - const Node **stack = (const Node **)alloca(ALLOCA_STACK_SIZE * sizeof(const Node *)); + const Node **alloca_stack = (const Node **)alloca(ALLOCA_STACK_SIZE * sizeof(const Node *)); + const Node **stack = alloca_stack; stack[0] = bvh_root; int32_t depth = 1; int32_t threshold = ALLOCA_STACK_SIZE - 2; @@ -343,7 +344,8 @@ void DynamicBVH::aabb_query(const AABB &p_box, QueryResult &r_result) { if (depth > threshold) { if (aux_stack.is_empty()) { aux_stack.resize(ALLOCA_STACK_SIZE * 2); - memcpy(aux_stack.ptr(), stack, ALLOCA_STACK_SIZE * sizeof(const Node *)); + memcpy(aux_stack.ptr(), alloca_stack, ALLOCA_STACK_SIZE * sizeof(const Node *)); + alloca_stack = nullptr; } else { aux_stack.resize(aux_stack.size() * 2); } @@ -384,7 +386,8 @@ void DynamicBVH::convex_query(const Plane *p_planes, int p_plane_count, const Ve } } - const Node **stack = (const Node **)alloca(ALLOCA_STACK_SIZE * sizeof(const Node *)); + const Node **alloca_stack = (const Node **)alloca(ALLOCA_STACK_SIZE * sizeof(const Node *)); + const Node **stack = alloca_stack; stack[0] = bvh_root; int32_t depth = 1; int32_t threshold = ALLOCA_STACK_SIZE - 2; @@ -399,7 +402,8 @@ void DynamicBVH::convex_query(const Plane *p_planes, int p_plane_count, const Ve if (depth > threshold) { if (aux_stack.is_empty()) { aux_stack.resize(ALLOCA_STACK_SIZE * 2); - memcpy(aux_stack.ptr(), stack, ALLOCA_STACK_SIZE * sizeof(const Node *)); + memcpy(aux_stack.ptr(), alloca_stack, ALLOCA_STACK_SIZE * sizeof(const Node *)); + alloca_stack = nullptr; } else { aux_stack.resize(aux_stack.size() * 2); } @@ -436,7 +440,8 @@ void DynamicBVH::ray_query(const Vector3 &p_from, const Vector3 &p_to, QueryResu Vector3 bounds[2]; - const Node **stack = (const Node **)alloca(ALLOCA_STACK_SIZE * sizeof(const Node *)); + const Node **alloca_stack = (const Node **)alloca(ALLOCA_STACK_SIZE * sizeof(const Node *)); + const Node **stack = alloca_stack; stack[0] = bvh_root; int32_t depth = 1; int32_t threshold = ALLOCA_STACK_SIZE - 2; @@ -456,7 +461,8 @@ void DynamicBVH::ray_query(const Vector3 &p_from, const Vector3 &p_to, QueryResu if (depth > threshold) { if (aux_stack.is_empty()) { aux_stack.resize(ALLOCA_STACK_SIZE * 2); - memcpy(aux_stack.ptr(), stack, ALLOCA_STACK_SIZE * sizeof(const Node *)); + memcpy(aux_stack.ptr(), alloca_stack, ALLOCA_STACK_SIZE * sizeof(const Node *)); + alloca_stack = nullptr; } else { aux_stack.resize(aux_stack.size() * 2); }