erts: Fix crash when invoking non-existent error_handler

Author: jhogberg

erts/emulator/beam/beam_common.c

@@ -1323,46 +1323,6 @@ call_error_handler(Process* p, const ErtsCodeMFA *mfa, Eterm* reg, Eterm func)
     return ep;
 }
 
-static Export*
-apply_setup_error_handler(Process* p, Eterm module, Eterm function, Uint arity, Eterm* reg)
-{
-    Export* ep;
-
-    /*
-     * Find the export table index for the error handler. Return NULL if
-     * there is no error handler module.
-     */
-
-    if ((ep = erts_active_export_entry(erts_proc_get_error_handler(p),
-				     am_undefined_function, 3)) == NULL) {
-	return NULL;
-    } else {
-	int i;
-	Uint sz = 2*arity;
-	Eterm* hp;
-	Eterm args = NIL;
-
-	/*
-	 * Always copy args from registers to a new list; this ensures
-	 * that we have the same behaviour whether or not this was
-	 * called from apply or fixed_apply (any additional last
-	 * THIS-argument will be included, assuming that arity has been
-	 * properly adjusted).
-	 */
-
-        hp = HAlloc(p, sz);
-	for (i = arity-1; i >= 0; i--) {
-	    args = CONS(hp, reg[i], args);
-	    hp += 2;
-	}
-	reg[0] = module;
-	reg[1] = function;
-	reg[2] = args;
-    }
-
-    return ep;
-}
-
 static ERTS_INLINE void
 apply_bif_error_adjustment(Process *p, Export *ep,
                            Eterm *reg, Uint arity,
@@ -1537,18 +1497,13 @@ apply(Process* p, Eterm* reg, ErtsCodePtr I, Uint stack_offset)
 	goto error;
     }
 
-    /*
-     * Get the index into the export table, or failing that the export
-     * entry for the error handler.
-     *
-     * Note: All BIFs have export entries; thus, no special case is needed.
-     */
+    /* Call the referenced function, if any: should the function not be found,
+     * create a stub entry which in turn calls the error handler. */
+    ep = erts_export_get_or_make_stub(module, function, arity);
 
-    if ((ep = erts_active_export_entry(module, function, arity)) == NULL) {
-	if ((ep = apply_setup_error_handler(p, module, function, arity, reg)) == NULL) goto error;
-    }
     apply_bif_error_adjustment(p, ep, reg, arity, I, stack_offset);
     DTRACE_GLOBAL_CALL_FROM_EXPORT(p, ep);
+
     return ep;
 }
 
@@ -1583,17 +1538,9 @@ fixed_apply(Process* p, Eterm* reg, Uint arity,
 	return apply(p, reg, I, stack_offset);
     }
 
-    /*
-     * Get the index into the export table, or failing that the export
-     * entry for the error handler module.
-     *
-     * Note: All BIFs have export entries; thus, no special case is needed.
-     */
-
-    if ((ep = erts_active_export_entry(module, function, arity)) == NULL) {
-	if ((ep = apply_setup_error_handler(p, module, function, arity, reg)) == NULL)
-	    goto error;
-    }
+    /* Call the referenced function, if any: should the function not be found,
+     * create a stub entry which in turn calls the error handler. */
+    ep = erts_export_get_or_make_stub(module, function, arity);
 
     apply_bif_error_adjustment(p, ep, reg, arity, I, stack_offset);
     DTRACE_GLOBAL_CALL_FROM_EXPORT(p, ep);

erts/emulator/beam/emu/beam_emu.c

@@ -529,19 +529,25 @@ void process_main(ErtsSchedulerData *esdp)
          * code[3]: &&call_error_handler
          * code[4]: Not used
          */
+        const ErtsCodeMFA *mfa;
         Export *error_handler;
 
         HEAVY_SWAPOUT;
-        error_handler = call_error_handler(c_p, erts_code_to_codemfa(I),
-                                           reg, am_undefined_function);
+        mfa = erts_code_to_codemfa(I);
+        error_handler = call_error_handler(c_p,
+                                           mfa,
+                                           reg,
+                                           am_undefined_function);
         HEAVY_SWAPIN;
 
         if (error_handler) {
             I = error_handler->dispatch.addresses[erts_active_code_ix()];
             Goto(*I);
         }
+
+        I = handle_error(c_p, I, reg, mfa);
+        goto post_error_handling;
     }
- /* Fall through */
  OpCase(error_action_code): {
     handle_error:
      SWAPOUT;

erts/emulator/beam/jit/arm/beam_asm_global.cpp

@@ -207,12 +207,15 @@ void BeamGlobalAssembler::emit_export_trampoline() {
 
     a.bind(error_handler);
     {
+        lea(ARG2, arm::Mem(ARG1, offsetof(Export, info.mfa)));
+        a.str(ARG2, TMP_MEM1q);
+
         emit_enter_runtime_frame();
         emit_enter_runtime<Update::eReductions | Update::eStack |
                            Update::eHeap | Update::eXRegs>();
 
-        lea(ARG2, arm::Mem(ARG1, offsetof(Export, info.mfa)));
         a.mov(ARG1, c_p);
+        /* ARG2 set above */
         load_x_reg_array(ARG3);
         mov_imm(ARG4, am_undefined_function);
         runtime_call<4>(call_error_handler);
@@ -223,7 +226,8 @@ void BeamGlobalAssembler::emit_export_trampoline() {
                            Update::eHeap | Update::eXRegs>();
         emit_leave_runtime_frame();
 
-        a.cbz(ARG1, labels[process_exit]);
+        a.ldr(ARG4, TMP_MEM1q);
+        a.cbz(ARG1, labels[raise_exception]);
 
         branch(emit_setup_dispatchable_call(ARG1));
     }

erts/emulator/beam/jit/x86/beam_asm_global.cpp

@@ -208,23 +208,26 @@ void BeamGlobalAssembler::emit_export_trampoline() {
 
     a.bind(error_handler);
     {
+        a.lea(ARG2, x86::qword_ptr(RET, offsetof(Export, info.mfa)));
+        a.mov(TMP_MEM1q, ARG2);
+
         emit_enter_frame();
         emit_enter_runtime<Update::eReductions | Update::eStack |
                            Update::eHeap>();
 
         a.mov(ARG1, c_p);
-        a.lea(ARG2, x86::qword_ptr(RET, offsetof(Export, info.mfa)));
+        /* ARG2 set above */
         load_x_reg_array(ARG3);
         mov_imm(ARG4, am_undefined_function);
         runtime_call<4>(call_error_handler);
 
         emit_leave_runtime<Update::eReductions | Update::eStack |
                            Update::eHeap>();
+        emit_leave_frame();
 
+        a.mov(ARG4, TMP_MEM1q);
         a.test(RET, RET);
-        a.je(labels[process_exit]);
-
-        emit_leave_frame();
+        a.je(labels[raise_exception]);
         a.jmp(emit_setup_dispatchable_call(RET));
     }
 }

erts/emulator/test/hello_SUITE_data/hello.erl

@@ -772,6 +772,34 @@ test_apply_errors() ->
             [{M,whatever,[42],_}|_] = Stk2
     end,
 
+    %% ERIERL-1220: Set a bad error handler on purpose, this used to crash the
+    %% emulator.
+    erlang:process_flag(error_handler, this_module_does_not_exist),
+
+    try known_bad_module:whatever(42) of
+        _ ->
+            error(expected_failure)
+    catch
+        error:undef:Stk3 ->
+            [{known_bad_module,whatever,[42],_}|_] = Stk3
+    end,
+
+    try M:whatever(42) of
+        _ ->
+            error(expected_failure)
+    catch
+        error:undef:Stk4 ->
+            [{bad_module,whatever,[42],_}|_] = Stk4
+    end,
+
+    try apply(M, whatever, id([42])) of
+        _ ->
+            error(expected_failure)
+    catch
+        error:undef:Stk5 ->
+            [{bad_module,whatever,[42],_}|_] = Stk5
+    end,
+
     erlang:process_flag(error_handler, OldErrorHandler),
     ok.