On Thu, Dec 5, 2013 at 7:32 AM, Jakub Jelinek <[email protected]> wrote:
>
> I'm trying to understand how the backtrace_vector_* APIs are meant to work
> and used, but at least for alloc.c don't see how it can work properly:
>
> Both backtrace_vector_grow and backtrace_vector_release use
> base = realloc (vec->base, alc);
> or
> vec->base = realloc (vec->base, vec->size);
> (note, in the latter case it is even a memory leak if realloc fails),
> but that assumes that that vec->base has been returned by malloc/realloc
> etc. But,
> void
> backtrace_vector_finish (struct backtrace_state *state ATTRIBUTE_UNUSED,
> struct backtrace_vector *vec)
> {
> vec->base = (char *) vec->base + vec->size;
> vec->size = 0;
> }
> will change vec->base so that it no longer is an address returned by
> malloc/realloc, so next time you call backtrace_vector_grow, if it will
> actually need to reallocate anything, it will crash in realloc or silently
> misbehave. If this works properly in mmap.c implementation, perhaps
> backtrace_vector_finish in alloc.c should just backtrace_vector_release
> and memset (*vec, 0, sizeof (*vec)); ?
You're quite right. That was dumb. Thanks for noticing. Fixed with
this patch. Committed to mainline and 4.8 branch.
Ian
2013-12-05 Ian Lance Taylor <[email protected]>
* alloc.c (backtrace_vector_finish): Add error_callback and data
parameters. Call backtrace_vector_release. Return address base.
* mmap.c (backtrace_vector_finish): Add error_callback and data
parameters. Return address base.
* dwarf.c (read_function_info): Get new address base from
backtrace_vector_finish.
* internal.h (backtrace_vector_finish): Update declaration.
Index: dwarf.c
===================================================================
--- dwarf.c (revision 205711)
+++ dwarf.c (working copy)
@@ -2535,19 +2535,23 @@ read_function_info (struct backtrace_sta
if (pfvec->count == 0)
return;
- addrs = (struct function_addrs *) pfvec->vec.base;
addrs_count = pfvec->count;
if (fvec == NULL)
{
if (!backtrace_vector_release (state, &lvec.vec, error_callback, data))
return;
+ addrs = (struct function_addrs *) pfvec->vec.base;
}
else
{
/* Finish this list of addresses, but leave the remaining space in
the vector available for the next function unit. */
- backtrace_vector_finish (state, &fvec->vec);
+ addrs = ((struct function_addrs *)
+ backtrace_vector_finish (state, &fvec->vec,
+ error_callback, data));
+ if (addrs == NULL)
+ return;
fvec->count = 0;
}
Index: internal.h
===================================================================
--- internal.h (revision 205711)
+++ internal.h (working copy)
@@ -233,13 +233,17 @@ extern void *backtrace_vector_grow (stru
struct backtrace_vector *vec);
/* Finish the current allocation on VEC. Prepare to start a new
- allocation. The finished allocation will never be freed. */
+ allocation. The finished allocation will never be freed. Returns
+ a pointer to the base of the finished entries, or NULL on
+ failure. */
-extern void backtrace_vector_finish (struct backtrace_state *state,
- struct backtrace_vector *vec);
+extern void* backtrace_vector_finish (struct backtrace_state *state,
+ struct backtrace_vector *vec,
+ backtrace_error_callback error_callback,
+ void *data);
-/* Release any extra space allocated for VEC. Returns 1 on success, 0
- on failure. */
+/* Release any extra space allocated for VEC. This may change
+ VEC->base. Returns 1 on success, 0 on failure. */
extern int backtrace_vector_release (struct backtrace_state *state,
struct backtrace_vector *vec,
Index: mmap.c
===================================================================
--- mmap.c (revision 205711)
+++ mmap.c (working copy)
@@ -230,12 +230,19 @@ backtrace_vector_grow (struct backtrace_
/* Finish the current allocation on VEC. */
-void
-backtrace_vector_finish (struct backtrace_state *state ATTRIBUTE_UNUSED,
- struct backtrace_vector *vec)
+void *
+backtrace_vector_finish (
+ struct backtrace_state *state ATTRIBUTE_UNUSED,
+ struct backtrace_vector *vec,
+ backtrace_error_callback error_callback ATTRIBUTE_UNUSED,
+ void *data ATTRIBUTE_UNUSED)
{
+ void *ret;
+
+ ret = vec->base;
vec->base = (char *) vec->base + vec->size;
vec->size = 0;
+ return ret;
}
/* Release any extra space allocated for VEC. */
Index: alloc.c
===================================================================
--- alloc.c (revision 205711)
+++ alloc.c (working copy)
@@ -113,12 +113,24 @@ backtrace_vector_grow (struct backtrace_
/* Finish the current allocation on VEC. */
-void
-backtrace_vector_finish (struct backtrace_state *state ATTRIBUTE_UNUSED,
- struct backtrace_vector *vec)
+void *
+backtrace_vector_finish (struct backtrace_state *state,
+ struct backtrace_vector *vec,
+ backtrace_error_callback error_callback,
+ void *data)
{
- vec->base = (char *) vec->base + vec->size;
+ void *ret;
+
+ /* With this allocator we call realloc in backtrace_vector_grow,
+ which means we can't easily reuse the memory here. So just
+ release it. */
+ if (!backtrace_vector_release (state, vec, error_callback, data))
+ return NULL;
+ ret = vec->base;
+ vec->base = NULL;
vec->size = 0;
+ vec->alc = 0;
+ return ret;
}
/* Release any extra space allocated for VEC. */
