From ddebc40400bd362b6df6361fc1841f48f0c99dca Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Sun, 13 Jun 2021 14:46:46 +0200 Subject: [PATCH 01/16] Start filling out the body. --- 0000-backtrace-in-core.md | 113 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) create mode 100644 0000-backtrace-in-core.md diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md new file mode 100644 index 00000000000..2be88ab96f8 --- /dev/null +++ b/0000-backtrace-in-core.md @@ -0,0 +1,113 @@ +- Feature Name: Backtrace in `core` +- Start Date: (fill me in with today's date, YYYY-MM-DD) +- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) +- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) + +# Summary +[summary]: #summary + +This RFC will move the `Backtrace` type to `core`, making it available for usage in `no_std` contexts. This is a stepping stone for the `Error` type in `core` and eventually both types will end there. + +# Motivation +[motivation]: #motivation + +The main reason behind moving `Backtrace` to `core` is to have essential types available for wider usage without the need to import `std`. While `Error` has a valid reason for not being in `core` (it relies on `Box` type - TODO: am I right though?), `Backtrace` does not have similar blockers. + +Additionally, having this type in `core` will allow for??? + +The outcome of this RFC will be a `Backtrace` type in `core` with implementation defined in `std` and compiler-generated implementation when `std` is not linked. + +Why are we doing this? What use cases does it support? What is the expected outcome? + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +In terms of Guide-level changes, there is not much to be discussed - only that it is moved to `core` and if `std` is not linked, automatic backtrace handlers will be generated. Otherwise, the regular implementation of `Backtrace` is present. + +TODO: what else should be here? + +Explain the proposal as if it was already included in the language and you were teaching it to another Rust programmer. That generally means: + +- Introducing new named concepts. +- Explaining the feature largely in terms of examples. +- Explaining how Rust programmers should *think* about the feature, and how it should impact the way they use Rust. It should explain the impact as concretely as possible. +- If applicable, provide sample error messages, deprecation warnings, or migration guidance. +- If applicable, describe the differences between teaching this to existing Rust programmers and new Rust programmers. + +For implementation-oriented RFCs (e.g. for compiler internals), this section should focus on how compiler contributors should think about the change, and give examples of its concrete impact. For policy RFCs, this section should provide an example-driven introduction to the policy, and explain its impact in concrete terms. + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +This is the technical portion of the RFC. Explain the design in sufficient detail that: + +- Its interaction with other features is clear. +- It is reasonably clear how the feature would be implemented. +- Corner cases are dissected by example. + +The section should return to the examples given in the previous section, and explain more fully how the detailed proposal makes those examples work. + +Implementation-wise, `Backtrace` is **declared** in the `core` module and **defined** in `std` via *lang_items* which act as function hooks which are resolved during link-time. Special type `StdBacktrace` is introduced for the `std` part of implementation which acts as a proxy for `Debug` and `Display` trait impls. + +// TODO: add examples + + +# Drawbacks +[drawbacks]: #drawbacks + +Why should we *not* do this? + +`no_std` code bloat - Mara mentioned out +`Error:backtrace` also seems to be blocking?? + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +- Why is this design the best in the space of possible designs? +- What other designs have been considered and what is the rationale for not choosing them? +- What is the impact of not doing this? + +# Prior art +[prior-art]: #prior-art + +Discuss prior art, both the good and the bad, in relation to this proposal. +A few examples of what this can include are: + +- For language, library, cargo, tools, and compiler proposals: Does this feature exist in other programming languages and what experience have their community had? +- For community proposals: Is this done by some other community and what were their experiences with it? +- For other teams: What lessons can we learn from what other communities have done here? +- Papers: Are there any published papers or great posts that discuss this? If you have some relevant papers to refer to, this can serve as a more detailed theoretical background. + +This section is intended to encourage you as an author to think about the lessons from other languages, provide readers of your RFC with a fuller picture. +If there is no prior art, that is fine - your ideas are interesting to us whether they are brand new or if it is an adaptation from other languages. + +Note that while precedent set by other languages is some motivation, it does not on its own motivate an RFC. +Please also take into consideration that rust sometimes intentionally diverges from common language features. + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +- What parts of the design do you expect to resolve through the RFC process before this gets merged? +- What parts of the design do you expect to resolve through the implementation of this feature before stabilization? +- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC? + +# Future possibilities +[future-possibilities]: #future-possibilities + +Think about what the natural extension and evolution of your proposal would +be and how it would affect the language and project as a whole in a holistic +way. Try to use this section as a tool to more fully consider all possible +interactions with the project and language in your proposal. +Also consider how this all fits into the roadmap for the project +and of the relevant sub-team. + +This is also a good place to "dump ideas", if they are out of scope for the +RFC you are writing but otherwise related. + +If you have tried and cannot think of any future possibilities, +you may simply state that you cannot think of anything. + +Note that having something written down in the future-possibilities section +is not a reason to accept the current or a future RFC; such notes should be +in the section on motivation or rationale in this or subsequent RFCs. +The section merely provides additional information. From 358dbd33f59db8285c941ebd35ea905582eff5ab Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Wed, 28 Jul 2021 18:23:18 +0200 Subject: [PATCH 02/16] Update 0000-backtrace-in-core.md --- 0000-backtrace-in-core.md | 139 +++++++++++++++++++++++--------------- 1 file changed, 84 insertions(+), 55 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index 2be88ab96f8..4d858a8a8b9 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -6,46 +6,101 @@ # Summary [summary]: #summary -This RFC will move the `Backtrace` type to `core`, making it available for usage in `no_std` contexts. This is a stepping stone for the `Error` type in `core` and eventually both types will end there. +This RFC will move the `Backtrace` type to `core`, making it available for usage in `no_std` contexts. This is a stepping stone for the `Error` type in `core` and eventually both types will end there. + +It is still unclear whether `Backtrace` itself does need to be moved to `core` and in time this RFC will be unanimous in this matter. # Motivation [motivation]: #motivation -The main reason behind moving `Backtrace` to `core` is to have essential types available for wider usage without the need to import `std`. While `Error` has a valid reason for not being in `core` (it relies on `Box` type - TODO: am I right though?), `Backtrace` does not have similar blockers. - -Additionally, having this type in `core` will allow for??? +The main reason behind moving `Backtrace` to `core` is to have essential types available for wider usage without the need to import `std`. While `Error` had a valid reason for not being in `core` (it relies on `Box` type - TODO: am I right though?), `Backtrace` does not have similar blockers. -The outcome of this RFC will be a `Backtrace` type in `core` with implementation defined in `std` and compiler-generated implementation when `std` is not linked. +Additionally, having this type in `core` will allow its users to provide their own implementations of the backtrace collection and reporting and not rely on the `std`-provided one if they don't want to. -Why are we doing this? What use cases does it support? What is the expected outcome? +The outcome of this RFC will be a `Backtrace` type in `core` with implementation defined in `std` and compiler-generated implementation when `std` is not linked and user did not provide their own implementation for the reporting functions. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation +Backtraces are an essential part of the Rust ecosystem, being close coupled with error handling and reporting, be it to the user or the developer. They are usually of a common form: +```rust + Compiling playground v0.0.1 (/playground) + Finished dev [unoptimized + debuginfo] target(s) in 1.66s + Running `target/debug/playground` +thread 'main' panicked at 'index out of bounds: the len is 4 but the index is 4', src/main.rs:5:5 +stack backtrace: + 0: rust_begin_unwind + at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/std/src/panicking.rs:493:5 + 1: core::panicking::panic_fmt + at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/panicking.rs:92:14 + 2: core::panicking::panic_bounds_check + at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/panicking.rs:69:5 + 3: >::index_mut + at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/slice/index.rs:190:14 + 4: core::slice::index:: for [T]>::index_mut + at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/slice/index.rs:26:9 + 5: as core::ops::index::IndexMut>::index_mut + at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/alloc/src/vec/mod.rs:2396:9 + 6: playground::main + at ./src/main.rs:5:5 + 7: core::ops::function::FnOnce::call_once + at /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b/library/core/src/ops/function.rs:227:5 +note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. +``` + +They allow for a detailed inspection of what crashed at runtime and which libraries and functions were involved in the process. Obviously reporting and formatting it is also of a big importance, so `Error` allows for backtrace printing with help of a `backtrace()` method. + +Currently, `Backtrace` is an essential part of the `std` library and user can control whether the backtrace is enabled or disabled using the environmental variable: `RUST_BACKTRACE`. + In terms of Guide-level changes, there is not much to be discussed - only that it is moved to `core` and if `std` is not linked, automatic backtrace handlers will be generated. Otherwise, the regular implementation of `Backtrace` is present. TODO: what else should be here? -Explain the proposal as if it was already included in the language and you were teaching it to another Rust programmer. That generally means: +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +The following changes need to be made to implement this proposal: -- Introducing new named concepts. -- Explaining the feature largely in terms of examples. -- Explaining how Rust programmers should *think* about the feature, and how it should impact the way they use Rust. It should explain the impact as concretely as possible. -- If applicable, provide sample error messages, deprecation warnings, or migration guidance. -- If applicable, describe the differences between teaching this to existing Rust programmers and new Rust programmers. +## Move the `Backtrace` to `core` and add a thin wrapper in `std` -For implementation-oriented RFCs (e.g. for compiler internals), this section should focus on how compiler contributors should think about the change, and give examples of its concrete impact. For policy RFCs, this section should provide an example-driven introduction to the policy, and explain its impact in concrete terms. +This way, a `StdBacktrace` struct is introduced in `std` that allows for `Debug` and `Display` formatting. -# Reference-level explanation -[reference-level-explanation]: #reference-level-explanation +The regular API of `Backtrace` comprising `enabled()`, `create()` and `status()` would be left in the `std` as free-standing functions. Since, they are lang items they need to have a default implementation in case `std` is not linked, so they will be provided in such a form in the `core` library (and overwritten once `std` is linked): + +```rust +/// Global implementation of backtrace functionality. Called to create +/// `RawBacktrace` trait objects. +#[cfg(not(bootstrap))] +extern "Rust" { + #[lang = "backtrace_create"] + fn backtrace_create(ip: usize) -> *mut dyn RawBacktrace; + + #[lang = "backtrace_enabled"] + fn backtrace_enabled() -> bool; -This is the technical portion of the RFC. Explain the design in sufficient detail that: + #[lang = "backtrace_status"] + fn backtrace_status(raw: *mut dyn RawBacktrace) -> BacktraceStatus; +} -- Its interaction with other features is clear. -- It is reasonably clear how the feature would be implemented. -- Corner cases are dissected by example. +#[cfg(bootstrap)] +unsafe fn backtrace_create(_ip: usize) -> *mut dyn RawBacktrace { + UnsupportedBacktrace::create().inner +} -The section should return to the examples given in the previous section, and explain more fully how the detailed proposal makes those examples work. +#[cfg(bootstrap)] +unsafe fn backtrace_enabled() -> bool { + false +} + +#[cfg(bootstrap)] +unsafe fn backtrace_status(_raw: *mut dyn RawBacktrace) -> BacktraceStatus { + BacktraceStatus::Unsupported +} + +``` + +This change will make the `Backtrace` an optional part of the library and +This way, if the `Backtrace` is not enabled (on the library level) there is no need for it to report Implementation-wise, `Backtrace` is **declared** in the `core` module and **defined** in `std` via *lang_items* which act as function hooks which are resolved during link-time. Special type `StdBacktrace` is introduced for the `std` part of implementation which acts as a proxy for `Debug` and `Display` trait impls. @@ -55,7 +110,10 @@ Implementation-wise, `Backtrace` is **declared** in the `core` module and **defi # Drawbacks [drawbacks]: #drawbacks -Why should we *not* do this? +There are actually many drawbacks, most important of them being `Backtrace` using a lot of `std` for the actual backtrace capturing. + +The other one is a potential code bloat in `no_std` contexts, so a possible alternative may be only enabling the `Backtrace` conditionally via `cfg` settings. (not so sure about this though) + `no_std` code bloat - Mara mentioned out `Error:backtrace` also seems to be blocking?? @@ -63,51 +121,22 @@ Why should we *not* do this? # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives -- Why is this design the best in the space of possible designs? -- What other designs have been considered and what is the rationale for not choosing them? -- What is the impact of not doing this? +The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented (link!) we would not have to move the `Backtrace` to `core` at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. + +Alternatively, since the actual implementation of `Backtrace` uses allocating structures like `Box` for backtrace creation, it would be prudent to move it to where this type resides. This will in turn allow users of `alloc` module in embedded and `no_std` contexts to use this functionality without `std`. _should we go for this though instead of the core when we introduce such big changes?_ # Prior art [prior-art]: #prior-art -Discuss prior art, both the good and the bad, in relation to this proposal. -A few examples of what this can include are: - -- For language, library, cargo, tools, and compiler proposals: Does this feature exist in other programming languages and what experience have their community had? -- For community proposals: Is this done by some other community and what were their experiences with it? -- For other teams: What lessons can we learn from what other communities have done here? -- Papers: Are there any published papers or great posts that discuss this? If you have some relevant papers to refer to, this can serve as a more detailed theoretical background. - -This section is intended to encourage you as an author to think about the lessons from other languages, provide readers of your RFC with a fuller picture. -If there is no prior art, that is fine - your ideas are interesting to us whether they are brand new or if it is an adaptation from other languages. +This type is already implemented, but it seems like no type was moved from `std` to `core` previously so we have no point of reference on this one. -Note that while precedent set by other languages is some motivation, it does not on its own motivate an RFC. -Please also take into consideration that rust sometimes intentionally diverges from common language features. +As for `no_std` and embedded contexts, there exists the [mini-backtrace](https://github.com/amanieu/mini-backtrace) library that provides backtrace support via LLVM's libunwind. # Unresolved questions [unresolved-questions]: #unresolved-questions -- What parts of the design do you expect to resolve through the RFC process before this gets merged? -- What parts of the design do you expect to resolve through the implementation of this feature before stabilization? -- What related issues do you consider out of scope for this RFC that could be addressed in the future independently of the solution that comes out of this RFC? # Future possibilities [future-possibilities]: #future-possibilities -Think about what the natural extension and evolution of your proposal would -be and how it would affect the language and project as a whole in a holistic -way. Try to use this section as a tool to more fully consider all possible -interactions with the project and language in your proposal. -Also consider how this all fits into the roadmap for the project -and of the relevant sub-team. - -This is also a good place to "dump ideas", if they are out of scope for the -RFC you are writing but otherwise related. - -If you have tried and cannot think of any future possibilities, -you may simply state that you cannot think of anything. -Note that having something written down in the future-possibilities section -is not a reason to accept the current or a future RFC; such notes should be -in the section on motivation or rationale in this or subsequent RFCs. -The section merely provides additional information. From 43ee2274974e6caf0890e76a60f93645b60a6218 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Wed, 28 Jul 2021 18:27:29 +0200 Subject: [PATCH 03/16] Update 0000-backtrace-in-core.md --- 0000-backtrace-in-core.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index 4d858a8a8b9..9d34a64f4f2 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -104,7 +104,8 @@ This way, if the `Backtrace` is not enabled (on the library level) there is no n Implementation-wise, `Backtrace` is **declared** in the `core` module and **defined** in `std` via *lang_items* which act as function hooks which are resolved during link-time. Special type `StdBacktrace` is introduced for the `std` part of implementation which acts as a proxy for `Debug` and `Display` trait impls. -// TODO: add examples +// TODO: add examples of how would one implement these functions themselves like panic hooks + # Drawbacks @@ -125,6 +126,8 @@ The proposed solution is the one which is currently implementable. However, if t Alternatively, since the actual implementation of `Backtrace` uses allocating structures like `Box` for backtrace creation, it would be prudent to move it to where this type resides. This will in turn allow users of `alloc` module in embedded and `no_std` contexts to use this functionality without `std`. _should we go for this though instead of the core when we introduce such big changes?_ +A viable solution to allocating functions of `Backtrace` might be adding an API where the users could provide themselves the memory in which the backtrace should reside and truncate/report a failure in case the backtrace does not fit this preallocated space. + # Prior art [prior-art]: #prior-art From 815ac4e2243c4a2c2259c738a738f45edd493c31 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Wed, 28 Jul 2021 18:40:24 +0200 Subject: [PATCH 04/16] Update 0000-backtrace-in-core.md --- 0000-backtrace-in-core.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index 9d34a64f4f2..ef5ac7942a9 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -13,7 +13,7 @@ It is still unclear whether `Backtrace` itself does need to be moved to `core` a # Motivation [motivation]: #motivation -The main reason behind moving `Backtrace` to `core` is to have essential types available for wider usage without the need to import `std`. While `Error` had a valid reason for not being in `core` (it relies on `Box` type - TODO: am I right though?), `Backtrace` does not have similar blockers. +The main reason behind moving `Backtrace` to `core` is to have essential types available for wider usage without the need to import `std`. While `Error` had a valid reason for not being in `core` (it relies on `Box` type for different conversions), `Backtrace` does not have similar blockers apart from its frame-allocating API Additionally, having this type in `core` will allow its users to provide their own implementations of the backtrace collection and reporting and not rely on the `std`-provided one if they don't want to. From f8a67a5baf2cfc2ac6efae5e65b9f5429534b378 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Tue, 3 Aug 2021 17:11:21 +0200 Subject: [PATCH 05/16] Update 0000-backtrace-in-core.md Co-authored-by: Nick Cameron --- 0000-backtrace-in-core.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index ef5ac7942a9..fffc27722ad 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -This RFC will move the `Backtrace` type to `core`, making it available for usage in `no_std` contexts. This is a stepping stone for the `Error` type in `core` and eventually both types will end there. +This RFC proposes moving the `Backtrace` type from `std` to `core` (and some changes to `Backtrace` to facilitate this). The change is motivated by the desire to move `Error` from `std` to `core`, which requires either moving `Backtrace` or abstracting it out of `Error`. It is still unclear whether `Backtrace` itself does need to be moved to `core` and in time this RFC will be unanimous in this matter. @@ -142,4 +142,3 @@ As for `no_std` and embedded contexts, there exists the [mini-backtrace](https:/ # Future possibilities [future-possibilities]: #future-possibilities - From 22894ae31ee9ac58fc4c753ec00228bc169ee8d2 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Tue, 3 Aug 2021 17:39:54 +0200 Subject: [PATCH 06/16] Fix several suggestions from Nick --- 0000-backtrace-in-core.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index fffc27722ad..f181b7c84ab 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -8,7 +8,9 @@ This RFC proposes moving the `Backtrace` type from `std` to `core` (and some changes to `Backtrace` to facilitate this). The change is motivated by the desire to move `Error` from `std` to `core`, which requires either moving `Backtrace` or abstracting it out of `Error`. -It is still unclear whether `Backtrace` itself does need to be moved to `core` and in time this RFC will be unanimous in this matter. +Backtrace is a (often specifically formatted) list of function calls which program was in at the moment of its generation. Backtraces in Rust are generated when a program reaches an unrecoverable error and panics. In the other case (recoverable errors) they are handled by choosing a "fail path" which informs the user that an error ocurred and the program proceeds. + +This RFC does not cover eventually expanding the `Backtrace` API to include more functions and it does not solve the way backtraces are collected (although it proposes different takes on the matter). # Motivation [motivation]: #motivation @@ -23,7 +25,7 @@ The outcome of this RFC will be a `Backtrace` type in `core` with implementation [guide-level-explanation]: #guide-level-explanation Backtraces are an essential part of the Rust ecosystem, being close coupled with error handling and reporting, be it to the user or the developer. They are usually of a common form: -```rust +``` Compiling playground v0.0.1 (/playground) Finished dev [unoptimized + debuginfo] target(s) in 1.66s Running `target/debug/playground` @@ -63,7 +65,7 @@ The following changes need to be made to implement this proposal: ## Move the `Backtrace` to `core` and add a thin wrapper in `std` -This way, a `StdBacktrace` struct is introduced in `std` that allows for `Debug` and `Display` formatting. +This wrapper is used to do the actual backtrace capturing from the `std` in the three functions described below. The regular API of `Backtrace` comprising `enabled()`, `create()` and `status()` would be left in the `std` as free-standing functions. Since, they are lang items they need to have a default implementation in case `std` is not linked, so they will be provided in such a form in the `core` library (and overwritten once `std` is linked): From 219e3c0ccf15b44737333da56def3cac4fdb2136 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Tue, 3 Aug 2021 18:26:33 +0200 Subject: [PATCH 07/16] Fix several more suggestions. --- 0000-backtrace-in-core.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index f181b7c84ab..f4d59548c33 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -15,7 +15,12 @@ This RFC does not cover eventually expanding the `Backtrace` API to include more # Motivation [motivation]: #motivation -The main reason behind moving `Backtrace` to `core` is to have essential types available for wider usage without the need to import `std`. While `Error` had a valid reason for not being in `core` (it relies on `Box` type for different conversions), `Backtrace` does not have similar blockers apart from its frame-allocating API +The main reason behind moving `Backtrace` to `core` is to have essential types available for wider usage without the need to import `std`. [Error Handling Group Post](https://blog.rust-lang.org/inside-rust/2021/07/01/What-the-error-handling-project-group-is-working-towards.html#1-error-trait--panic-runtime-integration) goes into details on why one would want the `Error` type (and consequently `Backtrace`) in `core`. The end goal is to have a `panic_error` function which would allow for generating panic errors with detailed error informations. + +The [original PR](https://github.com/rust-lang/rust/pull/72981) which aims to stabilize the `Backtrace` already described that it is desirable to have this type in `core`. + + +While `Error` had a valid reason for not being in `core` (it relies on `Box` type for different conversions), `Backtrace` does not have similar blockers apart from its frame-allocating API Additionally, having this type in `core` will allow its users to provide their own implementations of the backtrace collection and reporting and not rely on the `std`-provided one if they don't want to. @@ -58,6 +63,9 @@ In terms of Guide-level changes, there is not much to be discussed - only that i TODO: what else should be here? +## Trade-offs in this solution + + # Reference-level explanation [reference-level-explanation]: #reference-level-explanation @@ -104,8 +112,6 @@ unsafe fn backtrace_status(_raw: *mut dyn RawBacktrace) -> BacktraceStatus { This change will make the `Backtrace` an optional part of the library and This way, if the `Backtrace` is not enabled (on the library level) there is no need for it to report -Implementation-wise, `Backtrace` is **declared** in the `core` module and **defined** in `std` via *lang_items* which act as function hooks which are resolved during link-time. Special type `StdBacktrace` is introduced for the `std` part of implementation which acts as a proxy for `Debug` and `Display` trait impls. - // TODO: add examples of how would one implement these functions themselves like panic hooks From 3b015acd07145529894edc010f0a4f575161e02e Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Mon, 23 Aug 2021 12:19:57 +0200 Subject: [PATCH 08/16] Flesh out several sections. Fix naming. --- 0000-backtrace-in-core.md | 169 ++++++++++++++++++++++++++++++++++---- 1 file changed, 153 insertions(+), 16 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index f4d59548c33..82a003b20a5 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -1,12 +1,12 @@ - Feature Name: Backtrace in `core` -- Start Date: (fill me in with today's date, YYYY-MM-DD) +- Start Date: 2021-07-03 - RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) - Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) # Summary [summary]: #summary -This RFC proposes moving the `Backtrace` type from `std` to `core` (and some changes to `Backtrace` to facilitate this). The change is motivated by the desire to move `Error` from `std` to `core`, which requires either moving `Backtrace` or abstracting it out of `Error`. +This RFC proposes moving the `Backtrace` type from std to core (and some changes to `Backtrace` to facilitate this). The change is motivated by the desire to move `Error` from std to core, which requires either moving `Backtrace` or abstracting it out of `Error`. Backtrace is a (often specifically formatted) list of function calls which program was in at the moment of its generation. Backtraces in Rust are generated when a program reaches an unrecoverable error and panics. In the other case (recoverable errors) they are handled by choosing a "fail path" which informs the user that an error ocurred and the program proceeds. @@ -15,16 +15,18 @@ This RFC does not cover eventually expanding the `Backtrace` API to include more # Motivation [motivation]: #motivation -The main reason behind moving `Backtrace` to `core` is to have essential types available for wider usage without the need to import `std`. [Error Handling Group Post](https://blog.rust-lang.org/inside-rust/2021/07/01/What-the-error-handling-project-group-is-working-towards.html#1-error-trait--panic-runtime-integration) goes into details on why one would want the `Error` type (and consequently `Backtrace`) in `core`. The end goal is to have a `panic_error` function which would allow for generating panic errors with detailed error informations. +The main reason behind moving `Backtrace` to core is to have essential types available for wider usage without the need to import std. [Error Handling Group Post](https://blog.rust-lang.org/inside-rust/2021/07/01/What-the-error-handling-project-group-is-working-towards.html#1-error-trait--panic-runtime-integration) goes into details on why one would want the `Error` type (and consequently `Backtrace`) in core. The end goal is to have a `panic_error` function which would allow for generating panic errors with detailed error informations. -The [original PR](https://github.com/rust-lang/rust/pull/72981) which aims to stabilize the `Backtrace` already described that it is desirable to have this type in `core`. +The [original PR](https://github.com/rust-lang/rust/pull/72981) which aims to stabilize the `Backtrace` already described that it is desirable to have this type in core. +While `Error` had a valid reason for not being in core (it relies on `Box` type for different conversions), `Backtrace` does not have similar blockers apart from its frame-allocating API. -While `Error` had a valid reason for not being in `core` (it relies on `Box` type for different conversions), `Backtrace` does not have similar blockers apart from its frame-allocating API +There are several approaches we can take, each of them having their own drawbacks and advantages. [Current solution](https://github.com/rust-lang/rust/pull/77384) backing this RFC uses `lang_items` which require tight integration with the Rust compiler. Also, this introduces additional requirement on `no_std` users who have to implement additional functions to have a compiling binary. Another solution would be to leave the `Backtrace` as it is and instead just wait until the[Generic Member Access RFC](https://github.com/rust-lang/rfcs/pull/2895) gets accepted and merged. This way `Error` could be moved to core on its own and `Backtrace` would be [provided to it only when desired](https://github.com/rust-lang/rfcs/pull/2895/files#diff-bb91f37510dc7a6369183c40dbdcfb52164335efa7a04f005a906d0006754d27R88). Third alternative is moving `Backtrace` to alloc which would be an intermediate step, since it will be merged with core eventually (TODO: source?). -Additionally, having this type in `core` will allow its users to provide their own implementations of the backtrace collection and reporting and not rely on the `std`-provided one if they don't want to. +Additionally, having this type in core will allow its users to provide their own implementations of the backtrace collection and reporting and not rely on the std-provided one if they don't want to. + +The outcome of this RFC will be a `Backtrace` type in core with implementation defined in std and compiler-generated implementation when std is not linked and user did not provide their own implementation for the reporting functions. -The outcome of this RFC will be a `Backtrace` type in `core` with implementation defined in `std` and compiler-generated implementation when `std` is not linked and user did not provide their own implementation for the reporting functions. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation @@ -57,9 +59,9 @@ note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose bac They allow for a detailed inspection of what crashed at runtime and which libraries and functions were involved in the process. Obviously reporting and formatting it is also of a big importance, so `Error` allows for backtrace printing with help of a `backtrace()` method. -Currently, `Backtrace` is an essential part of the `std` library and user can control whether the backtrace is enabled or disabled using the environmental variable: `RUST_BACKTRACE`. +Currently, `Backtrace` is an essential part of the std library and user can control whether the backtrace is enabled or disabled using the environmental variable: `RUST_BACKTRACE`. -In terms of Guide-level changes, there is not much to be discussed - only that it is moved to `core` and if `std` is not linked, automatic backtrace handlers will be generated. Otherwise, the regular implementation of `Backtrace` is present. +In terms of Guide-level changes, there is not much to be discussed - only that it is moved to core and if std is not linked, automatic backtrace handlers will be generated. Otherwise, the regular implementation of `Backtrace` is present. TODO: what else should be here? @@ -71,13 +73,148 @@ TODO: what else should be here? The following changes need to be made to implement this proposal: -## Move the `Backtrace` to `core` and add a thin wrapper in `std` +## Move the `Backtrace` to core and add a thin wrapper in std + +The core part of `Backtrace` looks as follows: +```rust +/// The current status of a backtrace, indicating whether it was captured or +/// whether it is empty for some other reason. +#[non_exhaustive] +#[derive(Debug, PartialEq, Eq)] +pub enum BacktraceStatus { + /// Capturing a backtrace is not supported, likely because it's not + /// implemented for the current platform. + Unsupported, + /// Capturing a backtrace has been disabled through either the + /// `RUST_LIB_BACKTRACE` or `RUST_BACKTRACE` environment variables. + Disabled, + /// A backtrace has been captured and the `Backtrace` should print + /// reasonable information when rendered. + Captured, +} + +#[unstable(feature = "backtrace", issue = "53487")] +/// +pub struct Backtrace { + /// + inner: *mut dyn RawBacktrace, +} + +/// Global implementation of backtrace functionality. Called to create +/// `RawBacktrace` trait objects. +#[cfg(not(bootstrap))] +extern "Rust" { + #[lang = "backtrace_create"] + fn backtrace_create(ip: usize) -> *mut dyn RawBacktrace; -This wrapper is used to do the actual backtrace capturing from the `std` in the three functions described below. + #[lang = "backtrace_enabled"] + fn backtrace_enabled() -> bool; -The regular API of `Backtrace` comprising `enabled()`, `create()` and `status()` would be left in the `std` as free-standing functions. Since, they are lang items they need to have a default implementation in case `std` is not linked, so they will be provided in such a form in the `core` library (and overwritten once `std` is linked): + #[lang = "backtrace_status"] + fn backtrace_status(raw: *mut dyn RawBacktrace) -> BacktraceStatus; +} + +#[cfg(bootstrap)] +unsafe fn backtrace_create(_ip: usize) -> *mut dyn RawBacktrace { + UnsupportedBacktrace::create().inner +} + +#[cfg(bootstrap)] +unsafe fn backtrace_enabled() -> bool { + false +} + +#[cfg(bootstrap)] +unsafe fn backtrace_status(_raw: *mut dyn RawBacktrace) -> BacktraceStatus { + BacktraceStatus::Unsupported +} + +impl Backtrace { + fn create(ip: usize) -> Backtrace { + // SAFETY: trust me + let inner = unsafe { backtrace_create(ip) }; + Backtrace { inner } + } + + /// Returns whether backtrace captures are enabled through environment + /// variables. + fn enabled() -> bool { + // SAFETY: trust me + unsafe { backtrace_enabled() } + } + + /// Capture a stack backtrace of the current thread. + /// + /// This function will capture a stack backtrace of the current OS thread of + /// execution, returning a `Backtrace` type which can be later used to print + /// the entire stack trace or render it to a string. + /// + /// This function will be a noop if the `RUST_BACKTRACE` or + /// `RUST_LIB_BACKTRACE` backtrace variables are both not set. If either + /// environment variable is set and enabled then this function will actually + /// capture a backtrace. Capturing a backtrace can be both memory intensive + /// and slow, so these environment variables allow liberally using + /// `Backtrace::capture` and only incurring a slowdown when the environment + /// variables are set. + /// + /// To forcibly capture a backtrace regardless of environment variables, use + /// the `Backtrace::force_capture` function. + #[inline(never)] // want to make sure there's a frame here to remove + pub fn capture() -> Backtrace { + if !Backtrace::enabled() { + return Backtrace::disabled(); + } + + Self::create(Backtrace::capture as usize) + } + + /// Forcibly captures a full backtrace, regardless of environment variable + /// configuration. + /// + /// This function behaves the same as `capture` except that it ignores the + /// values of the `RUST_BACKTRACE` and `RUST_LIB_BACKTRACE` environment + /// variables, always capturing a backtrace. + /// + /// Note that capturing a backtrace can be an expensive operation on some + /// platforms, so this should be used with caution in performance-sensitive + /// parts of code. + #[inline(never)] // want to make sure there's a frame here to remove + pub fn force_capture() -> Backtrace { + Self::create(Backtrace::force_capture as usize) + } + + /// Forcibly captures a disabled backtrace, regardless of environment + /// variable configuration. + pub const fn disabled() -> Backtrace { + DisabledBacktrace::create() + } + + /// Returns the status of this backtrace, indicating whether this backtrace + /// request was unsupported, disabled, or a stack trace was actually + /// captured. + pub fn status(&self) -> BacktraceStatus { + // SAFETY: trust me + unsafe { backtrace_status(self.inner) } + } +} +``` + +The wrapper part of `Backtrace` in std: +```rust +struct StdBacktrace { + inner: Inner, +} +``` + +This wrapper is used to do the actual backtrace capturing from the std in the three functions described below. + +The regular API of `Backtrace` comprising `enabled()`, `create()` and `status()` would be left in the std as free-standing functions. Since, they are lang items they need to have a default implementation in case std is not linked, so they will be provided in such a form in the core library (and overwritten once std is linked): ```rust +pub use core::backtrace::Backtrace; +pub use core::backtrace::BacktraceStatus; +use core::backtrace::RawBacktrace; + /// Global implementation of backtrace functionality. Called to create /// `RawBacktrace` trait objects. #[cfg(not(bootstrap))] @@ -119,7 +256,7 @@ This way, if the `Backtrace` is not enabled (on the library level) there is no n # Drawbacks [drawbacks]: #drawbacks -There are actually many drawbacks, most important of them being `Backtrace` using a lot of `std` for the actual backtrace capturing. +There are actually many drawbacks, most important of them being `Backtrace` using a lot of std for the actual backtrace capturing. The other one is a potential code bloat in `no_std` contexts, so a possible alternative may be only enabling the `Backtrace` conditionally via `cfg` settings. (not so sure about this though) @@ -130,16 +267,16 @@ The other one is a potential code bloat in `no_std` contexts, so a possible alte # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives -The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented (link!) we would not have to move the `Backtrace` to `core` at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. +The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented (link!) we would not have to move the `Backtrace` to core at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. -Alternatively, since the actual implementation of `Backtrace` uses allocating structures like `Box` for backtrace creation, it would be prudent to move it to where this type resides. This will in turn allow users of `alloc` module in embedded and `no_std` contexts to use this functionality without `std`. _should we go for this though instead of the core when we introduce such big changes?_ +Alternatively, since the actual implementation of `Backtrace` uses allocating structures like `Box` for backtrace creation, it would be prudent to move it to where this type resides. This will in turn allow users of alloc module in embedded and `no_std` contexts to use this functionality without std. _should we go for this though instead of the core when we introduce such big changes?_ A viable solution to allocating functions of `Backtrace` might be adding an API where the users could provide themselves the memory in which the backtrace should reside and truncate/report a failure in case the backtrace does not fit this preallocated space. # Prior art [prior-art]: #prior-art -This type is already implemented, but it seems like no type was moved from `std` to `core` previously so we have no point of reference on this one. +This type is already implemented, but it seems like no type was moved from std to core previously so we have no point of reference on this one. As for `no_std` and embedded contexts, there exists the [mini-backtrace](https://github.com/amanieu/mini-backtrace) library that provides backtrace support via LLVM's libunwind. From 184aa9d19d561d60e16fc08cd0ae9006ef92d5bd Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Mon, 23 Aug 2021 14:15:52 +0200 Subject: [PATCH 09/16] Add alternatives. Elaborate on guide-level-explanation. --- 0000-backtrace-in-core.md | 117 ++++++++++++++++++++++++++++++++------ 1 file changed, 100 insertions(+), 17 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index 82a003b20a5..c0c1a54db47 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -59,12 +59,91 @@ note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose bac They allow for a detailed inspection of what crashed at runtime and which libraries and functions were involved in the process. Obviously reporting and formatting it is also of a big importance, so `Error` allows for backtrace printing with help of a `backtrace()` method. -Currently, `Backtrace` is an essential part of the std library and user can control whether the backtrace is enabled or disabled using the environmental variable: `RUST_BACKTRACE`. +Currently, `Backtrace` is an essential part of the std library and user can control whether the backtrace is enabled or disabled using the environmental variable: `RUST_BACKTRACE` and `RUST_LIB_BACKTRACE` - see the [documentation](https://doc.rust-lang.org/std/backtrace/index.html) for differences between them. -In terms of Guide-level changes, there is not much to be discussed - only that it is moved to core and if std is not linked, automatic backtrace handlers will be generated. Otherwise, the regular implementation of `Backtrace` is present. +More specifically, Rust's `Backtrace` is a struct which is a wrapper over a stack backtrace: +```rust +pub struct Backtrace { + inner: Inner, +} +``` +It can be captured or not, depending on the environment settings: +```rust +/// The current status of a backtrace, indicating whether it was captured or +/// whether it is empty for some other reason. +#[non_exhaustive] +#[derive(Debug, PartialEq, Eq)] +pub enum BacktraceStatus { + /// Capturing a backtrace is not supported, likely because it's not + /// implemented for the current platform. + Unsupported, + /// Capturing a backtrace has been disabled through either the + /// `RUST_LIB_BACKTRACE` or `RUST_BACKTRACE` environment variables. + Disabled, + /// A backtrace has been captured and the `Backtrace` should print + /// reasonable information when rendered. + Captured, +} + +enum Inner { + Unsupported, + Disabled, + Captured(LazilyResolvedCapture), +} +``` +The `Capture` option of our `Backtrace` looks like this, and contains stack frames: +```rust +struct Capture { + actual_start: usize, + resolved: bool, + frames: Vec, +} +``` +Once captured, it is filled with `BacktraceFrame`s which contain the actual frame and symbols relevant to this frame: +```rust +/// A single frame of a backtrace. +#[unstable(feature = "backtrace_frames", issue = "79676")] +pub struct BacktraceFrame { + frame: RawFrame, + symbols: Vec, +} +``` + + +Users are interested in utilizing `Backtrace` for several reasons, primarily to inspect or reformat the frames at the point of `Backtrace` generation or to present the frames in some different way to the user. While most for most Rust programmers this type is transparent, people who use it recently even got a new API which returns an [iterator over the captured frames](https://doc.rust-lang.org/src/std/backtrace.rs.html#367). -TODO: what else should be here? +Two main groups of users can be distinguished: regular and `no_std`. While former don't care whether this type is in core or in std (it gets re-exported by std), the latter do not have the possibility to use this functionality right now and would be interested in having it available. On the other hand, the consumers of the type are satisfied with simply having the `Backtrace` printed upon panicking and seeing where exactly the program failed. + +We use `Backtrace` right now as follows: +```rust +#![feature(backtrace)] + +fn main() { + let backtrace = std::backtrace::Backtrace::capture(); + println!("{}", backtrace); +} +``` + +For inspecting frames one-by-one, we can use this API: +```rust +#![feature(backtrace)] +#![feature(backtrace_frames)] + +fn main() { + let backtrace = std::backtrace::Backtrace::capture(); + for frame in backtrace.frames() { + println!("{:?}", frame); + } +} +``` +After the `Backtrace` is moved to core not much has to be changed: +```rust +fn main() { + let backtrace = core::backtrace::Backtrace::capture(); + println!("{}", backtrace); +} +``` ## Trade-offs in this solution @@ -208,7 +287,8 @@ struct StdBacktrace { This wrapper is used to do the actual backtrace capturing from the std in the three functions described below. -The regular API of `Backtrace` comprising `enabled()`, `create()` and `status()` would be left in the std as free-standing functions. Since, they are lang items they need to have a default implementation in case std is not linked, so they will be provided in such a form in the core library (and overwritten once std is linked): +## Add the 3 free-standing functions to std +The regular API of `Backtrace` comprising `enabled()`, `create()` and `status()` would be left in the std as free-standing functions.: ```rust pub use core::backtrace::Backtrace; @@ -246,38 +326,37 @@ unsafe fn backtrace_status(_raw: *mut dyn RawBacktrace) -> BacktraceStatus { ``` -This change will make the `Backtrace` an optional part of the library and -This way, if the `Backtrace` is not enabled (on the library level) there is no need for it to report - -// TODO: add examples of how would one implement these functions themselves like panic hooks - - - # Drawbacks [drawbacks]: #drawbacks -There are actually many drawbacks, most important of them being `Backtrace` using a lot of std for the actual backtrace capturing. +The solution proposed by this RFC needs to integrate `Backtrace` implementation tightly with the Rust compiler machinery via usage of `lang_items`. This adds maintenance cost and mental overhead required to remember why this functionality is implemented in such special way. Ideally we would add functionality to the language without edge cases and cutting corners, but it is not always possible (refer to panic hooks implementation). Also, moving `Backtrace` to core was met with moderate reluctance by the [#rust-embedded community on Matrix](https://github.com/rust-embedded/wg) because of how the capturing API uses allocating functions ([logs here](https://libera.irclog.whitequark.org/rust-embedded/2021-08-17)). +/* The other one is a potential code bloat in `no_std` contexts, so a possible alternative may be only enabling the `Backtrace` conditionally via `cfg` settings. (not so sure about this though) - - `no_std` code bloat - Mara mentioned out `Error:backtrace` also seems to be blocking?? +*/ # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives -The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented (link!) we would not have to move the `Backtrace` to core at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. +The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented as discussed in the Motivation section, we would not have to move the `Backtrace` to core at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. This has the obvious drawback - if we just wait for its stabilization, we might have to wait for a long time and remain without viable `Backtrace` to be used in `no_std` contexts. + +Alternatively, since the actual implementation of `Backtrace` uses allocating structures like `Box` for backtrace creation, it would be prudent to move it to where this type resides. This will in turn allow users of alloc module in embedded and `no_std` contexts to use this functionality without std. Similarly to the Generic Member Access solution, it is a time-costly solution. + +During the [conversation on #rust-embedded IRC](https://libera.irclog.whitequark.org/rust-embedded/2021-08-17), various takes on the matter from the embedded contexts were given. What was most threatening for people engaged in the discussion is the allocating capabilities of `Backtrace`. -Alternatively, since the actual implementation of `Backtrace` uses allocating structures like `Box` for backtrace creation, it would be prudent to move it to where this type resides. This will in turn allow users of alloc module in embedded and `no_std` contexts to use this functionality without std. _should we go for this though instead of the core when we introduce such big changes?_ +A viable solution to this concern might be adding an API where the users could provide themselves the memory in which the backtrace should reside and truncate/report a failure in case the backtrace does not fit this preallocated space. In case the user did not provide providing their `capture()` implementation, there should be a no-op provided by the language. -A viable solution to allocating functions of `Backtrace` might be adding an API where the users could provide themselves the memory in which the backtrace should reside and truncate/report a failure in case the backtrace does not fit this preallocated space. +There was also an idea of providing general backtrace capturing functions for each family of embedded devices, but that would be too difficult to implement cohesively due to differences in implementations between them. Thus, what is proposed above seems like a valid alternative. # Prior art [prior-art]: #prior-art This type is already implemented, but it seems like no type was moved from std to core previously so we have no point of reference on this one. +There exists a [`backtrace-rs` crate](https://github.com/rust-lang/backtrace-rs) which supports acquiring backtraces at runtime. + As for `no_std` and embedded contexts, there exists the [mini-backtrace](https://github.com/amanieu/mini-backtrace) library that provides backtrace support via LLVM's libunwind. # Unresolved questions @@ -286,4 +365,8 @@ As for `no_std` and embedded contexts, there exists the [mini-backtrace](https:/ # Future possibilities [future-possibilities]: #future-possibilities +Since the RFC proposes a solution based on `lang_items`, one could wish to implement these functions themselves. We could support such endeavours and provide dummy implementations if the compiler does not see the overrides. This would be implemented via weak linkage (though, unfortunately not all platforms support it). + +// TODO: add examples of how would one implement these functions themselves like panic hooks + //Since, they are lang items they need to have a default implementation in case std is not linked, so they will be provided in such a form in the core library (and overwritten once std is linked) From b322f589d0e114535891eae53e9d738c41ea2ff8 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Mon, 23 Aug 2021 14:23:12 +0200 Subject: [PATCH 10/16] Address code bloat. --- 0000-backtrace-in-core.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index c0c1a54db47..5e6aaeee1e3 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -329,7 +329,20 @@ unsafe fn backtrace_status(_raw: *mut dyn RawBacktrace) -> BacktraceStatus { # Drawbacks [drawbacks]: #drawbacks -The solution proposed by this RFC needs to integrate `Backtrace` implementation tightly with the Rust compiler machinery via usage of `lang_items`. This adds maintenance cost and mental overhead required to remember why this functionality is implemented in such special way. Ideally we would add functionality to the language without edge cases and cutting corners, but it is not always possible (refer to panic hooks implementation). Also, moving `Backtrace` to core was met with moderate reluctance by the [#rust-embedded community on Matrix](https://github.com/rust-embedded/wg) because of how the capturing API uses allocating functions ([logs here](https://libera.irclog.whitequark.org/rust-embedded/2021-08-17)). +The solution proposed by this RFC needs to integrate `Backtrace` implementation tightly with the Rust compiler machinery via usage of `lang_items`. This adds maintenance cost and mental overhead required to remember why this functionality is implemented in such special way. Ideally we would add functionality to the language without edge cases and cutting corners, but it is not always possible (refer to panic hooks implementation). + +Also, moving `Backtrace` to core was met with moderate reluctance by the [#rust-embedded community on Matrix](https://github.com/rust-embedded/wg) because of how the capturing API uses allocating functions ([logs here](https://libera.irclog.whitequark.org/rust-embedded/2021-08-17)). + +Current implementation uses a fat pointer for storing the `RawBacktrace` inside the `Backtrace`: +```rust +#[unstable(feature = "backtrace", issue = "53487")] +/// +pub struct Backtrace { + /// + inner: *mut dyn RawBacktrace, +} +``` +and this may induce some code bloat which we do not want. /* The other one is a potential code bloat in `no_std` contexts, so a possible alternative may be only enabling the `Backtrace` conditionally via `cfg` settings. (not so sure about this though) From b29d0084ad5d3eed0cf82d3a22a2d91b3d21630b Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Fri, 27 Aug 2021 10:33:35 +0200 Subject: [PATCH 11/16] Apply suggestions from code review Co-authored-by: Jane Lusby Co-authored-by: Nick Cameron --- 0000-backtrace-in-core.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index 5e6aaeee1e3..00e4d6912da 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -8,7 +8,7 @@ This RFC proposes moving the `Backtrace` type from std to core (and some changes to `Backtrace` to facilitate this). The change is motivated by the desire to move `Error` from std to core, which requires either moving `Backtrace` or abstracting it out of `Error`. -Backtrace is a (often specifically formatted) list of function calls which program was in at the moment of its generation. Backtraces in Rust are generated when a program reaches an unrecoverable error and panics. In the other case (recoverable errors) they are handled by choosing a "fail path" which informs the user that an error ocurred and the program proceeds. +Backtrace is a (often specifically formatted) list of function calls which the program was in at the moment of its generation. Backtraces in Rust are generated when a program reaches an unrecoverable error and panics or they can be captured and stored when constructing recoverable errors by manually calling `Backtrace::capture()`. This RFC does not cover eventually expanding the `Backtrace` API to include more functions and it does not solve the way backtraces are collected (although it proposes different takes on the matter). @@ -21,9 +21,9 @@ The [original PR](https://github.com/rust-lang/rust/pull/72981) which aims to st While `Error` had a valid reason for not being in core (it relies on `Box` type for different conversions), `Backtrace` does not have similar blockers apart from its frame-allocating API. -There are several approaches we can take, each of them having their own drawbacks and advantages. [Current solution](https://github.com/rust-lang/rust/pull/77384) backing this RFC uses `lang_items` which require tight integration with the Rust compiler. Also, this introduces additional requirement on `no_std` users who have to implement additional functions to have a compiling binary. Another solution would be to leave the `Backtrace` as it is and instead just wait until the[Generic Member Access RFC](https://github.com/rust-lang/rfcs/pull/2895) gets accepted and merged. This way `Error` could be moved to core on its own and `Backtrace` would be [provided to it only when desired](https://github.com/rust-lang/rfcs/pull/2895/files#diff-bb91f37510dc7a6369183c40dbdcfb52164335efa7a04f005a906d0006754d27R88). Third alternative is moving `Backtrace` to alloc which would be an intermediate step, since it will be merged with core eventually (TODO: source?). +There are several approaches we can take, each of them having their own drawbacks and advantages. [Current solution](https://github.com/rust-lang/rust/pull/77384) backing this RFC uses `lang_items` which require tight integration with the Rust compiler. Also, this introduces additional requirement on `no_std` users who have to implement additional functions to have a compiling binary. Another solution would be to leave the `Backtrace` as it is and instead just wait until the[Generic Member Access RFC](https://github.com/rust-lang/rfcs/pull/2895) gets accepted and merged. This way `Error` could be moved to core on its own and `Backtrace` would be [provided to it only when desired](https://github.com/rust-lang/rfcs/pull/2895/files#diff-bb91f37510dc7a6369183c40dbdcfb52164335efa7a04f005a906d0006754d27R88). -Additionally, having this type in core will allow its users to provide their own implementations of the backtrace collection and reporting and not rely on the std-provided one if they don't want to. +Additionally, having this type in core will allow its users to provide their own implementations of the backtrace capture and reporting and not rely on the std-provided one if they don't want to. The outcome of this RFC will be a `Backtrace` type in core with implementation defined in std and compiler-generated implementation when std is not linked and user did not provide their own implementation for the reporting functions. @@ -57,9 +57,9 @@ stack backtrace: note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. ``` -They allow for a detailed inspection of what crashed at runtime and which libraries and functions were involved in the process. Obviously reporting and formatting it is also of a big importance, so `Error` allows for backtrace printing with help of a `backtrace()` method. +They allow for a detailed inspection of what crashed at runtime and which libraries and functions were involved in the process. Backtraces act like a map describing the origin of an error within a program's source code. This is useful information when debugging errors but does not itself describe the reason for an error, and so it does not belong in the error message itself. To model this the `Error` trait provides separate methods for retrieving backtraces (`fn backtrace()`) and error messages (`Display`) to provide flexibility when constructing error reports. -Currently, `Backtrace` is an essential part of the std library and user can control whether the backtrace is enabled or disabled using the environmental variable: `RUST_BACKTRACE` and `RUST_LIB_BACKTRACE` - see the [documentation](https://doc.rust-lang.org/std/backtrace/index.html) for differences between them. +Currently, `Backtrace` is part of the std library and users can control whether the backtrace is enabled or disabled using the environmental variable: `RUST_BACKTRACE` and `RUST_LIB_BACKTRACE` - see the [documentation](https://doc.rust-lang.org/std/backtrace/index.html) for differences between them. More specifically, Rust's `Backtrace` is a struct which is a wrapper over a stack backtrace: ```rust @@ -287,8 +287,9 @@ struct StdBacktrace { This wrapper is used to do the actual backtrace capturing from the std in the three functions described below. -## Add the 3 free-standing functions to std -The regular API of `Backtrace` comprising `enabled()`, `create()` and `status()` would be left in the std as free-standing functions.: +## Add the 3 new lang-items + +The `std` and `core` sides of the `Backtrace` API would be connected via three new lang items: `enabled()`, `create()` and `status()`. These functions are declared and called from `core` and defined in `std`. `no_std` binaries would need to provide their own implementations of these lang items or conditionally compile `core` to not include `Backtrace` support. ```rust pub use core::backtrace::Backtrace; @@ -353,9 +354,8 @@ The other one is a potential code bloat in `no_std` contexts, so a possible alte # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives -The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented as discussed in the Motivation section, we would not have to move the `Backtrace` to core at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. This has the obvious drawback - if we just wait for its stabilization, we might have to wait for a long time and remain without viable `Backtrace` to be used in `no_std` contexts. +The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented as discussed in the Motivation section, we would not have to move the `Backtrace` to core at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. -Alternatively, since the actual implementation of `Backtrace` uses allocating structures like `Box` for backtrace creation, it would be prudent to move it to where this type resides. This will in turn allow users of alloc module in embedded and `no_std` contexts to use this functionality without std. Similarly to the Generic Member Access solution, it is a time-costly solution. During the [conversation on #rust-embedded IRC](https://libera.irclog.whitequark.org/rust-embedded/2021-08-17), various takes on the matter from the embedded contexts were given. What was most threatening for people engaged in the discussion is the allocating capabilities of `Backtrace`. From 625954e131fdf2ef6731b92e92cc7248b99578f4 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Fri, 27 Aug 2021 11:43:25 +0200 Subject: [PATCH 12/16] Address some remarks. Remove before/after comparisons. --- 0000-backtrace-in-core.md | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index 00e4d6912da..70e4b3cca9f 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -110,11 +110,11 @@ pub struct BacktraceFrame { ``` -Users are interested in utilizing `Backtrace` for several reasons, primarily to inspect or reformat the frames at the point of `Backtrace` generation or to present the frames in some different way to the user. While most for most Rust programmers this type is transparent, people who use it recently even got a new API which returns an [iterator over the captured frames](https://doc.rust-lang.org/src/std/backtrace.rs.html#367). +Users are interested in utilizing `Backtrace` for several reasons, primarily to inspect or reformat the frames at the point of `Backtrace` generation or to present the frames in some different way to the user. While most for most Rust programmers this type is transparent, people who use the recent versions of nightly even got a new API which returns an [iterator over the captured frames](https://doc.rust-lang.org/src/std/backtrace.rs.html#367). Two main groups of users can be distinguished: regular and `no_std`. While former don't care whether this type is in core or in std (it gets re-exported by std), the latter do not have the possibility to use this functionality right now and would be interested in having it available. On the other hand, the consumers of the type are satisfied with simply having the `Backtrace` printed upon panicking and seeing where exactly the program failed. -We use `Backtrace` right now as follows: +One uses this API like this: ```rust #![feature(backtrace)] @@ -136,23 +136,15 @@ fn main() { } } ``` - -After the `Backtrace` is moved to core not much has to be changed: -```rust -fn main() { - let backtrace = core::backtrace::Backtrace::capture(); - println!("{}", backtrace); -} -``` ## Trade-offs in this solution # Reference-level explanation [reference-level-explanation]: #reference-level-explanation -The following changes need to be made to implement this proposal: +After this RFC is merged the `Backtrace` type will reside in core and the interfacing lang items will be in std. -## Move the `Backtrace` to core and add a thin wrapper in std +## The `Backtrace` implementation in core The core part of `Backtrace` looks as follows: ```rust @@ -278,16 +270,16 @@ impl Backtrace { } ``` -The wrapper part of `Backtrace` in std: +The part of `Backtrace` in std: ```rust struct StdBacktrace { inner: Inner, } ``` -This wrapper is used to do the actual backtrace capturing from the std in the three functions described below. +This struct is used to interface the actual backtraces from the std in the three functions described below. -## Add the 3 new lang-items +## The 3 new lang-items The `std` and `core` sides of the `Backtrace` API would be connected via three new lang items: `enabled()`, `create()` and `status()`. These functions are declared and called from `core` and defined in `std`. `no_std` binaries would need to provide their own implementations of these lang items or conditionally compile `core` to not include `Backtrace` support. @@ -347,8 +339,6 @@ and this may induce some code bloat which we do not want. /* The other one is a potential code bloat in `no_std` contexts, so a possible alternative may be only enabling the `Backtrace` conditionally via `cfg` settings. (not so sure about this though) -`no_std` code bloat - Mara mentioned out -`Error:backtrace` also seems to be blocking?? */ # Rationale and alternatives @@ -368,13 +358,16 @@ There was also an idea of providing general backtrace capturing functions for ea This type is already implemented, but it seems like no type was moved from std to core previously so we have no point of reference on this one. -There exists a [`backtrace-rs` crate](https://github.com/rust-lang/backtrace-rs) which supports acquiring backtraces at runtime. +The [`backtrace-rs` crate](https://github.com/rust-lang/backtrace-rs) is a cornerstone of the `Backtrace` type in the std library. It contains all the nitty-gritty details of obtaining the actual backtraces. As for `no_std` and embedded contexts, there exists the [mini-backtrace](https://github.com/amanieu/mini-backtrace) library that provides backtrace support via LLVM's libunwind. # Unresolved questions [unresolved-questions]: #unresolved-questions +Will introducing this implementation be worth in the long term? - `no_std` users can implement their own backtrace capturing mechanisms either way and not care about `Backtrace` in core. + +Is this better than Generic Member Access? - This will be answered with either implementing this RFC or dropping it. # Future possibilities [future-possibilities]: #future-possibilities From dbf7bcd05ded9b1b751412a8989bf474e32d5cf1 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Fri, 27 Aug 2021 11:50:08 +0200 Subject: [PATCH 13/16] Add another possible approach. --- 0000-backtrace-in-core.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index 70e4b3cca9f..fb52f797d9f 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -21,7 +21,11 @@ The [original PR](https://github.com/rust-lang/rust/pull/72981) which aims to st While `Error` had a valid reason for not being in core (it relies on `Box` type for different conversions), `Backtrace` does not have similar blockers apart from its frame-allocating API. -There are several approaches we can take, each of them having their own drawbacks and advantages. [Current solution](https://github.com/rust-lang/rust/pull/77384) backing this RFC uses `lang_items` which require tight integration with the Rust compiler. Also, this introduces additional requirement on `no_std` users who have to implement additional functions to have a compiling binary. Another solution would be to leave the `Backtrace` as it is and instead just wait until the[Generic Member Access RFC](https://github.com/rust-lang/rfcs/pull/2895) gets accepted and merged. This way `Error` could be moved to core on its own and `Backtrace` would be [provided to it only when desired](https://github.com/rust-lang/rfcs/pull/2895/files#diff-bb91f37510dc7a6369183c40dbdcfb52164335efa7a04f005a906d0006754d27R88). +There are several approaches we can take, each of them having their own drawbacks and advantages. [Current solution](https://github.com/rust-lang/rust/pull/77384) backing this RFC uses `lang_items` which require tight integration with the Rust compiler. Also, this introduces additional requirement on `no_std` users who have to implement additional functions to have a compiling binary. + +Another solution would be to leave the `Backtrace` as it is and instead just wait until the[Generic Member Access RFC](https://github.com/rust-lang/rfcs/pull/2895) gets accepted and merged. This way `Error` could be moved to core on its own and `Backtrace` would be [provided to it only when desired](https://github.com/rust-lang/rfcs/pull/2895/files#diff-bb91f37510dc7a6369183c40dbdcfb52164335efa7a04f005a906d0006754d27R88). + +Yet another take on the subject is to store and capture the backtrace in core and do the actual generation in std. This way users could implement their own backtrace generation if they do not wish to use std. This would, however, require some bigger changes in the current implementation, but would not require the lang items. Additionally, having this type in core will allow its users to provide their own implementations of the backtrace capture and reporting and not rely on the std-provided one if they don't want to. @@ -136,8 +140,6 @@ fn main() { } } ``` -## Trade-offs in this solution - # Reference-level explanation [reference-level-explanation]: #reference-level-explanation From 06f3853e051ffd0db5ce0a241e132e742010e9ab Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Sat, 28 Aug 2021 13:57:22 +0200 Subject: [PATCH 14/16] Update 0000-backtrace-in-core.md --- 0000-backtrace-in-core.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index fb52f797d9f..96ecb3fe1f0 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -349,9 +349,9 @@ The other one is a potential code bloat in `no_std` contexts, so a possible alte The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented as discussed in the Motivation section, we would not have to move the `Backtrace` to core at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. -During the [conversation on #rust-embedded IRC](https://libera.irclog.whitequark.org/rust-embedded/2021-08-17), various takes on the matter from the embedded contexts were given. What was most threatening for people engaged in the discussion is the allocating capabilities of `Backtrace`. +During the [conversation on #rust-embedded IRC](https://libera.irclog.whitequark.org/rust-embedded/2021-08-17), various takes on the matter from the embedded contexts were given. What was most threatening for people engaged in the discussion is the allocating capabilities of `Backtrace`. Though the implementation in std uses `Vec` for allocating backtrace frames, the API declaration in core leaves the implementation to the user (if no std is supplied). -A viable solution to this concern might be adding an API where the users could provide themselves the memory in which the backtrace should reside and truncate/report a failure in case the backtrace does not fit this preallocated space. In case the user did not provide providing their `capture()` implementation, there should be a no-op provided by the language. +This implementation may look like this: provide the memory in which the backtrace should reside and truncate/report a failure in case the backtrace does not fit this preallocated space. In case the user did not provide providing their `capture()` implementation, there should be a no-op provided by the language. There was also an idea of providing general backtrace capturing functions for each family of embedded devices, but that would be too difficult to implement cohesively due to differences in implementations between them. Thus, what is proposed above seems like a valid alternative. From c4326773812037fc2eb551e4b7d9b079e7911b08 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Sat, 30 Oct 2021 11:29:49 +0200 Subject: [PATCH 15/16] Update 0000-backtrace-in-core.md --- 0000-backtrace-in-core.md | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index 96ecb3fe1f0..a998f0d0e7a 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -337,17 +337,14 @@ pub struct Backtrace { inner: *mut dyn RawBacktrace, } ``` -and this may induce some code bloat which we do not want. - -/* -The other one is a potential code bloat in `no_std` contexts, so a possible alternative may be only enabling the `Backtrace` conditionally via `cfg` settings. (not so sure about this though) -*/ +and this may induce some code bloat that is suboptimal but livable-with. It could be changed via some vtable sheaningans but we will leave it as it is for simplicity's sake. # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives The proposed solution is the one which is currently implementable. However, if the Generic Member Access RFC was implemented as discussed in the Motivation section, we would not have to move the `Backtrace` to core at all. In the alternative solution, we would leave the `Backtrace` as it is and instead the `Error` trait will provide a `backtrace()` method which will use the Generic Access to extract the concrete `Backtrace` out of the propagated error. +The `Backtrace` functionality could be implemented as a trait in order to be extensible in the future and would allow for backwards compatibility. However, since in the long goal we want to deprecate `Backtrace` in the `Error` trait we do not pursue that idea. During the [conversation on #rust-embedded IRC](https://libera.irclog.whitequark.org/rust-embedded/2021-08-17), various takes on the matter from the embedded contexts were given. What was most threatening for people engaged in the discussion is the allocating capabilities of `Backtrace`. Though the implementation in std uses `Vec` for allocating backtrace frames, the API declaration in core leaves the implementation to the user (if no std is supplied). @@ -374,7 +371,3 @@ Is this better than Generic Member Access? - This will be answered with either i # Future possibilities [future-possibilities]: #future-possibilities Since the RFC proposes a solution based on `lang_items`, one could wish to implement these functions themselves. We could support such endeavours and provide dummy implementations if the compiler does not see the overrides. This would be implemented via weak linkage (though, unfortunately not all platforms support it). - -// TODO: add examples of how would one implement these functions themselves like panic hooks - - //Since, they are lang items they need to have a default implementation in case std is not linked, so they will be provided in such a form in the core library (and overwritten once std is linked) From 4ef2cfc19c3a512abd775df2318114e4feb5bb58 Mon Sep 17 00:00:00 2001 From: Jakub Duchniewicz Date: Sat, 30 Oct 2021 14:12:35 +0200 Subject: [PATCH 16/16] Update 0000-backtrace-in-core.md --- 0000-backtrace-in-core.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/0000-backtrace-in-core.md b/0000-backtrace-in-core.md index a998f0d0e7a..70327cbc1ac 100644 --- a/0000-backtrace-in-core.md +++ b/0000-backtrace-in-core.md @@ -370,4 +370,4 @@ Is this better than Generic Member Access? - This will be answered with either i # Future possibilities [future-possibilities]: #future-possibilities -Since the RFC proposes a solution based on `lang_items`, one could wish to implement these functions themselves. We could support such endeavours and provide dummy implementations if the compiler does not see the overrides. This would be implemented via weak linkage (though, unfortunately not all platforms support it). +Since the RFC proposes a solution based on `lang_items`, one could wish to implement these functions themselves. We could support such endeavours and provide dummy implementations if the compiler does not see the overrides. This would be implemented via weak linkage (though, unfortunately not all platforms support it). It would be a breaking change, since we would require the linker to be run in `no_std` contexts and currently it is run only on several occasions, therefore this seems like a dead end or at least not worth the effort and overhead.