File 1192-erts-Fix-stack-inconsistency-in-debug-brea... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 1192-erts-Fix-stack-inconsistency-in-debug-breakpoints.patch of Package erlang (Revision da2dbf8c86da2c50cb07148e5a4ddd44)

Currently displaying revision da2dbf8c86da2c50cb07148e5a4ddd44 , Show latest

From f14c4c42cd7203417c27a8c11f0cdb490554abac Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?John=20H=C3=B6gberg?= <john@erlang.org>
Date: Tue, 10 Aug 2021 12:16:45 +0200
Subject: [PATCH 2/3] erts: Fix stack inconsistency in debug breakpoints

---
 erts/emulator/beam/jit/x86/instr_trace.cpp | 25 ++++++++++++----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/erts/emulator/beam/jit/x86/instr_trace.cpp b/erts/emulator/beam/jit/x86/instr_trace.cpp
index 06fed56d19..128fbbe615 100644
--- a/erts/emulator/beam/jit/x86/instr_trace.cpp
+++ b/erts/emulator/beam/jit/x86/instr_trace.cpp
@@ -57,8 +57,14 @@ void BeamGlobalAssembler::emit_generic_bp_local() {
     emit_assert_erlang_stack();
 
 #ifdef NATIVE_ERLANG_STACK
-    /* Since we've entered here on the Erlang stack, we need to stash our return
-     * addresses in case `erts_generic_breakpoint` pushes any trace frames. */
+    /* Since we've entered here on the Erlang stack, we need to stash our
+     * return addresses in case `erts_generic_breakpoint` pushes any trace
+     * frames.
+     *
+     * Note that both of these are return addresses even when frame pointers
+     * are enabled due to the way the breakpoint trampoline works. They must
+     * not be restored until we're ready to return to module code, lest we
+     * leave the stack in an inconsistent state. */
     a.pop(TMP_MEM2q);
     a.pop(ARG2);
 #else
@@ -100,14 +106,17 @@ void BeamGlobalAssembler::emit_generic_bp_local() {
      * corresponding comment in `generic_bp_global` */
     emit_leave_frame();
 
+    a.cmp(RET, imm(BeamOpCodeAddr(op_i_debug_breakpoint)));
+    a.je(labels[debug_bp]);
+
+    /* Note that we don't restore our return addresses in the `debug_bp` case
+     * above, since it tail calls the error handler and thus never returns to
+     * module code or `call_nif_early`. */
 #ifdef NATIVE_ERLANG_STACK
     a.push(TMP_MEM1q);
     a.push(TMP_MEM2q);
 #endif
 
-    a.cmp(RET, imm(BeamOpCodeAddr(op_i_debug_breakpoint)));
-    a.je(labels[debug_bp]);
-
     a.ret();
 }
 
@@ -136,12 +145,6 @@ void BeamGlobalAssembler::emit_debug_bp() {
     emit_leave_runtime<Update::eReductions | Update::eStack | Update::eHeap>();
     emit_leave_frame();
 
-    /* Skip two return addresses so we can make a direct jump to the error
-     * handler. This makes it so that if we are to do a call_nif_early, we skip
-     * that and call the error handler's code instead, mirroring the way the
-     * interpreter works. */
-    a.add(x86::rsp, imm(16));
-
     a.test(RET, RET);
     a.je(error);
 
-- 
2.31.1

Places

File 1192-erts-Fix-stack-inconsistency-in-debug-breakpoints.patch of Package erlang (Revision da2dbf8c86da2c50cb07148e5a4ddd44)

Places