Skip to content

Commit 97d9699

Browse files
committed
Add a compiler option to force jump veneers
Only intended to be used during testing/debugging. This also hooks up the option to fuzzers.
1 parent d33a94d commit 97d9699

File tree

6 files changed

+62
-45
lines changed

6 files changed

+62
-45
lines changed

cranelift/codegen/src/isa/aarch64/inst/emit.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@ pub fn memlabel_finalize(_insn_off: CodeOffset, label: &MemLabel) -> i32 {
2525
///
2626
/// This generates:
2727
///
28-
/// ldr x16, 16
29-
/// adr x17, 12
30-
/// add x16, x16, x17
31-
/// br x16
28+
/// ```ignore
29+
/// ldr x16, 16
30+
/// adr x17, 12
31+
/// add x16, x16, x17
32+
/// br x16
33+
/// ```
3234
///
3335
/// and the expectation is that the 8-byte immediate address to jump to is
3436
/// located after these instructions are encoded.

crates/cranelift/src/builder.rs

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,20 @@ use wasmtime_environ::{CompilerBuilder, Setting, SettingKind};
1313
struct Builder {
1414
flags: settings::Builder,
1515
isa_flags: isa::Builder,
16+
linkopts: LinkOptions,
17+
}
1618

17-
// A debug-only setting used to synthetically insert 0-byte padding between
18-
// compiled functions to simulate huge compiled artifacts and exercise logic
19-
// related to jump veneers.
20-
padding_between_functions: usize,
19+
#[derive(Clone, Default)]
20+
pub struct LinkOptions {
21+
/// A debug-only setting used to synthetically insert 0-byte padding between
22+
/// compiled functions to simulate huge compiled artifacts and exercise
23+
/// logic related to jump veneers.
24+
pub padding_between_functions: usize,
25+
26+
/// A debug-only setting used to force inter-function calls in a wasm module
27+
/// to always go through "jump veneers" which are typically only generated
28+
/// when functions are very far from each other.
29+
pub force_jump_veneers: bool,
2130
}
2231

2332
pub fn builder() -> Box<dyn CompilerBuilder> {
@@ -37,7 +46,7 @@ pub fn builder() -> Box<dyn CompilerBuilder> {
3746
Box::new(Builder {
3847
flags,
3948
isa_flags: cranelift_native::builder().expect("host machine is not a supported target"),
40-
padding_between_functions: 0,
49+
linkopts: LinkOptions::default(),
4150
})
4251
}
4352

@@ -56,12 +65,17 @@ impl CompilerBuilder for Builder {
5665
}
5766

5867
fn set(&mut self, name: &str, value: &str) -> Result<()> {
59-
// Special wasmtime-cranelift-only setting.
60-
if name == "padding_between_functions" {
61-
self.padding_between_functions = value.parse()?;
68+
// Special wasmtime-cranelift-only settings first
69+
if name == "linkopt_padding_between_functions" {
70+
self.linkopts.padding_between_functions = value.parse()?;
71+
return Ok(());
72+
}
73+
if name == "linkopt_force_jump_veneer" {
74+
self.linkopts.force_jump_veneers = value == "true";
6275
return Ok(());
6376
}
6477

78+
// ... then forward this to Cranelift
6579
if let Err(err) = self.flags.set(name, value) {
6680
match err {
6781
SetError::BadName(_) => {
@@ -92,10 +106,7 @@ impl CompilerBuilder for Builder {
92106
.isa_flags
93107
.clone()
94108
.finish(settings::Flags::new(self.flags.clone()));
95-
Box::new(crate::compiler::Compiler::new(
96-
isa,
97-
self.padding_between_functions,
98-
))
109+
Box::new(crate::compiler::Compiler::new(isa, self.linkopts.clone()))
99110
}
100111

101112
fn settings(&self) -> Vec<Setting> {

crates/cranelift/src/compiler.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::builder::LinkOptions;
12
use crate::debug::ModuleMemoryOffset;
23
use crate::func_environ::{get_func_name, FuncEnvironment};
34
use crate::obj::ObjectBuilder;
@@ -36,15 +37,15 @@ use wasmtime_environ::{
3637
pub(crate) struct Compiler {
3738
translators: Mutex<Vec<FuncTranslator>>,
3839
isa: Box<dyn TargetIsa>,
39-
padding_between_functions: usize,
40+
linkopts: LinkOptions,
4041
}
4142

4243
impl Compiler {
43-
pub(crate) fn new(isa: Box<dyn TargetIsa>, padding_between_functions: usize) -> Compiler {
44+
pub(crate) fn new(isa: Box<dyn TargetIsa>, linkopts: LinkOptions) -> Compiler {
4445
Compiler {
4546
translators: Default::default(),
4647
isa,
47-
padding_between_functions,
48+
linkopts,
4849
}
4950
}
5051

@@ -254,11 +255,12 @@ impl wasmtime_environ::Compiler for Compiler {
254255
}
255256

256257
let mut builder = ObjectBuilder::new(obj, &translation.module, &*self.isa);
258+
builder.force_jump_veneers = self.linkopts.force_jump_veneers;
257259

258260
for ((i, func), (_, body)) in funcs.iter().zip(bodies) {
259261
builder.func(i, func, body);
260-
if self.padding_between_functions > 0 {
261-
builder.append_synthetic_padding(self.padding_between_functions);
262+
if self.linkopts.padding_between_functions > 0 {
263+
builder.append_synthetic_padding(self.linkopts.padding_between_functions);
262264
}
263265
}
264266
for (i, (body, func)) in trampolines.iter_mut() {

crates/cranelift/src/obj.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@ pub struct ObjectBuilder<'a> {
162162
/// call instruction. These relocations will get filled in at the end
163163
/// with relative offsets to their target.
164164
relative_relocs: Vec<RelativeReloc>,
165+
166+
/// A debug-only option to indicate that all inter-function calls should go
167+
/// through veneers, ideally testing the veneer emission code.
168+
pub force_jump_veneers: bool,
165169
}
166170

167171
/// A pending relocation against a wasm-defined function that needs to be
@@ -255,6 +259,7 @@ impl<'a> ObjectBuilder<'a> {
255259
current_text_off: 0,
256260
text_locations: HashMap::new(),
257261
relative_relocs: Vec::new(),
262+
force_jump_veneers: false,
258263
}
259264
}
260265

@@ -475,7 +480,7 @@ impl<'a> ObjectBuilder<'a> {
475480
// added for a symbol but the symbol was defined very long
476481
// ago. The only way to reach the symbol at this point
477482
// is via a veneer.
478-
if distance < min_neg {
483+
if distance < min_neg || self.force_jump_veneers {
479484
self.emit_jump_veneer(r);
480485

481486
// This case, if hit, represents a programming error. If

crates/fuzzing/src/generators.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use arbitrary::{Arbitrary, Unstructured};
2020
pub struct DifferentialConfig {
2121
strategy: DifferentialStrategy,
2222
opt_level: OptLevel,
23+
force_jump_veneers: bool,
2324
}
2425

2526
impl DifferentialConfig {
@@ -30,6 +31,11 @@ impl DifferentialConfig {
3031
DifferentialStrategy::Lightbeam => wasmtime::Strategy::Lightbeam,
3132
})?;
3233
config.cranelift_opt_level(self.opt_level.to_wasmtime());
34+
if self.force_jump_veneers {
35+
unsafe {
36+
config.cranelift_flag_set("linkopt_force_jump_veneer", "true")?;
37+
}
38+
}
3339
Ok(config)
3440
}
3541
}

tests/all/relocs.rs

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ fn store_with_padding(padding: usize) -> Result<Store<()>> {
1414
// This is an internal debug-only setting specifically recognized for
1515
// basically just this set of tests.
1616
unsafe {
17-
config.cranelift_flag_set("padding_between_functions", &padding.to_string())?;
17+
config.cranelift_flag_set("linkopt_padding_between_functions", &padding.to_string())?;
1818
}
1919
let engine = Engine::new(&config)?;
2020
Ok(Store::new(&engine, ()))
@@ -64,8 +64,20 @@ fn backwards_call_works() -> Result<()> {
6464

6565
#[test]
6666
fn mixed() -> Result<()> {
67-
let mut store = store_with_padding(MB)?;
67+
test_many_call_module(store_with_padding(MB)?)
68+
}
69+
70+
#[test]
71+
fn mixed_forced() -> Result<()> {
72+
let mut config = Config::new();
73+
unsafe {
74+
config.cranelift_flag_set("linkopt_force_jump_veneer", &padding.to_string())?;
75+
}
76+
let engine = Engine::new(&config)?;
77+
test_many_call_module(Store::new(&engine, ()))
78+
}
6879

80+
fn test_many_call_module(store: Store<()>) -> Result<()> {
6981
const N: i32 = 200;
7082

7183
let mut wat = String::new();
@@ -96,24 +108,3 @@ fn mixed() -> Result<()> {
96108
}
97109
Ok(())
98110
}
99-
100-
#[test]
101-
fn forward_huge() -> Result<()> {
102-
let mut store = store_with_padding(2 * GB)?;
103-
let module = Module::new(
104-
store.engine(),
105-
r#"
106-
(module
107-
(func (export "foo") (result i32)
108-
call 1)
109-
(func (result i32)
110-
i32.const 4)
111-
)
112-
"#,
113-
)?;
114-
115-
let i = Instance::new(&mut store, &module, &[])?;
116-
let foo = i.get_typed_func::<(), i32, _>(&mut store, "foo")?;
117-
assert_eq!(foo.call(&mut store, ())?, 4);
118-
Ok(())
119-
}

0 commit comments

Comments
 (0)