Skip to content

Audit LLVM ABI size #12127

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

Jump to bottom
Merged
merged 5 commits into from
Jul 15, 2022
Merged

Audit LLVM ABI size #12127

merged 5 commits into from
Jul 15, 2022

Conversation

andrewrk
Copy link
Member

@andrewrk andrewrk commented Jul 15, 2022 •
edited
Loading

As @topolarity noticed, sometimes the LLVM backend lowers types in a way that do not have an LLVM ABI size that matches the Zig ABI size. This is essentially unwanted undefined behavior whenever it occurs.

This branch has a mechanism to detect when this occurs and log about it in a debug build of the compiler. I have it disabled in the very last commit because there are more instances to fix before turning it on. However this branch already fixes two instances, one in optional types and one in error unions. Both are fixed by adding padding bytes at the end.

Note that we never call LLVMABISizeOfType because this fails when lowering types that reference pointers of themselves. Rather than work around this, it is much simpler for Zig to compute the padding itself and add it unconditionally.

Current Status of Audit

Behavior tests cause many messages to be emitted and I do not have those listed here.

However here are the messages for building stage3, in the hopes that this branch could solve #11450:

error(codegen): when lowering builtin.Type.Struct, Zig ABI size = 32 but LLVM ABI size = 48
error(codegen): when lowering @Vector(5, usize), Zig ABI size = 40 but LLVM ABI size = 64
error(codegen): when lowering @Vector(3, usize), Zig ABI size = 24 but LLVM ABI size = 32
error(codegen): when lowering @Vector(3, u1), Zig ABI size = 3 but LLVM ABI size = 1
error(codegen): when lowering @Vector(2, u1), Zig ABI size = 2 but LLVM ABI size = 1
error(codegen): when lowering tuple{@Vector(3, usize), @Vector(3, u1)}, Zig ABI size = 32 but LLVM ABI size = 64

Finally the logging causes a crash because a type did not properly get resolveTypeFully called before it was sent to the linker for lowering:

thread 3921472 panic: reached unreachable code
Analyzing /home/andy/dev/zig/src/translate_c/ast.zig: translate_c/ast.zig:renderNode
      %9404 = dbg_block_begin())
      %9405 = ret_ptr() 
      %9406 = decl_val("renderRecord") 
      %9407 = dbg_stmt(980, 53)
      %9408 = param_type(%9406, 0)
      %9409 = param_type(%9406, 1)
      %9410 = call(.auto, %9406, [%2448, %2451]) 
    > %9411 = store_node(%9405, %9410) 
      %9413 = dbg_block_end())
      %9412 = ret_load(%9405) 
    For full context, use the command
      zig ast-check -t /home/andy/dev/zig/src/translate_c/ast.zig

  in /home/andy/dev/zig/src/translate_c/ast.zig: translate_c/ast.zig:renderNode
    > %2465 = switch_block(%2463,
        %2467 => {%2468..%2469},
        %2472 => {%2473..%2516},
        %2518 => {%2519..%2559},
        %2561 => {%2562..%2605},
        %2607 => {%2608..%2652},
        %2654 => {%2655..%2695},
        %2697 => {%2698..%2739},
        %2741 => {%2742..%2782},
        %2784 => {%2785..%2827},
        %2829 => {%2830..%2873},
        %2875 => {%2876..%2919},
        %2921 => {%2922..%2950},
        %2952 => {%2953..%2991},
        %2993 => {%2994..%3021},
        %3024 => {%3025..%3052},
        %3055 => {%3056..%3083},
        %3086 => {%3087..%3114},
        %3117 => {%3118..%3145},
        %3148 => {%3149..%3176},
        %3179 => {%3180..%3207},
        %3210 => {%3211..%3238},
        %3241 => {%3242..%3274},
        %3277 => {%3278..%3310},
        %3313 => {%3314..%3346},
        %3349 => {%3350..%3456},
        %3458 => {%3459..%3514},
        %3516 => {%3517..%3572},
        %3574 => {%3575..%3630},
        %3632 => {%3633..%3716},
        %3718 => {%3719..%3760},
        %3762 => {%3763..%3807},
        %3809 => {%3810..%3849},
        %3851 => {%3852..%3893},
        %3895 => {%3896..%3937},
        %3939 => {%3940..%3981},
        %3983 => {%3984..%4025},
        %4027 => {%4028..%4082},
        %4084 => {%4085..%4129},
        %4131 => {%4132..%4173},
        %4175 => {%4176..%4350},
        %4352 => {%4353..%4564},
        %4700 => {%4701..%4864},
        %4866 => {%4867..%5000},
        %5002 => {%5003..%5011},
        %5193 => {%5194..%5222},
        %5224 => {%5225..%5268},
        %5270 => {%5271..%5299},
        %5301 => {%5302..%5328},
        %5330 => {%5331..%5359},
        %5361 => {%5362..%5390},
        %5392 => {%5393..%5421},
        %5423 => {%5424..%5452},
        %5454 => {%5455..%5483},
        %5485 => {%5486..%5514},
        %5516 => {%5517..%5545},
        %5547 => {%5548..%5574},
        %5576 => {%5577..%5605},
        %5607 => {%5608..%5636},
        %5638 => {%5639..%5667},
        %5669 => {%5670..%5698},
        %5700 => {%5701..%5727},
        %5729 => {%5730..%5760},
        %5762 => {%5763..%5789},
        %5791 => {%5792..%5818},
        %5820 => {%5821..%5847},
        %5849 => {%5850..%5864},
        %5867 => {%5868..%5882},
        %5885 => {%5886..%5900},
        %5903 => {%5904..%5918},
        %5921 => {%5922..%5936},
        %5939 => {%5940..%5954},
        %5957 => {%5958..%6021},
        %6023 => {%6024..%6102},
        %6296 => {%6297..%6311},
        %6314 => {%6315..%6329},
        %6332 => {%6333..%6347},
        %6350 => {%6351..%6365},
        %6368 => {%6369..%6383},
        %6386 => {%6387..%6401},
        %6404 => {%6405..%6419},
        %6422 => {%6423..%6437},
        %6440 => {%6441..%6455},
        %6458 => {%6459..%6473},
        %6476 => {%6477..%6491},
        %6494 => {%6495..%6509},
        %6512 => {%6513..%6527},
        %6530 => {%6531..%6545},
        %6548 => {%6549..%6563},
        %6566 => {%6567..%6581},
        %6584 => {%6585..%6599},
        %6602 => {%6603..%6617},
        %6620 => {%6621..%6635},
        %6638 => {%6639..%6653},
        %6656 => {%6657..%6671},
        %6674 => {%6675..%6689},
        %6692 => {%6693..%6707},
        %6710 => {%6711..%6725},
        %6728 => {%6729..%6743},
        %6746 => {%6747..%6761},
        %6764 => {%6765..%6779},
        %6782 => {%6783..%6797},
        %6800 => {%6801..%6815},
        %6818 => {%6819..%6833},
        %6836 => {%6837..%6851},
        %6854 => {%6855..%6869},
        %6872 => {%6873..%6887},
        %6890 => {%6891..%6905},
        %6908 => {%6909..%6923},
        %6926 => {%6927..%6941},
        %6944 => {%6945..%6959},
        %6962 => {%6963..%7017},
        %7019 => {%7020..%7106},
        %7108 => {%7109..%7368},
        %7370 => {%7371..%7379},
        %7382 => {%7383..%7391},
        %7394 => {%7395..%7497},
        %7499 => {%7500..%7735},
        %7737 => {%7738..%7858},
        %7860 => {%7861..%8037},
        %8039 => {%8040..%8197},
        %8199 => {%8200..%8471},
        %8473 => {%8474..%8542},
        %8544 => {%8545..%8895},
        %8897 => {%8898..%8965},
        %8967 => {%8968..%9058},
        %9060 => {%9061..%9087},
        %9089 => {%9090..%9116},
        %9118 => {%9119..%9276},
        %9278 => {%9279..%9317},
        %9319 => {%9320..%9358},
        %9360 => {%9361..%9399},
        %9415 => {%9416..%9587},
        %9589 => {%9590..%9878},
        %9880 => {%9881..%10219},
        %10221 => {%10222..%10578},
        %10580 => {%10581..%10582},
        %4566, %4568 => {%4569..%4698},
        %5014, %5016 => {%5017..%5191},
        %6104, %6106 => {%6107..%6294},
        %9401, %9403 => {%9404..%9412}) 

/home/andy/dev/zig/src/type.zig:5097:39: 0x2fe177d in type.Type.comptimeOnly (zig)
                    .wip, .unknown => unreachable, // This function asserts types already resolved.
                                      ^
/home/andy/dev/zig/src/type.zig:5073:49: 0x2fe1505 in type.Type.comptimeOnly (zig)
                    return child_ty.comptimeOnly();
                                                ^
/home/andy/dev/zig/src/type.zig:2421:41: 0x32a1bd1 in type.Type.hasRuntimeBitsAdvanced (zig)
                    return !comptimeOnly(ty);
                                        ^
/home/andy/dev/zig/src/type.zig:2653:38: 0x2fe0f8a in type.Type.hasRuntimeBits (zig)
        return hasRuntimeBitsAdvanced(ty, false, null) catch unreachable;
                                     ^
/home/andy/dev/zig/src/codegen/llvm.zig:2430:34: 0x32c2c29 in codegen.llvm.DeclGen.lowerType (zig)
            if (!t.hasRuntimeBits()) break :check;
                                 ^
/home/andy/dev/zig/src/codegen/llvm.zig:5193:45: 0x3694daf in codegen.llvm.FuncGen.airFieldParentPtr (zig)
        const res_ty = try self.dg.lowerType(self.air.getRefType(ty_pl.ty));
                                            ^
/home/andy/dev/zig/src/codegen/llvm.zig:4131:64: 0x3666d64 in codegen.llvm.FuncGen.genBody (zig)
                .field_parent_ptr => try self.airFieldParentPtr(inst),
                                                               ^
/home/andy/dev/zig/src/codegen/llvm.zig:966:19: 0x36607cf in codegen.llvm.Object.updateFunc (zig)
        fg.genBody(air.getMainBody()) catch |err| switch (err) {
                  ^
/home/andy/dev/zig/src/link/Elf.zig:2377:74: 0x3441f77 in link.Elf.updateFunc (zig)
        if (self.llvm_object) |llvm_object| return llvm_object.updateFunc(module, func, air, liveness);
                                                                         ^
/home/andy/dev/zig/src/link.zig:509:77: 0x3273d2d in link.File.updateFunc (zig)
            .elf   => return @fieldParentPtr(Elf,   "base", base).updateFunc(module, func, air, liveness),
                                                                            ^
/home/andy/dev/zig/src/Module.zig:3858:41: 0x3254045 in Module.ensureFuncBodyAnalyzed (zig)
            mod.comp.bin_file.updateFunc(mod, func, air, liveness) catch |err| switch (err) {
                                        ^
/home/andy/dev/zig/src/Sema.zig:23218:36: 0x38bdbf3 in Sema.ensureFuncBodyAnalyzed (zig)
    sema.mod.ensureFuncBodyAnalyzed(func) catch |err| {
                                   ^
/home/andy/dev/zig/src/Sema.zig:25079:40: 0x38064d0 in Sema.resolveInferredErrorSet (zig)
        try sema.ensureFuncBodyAnalyzed(ies.func);
                                       ^
/home/andy/dev/zig/src/Sema.zig:21334:45: 0x38b6574 in Sema.coerceInMemoryAllowedErrorSets (zig)
            try sema.resolveInferredErrorSet(block, src_src, src_data);
                                            ^
/home/andy/dev/zig/src/Sema.zig:21174:55: 0x36d5c3a in Sema.coerceInMemoryAllowed (zig)
        return try sema.coerceInMemoryAllowedErrorSets(block, dest_ty, src_ty, dest_src, src_src);
                                                      ^
/home/andy/dev/zig/src/Sema.zig:21169:46: 0x36d5b09 in Sema.coerceInMemoryAllowed (zig)
        return try sema.coerceInMemoryAllowed(block, dest_ty.errorUnionSet(), src_ty.errorUnionSet(), dest_is_mut, target, dest_src, src_src);
                                             ^
/home/andy/dev/zig/src/Sema.zig:20180:58: 0x345f5fc in Sema.coerceExtra (zig)
    var in_memory_result = try sema.coerceInMemoryAllowed(block, dest_ty, inst_ty, false, target, dest_ty_src, inst_src);
                                                         ^
/home/andy/dev/zig/src/Sema.zig:21692:37: 0x364fb48 in Sema.storePtr2 (zig)
    const operand = sema.coerceExtra(block, elem_ty, uncasted_operand, operand_src, true, is_ret) catch |err| switch (err) {
                                    ^
/home/andy/dev/zig/src/Sema.zig:4273:26: 0x3639aff in Sema.zirStoreNode (zig)
    return sema.storePtr2(block, src, ptr, src, operand, src, if (is_ret) .ret_ptr else .store);
                         ^
/home/andy/dev/zig/src/Sema.zig:1035:38: 0x3456ddc in Sema.analyzeBodyInner (zig)
                try sema.zirStoreNode(block, inst);
                                     ^
/home/andy/dev/zig/src/Sema.zig:9062:38: 0x35e727b in Sema.zirSwitchBlock (zig)
            _ = sema.analyzeBodyInner(&case_block, body) catch |err| switch (err) {
                                     ^
/home/andy/dev/zig/src/Sema.zig:780:69: 0x344edbb in Sema.analyzeBodyInner (zig)
            .switch_block                 => try sema.zirSwitchBlock(block, inst),
                                                                    ^
/home/andy/dev/zig/src/Sema.zig:598:30: 0x343f54a in Sema.analyzeBody (zig)
    _ = sema.analyzeBodyInner(block, body) catch |err| switch (err) {
                             ^
/home/andy/dev/zig/src/Module.zig:5101:21: 0x3271126 in Module.analyzeFnBody (zig)
    sema.analyzeBody(&inner_block, fn_info.body) catch |err| switch (err) {
                    ^
/home/andy/dev/zig/src/Module.zig:3829:40: 0x3253c20 in Module.ensureFuncBodyAnalyzed (zig)
            var air = mod.analyzeFnBody(func, sema_arena) catch |err| switch (err) {
                                       ^
/home/andy/dev/zig/src/Compilation.zig:2963:42: 0x2fd027a in Compilation.processOneJob (zig)
            module.ensureFuncBodyAnalyzed(func) catch |err| switch (err) {
                                         ^
/home/andy/dev/zig/src/Compilation.zig:2895:30: 0x2fbbffd in Compilation.performAllTheWork (zig)
            try processOneJob(comp, work_item);
                             ^
/home/andy/dev/zig/src/Compilation.zig:2243:31: 0x2fb4652 in Compilation.update (zig)
    try comp.performAllTheWork(main_progress_node);
                              ^
/home/andy/dev/zig/src/main.zig:3288:20: 0x2f1f94f in updateModule (zig)
    try comp.update();
                   ^

andrewrk added 5 commits July 14, 2022 22:26
@andrewrk
LLVM: insert debug logging when LLVM ABI size is wrong
070e3ea
@andrewrk
LLVM: add padding to optional types when lowering
9c136be 
If the LLVM ABI size does not agree with the Zig ABI size.
@andrewrk
Revert "stage2 llvm: Use unpacked struct for unions and arrays"
8c14d17 
This reverts commit 2eaef84.

Here is a motivating example:

```zig
const E = union(enum) {
    A: [9]u8,
    B: u64,
};
```

```llvm
%test2.E = type { { i64, [1 x i8] }, i1, [6 x i8] }
```

```
error(codegen): when lowering test2.E, Zig ABI size = 16 but LLVM ABI size = 24
```
@andrewrk
LLVM: fix ABI size of optional and error union types
040cb58 
Previously, the Zig ABI size and LLVM ABI size of these types disagreed
sometimes. This code also corrects the logging messages to not trigger
LLVM assertions.
@andrewrk
LLVM: disable the ABI size safety check
9329b93 
There are many more instances of this check being tripped that we need
to fix before we can enable this.
@andrewrk andrewrk merged commit e867127 into master Jul 15, 2022
@andrewrk andrewrk deleted the llvm-abi-size branch July 15, 2022 18:48
@andrewrk andrewrk mentioned this pull request Jul 17, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@andrewrk