Skip to content

add @trap builtin#14782

Merged
andrewrk merged 3 commits intomasterfrom
unknown repository
Mar 4, 2023
Merged

add @trap builtin#14782
andrewrk merged 3 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Mar 3, 2023

This introduces a new @trap builtin, which is very similar to @breakpoint in that both cause some kind of exception/interrupt usually. The code that the two builtins compile down to may equally stop the program entirely, but not always. Usually @breakpoint would allow the debugger to resume the program after inspections into the program's state have finished, whereas @trap does not really have anything to do with debuggers and is just there to exit the program in some way, but in an abnormal way.

Zig's @trap = LLVM's llvm.trap = Rust's std::instrinsics::abort
Zig's @breakpoint = LLVM's llvm.debugtrap = Rust's std::instrinsics::breakpoint

For example, here's what I imagine the two builtins to compile down to on the 6502:

  • @trap -> JAM (link doesn't work if you don't have "Undocumented opcodes" checked at the top of the page)
  • @breakpoint -> BRK

Checklist

  • Ensure correct machine code output for:
    • x86_64 (zig run: Illegal instruction (core dumped))
    • wasm32 (wasm2wat)
    • wasm64 (wasm2wat)
    • arm (zig run: qemu: uncaught target signal 4 (Illegal instruction) - core dumped followed by Illegal instruction (core dumped))
    • aarch64 (zig run: qemu: uncaught target signal 5 (Trace/breakpoint trap) - core dumped followed by Trace/breakpoint trap (core dumped) (same for @breakpoint))
    • riscv64 (zig run: Illegal instruction (core dumped))
    • sparc64
      We get some very elaborate output (same for @breakpoint):
      Unhandled trap: 0x105 pc: 0000000008000004 npc: 0000000008000008 %g0-3: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 %g4-7: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 %o0-3: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 %o4-7: 0000000000000000 0000000000000000 00000040020000f1 0000000000000000 %l0-3: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 %l4-7: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 %i0-3: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 %i4-7: 0000000000000000 0000000000000000 00000040020001a1 0000000000000000 pstate: 00000092 ccr: 00 (icc: ---- xcc: ----) asi: 82 tl: 0 pil: 0 gl: 0 tbr: 0000000000000000 hpstate: 0000000000000000 htba: 0000000000000000 cansave: 5 canrestore: 1 otherwin: 0 wstate: 0 cleanwin: 6 cwp: 0 fsr: 0000000000000000 y: 0000000000000000 fprs: 0000000000000000
    • LLVM ( call coldcc void @llvm.trap(), !dbg !2275)
  • Improve documentation a bit? Some people will be confused about the difference between @breakpoint and @trap; make sure people understand.
  • Remove the reference comments in the code? Maybe we don't need to keep them.
  • Consider translating the content of the reference URLs to our codebase.
  • Make a follow-up PR after this is merged that makes use of @trap in the std (done: @trap fixups #14799)

Fixes #14765

If I could mark a builtin function as cold, I would mark @setCold as cold.
We have run out of `Zir.Inst.Tag`s so I had to move a tag from Zir.Inst.Tag to
Zir.Inst.Extended. This is because a new noreturn builtin will be added and
noreturn builtins cannot be part of Inst.Tag:
```
/// `noreturn` instructions may not go here; they must be part of the main `Tag` enum.
pub const Extended = enum(u16) {
```

Here's another reason I went for @setCold:
```
$ git grep setRuntimeSafety | wc -l
322
$ git grep setCold | wc -l
79
$ git grep setEvalBranchQuota | wc -l
82
```

This also simply removes @setCold from Autodoc and the docs frontend because
as far as I could understand it, builtins represented using Zir extended
instructions are not yet supported because I couldn't find
@setStackAlign or @setFloatMode there, either.
r00ster91 added 2 commits March 4, 2023 12:08
This introduces a new builtin function that compiles down to something that results in an illegal instruction exception/interrupt.
It can be used to exit a program abnormally.

This implements the builtin for all backends.
This should improve the developer debugging experience.
Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

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

Nice work! Happy to merge this as-is.

I will follow-up with my own PR to update zig1.wasm. Because this file contains opaque arbitrary code, this file should be updated by core team members only.

#elif defined(__i386__) || defined(__x86_64__)
#define zig_trap() __asm__ volatile("ud2");
#else
#define zig_trap() raise(SIGILL)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#define zig_trap() raise(SIGILL)
#define zig_trap() raise(SIGTRAP)

Copy link
Author

@ghost ghost Mar 4, 2023

Choose a reason for hiding this comment

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

The references I quoted and linked in this comment might be of interest.

The SIGTRAP signal is sent to a process when an exception (or trap) occurs: a condition that a debugger has requested to be informed of

(they talk about debuggers in SIGTRAP so this is used for debugtrap below)

and then SIGILL (used for trap):

The SIGILL signal is sent to a process when it attempts to execute an illegal, malformed, unknown, or privileged instruction

I know the signal names themselves make this seem like an odd choice but that's why I did it this way.

Unlike for {#syntax#}@breakpoint(){#endsyntax#}, execution does not continue after this point.
</p>
<p>
This function is only valid within function scope.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This function is only valid within function scope.
Outside function scope, this builtin causes a compile error.

Copy link
Author

Choose a reason for hiding this comment

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

Hmm, I don't think it does? I didn't check for that but I'm guessing it has or should have the same behavior as @breakpoint in that regard:

$ cat x.zig
const std = @import("std");

const x = @breakpoint();

test {
    std.debug.print("{}\n", .{x});
}
$ zig test x.zig
Test [1/1] test_0... void
All 1 tests passed.

So I don't know if that should be a compile error, but maybe because it returns noreturn instead of void. So, should it not be valid within test scope either? Not sure.

Copy link
Member

Choose a reason for hiding this comment

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

my suggested docs are correct; the compiler is wrong :-)

@andrewrk andrewrk merged commit e7f128c into ziglang:master Mar 4, 2023
@andrewrk andrewrk mentioned this pull request Mar 4, 2023
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.

add a @trap builtin

2 participants