> > Hello. > > > > As noticed in the PR, it's quite tricky to not run malloc (or calloc) > > in context of libgcov. I'm suggesting a new approach where we'll first > > use the pre-allocated static buffer in hope that malloc function is > > initialized > > and so every call to calloc can happen. That's why I increased number of KVP > > to 64 and I believe one reaches malloc pretty soon in an application run. > > > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > > > Ready to be installed? > > Thanks, > > Martin > > > > gcc/ChangeLog: > > > > PR gcov-profile/97461 > > * gcov-io.h (GCOV_PREALLOCATED_KVP): Pre-allocate 64 > > static counters. > > > > libgcc/ChangeLog: > > > > PR gcov-profile/97461 > > * libgcov.h (gcov_counter_add): Use first static counters > > as it should help to have malloc wrappers set up. > > > > gcc/testsuite/ChangeLog: > > > > PR gcov-profile/97461 > > * gcc.dg/tree-prof/pr97461.c: New test.
Looks reasonable, but I do not like very much the non-configurable preallocation since libgcov was meant to be useful for embedded targets and not consume too much. I guess we could handle that incrementally, how the llvm's option for preallocated pool size is implemented? Honza > > --- > > gcc/gcov-io.h | 2 +- > > gcc/testsuite/gcc.dg/tree-prof/pr97461.c | 58 ++++++++++++++++++++++++ > > libgcc/libgcov.h | 24 +++------- > > 3 files changed, 65 insertions(+), 19 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/tree-prof/pr97461.c > > > > diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h > > index 4dba01c78ce..4e95c7c82ee 100644 > > --- a/gcc/gcov-io.h > > +++ b/gcc/gcov-io.h > > @@ -293,7 +293,7 @@ GCOV_COUNTERS > > #define GCOV_TOPN_MAXIMUM_TRACKED_VALUES 32 > > > > /* Number of pre-allocated gcov_kvp structures. */ > > -#define GCOV_PREALLOCATED_KVP 16 > > +#define GCOV_PREALLOCATED_KVP 64 > > > > /* Convert a counter index to a tag. */ > > #define GCOV_TAG_FOR_COUNTER(COUNT) \ > > diff --git a/gcc/testsuite/gcc.dg/tree-prof/pr97461.c > > b/gcc/testsuite/gcc.dg/tree-prof/pr97461.c > > new file mode 100644 > > index 00000000000..8d21a3ef421 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/tree-prof/pr97461.c > > @@ -0,0 +1,58 @@ > > +/* PR gcov-profile/97461 */ > > +/* { dg-options "-O2 -ldl" } */ > > + > > +#define _GNU_SOURCE > > + > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > + > > +static int malloc_depth = 0; > > + > > +static char memory[128* 1024]; > > +static size_t memory_p = 0; > > + > > +void f1(void) {} > > +void f2(void) {} > > + > > +typedef void (*fun_t)(void); > > +static const fun_t funs[2] = { f1, f2, }; > > + > > +static void * malloc_impl(size_t size) { > > + void * r = &memory[memory_p]; > > + memory_p += size; > > + > > + // force TOPN profile > > + funs[size % 2](); > > + return r; > > +} > > + > > +// Override default malloc, check it it get s called recursively > > +void * malloc(size_t size) { > > + // Must not be called recursively. Malloc implementation does not > > support it. > > + if (malloc_depth != 0) __builtin_trap(); > > + > > + ++malloc_depth; > > + void * r = malloc_impl(size); > > + --malloc_depth; > > + return r; > > +} > > + > > +// Called from gcov > > +void *calloc(size_t nmemb, size_t size) { > > + // Must not be called recursively. Malloc implementation does not > > support it. > > + if (malloc_depth != 0) __builtin_trap(); > > + > > + ++malloc_depth; > > + void * r = malloc_impl(size * nmemb); > > + memset(r, 0, size * nmemb); > > + --malloc_depth; > > + return r; > > +} > > + > > +void free(void *ptr){} > > + > > +int main() { > > + void * p = malloc(8); > > + return p != 0 ? 0 : 1; > > +} > > diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h > > index 8be5bebcac0..e70cf63b414 100644 > > --- a/libgcc/libgcov.h > > +++ b/libgcc/libgcov.h > > @@ -404,22 +404,16 @@ gcov_counter_add (gcov_type *counter, gcov_type value, > > *counter += value; > > } > > > > -/* Allocate gcov_kvp from heap. If we are recursively called, then > > allocate > > - it from a list of pre-allocated pool. */ > > +/* Allocate gcov_kvp from statically pre-allocated pool, > > + or use heap otherwise. */ > > > > static inline struct gcov_kvp * > > allocate_gcov_kvp (void) > > { > > struct gcov_kvp *new_node = NULL; > > > > - static > > -#if defined(HAVE_CC_TLS) > > -__thread > > -#endif > > - volatile unsigned in_recursion ATTRIBUTE_UNUSED = 0; > > - > > #if !defined(IN_GCOV_TOOL) && !defined(L_gcov_merge_topn) > > - if (__builtin_expect (in_recursion, 0)) > > + if (__gcov_kvp_pool_index < GCOV_PREALLOCATED_KVP) > > { > > unsigned index; > > #if GCOV_SUPPORTS_ATOMIC > > @@ -430,17 +424,11 @@ __thread > > #endif > > if (index < GCOV_PREALLOCATED_KVP) > > new_node = &__gcov_kvp_pool[index]; > > - else > > - /* Do not crash in the situation. */ > > - return NULL; > > } > > - else > > #endif > > - { > > - in_recursion = 1; > > - new_node = (struct gcov_kvp *)xcalloc (1, sizeof (struct gcov_kvp)); > > - in_recursion = 0; > > - } > > + > > + if (new_node == NULL) > > + new_node = (struct gcov_kvp *)xcalloc (1, sizeof (struct gcov_kvp)); > > > > return new_node; > > } >