Skip to content

Commit

Permalink
Differentiate OBJ_PIN and PTR_PIN. Add more pinning. Trace all global…
Browse files Browse the repository at this point in the history
… roots. (mmtk#84)

This PR
* differentiates `OBJ_PIN` from `PTR_PIN`. In rare cases, we have to deal with internal pointers, and have to use `PTR_PIN`.
* pins more objects that cannot be moved.
* traces all the `JL_GLOBALLY_ROOTED` symbols.

This PR still transitively pins `jl_global_roots_list`. Without transitive pinning, we observed assertions failures in the precompilation step during Julia build, saying we reach objects without the valid object bit. This issue will be debugged and fixed after this PR.
  • Loading branch information
qinsoon committed Feb 6, 2025
1 parent 217fe9a commit 39a9bc4
Show file tree
Hide file tree
Showing 22 changed files with 381 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/aotcompile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ typedef struct {
SmallVector<GlobalValue*, 0> jl_sysimg_fvars;
SmallVector<GlobalValue*, 0> jl_sysimg_gvars;
std::map<jl_code_instance_t*, std::tuple<uint32_t, uint32_t>> jl_fvar_map;
// This holds references to the heap. Need to be pinned.
SmallVector<void*, 0> jl_value_to_llvm;
SmallVector<jl_code_instance_t*, 0> jl_external_to_llvm;
} jl_native_code_desc_t;
Expand Down
4 changes: 3 additions & 1 deletion src/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,9 @@ static value_t julia_to_list2_noalloc(fl_context_t *fl_ctx, jl_value_t *a, jl_va

static value_t julia_to_scm_(fl_context_t *fl_ctx, jl_value_t *v, int check_valid)
{
PTR_PIN(v);
// The following code will take internal pointers to v's fields. We need to make sure
// that v will not be moved by GC.
OBJ_PIN(v);
value_t retval;
if (julia_to_scm_noalloc1(fl_ctx, v, &retval))
return retval;
Expand Down
11 changes: 11 additions & 0 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,9 @@ static uintptr_t type_object_id_(jl_value_t *v, jl_varidx_t *env) JL_NOTSAFEPOIN
i++;
pe = pe->prev;
}
// FIXME: Pinning objects that get hashed
// until we implement address space hashing.
OBJ_PIN(v);
uintptr_t bits = jl_astaggedvalue(v)->header;
if (bits & GC_IN_IMAGE)
return ((uintptr_t*)v)[-2];
Expand Down Expand Up @@ -400,6 +403,10 @@ static uintptr_t immut_id_(jl_datatype_t *dt, jl_value_t *v, uintptr_t h) JL_NOT
// a few select pointers (notably symbol) also have special hash values
// which may affect the stability of the objectid hash, even though
// they don't affect egal comparison

// FIXME: Pinning objects that get hashed
// until we implement address space hashing.
PTR_PIN(v); // This has to be a pointer pin -- v could be an internal pointer
return bits_hash(v, sz) ^ h;
}
if (dt == jl_unionall_type)
Expand Down Expand Up @@ -460,6 +467,10 @@ static uintptr_t NOINLINE jl_object_id__cold(uintptr_t tv, jl_value_t *v) JL_NOT
uintptr_t bits = jl_astaggedvalue(v)->header;
if (bits & GC_IN_IMAGE)
return ((uintptr_t*)v)[-2];

// FIXME: Pinning objects that get hashed
// until we implement address space hashing.
OBJ_PIN(v);
return inthash((uintptr_t)v);
}
return immut_id_(dt, v, dt->hash);
Expand Down
4 changes: 3 additions & 1 deletion src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ static Constant *julia_pgv(jl_codectx_t &ctx, const char *cname, void *addr)
// emit a GlobalVariable for a jl_value_t named "cname"
// store the name given so we can reuse it (facilitating merging later)
// so first see if there already is a GlobalVariable for this address
OBJ_PIN(addr); // This will be stored in the native heap. We need to pin it.
GlobalVariable* &gv = ctx.emission_context.global_targets[addr];
Module *M = jl_Module;
StringRef localname;
Expand Down Expand Up @@ -570,7 +571,8 @@ static Value *literal_pointer_val(jl_codectx_t &ctx, jl_value_t *p)
{
if (p == NULL)
return Constant::getNullValue(ctx.types().T_pjlvalue);
PTR_PIN(p);
// Pointers to p will be emitted into the code. Make sure p won't be moved by GC.
OBJ_PIN(p);
Value *pgv = literal_pointer_val_slot(ctx, p);
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_const);
auto load = ai.decorateInst(maybe_mark_load_dereferenceable(
Expand Down
10 changes: 10 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1823,6 +1823,7 @@ struct jl_cgval_t {
promotion_point(nullptr),
promotion_ssa(-1)
{
OBJ_PIN(typ); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
assert(TIndex == nullptr || TIndex->getType() == getInt8Ty(TIndex->getContext()));
}
jl_cgval_t(Value *Vptr, bool isboxed, jl_value_t *typ, Value *tindex, MDNode *tbaa, Value* inline_roots) = delete;
Expand All @@ -1839,6 +1840,7 @@ struct jl_cgval_t {
promotion_point(nullptr),
promotion_ssa(-1)
{
OBJ_PIN(typ); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
if (Vboxed)
assert(Vboxed->getType() == JuliaType::get_prjlvalue_ty(Vboxed->getContext()));
assert(tbaa != nullptr);
Expand All @@ -1859,6 +1861,8 @@ struct jl_cgval_t {
promotion_point(nullptr),
promotion_ssa(-1)
{
OBJ_PIN(typ); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
OBJ_PIN(constant); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
assert(jl_is_datatype(typ));
assert(constant);
}
Expand All @@ -1875,6 +1879,8 @@ struct jl_cgval_t {
promotion_point(v.promotion_point),
promotion_ssa(v.promotion_ssa)
{
OBJ_PIN(typ); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
OBJ_PIN(constant); // jl_cgval_t could be in the native heap. We have to pin the object references in it.
if (Vboxed)
assert(Vboxed->getType() == JuliaType::get_prjlvalue_ty(Vboxed->getContext()));
// this constructor expects we had a badly or equivalently typed version
Expand Down Expand Up @@ -1947,6 +1953,7 @@ class jl_codectx_t {
std::map<int, jl_varinfo_t> phic_slots;
std::map<int, std::pair<Value*, Value*> > scope_restore;
SmallVector<jl_cgval_t, 0> SAvalues;
// The vector holds reference to Julia obj ref. We need to pin jl_value_t*.
SmallVector<std::tuple<jl_cgval_t, BasicBlock *, AllocaInst *, PHINode *, SmallVector<PHINode*,0>, jl_value_t *>, 0> PhiNodes;
SmallVector<bool, 0> ssavalue_assigned;
SmallVector<int, 0> ssavalue_usecount;
Expand Down Expand Up @@ -6254,6 +6261,7 @@ static void emit_phinode_assign(jl_codectx_t &ctx, ssize_t idx, jl_value_t *r)
decay_derived(ctx, phi));
jl_cgval_t val = mark_julia_slot(ptr, phiType, Tindex_phi, best_tbaa(ctx.tbaa(), phiType));
val.Vboxed = ptr_phi;
OBJ_PIN(r); // r will be saved to a data structure in the native heap, make sure it won't be moved by GC.
ctx.PhiNodes.push_back(std::make_tuple(val, BB, dest, ptr_phi, roots, r));
ctx.SAvalues[idx] = val;
ctx.ssavalue_assigned[idx] = true;
Expand All @@ -6263,6 +6271,7 @@ static void emit_phinode_assign(jl_codectx_t &ctx, ssize_t idx, jl_value_t *r)
PHINode *Tindex_phi = PHINode::Create(getInt8Ty(ctx.builder.getContext()), jl_array_nrows(edges), "tindex_phi");
Tindex_phi->insertInto(BB, InsertPt);
jl_cgval_t val = mark_julia_slot(NULL, phiType, Tindex_phi, ctx.tbaa().tbaa_stack);
OBJ_PIN(r); // r will be saved to a data structure in the native heap, make sure it won't be moved by GC.
ctx.PhiNodes.push_back(std::make_tuple(val, BB, dest, (PHINode*)nullptr, roots, r));
ctx.SAvalues[idx] = val;
ctx.ssavalue_assigned[idx] = true;
Expand Down Expand Up @@ -6313,6 +6322,7 @@ static void emit_phinode_assign(jl_codectx_t &ctx, ssize_t idx, jl_value_t *r)
value_phi->insertInto(BB, InsertPt);
slot = mark_julia_type(ctx, value_phi, isboxed, phiType);
}
OBJ_PIN(r); // r will be saved to a data structure in the native heap, make sure it won't be moved by GC.
ctx.PhiNodes.push_back(std::make_tuple(slot, BB, dest, value_phi, roots, r));
ctx.SAvalues[idx] = slot;
ctx.ssavalue_assigned[idx] = true;
Expand Down
6 changes: 6 additions & 0 deletions src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ JL_DLLEXPORT jl_typename_t *jl_new_typename_in(jl_sym_t *name, jl_module_t *modu
jl_typename_t *tn =
(jl_typename_t*)jl_gc_alloc(ct->ptls, sizeof(jl_typename_t),
jl_typename_type);
// Typenames should be pinned since they are used as metadata, and are
// read during scan_object
OBJ_PIN(tn);
tn->name = name;
tn->module = module;
tn->wrapper = NULL;
Expand Down Expand Up @@ -96,6 +99,9 @@ jl_datatype_t *jl_new_uninitialized_datatype(void)
{
jl_task_t *ct = jl_current_task;
jl_datatype_t *t = (jl_datatype_t*)jl_gc_alloc(ct->ptls, sizeof(jl_datatype_t), jl_datatype_type);
// Types should be pinned since they are used as metadata, and are
// read during scan_object
OBJ_PIN(t);
jl_set_typetagof(t, jl_datatype_tag, 0);
t->hash = 0;
t->hasfreetypevars = 0;
Expand Down
4 changes: 4 additions & 0 deletions src/gc-interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ JL_DLLEXPORT void jl_gc_set_max_memory(uint64_t max_mem);
JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection);
// Returns whether the thread with `tid` is a collector thread
JL_DLLEXPORT int gc_is_collector_thread(int tid) JL_NOTSAFEPOINT;
// Pinning objects; Returns whether the object has been pinned by this call.
JL_DLLEXPORT unsigned char jl_gc_pin_object(void* obj);
// Pinning objects through a potential internal pointer; Returns whether the object has been pinned by this call.
JL_DLLEXPORT unsigned char jl_gc_pin_pointer(void* ptr);
// Returns which GC implementation is being used and possibly its version according to the list of supported GCs
// NB: it should clearly identify the GC by including e.g. ‘stock’ or ‘mmtk’ as a substring.
JL_DLLEXPORT const char* jl_gc_active_impl(void);
Expand Down
Loading

0 comments on commit 39a9bc4

Please sign in to comment.