Stream: git-wasmtime

Topic: wasmtime / PR #2349 Fix off-by-one error looking up frame...


view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2020 at 15:27):

uweigand opened PR #2349 from fix-frameinfo to main:

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.

Instead, it seems we need to look up the range starting at pc + 1.
In addition, the *end < pc check also look incorrect: in the case
of *end == pc, the PC value is actually outside of the function
that was found, and we should therefore return failure.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 02 2020 at 16:07):

uweigand updated PR #2349 from fix-frameinfo to main:

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.

Instead, it seems we need to look up the range starting at pc + 1.
In addition, the *end < pc check also look incorrect: in the case
of *end == pc, the PC value is actually outside of the function
that was found, and we should therefore return failure.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2020 at 09:51):

uweigand updated PR #2349 from fix-frameinfo to main:

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.

Instead, it seems we need to look up the range starting at pc + 1.
In addition, the *end < pc check also look incorrect: in the case
of *end == pc, the PC value is actually outside of the function
that was found, and we should therefore return failure.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Nov 03 2020 at 19:54):

alexcrichton merged PR #2349.


Last updated: Dec 23 2024 at 12:05 UTC