Bugzilla

This surfaced while I was investigating some stack misalignment issues
possibly relating to sizeof(wasm::Frame).

The Ion pipeline for Wasm decides in CodeGenerator::generateWasm what kind of
stack reservation to make:

  if (omitOverRecursedCheck()) {
    masm.reserveStack(frameSize());
  } else {
    masm.wasmReserveStackChecked(frameSize(), trapOffset);
  }

Assuming the 'else' clause is taken, we end up here:

  void
  MacroAssembler::wasmReserveStackChecked(uint32_t amount,
                                          wasm::BytecodeOffset trapOffset) {
    if (!amount) {
      return;
    }

so if |amount == 0| the stack check is omitted.  I think this early exit
is incorrect and should be removed.

My guess about what happened is as follows: it appears that |amount|,
which == CodeGeneratorShared::frameSize(), holds the number of extra bytes
*in addition* to the sizeof(Wasm::frame), that are needed.  However, this
number is rounded up so that (it + sizeof(Wasm::frame)) are 0 % 16.

As an example, on x86(32)-linux, in an infinite-recursion test that should end
by a stack overflow check failing ...

   (module (func (call 0)) (export "" 0))

.. we have:

   sizeof(Frame) == 12
   no extra stack allocation needed
   so |amount| == 4 because it is the smallest number giving
        |amount| + sizeof(Frame) being 0 % 16.
   So the stack overflow is included (correctly).

When I increase sizeof(Frame) to 16, then

   sizeof(Frame) == 16
   no extra stack allocation needed
   so |amount| == 0
   So the stack overflow is incorrectly omitted.

and printfs verify the |amount| numbers in these cases.

Looking back at the early exit in MacroAssembler::wasmReserveStackChecked,
there's no way any non-leaf call could really use zero stack in total -- else
where would he return address be stored?  So the early exit can't be right
as-is.

I looked also at CodeGeneratorShared::frameSize(), but failed to deduce the
intended semantics from it:

  uint32_t frameSize() const {
    return frameClass_ == FrameSizeClass::None() ? frameDepth_
                                                 : frameClass_.frameSize();
  }

```
This surfaced while I was investigating some stack misalignment issues
possibly relating to sizeof(wasm::Frame).

The Ion pipeline for Wasm decides in CodeGenerator::generateWasm what kind of
stack reservation to make:

  if (omitOverRecursedCheck()) {
    masm.reserveStack(frameSize());
  } else {
    masm.wasmReserveStackChecked(frameSize(), trapOffset);
  }

Assuming the 'else' clause is taken, we end up here:

  void
  MacroAssembler::wasmReserveStackChecked(uint32_t amount,
                                          wasm::BytecodeOffset trapOffset) {
    if (!amount) {
      return;
    }

so if |amount == 0| the stack check is omitted.  I think this early exit
is incorrect and should be removed.

My guess about what happened is as follows: it appears that |amount|,
which == CodeGeneratorShared::frameSize(), holds the number of extra bytes
*in addition* to the sizeof(Wasm::frame), that are needed.  However, this
number is rounded up so that (it + sizeof(Wasm::frame)) are 0 % 16.

As an example, on x86(32)-linux, in an infinite-recursion test that should end
by a stack overflow check failing ...

   (module (func (call 0)) (export "" 0))

.. we have:

   sizeof(Frame) == 12
   no extra stack allocation needed
   so |amount| == 4 because it is the smallest number giving
        |amount| + sizeof(Frame) being 0 % 16.
   So the stack overflow is included (correctly).

When I increase sizeof(Frame) to 16, then

   sizeof(Frame) == 16
   no extra stack allocation needed
   so |amount| == 0
   So the stack overflow is incorrectly omitted.

and printfs verify the |amount| numbers in these cases.

Looking back at the early exit in MacroAssembler::wasmReserveStackChecked,
there's no way any non-leaf call could really use zero stack in total -- else
where would he return address be stored?  So the early exit can't be right
as-is.

I looked also at CodeGeneratorShared::frameSize(), but failed to deduce the
intended semantics from it:

  uint32_t frameSize() const {
    return frameClass_ == FrameSizeClass::None() ? frameDepth_
                                                 : frameClass_.frameSize();
  }
```