Skip to content

Commit c78ec31

Browse files
committed
erts: Fix crash in erts_validate_stack()
during call_memory tracing with +JPperf true. Problem: erts_validate_stack() checks BeamIsReturnCallAccTrace(p->i) but p->i was not updated after returning from erts_call_trace_return(), so it assumed it's still such a frame on the stack. Solution: Make sure p->i is updated after we "return" from trace stack frames.
1 parent d04f6a0 commit c78ec31

File tree

7 files changed

+45
-13
lines changed

7 files changed

+45
-13
lines changed

erts/emulator/beam/erl_gc.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3853,7 +3853,10 @@ void erts_validate_stack(Process *p, Eterm *frame_ptr, Eterm *stack_top) {
38533853
ASSERT((next_fp != NULL) ^ (stack_top == stack_bottom));
38543854

38553855
/* If the GC happens when we are about to execute a trace we
3856-
need to skip the trace instructions */
3856+
need to skip the trace instructions.
3857+
Note: It's not safe in general to assume p->i is up-to-date in GC.
3858+
However the return trace intructions do update p->i after returning.
3859+
*/
38573860
if (BeamIsReturnTrace(p->i)) {
38583861
/* Skip MFA and tracer. */
38593862
ASSERT_MFA((ErtsCodeMFA*)cp_val(scanner[0]));

erts/emulator/beam/jit/arm/beam_asm.hpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1374,7 +1374,8 @@ class BeamModuleAssembler : public BeamAssembler,
13741374
void emit_tuple_assertion(const ArgSource &Src, a64::Gp tuple_reg);
13751375
#endif
13761376

1377-
void emit_dispatch_return();
1377+
void emit_return_do(bool set_I);
1378+
void emit_dispatch_return(bool set_I);
13781379

13791380
#include "beamasm_protos.h"
13801381

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,13 @@ void BeamGlobalAssembler::emit_dispatch_return() {
3535
a.b(labels[context_switch_simplified]);
3636
}
3737

38-
void BeamModuleAssembler::emit_dispatch_return() {
38+
void BeamModuleAssembler::emit_dispatch_return(bool set_I) {
3939
#ifdef JIT_HARD_DEBUG
4040
/* Validate return address and {x,0} */
4141
emit_validate(ArgVal(ArgVal::Type::Word, 1));
4242
#endif
4343

44-
if (erts_alcu_enable_code_atags) {
44+
if (erts_alcu_enable_code_atags || set_I) {
4545
/* See emit_i_test_yield. */
4646
a.str(a64::x30, arm::Mem(c_p, offsetof(Process, i)));
4747
}
@@ -57,13 +57,17 @@ void BeamModuleAssembler::emit_dispatch_return() {
5757
}
5858

5959
void BeamModuleAssembler::emit_return() {
60+
emit_return_do(false);
61+
}
62+
63+
void BeamModuleAssembler::emit_return_do(bool set_I) {
6064
emit_leave_erlang_frame();
61-
emit_dispatch_return();
65+
emit_dispatch_return(set_I);
6266
}
6367

6468
void BeamModuleAssembler::emit_move_deallocate_return() {
6569
a.ldp(XREG0, a64::x30, arm::Mem(E).post(16));
66-
emit_dispatch_return();
70+
emit_dispatch_return(false);
6771
}
6872

6973
void BeamModuleAssembler::emit_i_call(const ArgLabel &CallTarget) {

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,10 @@ void BeamModuleAssembler::emit_return_trace() {
164164
emit_leave_runtime<Update::eHeapAlloc>(1);
165165

166166
emit_deallocate(ArgVal(ArgVal::Type::Word, BEAM_RETURN_TRACE_FRAME_SZ));
167-
emit_return();
167+
168+
/* Return and set c_p->i to avoid calls to BeamIsReturnTrace(c_p->i)
169+
to assume there is still a trace frame on the stack. */
170+
emit_return_do(true);
168171
}
169172

170173
void BeamModuleAssembler::emit_i_call_trace_return() {
@@ -189,7 +192,10 @@ void BeamModuleAssembler::emit_i_call_trace_return() {
189192

190193
emit_deallocate(
191194
ArgVal(ArgVal::Type::Word, BEAM_RETURN_CALL_ACC_TRACE_FRAME_SZ));
192-
emit_return();
195+
196+
/* Return and set c_p->i to avoid calls to BeamIsReturnCallAccTrace(c_p->i)
197+
to assume there is still a trace frame on the stack. */
198+
emit_return_do(true);
193199
}
194200

195201
void BeamModuleAssembler::emit_i_return_to_trace() {
@@ -207,7 +213,10 @@ void BeamModuleAssembler::emit_i_return_to_trace() {
207213
emit_leave_runtime<Update::eHeapAlloc>(1);
208214

209215
emit_deallocate(ArgVal(ArgVal::Type::Word, BEAM_RETURN_TO_TRACE_FRAME_SZ));
210-
emit_return();
216+
217+
/* Return and set c_p->i to avoid calls to BeamIsReturnToTrace(c_p->i)
218+
to assume there is still a trace frame on the stack. */
219+
emit_return_do(true);
211220
}
212221

213222
void BeamModuleAssembler::emit_i_hibernate() {

erts/emulator/beam/jit/x86/beam_asm.hpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,6 +1520,8 @@ class BeamModuleAssembler : public BeamAssembler,
15201520
void emit_tuple_assertion(const ArgSource &Src, x86::Gp tuple_reg);
15211521
#endif
15221522

1523+
void emit_return_do(bool set_I);
1524+
15231525
#include "beamasm_protos.h"
15241526

15251527
const Label &resolve_beam_label(const ArgLabel &Lbl) const {

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,10 @@ void BeamGlobalAssembler::emit_dispatch_return() {
4141
}
4242

4343
void BeamModuleAssembler::emit_return() {
44+
emit_return_do(false);
45+
}
46+
47+
void BeamModuleAssembler::emit_return_do(bool set_I) {
4448
#ifdef JIT_HARD_DEBUG
4549
/* Validate return address and {x,0} */
4650
emit_validate(ArgWord(1));
@@ -53,7 +57,7 @@ void BeamModuleAssembler::emit_return() {
5357
a.mov(getCPRef(), imm(NIL));
5458
#endif
5559

56-
if (erts_alcu_enable_code_atags) {
60+
if (erts_alcu_enable_code_atags || set_I) {
5761
/* See emit_i_test_yield. */
5862
#if defined(NATIVE_ERLANG_STACK)
5963
a.mov(ARG3, x86::qword_ptr(E));

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,10 @@ void BeamModuleAssembler::emit_return_trace() {
194194
emit_leave_runtime<Update::eHeapAlloc>();
195195

196196
emit_deallocate(ArgWord(BEAM_RETURN_TRACE_FRAME_SZ));
197-
emit_return();
197+
198+
/* Return and set c_p->i to avoid calls to BeamIsReturnTrace(c_p->i)
199+
to assume there is still a trace frame on the stack. */
200+
emit_return_do(true);
198201
}
199202

200203
void BeamModuleAssembler::emit_i_call_trace_return() {
@@ -217,7 +220,10 @@ void BeamModuleAssembler::emit_i_call_trace_return() {
217220
emit_leave_runtime<Update::eHeapAlloc>();
218221

219222
emit_deallocate(ArgWord(BEAM_RETURN_CALL_ACC_TRACE_FRAME_SZ));
220-
emit_return();
223+
224+
/* Return and set c_p->i to avoid calls to BeamIsReturnCallAccTrace(c_p->i)
225+
to assume there is still a trace frame on the stack. */
226+
emit_return_do(true);
221227
}
222228

223229
void BeamModuleAssembler::emit_i_return_to_trace() {
@@ -239,7 +245,10 @@ void BeamModuleAssembler::emit_i_return_to_trace() {
239245
emit_leave_runtime<Update::eReductions | Update::eHeapAlloc>();
240246

241247
emit_deallocate(ArgWord(BEAM_RETURN_TO_TRACE_FRAME_SZ));
242-
emit_return();
248+
249+
/* Return and set c_p->i to avoid calls to BeamIsReturnToTrace(c_p->i)
250+
to assume there is still a trace frame on the stack. */
251+
emit_return_do(true);
243252
}
244253

245254
void BeamModuleAssembler::emit_i_hibernate() {

0 commit comments

Comments
 (0)