Skip to content

Commit 884b714

Browse files
committed
Fix parsing named locals without an inline function type
This commit fixes a bug in `wast` where the generated `name` section was invalid and placed local names on the wrong location. This happened when a function type was not listed inline for a function (e.g. it was `(type $foo)`) meaning that the binary translation started with the wrong local offset. The fix here is to do a lookup of the function type to see how many parameters it has and start the indexing there, or skip emitting local names if the function type doesn't exist.
1 parent 01fc821 commit 884b714

File tree

7 files changed

+110
-65
lines changed

7 files changed

+110
-65
lines changed

crates/wast/src/core/binary.rs

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ pub(crate) fn encode(
177177
// Prepare to and emit the code section. This is where DWARF may optionally
178178
// be emitted depending on configuration settings. Note that `code_section`
179179
// will internally emit the branch hints section if necessary.
180-
let names = find_names(module_id, module_name, fields);
180+
let names = find_names(module_id, module_name, fields, &types);
181181
let num_import_funcs = imports
182182
.iter()
183183
.filter(|i| matches!(i.item.kind, ItemKind::Func(..)))
@@ -209,6 +209,23 @@ pub(crate) fn encode(
209209
}
210210
}
211211

212+
fn func_type<'a>(types: &'a [RecOrType<'a>], ty: u32) -> Option<&'a FunctionType<'a>> {
213+
// Iterate through `self.types` which is what was encoded into the
214+
// module and find the function type which gives access to the
215+
// parameters which gives access to their types.
216+
let ty = types
217+
.iter()
218+
.flat_map(|t| match t {
219+
RecOrType::Type(t) => std::slice::from_ref(*t),
220+
RecOrType::Rec(r) => &r.types,
221+
})
222+
.nth(ty as usize)?;
223+
match &ty.def.kind {
224+
InnerTypeKind::Func(ty) => Some(ty),
225+
_ => None,
226+
}
227+
}
228+
212229
struct Encoder<'a> {
213230
wasm: wasm_encoder::Module,
214231
customs: &'a [&'a Custom<'a>],
@@ -732,7 +749,9 @@ impl Func<'_> {
732749
) -> Vec<wasm_encoder::BranchHint> {
733750
assert!(self.exports.names.is_empty());
734751
let (expr, locals) = match &self.kind {
735-
FuncKind::Inline { expression, locals } => (expression, locals),
752+
FuncKind::Inline {
753+
expression, locals, ..
754+
} => (expression, locals),
736755
_ => panic!("should only have inline functions in emission"),
737756
};
738757

@@ -976,6 +995,7 @@ fn find_names<'a>(
976995
module_id: &Option<Id<'a>>,
977996
module_name: &Option<NameAnnotation<'a>>,
978997
fields: &[ModuleField<'a>],
998+
types: &'a [RecOrType<'a>],
979999
) -> Names<'a> {
9801000
fn get_name<'a>(id: &Option<Id<'a>>, name: &Option<NameAnnotation<'a>>) -> Option<&'a str> {
9811001
name.as_ref().map(|n| n.name).or(id.and_then(|id| {
@@ -1056,19 +1076,40 @@ fn find_names<'a>(
10561076
let mut label_names = Vec::new();
10571077
let mut local_idx = 0;
10581078
let mut label_idx = 0;
1079+
let mut discard_locals = false;
10591080

1060-
// Consult the inline type listed for local names of parameters.
1061-
// This is specifically preserved during the name resolution
1062-
// pass, but only for functions, so here we can look at the
1063-
// original source's names.
10641081
if let Some(ty) = &f.ty.inline {
1082+
// Consult the inline type listed for local names of parameters.
1083+
// This is specifically preserved during the name resolution
1084+
// pass, but only for functions, so here we can look at the
1085+
// original source's names.
10651086
for (id, name, _) in ty.params.iter() {
10661087
if let Some(name) = get_name(id, name) {
10671088
local_names.push((local_idx, name));
10681089
}
10691090
local_idx += 1;
10701091
}
1092+
} else {
1093+
// If the inline type isn't listed then it's either not present
1094+
// (e.g. no params or results) or it was referenced by index.
1095+
// Either way we've got the index here, so look it up in the
1096+
// list of types and see how many parameters this function's
1097+
// type has.
1098+
let index = match f.ty.index.as_ref().unwrap() {
1099+
Index::Num(n, _) => *n,
1100+
_ => unreachable!(),
1101+
};
1102+
1103+
match func_type(types, index) {
1104+
Some(ft) => local_idx = ft.params.len() as u32,
1105+
// If the function type index is invalid then skip
1106+
// preserving names since we don't know how many parameters
1107+
// this function will have so we don't know where to start
1108+
// indexing at.
1109+
None => discard_locals = true,
1110+
}
10711111
}
1112+
10721113
if let FuncKind::Inline {
10731114
locals, expression, ..
10741115
} = &f.kind
@@ -1096,7 +1137,7 @@ fn find_names<'a>(
10961137
}
10971138
}
10981139
}
1099-
if local_names.len() > 0 {
1140+
if !discard_locals && local_names.len() > 0 {
11001141
ret.locals.push((*idx, local_names));
11011142
}
11021143
if label_names.len() > 0 {

crates/wast/src/core/binary/dwarf.rs

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
//! easy/fun to play around with.
1212
1313
use crate::core::binary::{EncodeOptions, Encoder, GenerateDwarf, Names, RecOrType};
14-
use crate::core::{InnerTypeKind, Local, ValType};
14+
use crate::core::{Local, ValType};
1515
use crate::token::Span;
1616
use gimli::write::{
1717
self, Address, AttributeValue, DwarfUnit, Expression, FileId, LineProgram, LineString,
@@ -216,19 +216,8 @@ impl<'a> Dwarf<'a> {
216216
ty: u32,
217217
locals: &[Local<'_>],
218218
) {
219-
// Iterate through `self.types` which is what was encoded into the
220-
// module and find the function type which gives access to the
221-
// parameters which gives access to their types.
222-
let ty = self
223-
.types
224-
.iter()
225-
.flat_map(|t| match t {
226-
RecOrType::Type(t) => std::slice::from_ref(*t),
227-
RecOrType::Rec(r) => &r.types,
228-
})
229-
.nth(ty as usize);
230-
let ty = match ty.map(|t| &t.def.kind) {
231-
Some(InnerTypeKind::Func(ty)) => ty,
219+
let ty = match super::func_type(self.types, ty) {
220+
Some(ty) => ty,
232221
_ => return,
233222
};
234223

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
;; RUN: print %
2+
3+
(module
4+
(type $a (func (param i32)))
5+
6+
(func (type $a)
7+
(local $b i32)
8+
)
9+
)
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
(module
2+
(type $a (;0;) (func (param i32)))
3+
(func (;0;) (type $a) (param i32)
4+
(local $b i32)
5+
)
6+
)

tests/snapshots/cli/stack-switching/cont.wast/47.print

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,30 +21,30 @@
2121
i32.const 42
2222
return
2323
)
24-
(func $f (;2;) (type $ft) (param $nextk (ref null $ct))
25-
(local (ref null $ct))
26-
local.get $nextk
27-
local.set 1
24+
(func $f (;2;) (type $ft) (param (ref null $ct))
25+
(local $nextk (ref null $ct))
26+
local.get 0
27+
local.set $nextk
2828
global.get $fi
2929
call $print-i32
30-
local.get 1
30+
local.get $nextk
3131
switch $ct $swap
32-
local.set 1
32+
local.set $nextk
3333
global.get $fi
3434
call $print-i32
35-
local.get 1
35+
local.get $nextk
3636
switch $ct $swap
3737
drop
3838
)
39-
(func $g (;3;) (type $ft) (param $nextk (ref null $ct))
40-
(local (ref null $ct))
41-
local.get $nextk
42-
local.set 1
39+
(func $g (;3;) (type $ft) (param (ref null $ct))
40+
(local $nextk (ref null $ct))
41+
local.get 0
42+
local.set $nextk
4343
global.get $gi
4444
call $print-i32
45-
local.get 1
45+
local.get $nextk
4646
switch $ct $swap
47-
local.set 1
47+
local.set $nextk
4848
global.get $gi
4949
call $print-i32
5050
)

tests/snapshots/cli/stack-switching/cont.wast/49.print

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,48 +17,48 @@
1717
cont.new $ct
1818
resume $ct (on $swap switch)
1919
)
20-
(func $f (;2;) (type $ft) (param $i i32) (param $nextk (ref null $ct)) (result i32)
21-
(local i32 (ref null $ct))
20+
(func $f (;2;) (type $ft) (param i32 (ref null $ct)) (result i32)
21+
(local $i i32) (local $nextk (ref null $ct))
22+
local.get 0
23+
local.set $i
24+
local.get 1
25+
local.set $nextk
2226
local.get $i
23-
local.set 2
24-
local.get $nextk
25-
local.set 3
26-
local.get 2
2727
call $print-i32
2828
i32.const 1
29-
local.get 2
29+
local.get $i
3030
i32.add
31-
local.get 3
31+
local.get $nextk
3232
switch $ct $swap
33-
local.set 3
34-
local.set 2
35-
local.get 2
33+
local.set $nextk
34+
local.set $i
35+
local.get $i
3636
call $print-i32
3737
i32.const 1
38-
local.get 2
38+
local.get $i
3939
i32.add
40-
local.get 3
40+
local.get $nextk
4141
switch $ct $swap
4242
unreachable
4343
)
44-
(func $g (;3;) (type $ft) (param $i i32) (param $nextk (ref null $ct)) (result i32)
45-
(local i32 (ref null $ct))
44+
(func $g (;3;) (type $ft) (param i32 (ref null $ct)) (result i32)
45+
(local $i i32) (local $nextk (ref null $ct))
46+
local.get 0
47+
local.set $i
48+
local.get 1
49+
local.set $nextk
4650
local.get $i
47-
local.set 2
48-
local.get $nextk
49-
local.set 3
50-
local.get 2
5151
call $print-i32
5252
i32.const 1
53-
local.get 2
53+
local.get $i
5454
i32.add
55-
local.get 3
55+
local.get $nextk
5656
switch $ct $swap
57-
local.set 3
58-
local.set 2
59-
local.get 2
57+
local.set $nextk
58+
local.set $i
59+
local.get $i
6060
call $print-i32
61-
local.get 2
61+
local.get $i
6262
return
6363
)
6464
)

tests/snapshots/testsuite/func.wast/93.print

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44
(export "f" (func 0))
55
(export "g" (func 2))
66
(export "p" (func 3))
7-
(func (;0;) (type $sig) (param $var i32) (result i32)
8-
(local i32)
9-
local.get 1
7+
(func (;0;) (type $sig) (param i32) (result i32)
8+
(local $var i32)
9+
local.get $var
1010
)
11-
(func $g (;1;) (type $sig) (param $var i32) (result i32)
12-
(local i32)
13-
local.get 1
11+
(func $g (;1;) (type $sig) (param i32) (result i32)
12+
(local $var i32)
13+
local.get $var
1414
)
1515
(func (;2;) (type $sig) (param i32) (result i32)
1616
local.get 0

0 commit comments

Comments
 (0)