Skip to content

Commit c86889d

Browse files
committed
gc: "new" gc_realloc: Rewrite in plain C, fixing bunch of bugs.
There were typos, various rounding errors trying to do concurrent counting in bytes vs blocks, complex conditional paths, superfluous variables, etc., etc., all leading to obscure segfaults.
1 parent ed162b5 commit c86889d

1 file changed

Lines changed: 31 additions & 30 deletions

File tree

py/gc.c

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <assert.h>
12
#include <stdio.h>
23
#include <string.h>
34
#include <stdbool.h>
@@ -459,16 +460,18 @@ void *gc_realloc(void *ptr_in, machine_uint_t n_bytes) {
459460
}
460461

461462
void *ptr_out = NULL;
462-
machine_uint_t block = 0;
463463
machine_uint_t ptr = (machine_uint_t)ptr_in;
464464

465465
if (ptr_in == NULL) {
466466
return gc_alloc(n_bytes, false);
467467
}
468468

469-
if (VERIFY_PTR(ptr) // verify pointer
470-
&& (block = BLOCK_FROM_PTR(ptr)) // get first block
471-
&& ATB_GET_KIND(block) == AT_HEAD) { // make sure it's a HEAD block
469+
machine_uint_t new_blocks = (n_bytes + BYTES_PER_BLOCK) / BYTES_PER_BLOCK;
470+
// get first block
471+
machine_uint_t block = BLOCK_FROM_PTR(ptr);
472+
473+
// Sabity checks
474+
if (VERIFY_PTR(ptr) && ATB_GET_KIND(block) == AT_HEAD) {
472475

473476
byte block_type;
474477
machine_uint_t n_free = 0;
@@ -478,42 +481,39 @@ void *gc_realloc(void *ptr_in, machine_uint_t n_bytes) {
478481
// get the number of consecutive tail blocks and
479482
// the number of free blocks after last tail block
480483
// stop if we reach (or are at) end of heap
481-
while ((block + n_blocks + n_free) < max_block
484+
while (block + n_blocks + n_free < max_block) {
485+
if (n_blocks + n_free >= new_blocks) {
482486
// stop as soon as we find enough blocks for n_bytes
483-
&& (n_bytes > ((n_blocks+n_free) * BYTES_PER_BLOCK))
484-
// stop if block is HEAD
485-
&& (block_type = ATB_GET_KIND(block + n_blocks + n_free)) != AT_HEAD) {
487+
break;
488+
}
489+
block_type = ATB_GET_KIND(block + n_blocks + n_free);
486490
switch (block_type) {
487-
case AT_FREE: n_free++; break;
488-
case AT_TAIL: n_blocks++; break;
489-
default: break;
491+
case AT_FREE: n_free++; continue;
492+
case AT_TAIL: n_blocks++; continue;
493+
case AT_MARK: assert(0);
490494
}
495+
break;
491496
}
492-
// number of allocated bytes
493-
machine_uint_t n_existing = n_blocks * BYTES_PER_BLOCK;
494497

495-
// check if realloc'ing to a smaller size
496-
if (n_bytes <= n_existing) {
497-
ptr_out = ptr_in;
498+
if (new_blocks == n_blocks) {
499+
return ptr_in;
500+
}
501+
502+
if (new_blocks < n_blocks) {
498503
// free unneeded tail blocks
499-
for (machine_uint_t bl = block + n_blocks; ATB_GET_KIND(bl) == AT_TAIL; bl++) {
504+
for (machine_uint_t bl = block + new_blocks; ATB_GET_KIND(bl) == AT_TAIL; bl++) {
500505
ATB_ANY_TO_FREE(bl);
501506
}
507+
return ptr_in;
502508

503509
// check if we can expand in place
504-
} else if (n_bytes <= (n_existing + (n_free * BYTES_PER_BLOCK))) {
505-
// number of blocks needed to expand +1 if there's a remainder
506-
machine_uint_t n_diff = ( n_bytes - n_existing)/BYTES_PER_BLOCK+
507-
((n_bytes - n_existing)%BYTES_PER_BLOCK!=0);
508-
509-
DEBUG_printf("gc_realloc: expanding " UINT_FMT " blocks (" UINT_FMT " bytes) to " UINT_FMT " blocks (" UINT_FMT " bytes)\n",
510-
n_existing/BYTES_PER_BLOCK, n_existing, n_existing/BYTES_PER_BLOCK+n_diff, n_existing + n_diff*BYTES_PER_BLOCK);
511-
512-
// mark rest of blocks as used tail
513-
for (machine_uint_t bl = block + n_blocks; bl < (block + n_blocks + n_diff); bl++) {
510+
} else if (new_blocks <= n_blocks + n_free) {
511+
// mark few more blocks as used tail
512+
for (machine_uint_t bl = block + n_blocks; bl < block + new_blocks; bl++) {
513+
assert(ATB_GET_KIND(bl) == AT_FREE);
514514
ATB_FREE_TO_TAIL(bl);
515515
}
516-
ptr_out = ptr_in;
516+
return ptr_in;
517517

518518
// try to find a new contiguous chain
519519
} else if ((ptr_out = gc_alloc(n_bytes,
@@ -524,12 +524,13 @@ void *gc_realloc(void *ptr_in, machine_uint_t n_bytes) {
524524
#endif
525525
)) != NULL) {
526526
DEBUG_printf("gc_realloc: allocating new block\n");
527-
memcpy(ptr_out, ptr_in, n_existing);
527+
memcpy(ptr_out, ptr_in, n_blocks * BYTES_PER_BLOCK);
528528
gc_free(ptr_in);
529+
return ptr_out;
529530
}
530531
}
531532

532-
return ptr_out;
533+
return NULL;
533534
}
534535
#endif // Alternative gc_realloc impl
535536

0 commit comments

Comments
 (0)