Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/attributes.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question [GUARANTEE 1/3]: is this intended to be a hint, or a guarantee?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question:

This option enables the -fno-jump-tables flag for LLVM, which makes the codegen backend avoid generating jump tables when lowering switches.

This option adds the LLVM no-jump-tables=true attribute to every function.

The option can be used to help provide protection against jump-oriented-programming (JOP) attacks, such as with the linux kernel's IBT.

How does this interact with pre-compiled std? I.e. can you mix downstream user crates compiled with -Cjump-tables=no versus a pre-compiled std compiled and distributed with -Cjump-tables=yes?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there objections renaming -Zno-jump-tables to -Zjump-tables=?

Not having double-negatives is very nice 👍

Is it desirable to keep -Zno-jump-tables for a period of time?

No, I rather not have to keep around an unstable flag as an alias to a stable flag, that's very weird.

Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ fn instrument_function_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> SmallVec<[&'ll Attr
}

fn nojumptables_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> Option<&'ll Attribute> {
if !cx.sess().opts.unstable_opts.no_jump_tables {
if cx.sess().opts.cg.jump_tables {
return None;
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ fn test_codegen_options_tracking_hash() {
tracked!(force_frame_pointers, FramePointer::Always);
tracked!(force_unwind_tables, Some(true));
tracked!(instrument_coverage, InstrumentCoverage::Yes);
tracked!(jump_tables, false);
tracked!(link_dead_code, Some(true));
tracked!(linker_plugin_lto, LinkerPluginLto::LinkerPluginAuto);
tracked!(llvm_args, vec![String::from("1"), String::from("2")]);
Expand Down Expand Up @@ -830,7 +831,6 @@ fn test_unstable_options_tracking_hash() {
tracked!(mutable_noalias, false);
tracked!(next_solver, NextSolverConfig { coherence: true, globally: true });
tracked!(no_generate_arange_section, true);
tracked!(no_jump_tables, true);
tracked!(no_link, true);
tracked!(no_profiler_runtime, true);
tracked!(no_trait_vptr, true);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2022,6 +2022,8 @@ options! {
"instrument the generated code to support LLVM source-based code coverage reports \
(note, the compiler build config must include `profiler = true`); \
implies `-C symbol-mangling-version=v0`"),
jump_tables: bool = (true, parse_bool, [TRACKED],
"Allow jump table and lookup table generation from switch case lowering (default: yes)"),
link_arg: (/* redirected to link_args */) = ((), parse_string_push, [UNTRACKED],
"a single extra argument to append to the linker invocation (can be used several times)"),
link_args: Vec<String> = (Vec::new(), parse_list, [UNTRACKED],
Expand Down Expand Up @@ -2404,8 +2406,6 @@ options! {
"omit DWARF address ranges that give faster lookups"),
no_implied_bounds_compat: bool = (false, parse_bool, [TRACKED],
"disable the compatibility version of the `implied_bounds_ty` query"),
no_jump_tables: bool = (false, parse_no_value, [TRACKED],
"disable the jump tables and lookup tables that can be generated from a switch case lowering"),
no_leak_check: bool = (false, parse_no_value, [UNTRACKED],
"disable the 'leak check' for subtyping; unsound, but useful for tests"),
no_link: bool = (false, parse_no_value, [TRACKED],
Expand Down
13 changes: 13 additions & 0 deletions src/doc/rustc/src/codegen-options/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,19 @@ Note that while the `-C instrument-coverage` option is stable, the profile data
format produced by the resulting instrumentation may change, and may not work
with coverage tools other than those built and shipped with the compiler.

## jump-tables

This option is used to allow or prevent the codegen backend from creating
jump tables when lowering switches.
Comment on lines +214 to +215
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion: Hm, what happens if a different cg backend is selected?

cc @GuillaumeGomez @antoyo

Comment on lines +214 to +215
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question [GUARANTEE 2/3]: This wording reads like a guarantee -- but is it? Can we make that guarantee?


* `y`, `yes`, `on`, `true` or no value: allow jump tables (the default).
* `n`, `no`, `off` or `false`: disable jump tables.

Disabling jump tables can be used to help provide protection against
jump-oriented-programming (JOP) attacks, such as with the linux kernel's [IBT].
Comment on lines +220 to +221
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussion [GUARANTEE 3/3]: If the flag is intended to be a hint, then this sentence can be a bit misleading, because we may not always guarantee it. We may want to slightly caveat this wording to not convey a "false promise" so to speak.

Or, if a user do want such protection, then do they need to enforce it over the whole crate graph?


[IBT]: https://www.phoronix.com/news/Linux-IBT-By-Default-Tip

## link-arg

This flag lets you append a single extra argument to the linker invocation.
Expand Down
19 changes: 0 additions & 19 deletions src/doc/unstable-book/src/compiler-flags/no-jump-tables.md

This file was deleted.

4 changes: 2 additions & 2 deletions tests/assembly-llvm/x86_64-no-jump-tables.rs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: I'm assuming that if you inspect the assembly of an actual hello world binary that uses std in some way, then you might see jump stables still? 🤔

Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// Test that jump tables are (not) emitted when the `-Zno-jump-tables`
// Test that jump tables are (not) emitted when the `-Cjump-tables=no`
// flag is (not) set.

//@ revisions: unset set
//@ assembly-output: emit-asm
//@ compile-flags: -Copt-level=3
//@ [set] compile-flags: -Zno-jump-tables
//@ [set] compile-flags: -Cjump-tables=no
//@ only-x86_64
//@ ignore-sgx

Expand Down
10 changes: 6 additions & 4 deletions tests/codegen-llvm/no-jump-tables.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// Test that the `no-jump-tables` function attribute are (not) emitted when
// the `-Zno-jump-tables` flag is (not) set.
// the `-Cjump-tables=no` flag is (not) set.

//@ add-core-stubs
//@ revisions: unset set
//@ revisions: unset set_no set_yes
//@ needs-llvm-components: x86
//@ compile-flags: --target x86_64-unknown-linux-gnu
//@ [set] compile-flags: -Zno-jump-tables
//@ [set_no] compile-flags: -Cjump-tables=no
//@ [set_yes] compile-flags: -Cjump-tables=yes

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

// unset-NOT: attributes #0 = { {{.*}}"no-jump-tables"="true"{{.*}} }
// set: attributes #0 = { {{.*}}"no-jump-tables"="true"{{.*}} }
// set_yes-NOT: attributes #0 = { {{.*}}"no-jump-tables"="true"{{.*}} }
// set_no: attributes #0 = { {{.*}}"no-jump-tables"="true"{{.*}} }
}
Loading