From b7359bd91c7dfddba328a6678796ce77549287c7 Mon Sep 17 00:00:00 2001 From: ticki Date: Sat, 1 Oct 2016 15:20:45 +0200 Subject: [PATCH 1/5] RFC: Abort by default --- text/0000-abort-by-default.md | 70 +++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 text/0000-abort-by-default.md diff --git a/text/0000-abort-by-default.md b/text/0000-abort-by-default.md new file mode 100644 index 00000000000..e091a111ad1 --- /dev/null +++ b/text/0000-abort-by-default.md @@ -0,0 +1,70 @@ +- Feature Name: abort_by_default +- Start Date: 2016-10-01 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Use abort as the standard panic method rather than unwind. + +# Motivation +[motivation]: #motivation + +## Performance + +Generally, the performance of bigger programs [improves by 10%](https://www.youtube.com/watch?v=mRGb4hoGuPs), which is no small amount. When overflow checking is enabled, it is as high as 2x, making overflowing checking plausible in production. + +The performance gains mainly originates in the fact that it is a lot easier for the compiler to optimize, due to the unwind path disappearing, and hence reasoning about the code becomes easier. + +## Binary size + +Binary size improves by 10% as well. This is due to the lack of landing pads which are injected in the stack when unwinding is enabled. + +## Compile time + +Compile time in debug mode improves by 20% on average. In release mode, the number is around 10%. + +## Runtime size + +Unwinding requires a fair amount of runtime code, which can be seen as conflicting with the goals of Rust. + +## Correctness + +You often see people abusing `std::panic::catch_unwind` for exception handling. Forcing the user to explicitly opt in to unwinding will make it harder to unintentionally misuse it. + +# Detailed design +[design]: #detailed-design + +Default to `panic=abort`. + +## Backwards compatibility + +No code should rely on a particular default panicking strategy, and code which do is already broken. However, this broken code might start actually encountering the issues. In a sense, that is actually positive, since it reveals that the code is broken. + +No invariants are broken, and no code is going to emit Undefined Behavior due to this change (this can be seen by observing that aborting is globally diverging, and hence no code is run after). + +# Drawbacks +[drawbacks]: #drawbacks + +While not breaking, the behavior of certain broken programs can change. + +It makes panics global by default (i.e., panicking in some thread will abort the whole program). + +It might make it tempting to ignore the existence of an unwind option and consequently shaping the code after panicking being aborting. + +# Alternatives +[alternatives]: #alternatives + +Keep unwinding as default. + +Make Cargo set `panic=abort` in all new binaries. + +Use unwind in `debug` and abort in `release`. + +Make use of more exotic instructions like `bound`, allowing e.g. overflow or bound checking without branches. This comes at the expense of error output. + +# Unresolved questions +[unresolved]: #unresolved-questions + +None. From 61268059b2afab9ca82e685da970bba922f2d42a Mon Sep 17 00:00:00 2001 From: ticki Date: Wed, 5 Oct 2016 17:12:56 +0200 Subject: [PATCH 2/5] Remove all breaking changes and let Cargo handle the change entirely. --- text/0000-abort-by-default.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/text/0000-abort-by-default.md b/text/0000-abort-by-default.md index e091a111ad1..733c0ff2b19 100644 --- a/text/0000-abort-by-default.md +++ b/text/0000-abort-by-default.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -Use abort as the standard panic method rather than unwind. +Specify `panic=abort` in `Cargo.toml` when the user does `cargo new --bin`. # Motivation [motivation]: #motivation @@ -36,19 +36,17 @@ You often see people abusing `std::panic::catch_unwind` for exception handling. # Detailed design [design]: #detailed-design -Default to `panic=abort`. +Have `cargo` generate `panic=abort` to the `Cargo.toml`. Whenever the user inteacts with `cargo` (by using a command) and the `Cargo.toml` has no specified panicking strategy, it will add `panic=unwind` and leave a note to the user: -## Backwards compatibility + Warning: No panic strategy was specified, added unwinding to the `Cargo.toml` file, please consider change it to your needs. -No code should rely on a particular default panicking strategy, and code which do is already broken. However, this broken code might start actually encountering the issues. In a sense, that is actually positive, since it reveals that the code is broken. +For libraries, nothing is modified, as they inherit the strategy of the binary. -No invariants are broken, and no code is going to emit Undefined Behavior due to this change (this can be seen by observing that aborting is globally diverging, and hence no code is run after). +After several release cycles, an extension could be added, which makes specifying the strategy mandatory. # Drawbacks [drawbacks]: #drawbacks -While not breaking, the behavior of certain broken programs can change. - It makes panics global by default (i.e., panicking in some thread will abort the whole program). It might make it tempting to ignore the existence of an unwind option and consequently shaping the code after panicking being aborting. From b7653fa6811ca9c135e1a3108244830e8b6c5e8a Mon Sep 17 00:00:00 2001 From: ticki Date: Wed, 5 Oct 2016 17:16:17 +0200 Subject: [PATCH 3/5] Add advantages of unwinding. --- text/0000-abort-by-default.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/text/0000-abort-by-default.md b/text/0000-abort-by-default.md index 733c0ff2b19..58b8f874a2f 100644 --- a/text/0000-abort-by-default.md +++ b/text/0000-abort-by-default.md @@ -51,6 +51,12 @@ It makes panics global by default (i.e., panicking in some thread will abort the It might make it tempting to ignore the existence of an unwind option and consequently shaping the code after panicking being aborting. +## Unwinding is not bad per se + +Unwinding has numerous advantages. Especially for certain classes of applications. These includes better error handling and better cleanup. + +Unwinding is especially important to long-running applications. + # Alternatives [alternatives]: #alternatives From 624150b1b9842da95e5c49206b34e9bb27ba5b7c Mon Sep 17 00:00:00 2001 From: ticki Date: Wed, 5 Oct 2016 17:38:51 +0200 Subject: [PATCH 4/5] Produce an error with incompatible runtimes. --- text/0000-abort-by-default.md | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/text/0000-abort-by-default.md b/text/0000-abort-by-default.md index 58b8f874a2f..bd69a4e209e 100644 --- a/text/0000-abort-by-default.md +++ b/text/0000-abort-by-default.md @@ -36,11 +36,21 @@ You often see people abusing `std::panic::catch_unwind` for exception handling. # Detailed design [design]: #detailed-design -Have `cargo` generate `panic=abort` to the `Cargo.toml`. Whenever the user inteacts with `cargo` (by using a command) and the `Cargo.toml` has no specified panicking strategy, it will add `panic=unwind` and leave a note to the user: +Have `cargo` generate `panic=abort` to the `Cargo.toml`. Whenever the user inteacts with `cargo` (by using a command) and the `Cargo.toml` has no specified panicking strategy, it will add `panic=unwind` and leave a note to the user: Warning: No panic strategy was specified, added unwinding to the `Cargo.toml` file, please consider change it to your needs. -For libraries, nothing is modified, as they inherit the strategy of the binary. +## Libraries + +For libraries, the `Cargo.toml` is not modified, as they inherit the strategy of the binary. + +### Relying on unwinding + +If a library specifies `panic=unwind`, it will stored in a rlib metadata field, `unwind_needed`. If this field does not match with the crate which is linking against the library, `rustc` will produce an error. + +This is done in order to make sure that applications can rely on unwinding without leading to unsafety when being linked against by an aborting runtime. + +## Extensions After several release cycles, an extension could be added, which makes specifying the strategy mandatory. From 162582d393c00877d1777944d8c33263db0468c8 Mon Sep 17 00:00:00 2001 From: ticki Date: Wed, 5 Oct 2016 18:24:31 +0200 Subject: [PATCH 5/5] Add `any` attribute to smoothen the process for new-beginners --- text/0000-abort-by-default.md | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/text/0000-abort-by-default.md b/text/0000-abort-by-default.md index bd69a4e209e..4b96e50064d 100644 --- a/text/0000-abort-by-default.md +++ b/text/0000-abort-by-default.md @@ -6,7 +6,7 @@ # Summary [summary]: #summary -Specify `panic=abort` in `Cargo.toml` when the user does `cargo new --bin`. +Specify abort-by-default in `Cargo.toml` when the user does `cargo new --bin`, as well as various other refinements to the panick strategy system. # Motivation [motivation]: #motivation @@ -36,9 +36,22 @@ You often see people abusing `std::panic::catch_unwind` for exception handling. # Detailed design [design]: #detailed-design -Have `cargo` generate `panic=abort` to the `Cargo.toml`. Whenever the user inteacts with `cargo` (by using a command) and the `Cargo.toml` has no specified panicking strategy, it will add `panic=unwind` and leave a note to the user: +First of all, we add a new possible value to the `panic` field. We call this `any`. It specifies that the library or binary is compatible with any panicking strategy. `any` will default to the `abort` strategy, if compatible with all of its dependencies. If not, it will fall back to `unwind`, and leave a warning. - Warning: No panic strategy was specified, added unwinding to the `Cargo.toml` file, please consider change it to your needs. +When `cargo new` is invoked, it will generate `panic=any` in the `Cargo.toml`, both for new libraries and new binaries. + +Whenever the user inteacts with `cargo` (by using a command) in an existing (binary) crate and the `Cargo.toml` has no specified panicking strategy, it will add `panic=unwind` and leave a note to the user: + + Warning: No panic strategy was specified, added unwinding to the + `Cargo.toml` file, please consider change it to your needs. If + your crate's behavior does not depend on unwinding, please add + `panic=any` instead. + +This will not happen to libraries, since they must only rely on unwind, if the specify it. Instead, it will add `panic=any` to libraries and give warning: + + Warning: No panic strategy was specified, so we default to aborting. If + your crate depends on unwinding, please put `panic=unwind` in + `Cargo.toml`. ## Libraries @@ -46,7 +59,7 @@ For libraries, the `Cargo.toml` is not modified, as they inherit the strategy of ### Relying on unwinding -If a library specifies `panic=unwind`, it will stored in a rlib metadata field, `unwind_needed`. If this field does not match with the crate which is linking against the library, `rustc` will produce an error. +If a library specifies `panic=unwind`, it will stored in a rlib metadata field, `unwind_needed`. If this field does not match with the crate which is linking against the library (`abort`), `rustc` will produce an error. This is done in order to make sure that applications can rely on unwinding without leading to unsafety when being linked against by an aborting runtime. @@ -78,7 +91,9 @@ Use unwind in `debug` and abort in `release`. Make use of more exotic instructions like `bound`, allowing e.g. overflow or bound checking without branches. This comes at the expense of error output. +Use crate attributes instead. + # Unresolved questions [unresolved]: #unresolved-questions -None. +Is there any way we can detect crates which do not rely on unwinding? Search for `catch_unwind`?