Skip to content

Stabilize default_alloc_error_handler#102318

Merged
bors merged 2 commits intorust-lang:masterfrom
Amanieu:default_alloc_error_handler
Dec 17, 2022
Merged

Stabilize default_alloc_error_handler#102318
bors merged 2 commits intorust-lang:masterfrom
Amanieu:default_alloc_error_handler

Conversation

@Amanieu
Copy link
Member

@Amanieu Amanieu commented Sep 26, 2022

Tracking issue: #66741

This turns feature(default_alloc_error_handler) on by default, which causes the compiler to automatically generate a default OOM handler which panics if #[alloc_error_handler] is not provided.

The FCP completed over 2 years ago but the stabilization was blocked due to an issue with unwinding. This was fixed by #88098 so stabilization can be unblocked.

Closes #66741

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 26, 2022
@rust-highfive
Copy link
Contributor

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 26, 2022
@lcnr
Copy link
Contributor

lcnr commented Sep 26, 2022

considering that the FCP was 2 years ago, i would expect us to go through another FCP which explains the reason why stabilization was blocked. Won't block this without an FCP, but i definitely want someone to look at this who's a bit more knowledgeable about this than me

r? compiler

@rust-highfive rust-highfive assigned petrochenkov and unassigned lcnr Sep 26, 2022
@Amanieu Amanieu force-pushed the default_alloc_error_handler branch from e16fad7 to 15aba25 Compare September 26, 2022 15:45
@bjorn3
Copy link
Member

bjorn3 commented Sep 26, 2022

When using this with cg_clif I get:

           /home/gh-bjorn3/cg_clif/build_sysroot/sysroot_src/library/alloc/src/alloc.rs:419: undefined reference to `rust_oom'

It seems that the current implementation depends on -Zfunction-sections=yes, while cg_clif sets it to no for linker performance reasons. To elaborate on why it depends on this. The code in question is

pub mod __alloc_error_handler {
    use crate::alloc::Layout;

    // called via generated `__rust_alloc_error_handler`

    // if there is no `#[alloc_error_handler]`
    #[rustc_std_internal_symbol]
    pub unsafe fn __rdl_oom(size: usize, _align: usize) -> ! {
        panic!("memory allocation of {size} bytes failed")
    }

    // if there is an `#[alloc_error_handler]`
    #[rustc_std_internal_symbol]
    pub unsafe fn __rg_oom(size: usize, align: usize) -> ! {
        let layout = unsafe { Layout::from_size_align_unchecked(size, align) };
        extern "Rust" {
            #[lang = "oom"]
            fn oom_impl(layout: Layout) -> !;
        }
        unsafe { oom_impl(layout) }
    }
}

__rust_alloc_error_handler will be generated as a call to either of these functions. __rdl_oom when #![feature(default_alloc_error_handler)] is used, no problem here and __rg_oom when it isn't used. The problem is that in this case rust_oom (which is the actual symbol name of oom_impl) is not defined. In other words it only just so happens to work because __rg_oom is garbage collected by the linker by default. Both compiling liballoc with -Zfunction-sections=no and using -Clink-dead-code when compiling the executable or dylib will cause __rg_oom to not be garbage collected and thus give a linker error.

@Amanieu Amanieu marked this pull request as draft September 26, 2022 18:08
@Ericson2314
Copy link
Contributor

I guess this is fine, but for people using alloc without global error handling at all (the current unstable cfg stuff, a feature hopefully someday) I hope one doesn't get needlessly synthesized.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 27, 2022

We should probably have a test that uses this in a way we expect it to be used without requiring other feature gates. The tests touched by this PR all use other feature gates.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 6, 2022
@bors

This comment was marked as resolved.

Amanieu added a commit to Amanieu/rust that referenced this pull request Oct 31, 2022
The new implementation doesn't use weak lang items and instead changes
`#[alloc_error_handler]` to an attribute macro just like
`#[global_allocator]`.

The attribute will generate the `__rg_oom` function which is called by
the compiler-generated `__rust_alloc_error_handler`. If no `__rg_oom`
function is defined in any crate then the compiler shim will call
`__rdl_oom` in the alloc crate which will simply panic.

This also fixes link errors with `-C link-dead-code` with
`default_alloc_error_handler`: `__rg_oom` was previously defined in the
alloc crate and would attempt to reference the `oom` lang item, even if
it didn't exist. This worked as long as `__rg_oom` was excluded from
linking since it was not called.

This is a prerequisite for the stabilization of
`default_alloc_error_handler` (rust-lang#102318).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Nov 1, 2022
…r, r=bjorn3

Rewrite implementation of `#[alloc_error_handler]`

The new implementation doesn't use weak lang items and instead changes `#[alloc_error_handler]` to an attribute macro just like `#[global_allocator]`.

The attribute will generate the `__rg_oom` function which is called by the compiler-generated `__rust_alloc_error_handler`. If no `__rg_oom` function is defined in any crate then the compiler shim will call `__rdl_oom` in the alloc crate which will simply panic.

This also fixes link errors with `-C link-dead-code` with `default_alloc_error_handler`: `__rg_oom` was previously defined in the alloc crate and would attempt to reference the `oom` lang item, even if it didn't exist. This worked as long as `__rg_oom` was excluded from linking since it was not called.

This is a prerequisite for the stabilization of `default_alloc_error_handler` (rust-lang#102318).
@Amanieu Amanieu force-pushed the default_alloc_error_handler branch from 15aba25 to 2db1dec Compare November 3, 2022 07:05
@rustbot rustbot added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label Nov 3, 2022
@Amanieu Amanieu marked this pull request as ready for review November 3, 2022 07:06
@rustbot
Copy link
Collaborator

rustbot commented Nov 3, 2022

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

@Amanieu Amanieu force-pushed the default_alloc_error_handler branch from 2db1dec to f5e0b76 Compare November 3, 2022 07:13
@Amanieu
Copy link
Member Author

Amanieu commented Nov 3, 2022

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 3, 2022
bors bot added a commit to rust-embedded/embedded-alloc that referenced this pull request Feb 26, 2023
63: Remove alloc_error_handler r=adamgreig a=jannic

With rust-lang/rust#102318 default_alloc_error_handler has been stabilized, ie. the default error handler is enabled by default.

Therefore, it's no longer necessary to provide an alloc_error_handler if the desired error handling is equivalent to a panic.

Co-authored-by: Jan Niehusmann <jan@gondor.com>
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Feb 27, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Feb 27, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 2, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 4, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 4, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 4, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 4, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 4, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 4, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 4, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this pull request Mar 7, 2023
The new implementation doesn't use weak lang items and instead changes
`#[alloc_error_handler]` to an attribute macro just like
`#[global_allocator]`.

The attribute will generate the `__rg_oom` function which is called by
the compiler-generated `__rust_alloc_error_handler`. If no `__rg_oom`
function is defined in any crate then the compiler shim will call
`__rdl_oom` in the alloc crate which will simply panic.

This also fixes link errors with `-C link-dead-code` with
`default_alloc_error_handler`: `__rg_oom` was previously defined in the
alloc crate and would attempt to reference the `oom` lang item, even if
it didn't exist. This worked as long as `__rg_oom` was excluded from
linking since it was not called.

This is a prerequisite for the stabilization of
`default_alloc_error_handler` (rust-lang#102318).
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 7, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 7, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 9, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 9, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 12, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 13, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to nicholasbishop/uefi-rs that referenced this pull request Mar 19, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
nicholasbishop added a commit to rust-osdev/uefi-rs that referenced this pull request Mar 19, 2023
As of 2022-12-18, we can rely on the `default_alloc_error_handler` feature which
was stabilized in rust-lang/rust#102318. The behavior of
the default handler is to panic, essentially the same as our current custom one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tracking issue for handle_alloc_error defaulting to panic (for no_std + liballoc)