1 files changed, 80 insertions, 56 deletions

diff --git a/erts/emulator/beam/beam_bp.c b/erts/emulator/beam/beam_bp.c
index 49ec59c989..0832b3f374 100644
--- a/erts/emulator/beam/beam_bp.c
+++ b/erts/emulator/beam/beam_bp.c
@@ -203,7 +203,7 @@ erts_bp_match_functions(BpFunctions* f, ErtsCodeMFA *mfa, int specified)
 	for (fi = 0; fi < num_functions; fi++) {
 
 	    ci = code_hdr->functions[fi];
-	    ASSERT(ci->op == (BeamInstr) BeamOp(op_i_func_info_IaaI));
+	    ASSERT(BeamIsOpCode(ci->op, op_i_func_info_IaaI));
 	    if (erts_is_function_native(ci)) {
 		continue;
 	    }
@@ -265,11 +265,11 @@ erts_bp_match_export(BpFunctions* f, ErtsCodeMFA *mfa, int specified)
 
 	pc = ep->beam;
 	if (ep->addressv[code_ix] == pc) {
-	    if ((*pc == (BeamInstr) em_apply_bif ||
-		 *pc == (BeamInstr) em_call_error_handler)) {
-		continue;
+	    if (BeamIsOpCode(*pc, op_apply_bif) ||
+                BeamIsOpCode(*pc, op_call_error_handler)) {
+                continue;
 	    }
-	    ASSERT(*pc == (BeamInstr) BeamOp(op_i_generic_breakpoint));
+	    ASSERT(BeamIsOpCode(*pc, op_i_generic_breakpoint));
 	} else if (erts_is_function_native(erts_code_to_codeinfo(ep->addressv[code_ix]))) {
 	    continue;
 	}
@@ -366,8 +366,8 @@ consolidate_bp_data(Module* modp, ErtsCodeInfo *ci, int local)
 	    }
 	    ASSERT(modp->curr.num_breakpoints >= 0);
 	    ASSERT(modp->curr.num_traced_exports >= 0);
-	    ASSERT(*erts_codeinfo_to_code(ci) !=
-                   (BeamInstr) BeamOp(op_i_generic_breakpoint));
+	    ASSERT(! BeamIsOpCode(*erts_codeinfo_to_code(ci),
+                                  op_i_generic_breakpoint));
 	}
 	ci->u.gen_bp = NULL;
 	Free(g);
@@ -415,13 +415,15 @@ erts_install_breakpoints(BpFunctions* f)
 {
     Uint i;
     Uint n = f->matched;
-    BeamInstr br = (BeamInstr) BeamOp(op_i_generic_breakpoint);
+    BeamInstr br = BeamOpCodeAddr(op_i_generic_breakpoint);
 
     for (i = 0; i < n; i++) {
 	ErtsCodeInfo* ci = f->matching[i].ci;
-        BeamInstr *pc = erts_codeinfo_to_code(ci);
 	GenericBp* g = ci->u.gen_bp;
-	if (*pc != br && g) {
+        BeamInstr volatile *pc = erts_codeinfo_to_code(ci);
+        BeamInstr instr = *pc;
+
+	if (!BeamIsOpCode(instr, op_i_generic_breakpoint) && g) {
 	    Module* modp = f->matching[i].mod;
 
 	    /*
@@ -435,11 +437,16 @@ erts_install_breakpoints(BpFunctions* f)
 	    /*
 	     * The following write is not protected by any lock. We
 	     * assume that the hardware guarantees that a write of an
-	     * aligned word-size (or half-word) writes is atomic
-	     * (i.e. that other processes executing this code will not
-	     * see a half pointer).
+	     * aligned word-size writes is atomic (i.e. that other
+	     * processes executing this code will not see a half
+	     * pointer).
+             *
+             * The contents of *pc is marked 'volatile' to ensure that
+             * the compiler will do a single full-word write, and not
+             * try any fancy optimizations to write a half word.
 	     */
-	    *pc = br;
+            instr = BeamSetCodeAddr(instr, br);
+            *pc = instr;
 	    modp->curr.num_breakpoints++;
 	}
     }
@@ -460,7 +467,7 @@ static void
 uninstall_breakpoint(ErtsCodeInfo *ci)
 {
     BeamInstr *pc = erts_codeinfo_to_code(ci);
-    if (*pc == (BeamInstr) BeamOp(op_i_generic_breakpoint)) {
+    if (BeamIsOpCode(*pc, op_i_generic_breakpoint)) {
 	GenericBp* g = ci->u.gen_bp;
 	if (g->data[erts_active_bp_ix()].flags == 0) {
 	    /*
@@ -634,6 +641,49 @@ erts_clear_export_break(Module* modp, ErtsCodeInfo *ci)
     ASSERT(ci->u.gen_bp == NULL);
 }
 
+/*
+ * If c_p->cp is a trace return instruction, we set cp
+ * to be the place where we again start to execute code.
+ *
+ * cp is used by match spec {caller} to get the calling
+ * function, and if we don't do this fixup it will be
+ * 'undefined'. This has the odd side effect of {caller}
+ * not really being which function is the caller, but
+ * rather which function we are about to return to.
+ */
+static void fixup_cp_before_trace(Process *c_p, int *return_to_trace)
+{
+    Eterm *cpp, *E = c_p->stop;
+    BeamInstr w = *c_p->cp;
+    if (BeamIsOpCode(w, op_return_trace)) {
+        cpp = &E[2];
+    } else if (BeamIsOpCode(w, op_i_return_to_trace)) {
+        *return_to_trace = 1;
+        cpp = &E[0];
+    } else if (BeamIsOpCode(w, op_i_return_time_trace)) {
+        cpp = &E[0];
+    } else {
+        cpp = NULL;
+    }
+    if (cpp) {
+        for (;;) {
+            BeamInstr w = *cp_val(*cpp);
+            if (BeamIsOpCode(w, op_return_trace)) {
+                cpp += 3;
+            } else if (BeamIsOpCode(w, op_i_return_to_trace)) {
+                *return_to_trace = 1;
+                cpp += 1;
+            } else if (BeamIsOpCode(w, op_i_return_time_trace)) {
+                cpp += 2;
+            } else {
+                break;
+            }
+        }
+        c_p->cp = (BeamInstr *) cp_val(*cpp);
+        ASSERT(is_CP(*cpp));
+    }
+}
+
 BeamInstr
 erts_generic_breakpoint(Process* c_p, ErtsCodeInfo *info, Eterm* reg)
 {
@@ -642,7 +692,7 @@ erts_generic_breakpoint(Process* c_p, ErtsCodeInfo *info, Eterm* reg)
     Uint bp_flags;
     ErtsBpIndex ix = erts_active_bp_ix();
 
-    ASSERT(info->op == (BeamInstr) BeamOp(op_i_func_info_IaaI));
+    ASSERT(BeamIsOpCode(info->op, op_i_func_info_IaaI));
 
     g = info->u.gen_bp;
     bp = &g->data[ix];
@@ -695,9 +745,9 @@ erts_generic_breakpoint(Process* c_p, ErtsCodeInfo *info, Eterm* reg)
 	Eterm w;
 	erts_trace_time_call(c_p, info, bp->time);
 	w = (BeamInstr) *c_p->cp;
-	if (! (w == (BeamInstr) BeamOp(op_i_return_time_trace) ||
-	       w == (BeamInstr) BeamOp(op_return_trace) ||
-	       w == (BeamInstr) BeamOp(op_i_return_to_trace)) ) {
+	if (! (BeamIsOpCode(w, op_i_return_time_trace) ||
+	       BeamIsOpCode(w, op_return_trace) ||
+               BeamIsOpCode(w, op_i_return_to_trace)) ) {
 	    Eterm* E = c_p->stop;
 	    ASSERT(c_p->htop <= E && E <= c_p->hend);
 	    if (E - 2 < c_p->htop) {
@@ -717,7 +767,7 @@ erts_generic_breakpoint(Process* c_p, ErtsCodeInfo *info, Eterm* reg)
     }
 
     if (bp_flags & ERTS_BPF_DEBUG) {
-	return (BeamInstr) BeamOp(op_i_debug_breakpoint);
+	return BeamOpCodeAddr(op_i_debug_breakpoint);
     } else {
 	return g->orig_instr;
     }
@@ -744,6 +794,7 @@ erts_bif_trace(int bif_index, Process* p, Eterm* args, BeamInstr* I)
     GenericBp* g;
     GenericBpData* bp = NULL;
     Uint bp_flags = 0;
+    int return_to_trace = 0;
 
     ERTS_CHK_HAVE_ONLY_MAIN_PROC_LOCK(p);
 
@@ -759,6 +810,8 @@ erts_bif_trace(int bif_index, Process* p, Eterm* args, BeamInstr* I)
      */
     if (!applying) {
 	p->cp = I;
+    } else {
+        fixup_cp_before_trace(p, &return_to_trace);
     }
     if (bp_flags & (ERTS_BPF_LOCAL_TRACE|ERTS_BPF_GLOBAL_TRACE) &&
 	IS_TRACED_FL(p, F_TRACE_CALLS)) {
@@ -937,49 +990,20 @@ static ErtsTracer
 do_call_trace(Process* c_p, ErtsCodeInfo* info, Eterm* reg,
 	      int local, Binary* ms, ErtsTracer tracer)
 {
-    Eterm* cpp;
     int return_to_trace = 0;
-    BeamInstr w;
     BeamInstr *cp_save = c_p->cp;
     Uint32 flags;
     Uint need = 0;
     Eterm* E = c_p->stop;
 
-    w = *c_p->cp;
-    if (w == (BeamInstr) BeamOp(op_return_trace)) {
-	cpp = &E[2];
-    } else if (w == (BeamInstr) BeamOp(op_i_return_to_trace)) {
-	return_to_trace = 1;
-	cpp = &E[0];
-    } else if (w == (BeamInstr) BeamOp(op_i_return_time_trace)) {
-	cpp = &E[0];
-    } else {
-	cpp = NULL;
-    }
-    if (cpp) {
-	for (;;) {
-	    BeamInstr w = *cp_val(*cpp);
-	    if (w == (BeamInstr) BeamOp(op_return_trace)) {
-		cpp += 3;
-	    } else if (w == (BeamInstr) BeamOp(op_i_return_to_trace)) {
-		return_to_trace = 1;
-		cpp += 1;
-	    } else if (w == (BeamInstr) BeamOp(op_i_return_time_trace)) {
-		cpp += 2;
-	    } else {
-		break;
-	    }
-	}
-	cp_save = c_p->cp;
-	c_p->cp = (BeamInstr *) cp_val(*cpp);
-	ASSERT(is_CP(*cpp));
-    }
+    fixup_cp_before_trace(c_p, &return_to_trace);
+
     ERTS_UNREQ_PROC_MAIN_LOCK(c_p);
     flags = erts_call_trace(c_p, info, ms, reg, local, &tracer);
     ERTS_REQ_PROC_MAIN_LOCK(c_p);
-    if (cpp) {
-	c_p->cp = cp_save;
-    }
+
+    /* restore cp after potential fixup */
+    c_p->cp = cp_save;
 
     ASSERT(!ERTS_PROC_IS_EXITING(c_p));
     if ((flags & MATCH_SET_RETURN_TO_TRACE) && !return_to_trace) {
@@ -1293,7 +1317,7 @@ erts_find_local_func(ErtsCodeMFA *mfa) {
     n = (BeamInstr) code_hdr->num_functions;
     for (i = 0; i < n; ++i) {
 	ci = code_hdr->functions[i];
-	ASSERT(((BeamInstr) BeamOp(op_i_func_info_IaaI)) == ci->op);
+	ASSERT(BeamIsOpCode(ci->op, op_i_func_info_IaaI));
 	ASSERT(mfa->module == ci->mfa.module || is_nil(ci->mfa.module));
 	if (mfa->function == ci->mfa.function &&
 	    mfa->arity == ci->mfa.arity) {
@@ -1708,7 +1732,7 @@ check_break(ErtsCodeInfo *ci, Uint break_flags)
 {
     GenericBp* g = ci->u.gen_bp;
 
-    ASSERT(ci->op == (BeamInstr) BeamOp(op_i_func_info_IaaI));
+    ASSERT(BeamIsOpCode(ci->op, op_i_func_info_IaaI));
     if (erts_is_function_native(ci)) {
 	return 0;
     }