Skip to content

abort in core#154604

Open
CAD97 wants to merge 2 commits intorust-lang:mainfrom
CAD97:abort-immediate
Open

abort in core#154604
CAD97 wants to merge 2 commits intorust-lang:mainfrom
CAD97:abort-immediate

Conversation

@CAD97
Copy link
Copy Markdown
Contributor

@CAD97 CAD97 commented Mar 30, 2026

Implements core::process::abort_immediate as a wrapper around intrinsics::abort.

(This PR used to also add core::process::abort, but that's been deferred to a later addition.)

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 30, 2026

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 30, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Mar 30, 2026

r? @scottmcm

rustbot has assigned @scottmcm.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: @scottmcm, libs
  • @scottmcm, libs expanded to 8 candidates
  • Random selection from Mark-Simulacrum, scottmcm

@CAD97
Copy link
Copy Markdown
Contributor Author

CAD97 commented Mar 30, 2026

tests/ui/extern-flag and tests/ui/no_std caught that __rust_abort still wasn't being defined. I think this must be because std is available (and thus lang = "abort_impl" is defined, but #![no_std] means that we aren't linking std. I'm now looking for how to directly gate the shim on if std is getting linked, to avoid adding a new requirement to no_std binaries.

@RalfJung
Copy link
Copy Markdown
Member

Cc @bjorn3 who knows a lot about the alloc shims.

@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Copy Markdown
Member

bjorn3 commented Mar 30, 2026

Imagine the following scenario:

  • liba depends on libcore and gets linked as dylib
  • libb depends on liba and libstd

When building liba, we need to already generate the __rust_abort symbol, but would conflict with the one from libstd. This is not a breaking change on most targets as due to what is arguably a bug in rustc, in this scenario rustc will report an error about libcore getting linked twice. If however you remove libstd.so or link for a target for which we don't ship libstd as dylib yet do still support dynamic linking (i don't know if any such target exists. maybe a wasm target?), then this scenario would currently work fine, but break with this PR.

Also cc @anforowicz for Chromium which bypasses the allocator shim. I think it will be fine for you as you include libstd, but I'm not sure if you don't have any no_std executables. It is probably more of an issue for Bazel.

@anforowicz
Copy link
Copy Markdown
Contributor

Also cc @anforowicz for Chromium which bypasses the allocator shim. I think it will be fine for you as you include libstd, but I'm not sure if you don't have any no_std executables.

Thank you for the heads-up.

  • I think it doesn't impact Chromium, because Chromium product builds link everything statically (no dylibs, only rlibs that are at the end linked by non-rustc-driven linker).
  • OTOH we are currently working on building std into a dylib in developer builds (and later plan to expand this to some other libraries - e.g. the log crate). But this still seems ok, because I think that Chromium doesn't have a library like liba (linked as dylib, but depends on core and not on std).
  • That said, I don't fully understand the changes in this PR, so I defer to your judgement / I don't really understand if these changes may require a follow-up in Chromium when/if they get absorbed.

/cc @DKLoehr from the Chromium Toolchain team as FYI

@CAD97
Copy link
Copy Markdown
Contributor Author

CAD97 commented Mar 30, 2026

Given core abort is essentially blocked on EIIs being ready, would it make sense to PR abort_immediate separately first?

@CAD97 CAD97 force-pushed the abort-immediate branch from 807cd4e to 3a311ed Compare March 30, 2026 22:56
@CAD97
Copy link
Copy Markdown
Contributor Author

CAD97 commented Mar 30, 2026

I decided to rebase this branch/PR to just include abort_immediate, which is useful on its own. abort can be added later once EII can be utilized to do so properly.

@rust-log-analyzer

This comment has been minimized.

/// # Platform-specific behavior
///
/// `abort_immediate` lowers to a trap instruction on *most* architectures; on
/// some architectures it simply lowers to call the unmangled `abort` function.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"unmangled" you mean libc::abort?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I adapted this wording from the panic-abort crate docs.

extern "C" { #[no_mangle] fn abort() -> !; } will be libc::abort in most cases, yes, but I'm not personally aware of what platforms have what behavior here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

gotcha. cc @japaric, this PR is heavily based on your panic-abort crate.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think "unmangled abort" is very clear. Maybe refer to is as "the C abort function" or so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants