Skip to content

Conversation

@gbaraldi
Copy link
Member

@gbaraldi gbaraldi commented Jan 12, 2026

Fixes #60622

This was a very messy thing. But apparently the code we generate for value_to_pointer is liable to being transformed by LLVM, and under some transformations specifically LICM we can materialize a root that didn't exist at codegen time and that root is liable to being being undef which can cause segfaults
This conservativaly initializes the gc pointers to zero.

@oscardssmith oscardssmith added compiler:codegen Generation of LLVM IR and native code needs tests Unit tests are required for this change bugfix This change fixes an existing bug backport 1.12 Change should be backported to release-1.12 labels Jan 12, 2026
@gbaraldi
Copy link
Member Author

Only issue was. Writing a test for this was almost impossible.

src/codegen.cpp Outdated
auto tracked = TrackCompositeType(ty);
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe);
Instruction *last_inserted = ctx.topalloca;
for (const auto &indices : tracked) {
Copy link
Member

@topolarity topolarity Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be as simple as:

    jl_datatype_t *dt = (jl_datatype_t *)v.typ;
    bool hasptr = dt->layout->first_ptr >= 0;
    size_t npointers = hasptr ? dt->layout->npointers : 0;
    auto InsertPoint = ctx.builder.saveIP();
    ctx.builder.SetInsertPoint(ctx.topalloca->getParent(), ++ctx.topalloca->getIterator());
    for (size_t i = 0; i < npointers; i++) {
        // make sure these are nullptr early from LLVM's perspective, in case it decides to SROA it
        Value *ptr_field = emit_ptrgep(ctx, loc, jl_ptr_offset(dt, i) * sizeof(void *));
        StoreInst *store = ctx.builder.CreateAlignedStore(
            Constant::getNullValue(ctx.types().T_prjlvalue), ptr_field, Align(sizeof(void *)));
    }
    ctx.builder.restoreIP(InsertPoint);

although you can get fancier if you want to coalesce adjacent roots to a single memset.

This logic should probably be moved to cgutils.cpp and implemented more similar to split_value_into / undef_derived_strct

@topolarity topolarity added the backport 1.13 Change should be backported to release-1.13 label Jan 13, 2026
@topolarity
Copy link
Member

topolarity commented Jan 13, 2026

Writing a test for this was almost impossible.

It might be a reasonable addition to the GCinvariantPass?
Any alloca containing a ptr(10) should set that memory to 0 in the first basic block IIUC.

edit: Gabriel pointed out that DCE and (post-)dominating stores / loads will break that check, so that won't work sadly.

@topolarity topolarity removed the needs tests Unit tests are required for this change label Jan 13, 2026
Use Julia's datatype layout information (jl_ptr_offset) to iterate
through pointer fields instead of walking LLVM types with
TrackCompositeType. This approach is simpler and consistent with
similar code like undef_derived_strct.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@gbaraldi
Copy link
Member Author

Addressed the review feedback - simplified the code to use Julia's datatype layout information (jl_ptr_offset) instead of walking LLVM types with TrackCompositeType, as suggested.

@DilumAluthge DilumAluthge mentioned this pull request Jan 15, 2026
40 tasks
@gbaraldi gbaraldi merged commit f9d461f into master Jan 16, 2026
8 checks passed
@gbaraldi gbaraldi deleted the gb/evenlessundef branch January 16, 2026 13:23
DilumAluthge pushed a commit that referenced this pull request Jan 16, 2026
@KristofferC KristofferC mentioned this pull request Jan 26, 2026
43 tasks
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Feb 3, 2026
@KristofferC KristofferC mentioned this pull request Feb 4, 2026
21 tasks
KristofferC pushed a commit that referenced this pull request Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.13 Change should be backported to release-1.13 bugfix This change fixes an existing bug compiler:codegen Generation of LLVM IR and native code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GC corruption / segfault on 1.12

4 participants