VM never extends vp->sp beyond mapped region of stack

* libguile/vm-engine.c (ALLOC_FRAME): Fold CHECK_OVERFLOW into this
  routine, and rework to not extend vp->sp until the stack has been
  expanded.

* libguile/vm.c (vm_increase_sp): Likewise, don't extend vp->sp until
  the stack has expanded.
  (vm_expand_stack): Rework to take the new stack pointer as an
  argument, and also to update vp->sp_max_since_gc and vp->sp.

libguile/vm-engine.c

@@ -132,19 +132,7 @@
 
 /* Virtual Machine
 
-   This is Guile's new virtual machine.  When I say "new", I mean
+   The VM has three state bits: the instruction pointer (IP), the frame
-   relative to the current virtual machine.  At some point it will
-   become "the" virtual machine, and we'll delete this paragraph.  As
-   such, the rest of the comments speak as if there's only one VM.
-   In difference from the old VM, local 0 is the procedure, and the
-   first argument is local 1.  At some point in the future we should
-   change the fp to point to the procedure and not to local 1.
-
-   <more overview here>
- */
-
-
-/* The VM has three state bits: the instruction pointer (IP), the frame
    pointer (FP), and the top-of-stack pointer (SP).  We cache the first
    two of these in machine registers, local to the VM, because they are
    used extensively by the VM.  As the SP is used more by code outside
@@ -173,33 +161,30 @@
   } while (0)
 
 
-
-/* After advancing vp->sp, but before writing any stack slots, check
-   that it is actually in bounds.  If it is not in bounds, currently we
-   signal an error.  In the future we may expand the stack instead,
-   possibly by moving it elsewhere, therefore no pointer into the stack
-   besides FP is valid across a CHECK_OVERFLOW call.  Be careful!  */
-#define CHECK_OVERFLOW()                                            \
-  do {                                                              \
-    if (SCM_UNLIKELY (vp->sp >= vp->stack_limit))                   \
-      {                                                             \
-        SYNC_IP ();                                                 \
-        vm_expand_stack (vp);                                       \
-        CACHE_FP ();                                                \
-      }                                                             \
-  } while (0)
-
 /* Reserve stack space for a frame.  Will check that there is sufficient
    stack space for N locals, including the procedure.  Invoke after
-   preparing the new frame and setting the fp and ip.  */
+   preparing the new frame and setting the fp and ip.
+
+   If there is not enough space for this frame, we try to expand the
+   stack, possibly relocating it somewhere else in the address space.
+   Because of the possible relocation, no pointer into the stack besides
+   FP is valid across an ALLOC_FRAME call.  Be careful!  */
 #define ALLOC_FRAME(n)                                              \
   do {                                                              \
-    vp->sp = LOCAL_ADDRESS (n - 1);                                 \
+    SCM *new_sp = LOCAL_ADDRESS (n - 1);                            \
-    if (vp->sp > vp->sp_max_since_gc)                               \
+    if (new_sp > vp->sp_max_since_gc)                               \
       {                                                             \
-        vp->sp_max_since_gc = vp->sp;                               \
+        if (SCM_UNLIKELY (new_sp >= vp->stack_limit))               \
-        CHECK_OVERFLOW ();                                          \
+          {                                                         \
+            SYNC_IP ();                                             \
+            vm_expand_stack (vp, new_sp);                           \
+            CACHE_FP ();                                            \
+          }                                                         \
+        else                                                        \
+          vp->sp_max_since_gc = vp->sp = new_sp;                    \
       }                                                             \
+    else                                                            \
+      vp->sp = new_sp;                                              \
   } while (0)
 
 /* Reset the current frame to hold N locals.  Used when we know that no
@@ -3235,7 +3220,6 @@ VM_NAME (scm_i_thread *thread, struct scm_vm *vp,
 #undef BV_INT_REF
 #undef BV_INT_SET
 #undef CACHE_REGISTER
-#undef CHECK_OVERFLOW
 #undef END_DISPATCH_SWITCH
 #undef FREE_VARIABLE_REF
 #undef INIT

libguile/vm.c

@@ -64,7 +64,7 @@ static SCM sym_debug;
 
 /* #define VM_ENABLE_PARANOID_ASSERTIONS */
 
-static void vm_expand_stack (struct scm_vm *vp) SCM_NOINLINE;
+static void vm_expand_stack (struct scm_vm *vp, SCM *new_sp) SCM_NOINLINE;
 
 /* RESTORE is for the case where we know we have done a PUSH of equal or
    greater stack size in the past.  Otherwise PUSH is the thing, which
@@ -74,13 +74,16 @@ enum vm_increase_sp_kind { VM_SP_PUSH, VM_SP_RESTORE };
 static inline void
 vm_increase_sp (struct scm_vm *vp, SCM *new_sp, enum vm_increase_sp_kind kind)
 {
-  vp->sp = new_sp;
+  if (new_sp <= vp->sp_max_since_gc)
-  if (new_sp > vp->sp_max_since_gc)
     {
-      vp->sp_max_since_gc = new_sp;
+      vp->sp = new_sp;
-      if (kind == VM_SP_PUSH && new_sp >= vp->stack_limit)
+      return;
-        vm_expand_stack (vp);
     }
+
+  if (kind == VM_SP_PUSH && new_sp >= vp->stack_limit)
+    vm_expand_stack (vp, new_sp);
+  else
+    vp->sp_max_since_gc = vp->sp = new_sp;
 }
 
 static inline void
@@ -989,9 +992,9 @@ scm_i_vm_free_stack (struct scm_vm *vp)
 }
 
 static void
-vm_expand_stack (struct scm_vm *vp)
+vm_expand_stack (struct scm_vm *vp, SCM *new_sp)
 {
-  scm_t_ptrdiff stack_size = vp->sp + 1 - vp->stack_base;
+  scm_t_ptrdiff stack_size = new_sp + 1 - vp->stack_base;
 
   if (stack_size > hard_max_stack_size)
     {
@@ -1028,7 +1031,6 @@ vm_expand_stack (struct scm_vm *vp)
           SCM *fp;
           if (vp->fp)
             vp->fp += reloc;
-          vp->sp += reloc;
           vp->sp_max_since_gc += reloc;
           fp = vp->fp;
           while (fp)
@@ -1042,6 +1044,8 @@ vm_expand_stack (struct scm_vm *vp)
               fp = next_fp;
             }
         }
+
+      new_sp += reloc;
     }
 
   if (stack_size >= vp->max_stack_size)
@@ -1068,6 +1072,7 @@ vm_expand_stack (struct scm_vm *vp)
     }
 
   /* Otherwise continue, with the new enlarged stack.  */
+  vp->sp_max_since_gc = vp->sp = new_sp;
 }
 
 static struct scm_vm *