Browse Source

rework indirect realloc to use a callback instead of a type-punned pointer which broke strict aliasing rules

pull/1/head
Sami Vaarala 11 years ago
parent
commit
a9d80f4767
  1. 2
      src/duk_api.c
  2. 1
      src/duk_hbuffer.h
  3. 7
      src/duk_hbuffer_alloc.c
  4. 6
      src/duk_hbuffer_ops.c
  5. 28
      src/duk_heap.h
  6. 35
      src/duk_heap_memory.c
  7. 4
      src/duk_hthread.h
  8. 17
      src/duk_hthread_alloc.c
  9. 8
      src/duk_hthread_stacks.c

2
src/duk_api.c

@ -281,7 +281,7 @@ static int resize_valstack(duk_context *ctx, size_t new_size) {
*/
new_alloc_size = sizeof(duk_tval) * new_size;
new_valstack = (duk_tval *) DUK_REALLOC_INDIRECT(thr->heap, (void **) &thr->valstack, new_alloc_size);
new_valstack = (duk_tval *) DUK_REALLOC_INDIRECT(thr->heap, duk_hthread_get_valstack_ptr, (void *) thr, new_alloc_size);
if (!new_valstack) {
DUK_DPRINT("failed to resize valstack to %d entries (%d bytes)",
new_size, new_alloc_size);

1
src/duk_hbuffer.h

@ -101,6 +101,7 @@ struct duk_hbuffer_dynamic {
*/
duk_hbuffer *duk_hbuffer_alloc(duk_heap *heap, size_t size, int dynamic);
void *duk_hbuffer_get_dynalloc_ptr(void *ud); /* indirect allocs */
/* dynamic buffer ops */
void duk_hbuffer_resize(duk_hthread *thr, duk_hbuffer_dynamic *buf, size_t new_size, size_t new_usable_size);

7
src/duk_hbuffer_alloc.c

@ -61,3 +61,10 @@ duk_hbuffer *duk_hbuffer_alloc(duk_heap *heap, size_t size, int dynamic) {
return NULL;
}
/* For indirect allocs. */
void *duk_hbuffer_get_dynalloc_ptr(void *ud) {
duk_hbuffer_dynamic *buf = (duk_hbuffer_dynamic *) ud;
return (void *) buf->curr_alloc;
}

6
src/duk_hbuffer_ops.c

@ -28,7 +28,6 @@ static size_t add_spare(size_t size) {
void duk_hbuffer_resize(duk_hthread *thr, duk_hbuffer_dynamic *buf, size_t new_size, size_t new_usable_size) {
size_t new_alloc_size;
void **ptr;
void *res;
DUK_ASSERT(thr != NULL);
@ -44,8 +43,7 @@ void duk_hbuffer_resize(duk_hthread *thr, duk_hbuffer_dynamic *buf, size_t new_s
/* FIXME: maybe remove safety NUL term for buffers? */
new_alloc_size = new_usable_size + 1; /* +1 for safety nul term */
ptr = &buf->curr_alloc;
res = DUK_REALLOC_INDIRECT(thr->heap, ptr, new_alloc_size);
res = DUK_REALLOC_INDIRECT(thr->heap, duk_hbuffer_get_dynalloc_ptr, (void *) buf, new_alloc_size);
if (res) {
DUK_DDDPRINT("resized dynamic buffer %p:%d:%d -> %p:%d:%d",
buf->curr_alloc, buf->size, buf->usable_size,
@ -72,7 +70,7 @@ void duk_hbuffer_resize(duk_hthread *thr, duk_hbuffer_dynamic *buf, size_t new_s
buf->size = new_size;
buf->usable_size = new_usable_size;
*ptr = res;
buf->curr_alloc = res;
} else {
DUK_ERROR(thr, DUK_ERR_ALLOC_ERROR, "failed to resize dynamic buffer from %d:%d to %d:%d",
buf->size, buf->usable_size, new_size, new_usable_size);

28
src/duk_heap.h

@ -170,19 +170,21 @@
* by an allocation failure might invalidate the original 'ptr', thus
* causing a realloc retry to use an invalid pointer. Example: we're
* reallocating the value stack and a finalizer resizes the same value
* stack during mark-and-sweep. The indirect variant knows the storage
* location of the pointer being reallocated and looks it up on every
* attempt; the storage location must of course be stable, which is
* always the case for heap objects now.
*
* Note: the pointer in the storage location ('iptr') is read but is
* NOT updated; caller must do that.
* stack during mark-and-sweep. The indirect variant requests for the
* current location of the pointer being reallocated using a callback
* right before every realloc attempt; this circuitous approach is used
* to avoid strict aliasing issues in a more straightforward indirect
* pointer (void **) approach. Note: the pointer in the storage
* location is read but is NOT updated; the caller must do that.
*/
/* callback for indirect reallocs, request for current pointer */
typedef void *(*duk_mem_getptr)(void *ud);
#define DUK_ALLOC(heap,size) duk_heap_mem_alloc((heap), (size))
#define DUK_ALLOC_ZEROED(heap,size) duk_heap_mem_alloc_zeroed((heap), (size))
#define DUK_REALLOC(heap,ptr,newsize) duk_heap_mem_realloc((heap), (ptr), (newsize))
#define DUK_REALLOC_INDIRECT(heap,iptr,newsize) duk_heap_mem_realloc_indirect((heap), (iptr), (newsize))
#define DUK_REALLOC_INDIRECT(heap,cb,ud,newsize) duk_heap_mem_realloc_indirect((heap), (cb), (ud), (newsize))
#define DUK_FREE(heap,ptr) duk_heap_mem_free((heap), (ptr))
/*
@ -195,13 +197,13 @@
#define DUK_ALLOC_CHECKED(thr,size) duk_heap_mem_alloc_checked((thr), (size), DUK_FILE_MACRO, DUK_LINE_MACRO)
#define DUK_ALLOC_CHECKED_ZEROED(thr,size) duk_heap_mem_alloc_checked_zeroed((thr), (size), DUK_FILE_MACRO, DUK_LINE_MACRO)
#define DUK_REALLOC_CHECKED(thr,ptr,newsize) duk_heap_mem_realloc_checked((thr), (ptr), (newsize), DUK_FILE_MACRO, DUK_LINE_MACRO)
#define DUK_REALLOC_INDIRECT_CHECKED(thr,iptr,newsize) duk_heap_mem_realloc_indirect_checked((thr), (iptr), (newsize), DUK_FILE_MACRO, DUK_LINE_MACRO)
#define DUK_REALLOC_INDIRECT_CHECKED(thr,cb,ud,newsize) duk_heap_mem_realloc_indirect_checked((thr), (cb), (ud), (newsize), DUK_FILE_MACRO, DUK_LINE_MACRO)
#define DUK_FREE_CHECKED(thr,ptr) duk_heap_mem_free((thr)->heap, (ptr)) /* must not fail */
#else
#define DUK_ALLOC_CHECKED(thr,size) duk_heap_mem_alloc_checked((thr), (size))
#define DUK_ALLOC_CHECKED_ZEROED(thr,size) duk_heap_mem_alloc_checked_zeroed((thr), (size))
#define DUK_REALLOC_CHECKED(thr,ptr,newsize) duk_heap_mem_realloc_checked((thr), (ptr), (newsize))
#define DUK_REALLOC_INDIRECT_CHECKED(thr,iptr,newsize) duk_heap_mem_realloc_indirect_checked((thr), (iptr), (newsize))
#define DUK_REALLOC_INDIRECT_CHECKED(thr,cb,ud,newsize) duk_heap_mem_realloc_indirect_checked((thr), (cb), (ud), (newsize))
#define DUK_FREE_CHECKED(thr,ptr) duk_heap_mem_free((thr)->heap, (ptr)) /* must not fail */
#endif
@ -370,19 +372,19 @@ void duk_default_free_function(void *udata, void *ptr);
void *duk_heap_mem_alloc(duk_heap *heap, size_t size);
void *duk_heap_mem_alloc_zeroed(duk_heap *heap, size_t size);
void *duk_heap_mem_realloc(duk_heap *heap, void *ptr, size_t newsize);
void *duk_heap_mem_realloc_indirect(duk_heap *heap, void **iptr, size_t newsize);
void *duk_heap_mem_realloc_indirect(duk_heap *heap, duk_mem_getptr cb, void *ud, size_t newsize);
void duk_heap_mem_free(duk_heap *heap, void *ptr);
#ifdef DUK_USE_VERBOSE_ERRORS
void *duk_heap_mem_alloc_checked(duk_hthread *thr, size_t size, const char *filename, int line);
void *duk_heap_mem_alloc_checked_zeroed(duk_hthread *thr, size_t size, const char *filename, int line);
void *duk_heap_mem_realloc_checked(duk_hthread *thr, void *ptr, size_t newsize, const char *filename, int line);
void *duk_heap_mem_realloc_indirect_checked(duk_hthread *thr, void **iptr, size_t newsize, const char *filename, int line);
void *duk_heap_mem_realloc_indirect_checked(duk_hthread *thr, duk_mem_getptr cb, void *ud, size_t newsize, const char *filename, int line);
#else
void *duk_heap_mem_alloc_checked(duk_hthread *thr, size_t size);
void *duk_heap_mem_alloc_checked_zeroed(duk_hthread *thr, size_t size);
void *duk_heap_mem_realloc_checked(duk_hthread *thr, void *ptr, size_t newsize);
void *duk_heap_mem_realloc_indirect_checked(duk_hthread *thr, void **iptr, size_t newsize);
void *duk_heap_mem_realloc_indirect_checked(duk_hthread *thr, duk_mem_getptr cb, void *ud, size_t newsize);
#endif
#ifdef DUK_USE_REFERENCE_COUNTING

35
src/duk_heap_memory.c

@ -239,21 +239,18 @@ void *duk_heap_mem_realloc(duk_heap *heap, void *ptr, size_t newsize) {
#endif /* DUK_USE_MARK_AND_SWEEP */
/*
* Reallocate memory with garbage collection, using an indirect pointer
*
* This variant is used when a mark-and-sweep (finalizers) might change
* the original pointer. The indirect 'iptr' must have a stable location.
* Reallocate memory with garbage collection, using a callback to provide
* the current allocated pointer. This variant is used when a mark-and-sweep
* (e.g. finalizers) might change the original pointer.
*/
#ifdef DUK_USE_MARK_AND_SWEEP
void *duk_heap_mem_realloc_indirect(duk_heap *heap, void **iptr, size_t newsize) {
void *duk_heap_mem_realloc_indirect(duk_heap *heap, duk_mem_getptr cb, void *ud, size_t newsize) {
void *res;
int rc;
int i;
DUK_ASSERT(heap != NULL);
DUK_ASSERT(iptr != NULL);
/* *iptr may be NULL */
DUK_ASSERT(newsize >= 0);
/*
@ -275,7 +272,7 @@ void *duk_heap_mem_realloc_indirect(duk_heap *heap, void **iptr, size_t newsize)
goto skip_attempt;
}
#endif
res = heap->realloc_func(heap->alloc_udata, *iptr, newsize);
res = heap->realloc_func(heap->alloc_udata, cb(ud), newsize);
if (res || newsize == 0) {
/* for zero size allocations NULL is allowed */
return res;
@ -306,10 +303,11 @@ void *duk_heap_mem_realloc_indirect(duk_heap *heap, void **iptr, size_t newsize)
#ifdef DUK_USE_ASSERTIONS
void *ptr_pre; /* ptr before mark-and-sweep */
void *ptr_post;
#endif
#ifdef DUK_USE_ASSERTIONS
ptr_pre = *iptr;
ptr_pre = cb(ud);
#endif
flags = 0;
if (i >= DUK_HEAP_ALLOC_FAIL_MARKANDSWEEP_EMERGENCY_LIMIT - 1) {
@ -319,17 +317,18 @@ void *duk_heap_mem_realloc_indirect(duk_heap *heap, void **iptr, size_t newsize)
rc = duk_heap_mark_and_sweep(heap, flags);
DUK_UNREF(rc);
#ifdef DUK_USE_ASSERTIONS
if (ptr_pre != *iptr) {
ptr_post = cb(ud);
if (ptr_pre != ptr_post) {
/* useful for debugging */
DUK_DDPRINT("note: *iptr changed by mark-and-sweep: %p -> %p", ptr_pre, *iptr);
DUK_DDPRINT("note: base pointer changed by mark-and-sweep: %p -> %p", ptr_pre, ptr_post);
}
#endif
/* Note: key issue here is to re-lookup *iptr on every attempt -- the
* value behind iptr may change after every mark-and-sweep.
/* Note: key issue here is to re-lookup the base pointer on every attempt.
* The pointer being reallocated may change after every mark-and-sweep.
*/
res = heap->realloc_func(heap->alloc_udata, *iptr, newsize);
res = heap->realloc_func(heap->alloc_udata, cb(ud), newsize);
if (res) {
DUK_DPRINT("duk_heap_mem_realloc_indirect() succeeded after gc (pass %d), alloc size %d",
i + 1, newsize);
@ -438,16 +437,14 @@ void *duk_heap_mem_realloc_checked(duk_hthread *thr, void *ptr, size_t newsize)
}
#ifdef DUK_USE_VERBOSE_ERRORS
void *duk_heap_mem_realloc_indirect_checked(duk_hthread *thr, void **iptr, size_t newsize, const char *filename, int line) {
void *duk_heap_mem_realloc_indirect_checked(duk_hthread *thr, duk_mem_getptr cb, void *ud, size_t newsize, const char *filename, int line) {
#else
void *duk_heap_mem_realloc_indirect_checked(duk_hthread *thr, void **iptr, size_t newsize) {
void *duk_heap_mem_realloc_indirect_checked(duk_hthread *thr, duk_mem_getptr cb, void *ud, size_t newsize) {
#endif
DUK_ASSERT(thr != NULL);
DUK_ASSERT(iptr != NULL);
/* *iptr may be NULL */
DUK_ASSERT(newsize >= 0);
void *res = DUK_REALLOC_INDIRECT(thr->heap, iptr, newsize);
void *res = DUK_REALLOC_INDIRECT(thr->heap, cb, ud, newsize);
if (!res) {
DUK_ERROR(thr, DUK_ERR_ALLOC_ERROR, "memory realloc failed");
}

4
src/duk_hthread.h

@ -240,6 +240,8 @@ void duk_hthread_catchstack_shrink_check(duk_hthread *thr);
void duk_hthread_catchstack_unwind(duk_hthread *thr, int new_top);
duk_activation *duk_hthread_get_current_activation(duk_hthread *thr);
void *duk_hthread_get_valstack_ptr(void *ud); /* indirect allocs */
void *duk_hthread_get_callstack_ptr(void *ud); /* indirect allocs */
void *duk_hthread_get_catchstack_ptr(void *ud); /* indirect allocs */
#endif /* DUK_HTHREAD_H_INCLUDED */

17
src/duk_hthread_alloc.c

@ -71,3 +71,20 @@ int duk_hthread_init_stacks(duk_heap *heap, duk_hthread *thr) {
return 0;
}
/* For indirect allocs. */
void *duk_hthread_get_valstack_ptr(void *ud) {
duk_hthread *thr = (duk_hthread *) ud;
return (void *) thr->valstack;
}
void *duk_hthread_get_callstack_ptr(void *ud) {
duk_hthread *thr = (duk_hthread *) ud;
return (void *) thr->callstack;
}
void *duk_hthread_get_catchstack_ptr(void *ud) {
duk_hthread *thr = (duk_hthread *) ud;
return (void *) thr->catchstack;
}

8
src/duk_hthread_stacks.c

@ -50,7 +50,7 @@ void duk_hthread_callstack_grow(duk_hthread *thr) {
* pointer may be changed by mark-and-sweep.
*/
thr->callstack = (duk_activation *) DUK_REALLOC_INDIRECT_CHECKED(thr, (void **) &thr->callstack, sizeof(duk_activation) * new_size);
thr->callstack = (duk_activation *) DUK_REALLOC_INDIRECT_CHECKED(thr, duk_hthread_get_callstack_ptr, (void *) thr, sizeof(duk_activation) * new_size);
thr->callstack_size = new_size;
/* note: any entries above the callstack top are garbage and not zeroed */
@ -79,7 +79,7 @@ void duk_hthread_callstack_shrink_check(duk_hthread *thr) {
*/
/* shrink failure is not fatal */
p = (duk_activation *) DUK_REALLOC_INDIRECT(thr->heap, (void **) &thr->callstack, sizeof(duk_activation) * new_size);
p = (duk_activation *) DUK_REALLOC_INDIRECT(thr->heap, duk_hthread_get_callstack_ptr, (void *) thr, sizeof(duk_activation) * new_size);
if (p) {
thr->callstack = p;
thr->callstack_size = new_size;
@ -269,7 +269,7 @@ void duk_hthread_catchstack_grow(duk_hthread *thr) {
* pointer may be changed by mark-and-sweep.
*/
thr->catchstack = (duk_catcher *) DUK_REALLOC_INDIRECT_CHECKED(thr, (void **) &thr->catchstack, sizeof(duk_catcher) * new_size);
thr->catchstack = (duk_catcher *) DUK_REALLOC_INDIRECT_CHECKED(thr, duk_hthread_get_catchstack_ptr, (void *) thr, sizeof(duk_catcher) * new_size);
thr->catchstack_size = new_size;
/* note: any entries above the catchstack top are garbage and not zeroed */
@ -298,7 +298,7 @@ void duk_hthread_catchstack_shrink_check(duk_hthread *thr) {
*/
/* shrink failure is not fatal */
p = (duk_catcher *) DUK_REALLOC_INDIRECT(thr->heap, (void **) &thr->catchstack, sizeof(duk_catcher) * new_size);
p = (duk_catcher *) DUK_REALLOC_INDIRECT(thr->heap, duk_hthread_get_catchstack_ptr, (void *) thr, sizeof(duk_catcher) * new_size);
if (p) {
thr->catchstack = p;
thr->catchstack_size = new_size;

Loading…
Cancel
Save