Fix off-by-one error looking up frame info for a function (#2349)

The ModuleFrameInfo and FunctionInfo data structures maintain
a list of ranges via a BTreeMap.  The key to that map is one
past the end of the module/function in question.  This causes
a problem in the case of immediately adjacent ranges.  For
example, if we have two functions occupying adjacent ranges:
  A:   0-100
  B: 100-200
function A is stored with a key of 100 and B with a key of 200.

Now, when looking up the function associated with address 100,
we'd expect to find B.  However the current code:

       let (end, func) = info.functions.range(pc..).next()?;
       if pc < func.start || *end < pc {

will look up the value 100 in the map and return function A,
which will then fail the pc < func.start check in the next
line, so the result will be failure.

To fix this problem, make sure that the key used when
registering functions or modules is the address of the
last byte, not one past the end.
This commit is contained in:
Ulrich Weigand
2020-11-03 20:54:27 +01:00
committed by GitHub
parent c210f4d6a6
commit fa9c2a5172

View File

@@ -145,8 +145,13 @@ pub fn register(module: &CompiledModule) -> Option<GlobalFrameInfoRegistration>
let (start, end) = unsafe {
let ptr = (*allocated).as_ptr();
let len = (*allocated).len();
(ptr as usize, ptr as usize + len)
// First and last byte of the function text.
(ptr as usize, ptr as usize + len - 1)
};
// Skip empty functions.
if end < start {
continue;
}
min = cmp::min(min, start);
max = cmp::max(max, end);
let func = FunctionInfo {
@@ -260,3 +265,38 @@ impl FrameInfo {
(self.instr.bits() - self.func_start.bits()) as usize
}
}
#[test]
fn test_frame_info() -> Result<(), anyhow::Error> {
use crate::*;
let store = Store::default();
let module = Module::new(
store.engine(),
r#"
(module
(func (export "add") (param $x i32) (param $y i32) (result i32) (i32.add (local.get $x) (local.get $y)))
(func (export "sub") (param $x i32) (param $y i32) (result i32) (i32.sub (local.get $x) (local.get $y)))
(func (export "mul") (param $x i32) (param $y i32) (result i32) (i32.mul (local.get $x) (local.get $y)))
(func (export "div_s") (param $x i32) (param $y i32) (result i32) (i32.div_s (local.get $x) (local.get $y)))
(func (export "div_u") (param $x i32) (param $y i32) (result i32) (i32.div_u (local.get $x) (local.get $y)))
(func (export "rem_s") (param $x i32) (param $y i32) (result i32) (i32.rem_s (local.get $x) (local.get $y)))
(func (export "rem_u") (param $x i32) (param $y i32) (result i32) (i32.rem_u (local.get $x) (local.get $y)))
)
"#,
)?;
// Create an instance to ensure the frame information is registered.
let _ = Instance::new(&store, &module, &[])?;
let info = FRAME_INFO.read().unwrap();
for (i, alloc) in module.compiled_module().finished_functions() {
let (start, end) = unsafe {
let ptr = (**alloc).as_ptr();
let len = (**alloc).len();
(ptr as usize, ptr as usize + len)
};
for pc in start..end {
let frame = info.lookup_frame_info(pc).unwrap();
assert!(frame.func_index() == i.as_u32());
}
}
Ok(())
}