Skip to content
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.

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

Compilation: check if there is anything in the clang error bundle #20031

Closed
wants to merge 1 commit into from

Conversation

kubkon
Copy link
Member

@kubkon kubkon commented May 22, 2024

As far as I can tell, clang emits empty diagnostics for hand-crafted assembly files, however the diagnostics bundle is still well-formed, it just does not contain any block beyond META comprising VERSION record. This is most unfortunate as any errors in assembly source files will not be caught and lead to panics down the line. The solution seems to be to check for the number of parsed diagnostics before returning back to the frontend. I based this on the existence of a similar C exposed function in libclang.

For more context, here's what we see for some malformed C source file in terms of serialized diagnostics:

* magic is fine
* Item{  .start_block=Block{ .name=Meta, .id=8, .len=2 } }
* Item{  .record=Rec{ .name=Version, .id=1, .operands={ 2 }, .blob= } }
* Item{  .end_block=Block{ .name=Meta, .id=8, .len=0 } }
* Item{  .start_block=Block{ .name=Diag, .id=9, .len=34 } }
* Item{  .record=Rec{ .name=FileName, .id=6, .operands={ 1, 0, 0, 7 }, .blob=68656c6c6f2e63 } }
* Item{  .record=Rec{ .name=CatName, .id=5, .operands={ 1, 29 }, .blob=4c65786963616c206f722050726570726f636573736f72204973737565 } }
* Item{  .record=Rec{ .name=DiagInfo, .id=2, .operands={ 4, 1, 1, 10, 9, 1, 0, 24 }, .blob=27737464696f2e68272066696c65206e6f7420666f756e64 } }
* Item{  .record=Rec{ .name=SrcRange, .id=3, .operands={ 1, 1, 10, 9, 1, 1, 19, 18 }, .blob= } }
* Item{  .end_block=Block{ .name=Diag, .id=9, .len=0 } }
Number of diagnostics: 1

And here's what we see for malformed ASM source file:

* magic is fine
* Item{  .start_block=Block{ .name=Meta, .id=8, .len=2 } }
* Item{  .record=Rec{ .name=Version, .id=1, .operands={ 2 }, .blob= } }
* Item{  .end_block=Block{ .name=Meta, .id=8, .len=0 } }
Number of diagnostics: 0

EDIT: forgot to add that without any check like the above, when compiling malformed ASM source, we are greeted with a rather unhelpful panic:

error: thread 2173406 panic: access of union field 'success' while field 'failure' is active
/Users/kubkon/dev/zig/src/Compilation.zig:2531:58: 0x10c88ce3b in emitOthers (zig)
    const obj_path = comp.c_object_table.keys()[0].status.success.object_path;
                                                         ^
/Users/kubkon/dev/zig/src/Compilation.zig:2168:20: 0x10c888367 in update (zig)
    comp.emitOthers();
                   ^
/Users/kubkon/dev/zig/src/main.zig:4091:36: 0x10c8f5f47 in serve (zig)
                    try comp.update(main_progress_node);
                                   ^
/Users/kubkon/dev/zig/src/main.zig:3368:22: 0x10c9124c7 in buildOutputType (zig)
            try serve(
                     ^
/Users/kubkon/dev/zig/src/main.zig:266:31: 0x10c78f4b7 in mainArgs (zig)
        return buildOutputType(gpa, arena, args, .{ .build = .Obj });
                              ^
/Users/kubkon/dev/zig/src/main.zig:208:20: 0x10c78c8fb in main (zig)
    return mainArgs(gpa, arena, args);
                   ^
/Users/kubkon/dev/zig/build/stage3/lib/zig/std/start.zig:524:37: 0x10c78c53f in main (zig)
            const result = root.main() catch |err| {
                                    ^
???:?:?: 0x19571a0df in ??? (???)
???:?:?: 0xa10ffffffffffff in ??? (???)

when there is an actual error printed by clang frontend:

/Users/kubkon/dev/zig/zig-cache/o/9881456f058d6b83f5ca2b78b3eee487/a.s:6:7: error: invalid operand for instruction
  mov rax, L._q1

As far as I can tell, clang emits empty diagnostics for hand-crafted
assembly files, however the diagnostics bundle is still well-formed,
it just does not contain any block beyond META comprising VERSION
record. This is most unfortunate as any errors in assembly source files
will not be caught and lead to panics down the line. The solution seems
to be to check for the number of parsed diagnostics before returning
back to the frontend. I based this on the existence of a similar
C exposed function in libclang.

For more context, here's what we see for some malformed C source file
in terms of serialized diagnostics:

```
* magic is fine
* Item{  .start_block=Block{ .name=Meta, .id=8, .len=2 } }
* Item{  .record=Rec{ .name=Version, .id=1, .operands={ 2 }, .blob= } }
* Item{  .end_block=Block{ .name=Meta, .id=8, .len=0 } }
* Item{  .start_block=Block{ .name=Diag, .id=9, .len=34 } }
* Item{  .record=Rec{ .name=FileName, .id=6, .operands={ 1, 0, 0, 7 }, .blob=68656c6c6f2e63 } }
* Item{  .record=Rec{ .name=CatName, .id=5, .operands={ 1, 29 }, .blob=4c65786963616c206f722050726570726f636573736f72204973737565 } }
* Item{  .record=Rec{ .name=DiagInfo, .id=2, .operands={ 4, 1, 1, 10, 9, 1, 0, 24 }, .blob=27737464696f2e68272066696c65206e6f7420666f756e64 } }
* Item{  .record=Rec{ .name=SrcRange, .id=3, .operands={ 1, 1, 10, 9, 1, 1, 19, 18 }, .blob= } }
* Item{  .end_block=Block{ .name=Diag, .id=9, .len=0 } }
Number of diagnostics: 1
```

And here's what we see for malformed ASM source file:

```
* magic is fine
* Item{  .start_block=Block{ .name=Meta, .id=8, .len=2 } }
* Item{  .record=Rec{ .name=Version, .id=1, .operands={ 2 }, .blob= } }
* Item{  .end_block=Block{ .name=Meta, .id=8, .len=0 } }
Number of diagnostics: 0
```
@jacobly0
Copy link
Member

Closing in favor of #20068

@jacobly0 jacobly0 closed this May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants