Stream: git-wasmtime

Topic: wasmtime / PR #3180 Reimplement how unwind information is...

Wasmtime GitHub notifications bot (Aug 11 2021 at 20:21):

alexcrichton opened PR #3180 from refactor-unwind to main:

This commit is a major refactoring of how unwind information is stored
after compilation of a function has finished. Previously we would store
the raw UnwindInfo as a result of compilation and this would get
serialized/deserialized alongside the rest of the ELF object that
compilation creates. Whenever functions were registered with
CodeMemory this would also result in registering unwinding information
dynamically at runtime, which in the case of Unix, for example, would
dynamically created FDE/CIE entries on-the-fly.

Eventually I'd like to support compiling Wasmtime without Cranelift, but
this means that UnwindInfo wouldn't be easily available to decode into
and create unwinding information from. To solve this I've changed the
ELF object created to have the unwinding information encoded into it
ahead-of-time so loading code into memory no longer needs to create
unwinding tables. This change has two different implementations for
Windows/Unix:

• On Windows the implementation was much easier. The unwinding
  information on Windows is already stored after the function itself in
  the text section. This was actually slightly duplicated in object
  building and in code memory allocation. Now the object building
  continues to do the same, recording unwinding information after
  functions, and code memory no longer manually tracks this.
  Additionally Wasmtime will emit a special custom section in the object
  file with unwinding information which is the list of
  RUNTIME_FUNCTION structures that RtlAddFunctionTable expects. This
  means that the object file has all the information precompiled into it
  and registration at runtime is simply passing a few pointers around to
  the runtime.

• Unix was a little bit more difficult than Windows. Today a .eh_frame
  section is created on-the-fly with offsets in FDEs specified as the
  absolute address that functions are loaded at. This absolute
  address hindered the ability to precompile the FDE into the object
  file itself. I've switched how addresses are encoded, though, to using
  DW_EH_PE_pcrel which means that FDE addresses are now specified
  relative to the FDE itself. This means that we can maintain a fixed
  offset between the .eh_frame loaded in memory and the beginning of
  code memory. When doing so this enables precompiling the .eh_frame
  section into the object file and at runtime when loading an object no
  further construction of unwinding information is needed.

The overall result of this commit is that unwinding information is no
longer stored in its cranelift-data-structure form on disk. This means
that this unwinding information format is only present during
compilation, which will make it that much easier to compile out
cranelift in the future.

This commit also significantly refactors CodeMemory since the way
unwinding information is handled is not much different from before.
Previously CodeMemory was suitable for incrementally adding more and
more functions to it, but nowadays a CodeMemory either lives per
module (in which case all functions are known up front) or it's created
once-per-Func::new with two trampolines. In both cases we know all
functions up front so the functionality of incrementally adding more and
more segments is no longer needed. This commit removes the ability to
add a function-at-a-time in CodeMemory and instead it can now only
load objects in their entirety. A small helper function is added to
build a small object file for trampolines in Func::new to handle
allocation there.

Finally, this commit also refactors the wasmtime-obj crate and its
builder structure to be more amenable to this strategy of managing
unwinding tables.

It is not intentional to have any real functional change as a result of
this commit. This might accelerate loading a module from cache slightly
since less work is needed to manage the unwinding information, but
that's just a side benefit from the main goal of this commit which is to
remove the dependence on cranelift unwinding information being available
at runtime.

On a procedural note this draws from https://github.com/bytecodealliance/wasmtime/pull/3179 and https://github.com/bytecodealliance/wasmtime/pull/3176 so only the last commit here is what's to be reviewed.

Wasmtime GitHub notifications bot (Aug 11 2021 at 20:22):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 11 2021 at 20:30):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 11 2021 at 21:25):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 11 2021 at 21:26):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 11 2021 at 21:33):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 12 2021 at 14:39):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 12 2021 at 20:16):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 12 2021 at 20:16):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 12 2021 at 20:39):

alexcrichton requested fitzgen for a review on PR #3180.

Wasmtime GitHub notifications bot (Aug 12 2021 at 21:59):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 12 2021 at 21:59):

alexcrichton edited PR #3180 from refactor-unwind to main:

This commit is a major refactoring of how unwind information is stored
after compilation of a function has finished. Previously we would store
the raw UnwindInfo as a result of compilation and this would get
serialized/deserialized alongside the rest of the ELF object that
compilation creates. Whenever functions were registered with
CodeMemory this would also result in registering unwinding information
dynamically at runtime, which in the case of Unix, for example, would
dynamically created FDE/CIE entries on-the-fly.

Eventually I'd like to support compiling Wasmtime without Cranelift, but
this means that UnwindInfo wouldn't be easily available to decode into
and create unwinding information from. To solve this I've changed the
ELF object created to have the unwinding information encoded into it
ahead-of-time so loading code into memory no longer needs to create
unwinding tables. This change has two different implementations for
Windows/Unix:

• On Windows the implementation was much easier. The unwinding
  information on Windows is already stored after the function itself in
  the text section. This was actually slightly duplicated in object
  building and in code memory allocation. Now the object building
  continues to do the same, recording unwinding information after
  functions, and code memory no longer manually tracks this.
  Additionally Wasmtime will emit a special custom section in the object
  file with unwinding information which is the list of
  RUNTIME_FUNCTION structures that RtlAddFunctionTable expects. This
  means that the object file has all the information precompiled into it
  and registration at runtime is simply passing a few pointers around to
  the runtime.

• Unix was a little bit more difficult than Windows. Today a .eh_frame
  section is created on-the-fly with offsets in FDEs specified as the
  absolute address that functions are loaded at. This absolute
  address hindered the ability to precompile the FDE into the object
  file itself. I've switched how addresses are encoded, though, to using
  DW_EH_PE_pcrel which means that FDE addresses are now specified
  relative to the FDE itself. This means that we can maintain a fixed
  offset between the .eh_frame loaded in memory and the beginning of
  code memory. When doing so this enables precompiling the .eh_frame
  section into the object file and at runtime when loading an object no
  further construction of unwinding information is needed.

The overall result of this commit is that unwinding information is no
longer stored in its cranelift-data-structure form on disk. This means
that this unwinding information format is only present during
compilation, which will make it that much easier to compile out
cranelift in the future.

This commit also significantly refactors CodeMemory since the way
unwinding information is handled is not much different from before.
Previously CodeMemory was suitable for incrementally adding more and
more functions to it, but nowadays a CodeMemory either lives per
module (in which case all functions are known up front) or it's created
once-per-Func::new with two trampolines. In both cases we know all
functions up front so the functionality of incrementally adding more and
more segments is no longer needed. This commit removes the ability to
add a function-at-a-time in CodeMemory and instead it can now only
load objects in their entirety. A small helper function is added to
build a small object file for trampolines in Func::new to handle
allocation there.

Finally, this commit also refactors the wasmtime-obj crate and its
builder structure to be more amenable to this strategy of managing
unwinding tables.

It is not intentional to have any real functional change as a result of
this commit. This might accelerate loading a module from cache slightly
since less work is needed to manage the unwinding information, but
that's just a side benefit from the main goal of this commit which is to
remove the dependence on cranelift unwinding information being available
at runtime.

On a procedural note this draws from https://github.com/bytecodealliance/wasmtime/pull/3179 and https://github.com/bytecodealliance/wasmtime/pull/3176 so only the last commit here is what's to be reviewed.

Wasmtime GitHub notifications bot (Aug 16 2021 at 17:08):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 16 2021 at 17:08):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 16 2021 at 17:28):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 16 2021 at 20:33):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 16 2021 at 20:33):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 16 2021 at 20:37):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 16 2021 at 22:48):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen created PR review comment:

I think we could do a slightly more helpful chained error message here:

failed to parse internal ELF compilation artifact

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen created PR review comment:

Nitpick: this can be removed, I guess?

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen created PR review comment:

ugh I forgot about this bug

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen created PR review comment:

To clarify: the alternative to this whole DW_EH_PE_pcrel approach would be to emit relocs for the FDEs, right?

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen created PR review comment:

Nitpick, but I find it more confusing to turbo fish unzip than to give an explicit type to the LHS of the assignment, since unzip has those two extra type parameters that I don't remember what is going on with them and have to look up in the docs again. On the other hand, when the explicit types are in the LHS of the assignment, I don't get side tracked by these meaningless type parameters.

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen created PR review comment:

//! symbols that refer to libcalls.

Wasmtime GitHub notifications bot (Aug 17 2021 at 20:45):

fitzgen created PR review comment:

//! relocation records for the linking stage. If DWARF sections exist, their

Wasmtime GitHub notifications bot (Aug 17 2021 at 21:20):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Aug 17 2021 at 21:20):

alexcrichton created PR review comment:

Indeed yeah. I'll admit that I know so little about relocations I didn't really try to do this at all and I didn't feel I needed to pursue this much because I got the pcrel bits working. Not having relocations is somewhat beneficial in that loading these tables is that much faster (it's just what's in-memory) and in theory amenable to the hypothetical mmap-and-go world.

Wasmtime GitHub notifications bot (Aug 17 2021 at 21:21):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 17 2021 at 21:22):

alexcrichton updated PR #3180 from refactor-unwind to main.

Wasmtime GitHub notifications bot (Aug 17 2021 at 22:14):

alexcrichton merged PR #3180.

Last updated: Nov 22 2024 at 17:03 UTC