From f85852f42d1fd98c5fe19b690fd5c286abfd7516 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Wed, 5 Jun 2024 11:01:00 -0700 Subject: [PATCH 1/5] stacks on stacks on stacks --- source/memtrace.c | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/source/memtrace.c b/source/memtrace.c index fbdcf86df..b7080673a 100644 --- a/source/memtrace.c +++ b/source/memtrace.c @@ -134,7 +134,7 @@ static void s_alloc_tracer_track(struct alloc_tracer *tracer, void *ptr, size_t /* capture stack frames, skip 2 for this function and the allocation vtable function */ AWS_VARIABLE_LENGTH_ARRAY(void *, stack_frames, (FRAMES_TO_SKIP + tracer->frames_per_stack)); size_t stack_depth = aws_backtrace(stack_frames, FRAMES_TO_SKIP + tracer->frames_per_stack); - if (stack_depth >= FRAMES_TO_SKIP) { + if (stack_depth > 0) { /* hash the stack pointers */ struct aws_byte_cursor stack_cursor = aws_byte_cursor_from_array(stack_frames, stack_depth * sizeof(void *)); @@ -154,12 +154,27 @@ static void s_alloc_tracer_track(struct alloc_tracer *tracer, void *ptr, size_t 1, sizeof(struct stack_trace) + (sizeof(void *) * tracer->frames_per_stack)); AWS_FATAL_ASSERT(stack); - memcpy( - (void **)&stack->frames[0], - &stack_frames[FRAMES_TO_SKIP], - (stack_depth - FRAMES_TO_SKIP) * sizeof(void *)); - stack->depth = stack_depth - FRAMES_TO_SKIP; - item->value = stack; + /** + * Optimizations can affect the number the number of frames we get and in pathological cases we can + * get fewer than FRAMES_TO_SKIP frames, but always at least 1 because code has to start somewhere. + * (looking at you gcc with -O3 on aarch64) + * With optimizations on we cannot trust the stack trace too much. + * Memtracer makes an assumption that stack trace will be available in all cases if stack trace api + * works. So in the pathological case of stack_depth <= FRAMES_TO_SKIP lets record all the frames we + * have, to at least have an anchor for where allocation is comming from, however inaccurate it is. + */ + if (stack_depth <= FRAMES_TO_SKIP) { + memcpy((void **)&stack->frames[0], &stack_frames[0], (stack_depth) * sizeof(void *)); + stack->depth = stack_depth; + item->value = stack; + } else { + memcpy( + (void **)&stack->frames[0], + &stack_frames[FRAMES_TO_SKIP], + (stack_depth - FRAMES_TO_SKIP) * sizeof(void *)); + stack->depth = stack_depth - FRAMES_TO_SKIP; + item->value = stack; + } } aws_mutex_unlock(&tracer->mutex); } From e270eedc5f359671c9944e40fa335843a6d4f12e Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Wed, 5 Jun 2024 11:10:44 -0700 Subject: [PATCH 2/5] if only i could spell --- source/memtrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/memtrace.c b/source/memtrace.c index b7080673a..c3dc43b8c 100644 --- a/source/memtrace.c +++ b/source/memtrace.c @@ -155,7 +155,7 @@ static void s_alloc_tracer_track(struct alloc_tracer *tracer, void *ptr, size_t sizeof(struct stack_trace) + (sizeof(void *) * tracer->frames_per_stack)); AWS_FATAL_ASSERT(stack); /** - * Optimizations can affect the number the number of frames we get and in pathological cases we can + * Optimizations can affect the number of frames we get and in pathological cases we can * get fewer than FRAMES_TO_SKIP frames, but always at least 1 because code has to start somewhere. * (looking at you gcc with -O3 on aarch64) * With optimizations on we cannot trust the stack trace too much. From 9f2537883cb4616abb0b6807f344c67358b29cd8 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Wed, 5 Jun 2024 11:32:33 -0700 Subject: [PATCH 3/5] comments --- source/memtrace.c | 88 +++++++++++++++++++++++++---------------------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/source/memtrace.c b/source/memtrace.c index c3dc43b8c..322128d55 100644 --- a/source/memtrace.c +++ b/source/memtrace.c @@ -131,51 +131,55 @@ static void s_alloc_tracer_track(struct alloc_tracer *tracer, void *ptr, size_t aws_high_res_clock_get_ticks(&alloc->time); if (tracer->level == AWS_MEMTRACE_STACKS) { - /* capture stack frames, skip 2 for this function and the allocation vtable function */ + /* capture stack frames, + * skip 2 for this function and the allocation vtable function if we have a full stack trace + * and otherwise just capture what ever stack trace we got + */ AWS_VARIABLE_LENGTH_ARRAY(void *, stack_frames, (FRAMES_TO_SKIP + tracer->frames_per_stack)); size_t stack_depth = aws_backtrace(stack_frames, FRAMES_TO_SKIP + tracer->frames_per_stack); - if (stack_depth > 0) { - /* hash the stack pointers */ - struct aws_byte_cursor stack_cursor = - aws_byte_cursor_from_array(stack_frames, stack_depth * sizeof(void *)); - uint64_t stack_id = aws_hash_byte_cursor_ptr(&stack_cursor); - alloc->stack = stack_id; /* associate the stack with the alloc */ - - aws_mutex_lock(&tracer->mutex); - struct aws_hash_element *item = NULL; - int was_created = 0; - AWS_FATAL_ASSERT( - AWS_OP_SUCCESS == - aws_hash_table_create(&tracer->stacks, (void *)(uintptr_t)stack_id, &item, &was_created)); - /* If this is a new stack, save it to the hash */ - if (was_created) { - struct stack_trace *stack = aws_mem_calloc( - aws_default_allocator(), - 1, - sizeof(struct stack_trace) + (sizeof(void *) * tracer->frames_per_stack)); - AWS_FATAL_ASSERT(stack); - /** - * Optimizations can affect the number of frames we get and in pathological cases we can - * get fewer than FRAMES_TO_SKIP frames, but always at least 1 because code has to start somewhere. - * (looking at you gcc with -O3 on aarch64) - * With optimizations on we cannot trust the stack trace too much. - * Memtracer makes an assumption that stack trace will be available in all cases if stack trace api - * works. So in the pathological case of stack_depth <= FRAMES_TO_SKIP lets record all the frames we - * have, to at least have an anchor for where allocation is comming from, however inaccurate it is. - */ - if (stack_depth <= FRAMES_TO_SKIP) { - memcpy((void **)&stack->frames[0], &stack_frames[0], (stack_depth) * sizeof(void *)); - stack->depth = stack_depth; - item->value = stack; - } else { - memcpy( - (void **)&stack->frames[0], - &stack_frames[FRAMES_TO_SKIP], - (stack_depth - FRAMES_TO_SKIP) * sizeof(void *)); - stack->depth = stack_depth - FRAMES_TO_SKIP; - item->value = stack; - } + AWS_FATAL_ASSERT(stack_depth > 0); + + /* hash the stack pointers */ + struct aws_byte_cursor stack_cursor = + aws_byte_cursor_from_array(stack_frames, stack_depth * sizeof(void *)); + uint64_t stack_id = aws_hash_byte_cursor_ptr(&stack_cursor); + alloc->stack = stack_id; /* associate the stack with the alloc */ + + aws_mutex_lock(&tracer->mutex); + struct aws_hash_element *item = NULL; + int was_created = 0; + AWS_FATAL_ASSERT( + AWS_OP_SUCCESS == + aws_hash_table_create(&tracer->stacks, (void *)(uintptr_t)stack_id, &item, &was_created)); + /* If this is a new stack, save it to the hash */ + if (was_created) { + struct stack_trace *stack = aws_mem_calloc( + aws_default_allocator(), + 1, + sizeof(struct stack_trace) + (sizeof(void *) * tracer->frames_per_stack)); + AWS_FATAL_ASSERT(stack); + /** + * Optimizations can affect the number of frames we get and in pathological cases we can + * get fewer than FRAMES_TO_SKIP frames, but always at least 1 because code has to start somewhere. + * (looking at you gcc with -O3 on aarch64) + * With optimizations on we cannot trust the stack trace too much. + * Memtracer makes an assumption that stack trace will be available in all cases if stack trace api + * works. So in the pathological case of stack_depth <= FRAMES_TO_SKIP lets record all the frames we + * have, to at least have an anchor for where allocation is comming from, however inaccurate it is. + */ + if (stack_depth <= FRAMES_TO_SKIP) { + memcpy((void **)&stack->frames[0], &stack_frames[0], (stack_depth) * sizeof(void *)); + stack->depth = stack_depth; + item->value = stack; + } else { + memcpy( + (void **)&stack->frames[0], + &stack_frames[FRAMES_TO_SKIP], + (stack_depth - FRAMES_TO_SKIP) * sizeof(void *)); + stack->depth = stack_depth - FRAMES_TO_SKIP; + item->value = stack; } + aws_mutex_unlock(&tracer->mutex); } } From d32cd09dbea1c78d42b89be538761ce640356799 Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Wed, 5 Jun 2024 11:41:10 -0700 Subject: [PATCH 4/5] lint --- source/memtrace.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/source/memtrace.c b/source/memtrace.c index 322128d55..90afd0eab 100644 --- a/source/memtrace.c +++ b/source/memtrace.c @@ -131,17 +131,16 @@ static void s_alloc_tracer_track(struct alloc_tracer *tracer, void *ptr, size_t aws_high_res_clock_get_ticks(&alloc->time); if (tracer->level == AWS_MEMTRACE_STACKS) { - /* capture stack frames, - * skip 2 for this function and the allocation vtable function if we have a full stack trace - * and otherwise just capture what ever stack trace we got - */ + /* capture stack frames, + * skip 2 for this function and the allocation vtable function if we have a full stack trace + * and otherwise just capture what ever stack trace we got + */ AWS_VARIABLE_LENGTH_ARRAY(void *, stack_frames, (FRAMES_TO_SKIP + tracer->frames_per_stack)); size_t stack_depth = aws_backtrace(stack_frames, FRAMES_TO_SKIP + tracer->frames_per_stack); AWS_FATAL_ASSERT(stack_depth > 0); /* hash the stack pointers */ - struct aws_byte_cursor stack_cursor = - aws_byte_cursor_from_array(stack_frames, stack_depth * sizeof(void *)); + struct aws_byte_cursor stack_cursor = aws_byte_cursor_from_array(stack_frames, stack_depth * sizeof(void *)); uint64_t stack_id = aws_hash_byte_cursor_ptr(&stack_cursor); alloc->stack = stack_id; /* associate the stack with the alloc */ @@ -149,14 +148,11 @@ static void s_alloc_tracer_track(struct alloc_tracer *tracer, void *ptr, size_t struct aws_hash_element *item = NULL; int was_created = 0; AWS_FATAL_ASSERT( - AWS_OP_SUCCESS == - aws_hash_table_create(&tracer->stacks, (void *)(uintptr_t)stack_id, &item, &was_created)); + AWS_OP_SUCCESS == aws_hash_table_create(&tracer->stacks, (void *)(uintptr_t)stack_id, &item, &was_created)); /* If this is a new stack, save it to the hash */ if (was_created) { struct stack_trace *stack = aws_mem_calloc( - aws_default_allocator(), - 1, - sizeof(struct stack_trace) + (sizeof(void *) * tracer->frames_per_stack)); + aws_default_allocator(), 1, sizeof(struct stack_trace) + (sizeof(void *) * tracer->frames_per_stack)); AWS_FATAL_ASSERT(stack); /** * Optimizations can affect the number of frames we get and in pathological cases we can From d2612146a5a9f48747ae136a311cb736ce1a2b4c Mon Sep 17 00:00:00 2001 From: DmitriyMusatkin Date: Wed, 5 Jun 2024 11:49:01 -0700 Subject: [PATCH 5/5] wrong curly --- source/memtrace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/memtrace.c b/source/memtrace.c index 90afd0eab..ee1f291d7 100644 --- a/source/memtrace.c +++ b/source/memtrace.c @@ -175,9 +175,9 @@ static void s_alloc_tracer_track(struct alloc_tracer *tracer, void *ptr, size_t stack->depth = stack_depth - FRAMES_TO_SKIP; item->value = stack; } - - aws_mutex_unlock(&tracer->mutex); } + + aws_mutex_unlock(&tracer->mutex); } aws_mutex_lock(&tracer->mutex);