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

Correct and expand documentation of handle_alloc_error and set_alloc_error_hook. #115007

Merged
merged 1 commit into from
Aug 27, 2023

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Aug 19, 2023

The primary goal of this change is to remove the false claim that handle_alloc_error always aborts; instead, code should be prepared for handle_alloc_error to possibly unwind, and be sound under that condition.

I saw other opportunities for improvement, so I have added all the following information:

  • handle_alloc_error may panic instead of aborting. (Fixes Clarify that handle_alloc_error may unwind #114898)
  • What happens if a hook returns rather than diverging.
  • A hook may panic. (This was already demonstrated in an example, but not stated in prose.)
  • A hook must be sound to call — it cannot assume that it is only called by the runtime, since its function pointer can be retrieved by safe code.

I've checked these statements against the source code of alloc and std, but there may be nuances I haven't caught, so a careful review is welcome.

…oc_error_hook`.

Add the following facts:

* `handle_alloc_error` may panic instead of aborting.
* What happens if a hook returns rather than diverging.
* A hook may panic. (This was already demonstrated in an example,
  but not stated in prose.)
* A hook must be sound to call — it cannot assume that it is only
  called by the runtime, since its function pointer can be retrieved by
  safe code.
@rustbot
Copy link
Collaborator

rustbot commented Aug 19, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 19, 2023
@Mark-Simulacrum
Copy link
Member

This looks right to me, but cc @Amanieu as well.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Aug 26, 2023

📌 Commit 3dde25e has been approved by Mark-Simulacrum

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 27, 2023
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#114924 (coverage: Tidy up `run-coverage` tests in several small ways)
 - rust-lang#114927 (CI: add more debug logging to Docker caching)
 - rust-lang#114957 (tests: Fix tests for LoongArch64)
 - rust-lang#115007 (Correct and expand documentation of `handle_alloc_error` and `set_alloc_error_hook`.)
 - rust-lang#115098 (rust-gdbgui: remove excessive quotes)
 - rust-lang#115111 (compile rust-anaylzer with `x check` if it's enabled)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 922b827 into rust-lang:master Aug 27, 2023
11 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Aug 27, 2023
@kpreid kpreid deleted the alloc branch August 29, 2023 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Clarify that handle_alloc_error may unwind
4 participants