Stream: git-wasmtime

Topic: wasmtime / PR #2462 Debug info: two fixes in x64 backend.

Wasmtime GitHub notifications bot (Dec 02 2020 at 08:27):

cfallin edited PR #2462 from fix-sorted-debug-addr-transform to main:

• Sort by generated-code offset to maintain invariant and avoid gimli
  panic.

• Fix srcloc interaction with branch peephole optimization in
  MachBuffer: if a srcloc range overlaps with a branch that is
  truncated, remove that srcloc range.

These issues were found while fuzzing the new backend (#2453); I suspect
that they arise with the new backend because we can sink instructions
(e.g. loads or extends) in more interesting ways than before, but I'm
not entirely sure.

Test coverage will be via the fuzz corpus once #2453 lands.

Wasmtime GitHub notifications bot (Dec 02 2020 at 14:55):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Dec 02 2020 at 14:55):

alexcrichton submitted PR Review.

Wasmtime GitHub notifications bot (Dec 02 2020 at 14:55):

alexcrichton created PR Review Comment:

Out of curiosity, is there a speedier accesssor for non-sorted? If we do our own sort afterwards the sort here shouldn't matter too much.

Wasmtime GitHub notifications bot (Dec 02 2020 at 15:14):

yurydelendik submitted PR Review.

Wasmtime GitHub notifications bot (Dec 02 2020 at 15:14):

yurydelendik created PR Review Comment:

As documented: MachBufferFinalized::get_srclocs_sorted produces sorted-by-start-offset tuples. Do we need to move this logic to the MachBuffer::finish?

Wasmtime GitHub notifications bot (Dec 02 2020 at 18:20):

cfallin updated PR #2462 from fix-sorted-debug-addr-transform to main:

• Sort by generated-code offset to maintain invariant and avoid gimli
  panic.

• Fix srcloc interaction with branch peephole optimization in
  MachBuffer: if a srcloc range overlaps with a branch that is
  truncated, remove that srcloc range.

These issues were found while fuzzing the new backend (#2453); I suspect
that they arise with the new backend because we can sink instructions
(e.g. loads or extends) in more interesting ways than before, but I'm
not entirely sure.

Test coverage will be via the fuzz corpus once #2453 lands.

Wasmtime GitHub notifications bot (Dec 02 2020 at 18:21):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Dec 02 2020 at 18:21):

cfallin created PR Review Comment:

Ah, yes, good call; for some reason I had assumed that this was sorted by srcloc, not offset. After fixing that (with a sort in MachBuffer::finish() the sort here is not needed anymore. Thanks!

Wasmtime GitHub notifications bot (Dec 02 2020 at 18:21):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Dec 02 2020 at 18:21):

cfallin created PR Review Comment:

Fixed instead by actually sorting by the right thing in get_srclocs_sorted()!

Wasmtime GitHub notifications bot (Dec 02 2020 at 18:41):

cfallin updated PR #2462 from fix-sorted-debug-addr-transform to main:

• Sort by generated-code offset to maintain invariant and avoid gimli
  panic.

• Fix srcloc interaction with branch peephole optimization in
  MachBuffer: if a srcloc range overlaps with a branch that is
  truncated, remove that srcloc range.

These issues were found while fuzzing the new backend (#2453); I suspect
that they arise with the new backend because we can sink instructions
(e.g. loads or extends) in more interesting ways than before, but I'm
not entirely sure.

Test coverage will be via the fuzz corpus once #2453 lands.

Wasmtime GitHub notifications bot (Dec 02 2020 at 19:13):

yurydelendik submitted PR Review.

Wasmtime GitHub notifications bot (Dec 02 2020 at 19:24):

cfallin merged PR #2462.

Last updated: Oct 23 2024 at 20:03 UTC