Merge branch 'fix-trycatch-scrub-gh287'

Scrub TRYCATCH catch registers on TRYCATCH setup to avoid a previous
temporary value with a finalizer triggering harmful side effects.  The
concrete problem is that such a finalizer call would overwrite the
heap->lj.type value which then causes finally handling to fail, see
svaaralaGH-287.

RELEASES.rst

@@ -1061,6 +1061,9 @@ Planned
   DUK_USE_xxx config options for native recursion limits; C stacks are assumed
   to be deep by default for all targets (GH-165)
 
+* Fix a try-finally finalizer side effect issue by scrubbing TRYCATCH catch
+  registers before executing the try code block (GH-287)
+
 * Fix missing activation lookup from call handling after an Arguments object
   was created, this could in theory lead to memory unsafe behavior (GH-305)
 

src/duk_js_executor.c

@@ -3585,6 +3585,21 @@ DUK_INTERNAL void duk_js_execute_bytecode(duk_hthread *exec_thr) {
 				;
 			}
 
+			/* Registers 'bc' and 'bc + 1' are written in longjmp handling
+			 * and if their previous values (which are temporaries) become
+			 * unreachable -and- have a finalizer, there'll be a function
+			 * call during error handling which is not supported now (GH-287).
+			 * Ensure that both 'bc' and 'bc + 1' have primitive values to
+			 * guarantee no finalizer calls in error handling.  Scrubbing also
+			 * ensures finalizers for the previous values run here rather than
+			 * later.  Error handling related values are also written to 'bc'
+			 * and 'bc + 1' but those values never become unreachable during
+			 * error handling, so there's no side effect problem even if the
+			 * error value has a finalizer.
+			 */
+			duk_to_undefined(ctx, bc);
+			duk_to_undefined(ctx, bc + 1);
+
 			cat = thr->catchstack + thr->catchstack_top - 1;  /* relookup (side effects) */
 			cat->callstack_index = thr->callstack_top - 1;
 			cat->pc_base = (duk_instr_t *) curr_pc;  /* pre-incremented, points to first jump slot */

tests/ecmascript/test-bug-finally-ljtype-gh287.js

@@ -0,0 +1,35 @@
+/*
+ *  Bug in Duktape 1.2 and before where finalization just before
+ *  'finally' caused an internal error (GH-287).
+ */
+
+/*===
+The pig ate all the food, and then...
+*munch*
+Error: A pig ate everyone at 8:12
+done
+===*/
+
+function test() {
+    function Food() {
+        this.getEaten = function() { print("The pig ate all the food, and then..."); };
+    }
+    Duktape.fin(Food.prototype, function() {});
+    new Food().getEaten();
+
+    // The above line is critical to triggering the bug: on entry to TRYCATCH
+    // the first catch register must contain a value which (1) has only one
+    // reference left, and (2) has a finalizer.  When the error value is written
+    // to the catch register when setting up for the finally block, the value
+    // becomes unreachable and the finalizer is executed.
+
+    try { throw new Error("A pig ate everyone at 8:12"); }
+    finally { print("*munch*"); }
+}
+
+try {
+    test();
+} catch (e) {
+    print(e);
+}
+print('done');