From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Sat, 19 Feb 2022 19:55:39 +0000 (-0500)
Subject: (specpdl_unrewind): Fix corner case
X-Git-Tag: emacs-29.0.90~2217
X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=ecaedf2117cb015ad4028e4d6fc7058608c98096;p=emacs.git

(specpdl_unrewind): Fix corner case

* src/lisp.h (enum specbind_tag): New elem SPECPDL_NOP.

* src/eval.c (specpdl_unrewind): Zap entries that can't be applied any more,
and simplify.
(default_toplevel_binding, lexbound_p, Fbacktrace__locals): Simplify.
(do_one_unbind, mark_specpdl): Handle SPECPDL_NOP.
---

diff --git a/src/eval.c b/src/eval.c
index d3342289fbb..294d79e67a0 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -664,23 +664,7 @@ default_toplevel_binding (Lisp_Object symbol)
 	    binding = pdl;
 	  break;
 
-	case SPECPDL_UNWIND:
-	case SPECPDL_UNWIND_ARRAY:
-	case SPECPDL_UNWIND_PTR:
-	case SPECPDL_UNWIND_INT:
-	case SPECPDL_UNWIND_INTMAX:
-	case SPECPDL_UNWIND_EXCURSION:
-	case SPECPDL_UNWIND_VOID:
-	case SPECPDL_BACKTRACE:
-#ifdef HAVE_MODULES
-        case SPECPDL_MODULE_RUNTIME:
-        case SPECPDL_MODULE_ENVIRONMENT:
-#endif
-	case SPECPDL_LET_LOCAL:
-	  break;
-
-	default:
-	  emacs_abort ();
+	default: break;
 	}
     }
   return binding;
@@ -707,23 +691,7 @@ lexbound_p (Lisp_Object symbol)
 	    }
 	  break;
 
-	case SPECPDL_UNWIND:
-	case SPECPDL_UNWIND_ARRAY:
-	case SPECPDL_UNWIND_PTR:
-	case SPECPDL_UNWIND_INT:
-	case SPECPDL_UNWIND_INTMAX:
-	case SPECPDL_UNWIND_EXCURSION:
-	case SPECPDL_UNWIND_VOID:
-	case SPECPDL_BACKTRACE:
-#ifdef HAVE_MODULES
-        case SPECPDL_MODULE_RUNTIME:
-        case SPECPDL_MODULE_ENVIRONMENT:
-#endif
-	case SPECPDL_LET_LOCAL:
-	  break;
-
-	default:
-	  emacs_abort ();
+	default: break;
 	}
     }
   return false;
@@ -3721,6 +3689,7 @@ do_one_unbind (union specbinding *this_binding, bool unwinding,
 			      this_binding->unwind_excursion.window);
       break;
     case SPECPDL_BACKTRACE:
+    case SPECPDL_NOP:
       break;
 #ifdef HAVE_MODULES
     case SPECPDL_MODULE_RUNTIME:
@@ -4044,17 +4013,6 @@ specpdl_unrewind (union specbinding *pdl, int distance, bool vars_only)
 	    save_excursion_restore (marker, window);
 	  }
 	  break;
-	case SPECPDL_UNWIND_ARRAY:
-	case SPECPDL_UNWIND_PTR:
-	case SPECPDL_UNWIND_INT:
-	case SPECPDL_UNWIND_INTMAX:
-	case SPECPDL_UNWIND_VOID:
-	case SPECPDL_BACKTRACE:
-#ifdef HAVE_MODULES
-        case SPECPDL_MODULE_RUNTIME:
-        case SPECPDL_MODULE_ENVIRONMENT:
-#endif
-	  break;
 	case SPECPDL_LET:
 	  { /* If variable has a trivial value (no forwarding), we can
 	       just set it.  No need to check for constant symbols here,
@@ -4097,14 +4055,16 @@ specpdl_unrewind (union specbinding *pdl, int distance, bool vars_only)
                               SET_INTERNAL_THREAD_SWITCH);
 	      }
 	    else
-	      /* FIXME: If the var is not local any more, we failed
-                 to swap the old and new values.  As long as the var remains
-                 non-local, this is fine, but if it ever reverts to being
-                 local we may end up using this entry "in the wrong
-                 direction".  */
-	      {}
+	      /* If the var is not local any more, it can't be undone nor
+                 redone, so just zap it.
+                 This is important in case the buffer re-gains a local value
+                 before we unrewind again, in which case we'd risk applying
+                 this entry in the wrong direction.  */
+	      tmp->kind = SPECPDL_NOP;
 	  }
 	  break;
+
+	default: break;
 	}
     }
 }
@@ -4195,22 +4155,7 @@ NFRAMES and BASE specify the activation frame to use, as in `backtrace-frame'.
 	    }
 	    break;
 
-	  case SPECPDL_UNWIND:
-	  case SPECPDL_UNWIND_ARRAY:
-	  case SPECPDL_UNWIND_PTR:
-	  case SPECPDL_UNWIND_INT:
-	  case SPECPDL_UNWIND_INTMAX:
-	  case SPECPDL_UNWIND_EXCURSION:
-	  case SPECPDL_UNWIND_VOID:
-	  case SPECPDL_BACKTRACE:
-#ifdef HAVE_MODULES
-          case SPECPDL_MODULE_RUNTIME:
-          case SPECPDL_MODULE_ENVIRONMENT:
-#endif
-	    break;
-
-	  default:
-	    emacs_abort ();
+	  default: break;
 	  }
       }
   }
@@ -4274,8 +4219,12 @@ mark_specpdl (union specbinding *first, union specbinding *ptr)
 	case SPECPDL_UNWIND_INT:
 	case SPECPDL_UNWIND_INTMAX:
         case SPECPDL_UNWIND_VOID:
+	case SPECPDL_NOP:
 	  break;
 
+	/* While other loops that scan the specpdl use "default: break;"
+	   for simplicity, here we explicitly list all cases and abort
+	   if we find an unexpected value, as a sanity check.  */
 	default:
 	  emacs_abort ();
 	}
diff --git a/src/lisp.h b/src/lisp.h
index 19788ef07cc..deeca9bc86b 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3284,6 +3284,7 @@ enum specbind_tag {
   SPECPDL_UNWIND_EXCURSION,	/* Likewise, on an excursion.  */
   SPECPDL_UNWIND_VOID,		/* Likewise, with no arg.  */
   SPECPDL_BACKTRACE,		/* An element of the backtrace.  */
+  SPECPDL_NOP,			/* A filler.  */
 #ifdef HAVE_MODULES
   SPECPDL_MODULE_RUNTIME,       /* A live module runtime.  */
   SPECPDL_MODULE_ENVIRONMENT,   /* A live module environment.  */