Skip to content

Commit

Permalink
Fix tbaa annotation on union selector bytes inside of structs (#54604)
Browse files Browse the repository at this point in the history
We currently cause a alias analysis contradiction by saying that the
unionselbytes are on the stack, even if they are on a struct. LLVM is
then able to figure out that we giving it a impossible alias situation
(the object doesn't alias itself) and triggers UB.

https://godbolt.org/z/ssEKMzsPf

We may want to do a benchmarks run on this to see if anything too
critical hasn't regressed.

Fixes #54599

---------

Co-authored-by: Cody Tapscott <84105208+topolarity@users.noreply.github.com>
  • Loading branch information
gbaraldi and topolarity authored Jun 5, 2024
1 parent a5d6b50 commit 30542e0
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/cgutils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2806,7 +2806,8 @@ static MDNode *best_field_tbaa(jl_codectx_t &ctx, const jl_cgval_t &strct, jl_da
}
}
if (strct.V && jl_field_isconst(jt, idx) && isLoadFromConstGV(strct.V))
return ctx.tbaa().tbaa_const;
return ctx.tbaa().tbaa_const; //TODO: it seems odd to have a field with a tbaa that doesn't alias it's containing struct's tbaa
//Does the fact that this is marked as constant make this fine?
return tbaa;
}

Expand Down Expand Up @@ -2903,7 +2904,7 @@ static jl_cgval_t emit_getfield_knownidx(jl_codectx_t &ctx, const jl_cgval_t &st
else {
ptindex = emit_struct_gep(ctx, cast<StructType>(lt), staddr, byte_offset + fsz1);
}
auto val = emit_unionload(ctx, addr, ptindex, jfty, fsz, al, tbaa, !jl_field_isconst(jt, idx), union_max, ctx.tbaa().tbaa_unionselbyte);
auto val = emit_unionload(ctx, addr, ptindex, jfty, fsz, al, tbaa, !jl_field_isconst(jt, idx), union_max, strct.tbaa);
if (val.V && val.V != addr) {
setNameWithField(ctx.emission_context, val.V, get_objname, jt, idx, Twine());
}
Expand Down
9 changes: 9 additions & 0 deletions test/compiler/codegen.jl
Original file line number Diff line number Diff line change
Expand Up @@ -938,3 +938,12 @@ BigStructAnyInt() = BigStructAnyInt((Union{Base.inferencebarrier(Float64), Int}=
@test egal_any54109(Torture1_54109(), Torture1_54109())
@test egal_any54109(Torture2_54109(), Torture2_54109())
@test !egal_any54109(Torture1_54109(), Torture1_54109((DefaultOr54109(2.0, false) for i = 1:897)...))

function foo54599()
pkgid = Base.identify_package("Test")
println(devnull,pkgid)
println(devnull, pkgid.uuid)
pkgid.uuid
end

@test foo54599() !== nothing

0 comments on commit 30542e0

Please sign in to comment.