Use-after-free bug:
In stack_pop()
:
--s->size;
void *const top = (char *) s->data + (s->size * s->memb_size);
if (s->size && (s->size <= s->cap / 4)) {
void *const tmp = realloc(s->data, s->cap / 2 * s->memb_size);
if (tmp) {
s->data = tmp;
s->cap /= 2;
}
/* Else do nothing. The original memory is left intact. */
}
The lifetime of s->data
ends with the call to realloc()
, assuming it succeeded, so top
is pointing to a block of memory that might have already been freed whilst shrinking.
GCC 12.3 caught the bug with -O1 -Wall -Werror -Wpedantic
.
In file included from <source>:269:
<source>: In function 'main':
<source>:291:16: error: pointer used after 'realloc' [-Werror=use-after-free]
291 | assert(*(size_t *) gstack_pop(stack) == i);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function 'stack_pop',
inlined from 'main' at <source>:291:9:
<source>:238:27: note: call to 'realloc' here
238 | void *const tmp = realloc(s->data, s->memb_size * new_cap);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
If you remove -O1
it doesn't detect anything. GCC 13.1 and 13.2 did not detect the bug, ofor if they did, they did not output anything.
Valgrind also reports a read error for this in stack_pop()
.
The solution is to move the statement to the end of the function:
return (char *) s->data + (s->size * s->memb_size);
This also eliminated the variable top
.
Aside:
stack_is_empty()
, stack_is_full()
, stack_size()
, and stack_peek()
are pure functions and may benefit from __attribute__((pure))
. They are not constant functions because they're accessing global memory.