Skip to content

Commit b618119

Browse files
authored
Rollup merge of #145974 - pmur:murp/stabilize-zno-jump-tables, r=wesleywiser
Stabilize -Zno-jump-tables into -Cjump-tables=bool I propose stabilizing the -Zno-jump-tables option into -Cjump-tables=<bool>. # `-Zno-jump-tables` stabilization report ## What is the RFC for this feature and what changes have occurred to the user-facing design since the RFC was finalized? No RFC was created for this option. This was a narrowly scoped option introduced in #105812 to support code generation requirements of the x86-64 linux kernel, and eventually other targets as Rust For Linux grows. The tracking is #116592. ## What behavior are we committing to that has been controversial? Summarize the major arguments pro/con. The behavior of this flag is well defined, and mimics the existing `-fno-jump-tables` option currently available with LLVM and GCC with some caveats: * Unlike clang or gcc, this option may be ignored by the code generation backend. Rust can support multiple code-generation backends. For stabilization, only the LLVM backend honors this option. * The usage of this option will not guarantee a library or binary is free of jump tables. To ensure a jump-table free binary, all crates in the build graph must be compiled with this option. This includes implicitly linked crates such as std or core. * This option only enforces the crate being compiled is free of jump tables. * No verification is done to ensure other crates are compiled with this option. Enforcing code generation options are applied across the crate graph is out of scope for this option. What should the flag name be? * As introduced, this option was named `-Zno-jump-tables`. However, other major toolchains allow both positive and negative variants of this option to toggle this feature. Renaming the option to `-Cjump-tables=<bool>` makes this option consistent, and if for some reason, expandable to other arguments in the future. Notably, many LLVM targets have a configurable and different thresholds for when to lower into a jump table. ## Are there extensions to this feature that remain unstable? How do we know that we are not accidentally committing to those. No. This option is used exclusively to gate a very specific class of optimization. ## Summarize the major parts of the implementation and provide links into the code (or to PRs) * The original PR #105812 by ```@ojeda``` * The stabilized CLI option is parsed as a bool: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_session/src/options.rs#L2025-L2026 * This options adds an attribute to each llvm function via: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/compiler/rustc_codegen_llvm/src/attributes.rs#L210-L215 * Finally, the rustc book is updated with the new option: https://github.com/pmur/rust/blob/68bfda9025ccb2778e2606e12e8021b9918f40d3/src/doc/rustc/src/codegen-options/index.md?plain=1#L212-L223 ## Has a call-for-testing period been conducted? If so, what feedback was received? No. The option has originally created is being used by Rust For Linux to build the x86-64 kernel without issue. ## What outstanding bugs in the issue tracker involve this feature? Are they stabilization-blocking? There are no outstanding issues. ## Summarize contributors to the feature by name for recognition and assuredness that people involved in the feature agree with stabilization * ```@ojeda``` implemented this feature in #105815 as `-Zno-jump-tables`. * ```@tgross35``` created and maintained the tracking issue #116592, and provided feedback about the naming of the cli option. ## What FIXMEs are still in the code for that feature and why is it ok to leave them there? There are none. ## What static checks are done that are needed to prevent undefined behavior? This option cannot cause undefined behavior. It is a boolean option with well defined behavior in both cases. ## In what way does this feature interact with the reference/specification, and are those edits prepared? This adds a new cli option to `rustc`. The documentation is updated, and the unstable documentation cleaned up in this PR. ## Does this feature introduce new expressions and can they produce temporaries? What are the lifetimes of those temporaries? No. ## What other unstable features may be exposed by this feature? None. ## What is tooling support like for this feature, w.r.t rustdoc, clippy, rust-analzyer, rustfmt, etc.? No support is required from other rust tooling. ## Open Items - [x] Are there objections renaming `-Zno-jump-tables` to `-Cjump-tables=<bool>`? The consensus is no. - [x] Is it desirable to keep `-Zno-jump-tables` for a period of time? The consensus is no. --- Closes #116592
2 parents 22e4575 + fd11125 commit b618119

File tree

8 files changed

+35
-30
lines changed

8 files changed

+35
-30
lines changed

compiler/rustc_codegen_llvm/src/attributes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ fn instrument_function_attr<'ll>(
229229
}
230230

231231
fn nojumptables_attr<'ll>(cx: &SimpleCx<'ll>, sess: &Session) -> Option<&'ll Attribute> {
232-
if !sess.opts.unstable_opts.no_jump_tables {
232+
if sess.opts.cg.jump_tables {
233233
return None;
234234
}
235235

compiler/rustc_interface/src/tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,7 @@ fn test_codegen_options_tracking_hash() {
620620
tracked!(force_frame_pointers, FramePointer::Always);
621621
tracked!(force_unwind_tables, Some(true));
622622
tracked!(instrument_coverage, InstrumentCoverage::Yes);
623+
tracked!(jump_tables, false);
623624
tracked!(link_dead_code, Some(true));
624625
tracked!(linker_plugin_lto, LinkerPluginLto::LinkerPluginAuto);
625626
tracked!(llvm_args, vec![String::from("1"), String::from("2")]);
@@ -831,7 +832,6 @@ fn test_unstable_options_tracking_hash() {
831832
tracked!(mutable_noalias, false);
832833
tracked!(next_solver, NextSolverConfig { coherence: true, globally: true });
833834
tracked!(no_generate_arange_section, true);
834-
tracked!(no_jump_tables, true);
835835
tracked!(no_link, true);
836836
tracked!(no_profiler_runtime, true);
837837
tracked!(no_trait_vptr, true);

compiler/rustc_session/src/options.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2093,6 +2093,8 @@ options! {
20932093
"instrument the generated code to support LLVM source-based code coverage reports \
20942094
(note, the compiler build config must include `profiler = true`); \
20952095
implies `-C symbol-mangling-version=v0`"),
2096+
jump_tables: bool = (true, parse_bool, [TRACKED],
2097+
"allow jump table and lookup table generation from switch case lowering (default: yes)"),
20962098
link_arg: (/* redirected to link_args */) = ((), parse_string_push, [UNTRACKED],
20972099
"a single extra argument to append to the linker invocation (can be used several times)"),
20982100
link_args: Vec<String> = (Vec::new(), parse_list, [UNTRACKED],
@@ -2475,8 +2477,6 @@ options! {
24752477
"omit DWARF address ranges that give faster lookups"),
24762478
no_implied_bounds_compat: bool = (false, parse_bool, [TRACKED],
24772479
"disable the compatibility version of the `implied_bounds_ty` query"),
2478-
no_jump_tables: bool = (false, parse_no_value, [TRACKED],
2479-
"disable the jump tables and lookup tables that can be generated from a switch case lowering"),
24802480
no_leak_check: bool = (false, parse_no_value, [UNTRACKED],
24812481
"disable the 'leak check' for subtyping; unsound, but useful for tests"),
24822482
no_link: bool = (false, parse_no_value, [TRACKED],

src/ci/docker/scripts/rfl-build.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
set -euo pipefail
44

5-
LINUX_VERSION=v6.17-rc5
5+
# https://github.com/rust-lang/rust/pull/145974
6+
LINUX_VERSION=842cfd8e5aff3157cb25481b2900b49c188d628a
67

78
# Build rustc, rustdoc, cargo, clippy-driver and rustfmt
89
../x.py build --stage 2 library rustdoc clippy rustfmt

src/doc/rustc/src/codegen-options/index.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,27 @@ Note that while the `-C instrument-coverage` option is stable, the profile data
209209
format produced by the resulting instrumentation may change, and may not work
210210
with coverage tools other than those built and shipped with the compiler.
211211

212+
## jump-tables
213+
214+
This option is used to allow or prevent the LLVM codegen backend from creating
215+
jump tables when lowering switches from Rust code.
216+
217+
* `y`, `yes`, `on`, `true` or no value: allow jump tables (the default).
218+
* `n`, `no`, `off` or `false`: disable jump tables.
219+
220+
To prevent jump tables being created from Rust code, a target must ensure
221+
all crates are compiled with jump tables disabled.
222+
223+
Note, in many cases the Rust toolchain is distributed with precompiled
224+
crates, such as the core and std crates, which could possibly include
225+
jump tables. Furthermore, this option does not guarantee a target will
226+
be free of jump tables. They could arise from external dependencies,
227+
inline asm, or other complicated interactions when using crates which
228+
are compiled with jump table support.
229+
230+
Disabling jump tables can be used to help provide protection against
231+
jump-oriented-programming (JOP) attacks.
232+
212233
## link-arg
213234

214235
This flag lets you append a single extra argument to the linker invocation.

src/doc/unstable-book/src/compiler-flags/no-jump-tables.md

Lines changed: 0 additions & 19 deletions
This file was deleted.

tests/assembly-llvm/x86_64-no-jump-tables.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
// Test that jump tables are (not) emitted when the `-Zno-jump-tables`
1+
// Test that jump tables are (not) emitted when the `-Cjump-tables=no`
22
// flag is (not) set.
33

44
//@ revisions: unset set
55
//@ assembly-output: emit-asm
66
//@ compile-flags: -Copt-level=3
7-
//@ [set] compile-flags: -Zno-jump-tables
7+
//@ [set] compile-flags: -Cjump-tables=no
88
//@ only-x86_64
99
//@ ignore-sgx
1010

tests/codegen-llvm/no-jump-tables.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
// Test that the `no-jump-tables` function attribute are (not) emitted when
2-
// the `-Zno-jump-tables` flag is (not) set.
2+
// the `-Cjump-tables=no` flag is (not) set.
33

44
//@ add-minicore
5-
//@ revisions: unset set
5+
//@ revisions: unset set_no set_yes
66
//@ needs-llvm-components: x86
77
//@ compile-flags: --target x86_64-unknown-linux-gnu
8-
//@ [set] compile-flags: -Zno-jump-tables
8+
//@ [set_no] compile-flags: -Cjump-tables=no
9+
//@ [set_yes] compile-flags: -Cjump-tables=yes
910

1011
#![crate_type = "lib"]
1112
#![feature(no_core, lang_items)]
@@ -19,5 +20,6 @@ pub fn foo() {
1920
// CHECK: @foo() unnamed_addr #0
2021

2122
// unset-NOT: attributes #0 = { {{.*}}"no-jump-tables"="true"{{.*}} }
22-
// set: attributes #0 = { {{.*}}"no-jump-tables"="true"{{.*}} }
23+
// set_yes-NOT: attributes #0 = { {{.*}}"no-jump-tables"="true"{{.*}} }
24+
// set_no: attributes #0 = { {{.*}}"no-jump-tables"="true"{{.*}} }
2325
}

0 commit comments

Comments
 (0)