Skip to content

Commit

Permalink
Fix clang asan reported leak
Browse files Browse the repository at this point in the history
Fix a realloc() memory leak which would happen when:
- a previous allocation exists ('ptr' is non-NULL); and
- the new 'size' is zero; and
- a voluntary GC (or GC torture) causes the initial realloc() attempt
  to be bypassed

In this case the slow path would incorrectly assume that it was entered
after realloc() had returned a NULL, which for a zero new 'size' would
mean that 'ptr' was successfully freed and no further action was necessary.
But because the realloc() had actually been bypassed, this would cause the
old 'ptr' to leak.
  • Loading branch information
svaarala committed Feb 11, 2022
1 parent a851d8a commit d00bee4
Showing 1 changed file with 25 additions and 35 deletions.
60 changes: 25 additions & 35 deletions src-input/duk_heap_memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,13 +178,7 @@ DUK_LOCAL DUK_NOINLINE_PERF DUK_COLD void *duk__heap_mem_realloc_slowpath(duk_he
/* ptr may be NULL */
DUK_ASSERT_DISABLE(newsize >= 0);

/* Newsize was 0 and realloc() returned NULL, this has the semantics
* of free(oldptr), i.e. memory was successfully freed.
*/
if (newsize == 0) {
DUK_D(DUK_DPRINT("zero size realloc in slow path, return NULL"));
return NULL;
}
/* Unlike for malloc(), zero size NULL result check happens at the call site. */

DUK_D(DUK_DPRINT("first realloc attempt failed, attempt to gc and retry"));

Expand Down Expand Up @@ -217,7 +211,7 @@ DUK_LOCAL DUK_NOINLINE_PERF DUK_COLD void *duk__heap_mem_realloc_slowpath(duk_he

DUK_ASSERT(newsize > 0);
res = heap->realloc_func(heap->heap_udata, ptr, newsize);
if (res || newsize == 0) {
if (res != NULL || newsize == 0) {
DUK_D(DUK_DPRINT("duk_heap_mem_realloc() succeeded after gc (pass %ld), alloc size %ld",
(long) (i + 1),
(long) newsize));
Expand All @@ -240,7 +234,7 @@ DUK_INTERNAL DUK_INLINE_PERF DUK_HOT void *duk_heap_mem_realloc(duk_heap *heap,
#if defined(DUK_USE_VOLUNTARY_GC)
/* Voluntary periodic GC (if enabled). */
if (DUK_UNLIKELY(--(heap)->ms_trigger_counter < 0)) {
goto slowpath;
goto gc_retry;
}
#endif

Expand All @@ -252,22 +246,22 @@ DUK_INTERNAL DUK_INLINE_PERF DUK_HOT void *duk_heap_mem_realloc(duk_heap *heap,
DUK_DDD(DUK_DDDPRINT("gc torture enabled, pretend that first realloc attempt fails"));
res = NULL;
DUK_UNREF(res);
goto slowpath;
goto gc_retry;
}
#endif

res = heap->realloc_func(heap->heap_udata, ptr, newsize);
if (DUK_LIKELY(res != NULL)) {
if (DUK_LIKELY(res != NULL) || newsize == 0) {
if (res != NULL && newsize == 0) {
DUK_DD(DUK_DDPRINT("first realloc attempt returned NULL for zero size realloc, accept and return NULL"));
}
return res;
}

slowpath:

if (newsize == 0) {
DUK_D(DUK_DPRINT("first realloc attempt returned NULL for zero size realloc, use slow path to deal with it"));
} else {
DUK_D(DUK_DPRINT("first realloc attempt failed, attempt to gc and retry"));
goto gc_retry;
}
/* Never here. */

gc_retry:
return duk__heap_mem_realloc_slowpath(heap, ptr, newsize);
}

Expand All @@ -289,10 +283,7 @@ DUK_LOCAL DUK_NOINLINE_PERF DUK_COLD void *duk__heap_mem_realloc_indirect_slowpa
DUK_ASSERT(heap->realloc_func != NULL);
DUK_ASSERT_DISABLE(newsize >= 0);

if (newsize == 0) {
DUK_D(DUK_DPRINT("zero size indirect realloc in slow path, return NULL"));
return NULL;
}
/* Unlike for malloc(), zero size NULL result check happens at the call site. */

DUK_D(DUK_DPRINT("first indirect realloc attempt failed, attempt to gc and retry"));

Expand Down Expand Up @@ -343,9 +334,8 @@ DUK_LOCAL DUK_NOINLINE_PERF DUK_COLD void *duk__heap_mem_realloc_indirect_slowpa
* The pointer being reallocated may change after every mark-and-sweep.
*/

DUK_ASSERT(newsize > 0);
res = heap->realloc_func(heap->heap_udata, cb(heap, ud), newsize);
if (res || newsize == 0) {
if (res != NULL || newsize == 0) {
DUK_D(DUK_DPRINT("duk_heap_mem_realloc_indirect() succeeded after gc (pass %ld), alloc size %ld",
(long) (i + 1),
(long) newsize));
Expand All @@ -370,7 +360,7 @@ DUK_INTERNAL DUK_INLINE_PERF DUK_HOT void *duk_heap_mem_realloc_indirect(duk_hea
#if defined(DUK_USE_VOLUNTARY_GC)
/* Voluntary periodic GC (if enabled). */
if (DUK_UNLIKELY(--(heap)->ms_trigger_counter < 0)) {
goto slowpath;
goto gc_retry;
}
#endif

Expand All @@ -382,23 +372,23 @@ DUK_INTERNAL DUK_INLINE_PERF DUK_HOT void *duk_heap_mem_realloc_indirect(duk_hea
DUK_DDD(DUK_DDDPRINT("gc torture enabled, pretend that first indirect realloc attempt fails"));
res = NULL;
DUK_UNREF(res);
goto slowpath;
goto gc_retry;
}
#endif

res = heap->realloc_func(heap->heap_udata, cb(heap, ud), newsize);
if (DUK_LIKELY(res != NULL)) {
if (DUK_LIKELY(res != NULL) || newsize == 0) {
if (res != NULL && newsize == 0) {
DUK_DD(DUK_DDPRINT(
"first indirect realloc attempt returned NULL for zero size realloc, accept and return NULL"));
}
return res;
}

slowpath:

if (newsize == 0) {
DUK_D(DUK_DPRINT(
"first indirect realloc attempt returned NULL for zero size realloc, use slow path to deal with it"));
} else {
DUK_D(DUK_DPRINT("first indirect realloc attempt failed, attempt to gc and retry"));
goto gc_retry;
}
/* Never here. */

gc_retry:
return duk__heap_mem_realloc_indirect_slowpath(heap, cb, ud, newsize);
}

Expand Down

0 comments on commit d00bee4

Please sign in to comment.