Skip to content

codegen: fix alignment of source in typed_load from a unsafe_load #57845

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 21, 2025
Merged

codegen: fix alignment of source in typed_load from a unsafe_load #57845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ccall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ static Value *julia_to_native(
Align align(julia_alignment(jlto));
Value *slot = emit_static_alloca(ctx, to, align);
setName(ctx.emission_context, slot, "native_convert_buffer");
emit_unbox_store(ctx, jvinfo, slot, ctx.tbaa().tbaa_stack, align);
emit_unbox_store(ctx, jvinfo, slot, ctx.tbaa().tbaa_stack, align, align);
return slot;
}

Expand Down
31 changes: 17 additions & 14 deletions src/cgutils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V)
}

static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt);
static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value* dest, MDNode *tbaa_dest, Align alignment, bool isVolatile=false);
static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value* dest, MDNode *tbaa_dest, MaybeAlign align_src, Align align_dst, bool isVolatile=false);

static bool type_is_permalloc(jl_value_t *typ)
{
Expand Down Expand Up @@ -1090,7 +1090,7 @@ static void split_value_into(jl_codectx_t &ctx, const jl_cgval_t &x, Align align
return;
}
if (inline_roots_ptr == nullptr) {
emit_unbox_store(ctx, x, dst, ctx.tbaa().tbaa_stack, align_dst, isVolatileStore);
emit_unbox_store(ctx, x, dst, ctx.tbaa().tbaa_stack, align_src, align_dst, isVolatileStore);
return;
}
Value *src = data_pointer(ctx, value_to_pointer(ctx, x));
Expand Down Expand Up @@ -1152,7 +1152,7 @@ static void split_value_into(jl_codectx_t &ctx, const jl_cgval_t &x, Align align
return;
}
if (inline_roots.empty()) {
emit_unbox_store(ctx, x, dst, ctx.tbaa().tbaa_stack, align_dst);
emit_unbox_store(ctx, x, dst, ctx.tbaa().tbaa_stack, align_src, align_dst, false);
return;
}
Value *src = data_pointer(ctx, value_to_pointer(ctx, x));
Expand Down Expand Up @@ -2351,7 +2351,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
r = boxed(ctx, rhs);
}
else if (intcast) {
emit_unbox_store(ctx, rhs, intcast, ctx.tbaa().tbaa_stack, intcast->getAlign());
emit_unbox_store(ctx, rhs, intcast, ctx.tbaa().tbaa_stack, MaybeAlign(), intcast->getAlign());
r = ctx.builder.CreateLoad(realelty, intcast);
}
else if (aliasscope || Order != AtomicOrdering::NotAtomic || (tracked_pointers && rhs.inline_roots.empty())) {
Expand Down Expand Up @@ -2389,7 +2389,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
}
else {
assert(Order == AtomicOrdering::NotAtomic && !isboxed && rhs.typ == jltype);
emit_unbox_store(ctx, rhs, ptr, tbaa, Align(alignment));
emit_unbox_store(ctx, rhs, ptr, tbaa, MaybeAlign(), Align(alignment));
}
}
else if (isswapfield) {
Expand Down Expand Up @@ -2438,7 +2438,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
}
cmp = update_julia_type(ctx, cmp, jltype);
if (intcast) {
emit_unbox_store(ctx, cmp, intcast, ctx.tbaa().tbaa_stack, intcast->getAlign());
emit_unbox_store(ctx, cmp, intcast, ctx.tbaa().tbaa_stack, MaybeAlign(), intcast->getAlign());
Compare = ctx.builder.CreateLoad(realelty, intcast);
}
else {
Expand Down Expand Up @@ -2509,7 +2509,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
r = boxed(ctx, rhs);
}
else if (intcast) {
emit_unbox_store(ctx, rhs, intcast, ctx.tbaa().tbaa_stack, intcast->getAlign());
emit_unbox_store(ctx, rhs, intcast, ctx.tbaa().tbaa_stack, MaybeAlign(), intcast->getAlign());
r = ctx.builder.CreateLoad(realelty, intcast);
if (!tracked_pointers) // oldval is a slot, so put the oldval back
ctx.builder.CreateStore(realCompare, intcast);
Expand Down Expand Up @@ -2556,7 +2556,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
}
else {
assert(!isboxed && rhs.typ == jltype);
emit_unbox_store(ctx, rhs, ptr, tbaa, Align(alignment));
emit_unbox_store(ctx, rhs, ptr, tbaa, MaybeAlign(), Align(alignment));
}
ctx.builder.CreateBr(DoneBB);
instr = load;
Expand Down Expand Up @@ -3352,9 +3352,10 @@ static void init_bits_value(jl_codectx_t &ctx, Value *newv, Value *v, MDNode *tb
static void init_bits_cgval(jl_codectx_t &ctx, Value *newv, const jl_cgval_t &v)
{
MDNode *tbaa = jl_is_mutable(v.typ) ? ctx.tbaa().tbaa_mutab : ctx.tbaa().tbaa_immut;
Align newv_align{std::max(julia_alignment(v.typ), (unsigned)sizeof(void*))};
unsigned alignment = julia_alignment(v.typ);
unsigned newv_align = std::max(alignment, (unsigned)sizeof(void*));
newv = maybe_decay_tracked(ctx, newv);
emit_unbox_store(ctx, v, newv, tbaa, newv_align);
emit_unbox_store(ctx, v, newv, tbaa, Align(alignment), Align(newv_align));
}

static jl_value_t *static_constant_instance(const llvm::DataLayout &DL, Constant *constant, jl_value_t *jt)
Expand Down Expand Up @@ -3816,7 +3817,7 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
if (jl_is_pointerfree(typ)) {
emit_guarded_test(ctx, skip, nullptr, [&] {
unsigned alignment = julia_alignment(typ);
emit_unbox_store(ctx, mark_julia_const(ctx, src.constant), dest, tbaa_dst, Align(alignment), isVolatile);
emit_unbox_store(ctx, mark_julia_const(ctx, src.constant), dest, tbaa_dst, Align(alignment), Align(alignment), isVolatile);
return nullptr;
});
}
Expand All @@ -3826,7 +3827,7 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
if (jl_is_pointerfree(src.typ)) {
emit_guarded_test(ctx, skip, nullptr, [&] {
unsigned alignment = julia_alignment(src.typ);
emit_unbox_store(ctx, src, dest, tbaa_dst, Align(alignment), isVolatile);
emit_unbox_store(ctx, src, dest, tbaa_dst, Align(alignment), Align(alignment), isVolatile);
return nullptr;
});
}
Expand Down Expand Up @@ -4289,6 +4290,8 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
}
}
else {
Align align_dst(jl_field_align(sty, i));
Align align_src(julia_alignment(jtype));
if (field_promotable) {
fval_info.V->replaceAllUsesWith(dest);
cast<Instruction>(fval_info.V)->eraseFromParent();
Expand All @@ -4297,10 +4300,10 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
fval = emit_unbox(ctx, fty, fval_info, jtype);
}
else if (!roots.empty()) {
split_value_into(ctx, fval_info, Align(julia_alignment(jtype)), dest, Align(jl_field_align(sty, i)), jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), roots);
split_value_into(ctx, fval_info, align_src, dest, align_dst, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), roots);
}
else {
emit_unbox_store(ctx, fval_info, dest, ctx.tbaa().tbaa_stack, Align(jl_field_align(sty, i)));
emit_unbox_store(ctx, fval_info, dest, ctx.tbaa().tbaa_stack, align_src, align_dst);
}
}
if (init_as_value) {
Expand Down
17 changes: 10 additions & 7 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5724,7 +5724,7 @@ static void emit_vi_assignment_unboxed(jl_codectx_t &ctx, jl_varinfo_t &vi, Valu
if (vi.inline_roots)
split_value_into(ctx, rval_info, align, vi.value.V, align, jl_aliasinfo_t::fromTBAA(ctx, tbaa), vi.inline_roots, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe), vi.isVolatile);
else
emit_unbox_store(ctx, rval_info, vi.value.V, tbaa, align, vi.isVolatile);
emit_unbox_store(ctx, rval_info, vi.value.V, tbaa, align, align, vi.isVolatile);
}
}
}
Expand Down Expand Up @@ -6947,7 +6947,7 @@ static void emit_specsig_to_specsig(
split_value_into(ctx, gf_retval, align, sret, align, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), roots, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe));
}
else {
emit_unbox_store(ctx, gf_retval, sret, ctx.tbaa().tbaa_stack, align);
emit_unbox_store(ctx, gf_retval, sret, ctx.tbaa().tbaa_stack, align, align);
}
ctx.builder.CreateRetVoid();
break;
Expand Down Expand Up @@ -8730,11 +8730,12 @@ static jl_llvm_functions_t
++AI; // both specsig (derived) and fptr1 (box) pass this argument as a distinct argument
// Load closure world
Value *worldaddr = emit_ptrgep(ctx, oc_this, offsetof(jl_opaque_closure_t, world));
Align alignof_ptr(ctx.types().alignof_ptr);
jl_cgval_t closure_world = typed_load(ctx, worldaddr, NULL, (jl_value_t*)jl_long_type,
nullptr, nullptr, false, AtomicOrdering::NotAtomic, false, ctx.types().alignof_ptr.value());
nullptr, nullptr, false, AtomicOrdering::NotAtomic, false, alignof_ptr.value());
assert(ctx.world_age_at_entry == nullptr);
ctx.world_age_at_entry = closure_world.V; // The tls world in a OC is the world of the closure
emit_unbox_store(ctx, closure_world, world_age_field, ctx.tbaa().tbaa_gcframe, ctx.types().alignof_ptr);
emit_unbox_store(ctx, closure_world, world_age_field, ctx.tbaa().tbaa_gcframe, alignof_ptr, alignof_ptr);

if (s == jl_unused_sym || vi.value.constant)
continue;
Expand Down Expand Up @@ -9502,7 +9503,7 @@ static jl_llvm_functions_t
if (tracked)
split_value_into(ctx, typedval, align, dest, align, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), mayberoots);
else
emit_unbox_store(ctx, typedval, dest, ctx.tbaa().tbaa_stack, align);
emit_unbox_store(ctx, typedval, dest, ctx.tbaa().tbaa_stack, align, align);
}
return mayberoots;
});
Expand Down Expand Up @@ -9537,8 +9538,10 @@ static jl_llvm_functions_t
else {
if (VN)
V = Constant::getNullValue(ctx.types().T_prjlvalue);
if (dest)
emit_unbox_store(ctx, val, dest, ctx.tbaa().tbaa_stack, Align(julia_alignment(val.typ)));
if (dest) {
Align align(julia_alignment(val.typ));
emit_unbox_store(ctx, val, dest, ctx.tbaa().tbaa_stack, align, align);
}
RTindex = ConstantInt::get(getInt8Ty(ctx.builder.getContext()), tindex);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/intrinsics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_va
}

// emit code to store a raw value into a destination
static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value *dest, MDNode *tbaa_dest, Align alignment, bool isVolatile)
static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value *dest, MDNode *tbaa_dest, MaybeAlign align_src, Align align_dst, bool isVolatile)
{
if (x.isghost) {
// this can happen when a branch yielding a different type ends
Expand All @@ -507,22 +507,22 @@ static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value *dest
auto dest_ai = jl_aliasinfo_t::fromTBAA(ctx, tbaa_dest);

if (!x.inline_roots.empty()) {
recombine_value(ctx, x, dest, dest_ai, alignment, isVolatile);
recombine_value(ctx, x, dest, dest_ai, align_dst, isVolatile);
return;
}

if (!x.ispointer()) { // already unboxed, but sometimes need conversion (e.g. f32 -> i32)
assert(x.V);
Value *unboxed = zext_struct(ctx, x.V);
StoreInst *store = ctx.builder.CreateAlignedStore(unboxed, dest, alignment);
StoreInst *store = ctx.builder.CreateAlignedStore(unboxed, dest, align_dst);
store->setVolatile(isVolatile);
dest_ai.decorateInst(store);
return;
}

Value *src = data_pointer(ctx, x);
auto src_ai = jl_aliasinfo_t::fromTBAA(ctx, x.tbaa);
emit_memcpy(ctx, dest, dest_ai, src, src_ai, jl_datatype_size(x.typ), Align(alignment), Align(julia_alignment(x.typ)), isVolatile);
emit_memcpy(ctx, dest, dest_ai, src, src_ai, jl_datatype_size(x.typ), Align(align_dst), align_src ? *align_src : Align(julia_alignment(x.typ)), isVolatile);
}

static jl_datatype_t *staticeval_bitstype(const jl_cgval_t &targ)
Expand Down