Stabilize the breakpoint function

[joshtriplett]

Stabilization report and FCP in
#133724.

[rustbot]

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

[jhpratt]

@joshtriplett Was this discussed by T-lang as suggested here? I see that the issue was nominated until a few hours before this PR was opened, so it seems likely that that is the case.

r=me if that is the case.

[RalfJung]

From the docs:

this may compile to a trapping instruction (e.g. an undefined instruction) instead

I don't know what an "undefined instruction" is, but I hope it has nothing to do with undefined behavior?

[clarfonthey]

I would assume that "illegal instruction" is the intended wording here, since such instructions always trigger interrupts on hardware regardless of whether an OS is present. Although, I think that "trapping instruction" by itself is clearer wording and adding the "clarification" here only makes it more confusing. The fact that I'm probably wrong in this explanation helps emphasise how confusing it is: to me, a trapping instruction and an undefined instruction are two separate things.

[joshtriplett]

@joshtriplett Was this discussed by T-lang as suggested here?

No, it hasn't yet. I've nominated it for discussion in a meeting.

[joshtriplett]

From the docs:

this may compile to a trapping instruction (e.g. an undefined instruction) instead

I don't know what an "undefined instruction" is, but I hope it has nothing to do with undefined behavior?

"trapping instruction" refers to an instruction that produces a trap of some kind, and "undefined instruction" in this context refers to instruction opcodes explicitly reserved for this purpose. See https://www.felixcloutier.com/x86/ud for example. This is entirely unrelated to undefined behavior.

[traviscross]

It's correct, I think, for us to review this, as with exposing other intrinsics, to be sure we're happy with any effects on our language definition. As it is, I think this is OK, so let's...

@rfcbot fcp merge

[rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

Concerns:

• settle-on-specification (Stabilize the breakpoint function #142325 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[nikomatsakis]

@rfcbot reviewed

To the point of semantics, @RalfJung, it seems to me that the API docs are "sufficiently clear" for the spec team to do its work. Based on what it says, it seems clear that breakpoint() without a debugger attached cannot cause UB since it is guaranteed not to continue execution. I presume in practice it will signal or trap on an illegal instruction. (Though I gather @joshtriplett has a plan to make them more precise, which is good.)

[tmandry]

The exact semantics (abort if no debugger) are somewhat surprising to me. But I understand that this is the most straightforward low level operation that we can stabilize, because there is no portable way to implement "break but only if there is a debugger attached".

@rfcbot reviewed

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[scottmcm]

I suppose in a sense, then, the semantics here is similar to

if platform_specific_volatile_read() {
    abort()
}

and it needs to be treated as similarly immovable as volatile operations?

[hanna-kruppe]

Given discussions on the tracking issue, I’m not actually sure if everyone’s on the same page about what the language spec would be if it said something about this intrinsic. I imagine something like (in spirit if not spec-verbiage)

Any time breakpoint is called, it non-deterministically chooses between (1) terminating the program or (2) having no observable effect.

But that would technically imply “always do nothing” is a valid implementation and many months ago @joshtriplett wrote:

I don't think "do nothing" should be a valid implementation of this. "abort" is a valid implementation. Given that, I don't think this belongs in hint.

Also, my off the cuff proposal appears subtly different from what @scottmcm just posted while I was writing this.

[nikomatsakis]

I think it would be useful for there to be a warning when this is called.

[nikomatsakis]

@hanna-kruppe OK, fair enough. We discussed this some more in the meeting. I am aligned with your definition and I think it means that, indeed, a no-op would be a valid implementation, though not a very useful one.

(In reality, the semantics are platform dependent but must be compatible with that definition.)

[nikomatsakis]

Well, actually, the more I think on this, the less sure I am. Here's the thing. I can imagine architectures where it is possible for the breakpoint instruction to be intercepted. I think it is even possible, it's a signal, you can do signal handlers (how do debuggers work, after all). Which suggests that there would be programs that may indeed leverage this in twisted ways and therefore not ABORT nor be a no-op and yet not involve the user of a debugger.

It seems like the "specification" just wants to be entirely architecture dependent. Whatever spec we write would say "the defined behavior of this fn is entirely dependent on the target architecture" with a non-normative note that "it may reasonable be modeled as a conditional abort for the purposes of proving properties on programs".

[scottmcm]

Further conversation in the call had me thinking about what it means to use this. Because it returns () I think the code that calls it has to treat it like it might be a NOP no matter what the specification says. Caller code that's using it in some kind of non-unit _ => breakpoint() will have to add a panic!() or Default::default() or similar to satisfy the type checker.

So why isn't ok to just say "well this might be a NOP on some targets", since the caller has to handle that anyway?

---

It really sounds to me like the Specification for the intrinsic is essentially "might return, might never return" and it's a target Quality-of-Implementation issue how exactly that choice is made and how any non-return happens.

As potentially-elucidating discussions:

• would an implementation which just loop {}s be a legal implementation?
• would an implementation that uploads a dump to WER then continues executing be a legal implementation?

---

From a different perspective, I get the impression that people would be annoyed -- understandably so, given the intent expressed by libs-api for the function -- if a MIR optimization removed all the code after a call to this. But if the specification doesn't allow for it to return to the caller, then removing that code would be an entirely legal optimization. So if the intent is that that's not a legal optimization, then the spec must be that it might return, implying that it might have "done" nothing since there doesn't seem to be any other Observable Behaviour possible from it.

[traviscross]

@rfcbot concern settle-on-specification

We should settle on the language definition for this before stabilizing. I debated whether or not to file the concern, but it just seems better to settle this first so we don't confuse the people who are trying to write down the meaning of our language.

In my view, the language definition for this intrinsic is that its behavior is implementation defined, and that valid choices include (but are not limited to) emitting a no-op, emitting an unconditional abort, and emitting if rand() % 2 == 0 { abort() }.

Since we weren't able to settle on that unanimously, though, let's continue discussing, as above, and leave this nominated.

cc @ehuss @RalfJung

[RalfJung]

In particular, if a debugger is attached and one resumes execution of the program after the breakpoint, that basically makes it a NOP from the perspective of the program. (If it's not a NOP, then what did it do? The AM state is entirely unchanged. And it's not like we make any guarantees about what you can observe with a debugger anyway.) So while @joshtriplett has argued before that a NOP would not be a valid implementation, I think that's not semantically coherent.

[workingjubilee]

The exact semantics (abort if no debugger) are somewhat surprising to me. But I understand that this is the most straightforward low level operation that we can stabilize, because there is no portable way to implement "break but only if there is a debugger attached".

This is more a "fun fact" than anything necessarily decisive regarding the decision here, but a while ago we actually tried to implement this specific thing in the Rust standard library for panics. The idea was that, optimistically, your debugger would stop every time it hit a panic, without having to train debuggers to recognize "oh, that's a Rust exception being thrown".

It had the embarrassing consequence that programs would abort if you used strace on them, which is a diagnostic utility that usually people think of as a debugging aid, but not a debugger. But in the eyes of the kernel, strace and gdb are doing the same thing, so when we detected "debugger attached", we detected that we were under strace. Then we tried to fling ourselves onto a ud2... the sort of thing gdb would catch us from... but our trust-fall into the waiting arms of strace was answered by it continuing to simply faithfully report to the programmer that our Rust process had decided to give up on the world.

Obviously, we reverted that, as it was well-intentioned but a bit silly.

[traviscross]

@rustbot labels +I-libs-api-nominated

Since the libs-api call will precede the lang call next week, let's nominate this for @rust-lang/libs-api to review the feedback by lang members on how this intrinsic might be defined as a language matter in the event that affects at all (and it may not) anything on the library side.

[ChrisDenton]

So if I'm understanding correctly, one of the lang questions is on how this should affect optimizations (or not affect them as the case may be)? You want it to be specified as either always aborting (i.e. returning !) or maybe returning so the compiler can optimise accordingly. This is independent of any requirements libs-api may have for acceptable implementations (e.g. they may reject a PR that implements breakpoint as a literal no-op).

The other lang question is on SIGTRAP handling. I guess someone can handle SIGTRAP in twisted ways but I'm unsure how that is meaningfully different from having fun with other signals?

[workingjubilee]

Let's consolidate discussion into the tracking issue for now: #133724