Skip to content

Commit

Permalink
Encapsulate accesses to the hidden header.
Browse files Browse the repository at this point in the history
This fixes a bug where we load the payload size from the hidden header
without masking out the HAS_MOVED_GIVTBL flag.

Fixes: mmtk/mmtk-ruby#103
  • Loading branch information
wks committed Oct 9, 2024
1 parent fdd6b4e commit c300b86
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 9 deletions.
5 changes: 3 additions & 2 deletions gc/default.c
Original file line number Diff line number Diff line change
Expand Up @@ -2533,7 +2533,7 @@ rb_gc_impl_obj_slot_size(VALUE obj)
#if USE_MMTK
if (rb_mmtk_enabled_p()) {
// Load from our hidden field before the object.
return *(size_t*)(obj - MMTK_OBJREF_OFFSET);
return rb_mmtk_get_payload_size(obj);
}
#endif

Expand Down Expand Up @@ -3290,7 +3290,8 @@ rb_mmtk_each_objects_safe(each_obj_callback *callback, void *data)
// If GC is triggered in `callback`, `tmpbuf` will keep elements of `array` alive.
for (size_t i = 0; i < build_array_data.len; i++) {
volatile VALUE object = array[i];
size_t object_size = rb_mmtk_get_object_size(object);
// This is the "object size" seen by the callback. It's only the payload size.
size_t object_size = rb_mmtk_get_payload_size(object);
uintptr_t object_end = object + object_size;

RUBY_DEBUG_LOG("Enumerating object: %p\n", (void*)object);
Expand Down
8 changes: 8 additions & 0 deletions internal/mmtk.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ typedef uint32_t MMTk_AllocationSemantics;

#define MMTK_GC_THREAD_KIND_WORKER 1

#define MMTK_HAS_MOVED_GIVTBL 9223372036854775808ull

#define MMTK_HIDDEN_SIZE_MASK 281474976710655

typedef struct st_table st_table;

typedef struct RubyBindingOptions {
Expand Down Expand Up @@ -112,6 +116,10 @@ typedef struct MMTk_RawVecOfObjRef {
size_t capa;
} MMTk_RawVecOfObjRef;

typedef struct MMTk_HiddenHeader {
size_t prefix;
} MMTk_HiddenHeader;

/**
* Create an MMTKBuilder instance with default options.
* This instance shall be consumed by `mmtk_init_binding`.
Expand Down
3 changes: 2 additions & 1 deletion internal/mmtk_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ void rb_mmtk_destroy_mutator(MMTk_VMMutatorThread cur_thread, bool at_fork);
// Object layout
size_t rb_mmtk_prefix_size(void);
size_t rb_mmtk_suffix_size(void);
size_t rb_mmtk_get_object_size(VALUE object);
void rb_mmtk_init_hidden_header(VALUE object, size_t payload_size);
size_t rb_mmtk_get_payload_size(VALUE object);

// Allocation
VALUE rb_mmtk_alloc_obj(size_t mmtk_alloc_size, size_t size_pool_size, size_t prefix_size);
Expand Down
27 changes: 21 additions & 6 deletions mmtk_support.c
Original file line number Diff line number Diff line change
Expand Up @@ -395,12 +395,27 @@ rb_mmtk_suffix_size(void)
return ruby_binding_options.suffix_size;
}

size_t
rb_mmtk_get_object_size(VALUE object)
void
rb_mmtk_init_hidden_header(VALUE object, size_t payload_size)
{
return *(size_t*)(object - sizeof(VALUE));
RUBY_ASSERT(payload_size <= MMTK_HIDDEN_SIZE_MASK,
"payload size greater than MMTK_HIDDEN_SIZE_MASK. payload_size: %zu", payload_size);

struct MMTk_HiddenHeader *hidden_header = (struct MMTk_HiddenHeader*)(object - MMTK_OBJREF_OFFSET);
hidden_header->prefix = payload_size;
}

size_t
rb_mmtk_get_payload_size(VALUE object)
{
struct MMTk_HiddenHeader *hidden_header = (struct MMTk_HiddenHeader*)(object - MMTK_OBJREF_OFFSET);
size_t prefix = hidden_header->prefix;

RUBY_ASSERT((prefix & ~(MMTK_HIDDEN_SIZE_MASK | MMTK_HAS_MOVED_GIVTBL)) == 0,
"Hidden field is corrupted. Object: %p, prefix: %zx", (void*) object, prefix);

return prefix & MMTK_HIDDEN_SIZE_MASK;
}

////////////////////////////////////////////////////////////////////////////////
// Allocation
Expand Down Expand Up @@ -495,13 +510,13 @@ rb_mmtk_alloc_obj(size_t mmtk_alloc_size, size_t size_pool_size, size_t prefix_s
// Allocate the object.
void *addr = rb_mmtk_alloc(mmtk_alloc_size, semantics);

// Store the Ruby-level object size before the object.
*(size_t*)addr = size_pool_size;

// The Ruby-level object reference (i.e. VALUE) is at an offset from the MMTk-level
// allocation unit.
VALUE obj = (VALUE)addr + prefix_size;

// Store the Ruby-level object size before the object.
rb_mmtk_init_hidden_header(obj, size_pool_size);

rb_mmtk_post_alloc(obj, mmtk_alloc_size, semantics);

return obj;
Expand Down

0 comments on commit c300b86

Please sign in to comment.