File 1171-jit-func_line-alignment-for-line-table.patch of Package erlang
From 52959810fc0f43668753ac5653401bb7a1f25f5d Mon Sep 17 00:00:00 2001
From: Daniel Finke <danielfinke2011@gmail.com>
Date: Tue, 29 Oct 2024 19:07:34 +0100
Subject: [PATCH] jit: func_line alignment for line table
The changes from 5b4457e cause the line table to have extra entries when
handling the last_error_offset. The line number from the subsequent
function is mapped to the nop instruction following the error, and as a
result, the line table suggests that the line is mapped in the former
and current range.
As an example, if you enable logging in jit-reader.c, and look for
sets.erl, you would see something like:
```
Add range `sets:new/0-CodeInfoPrologue` (0x7f5fddf03f18, 0x7f5fddf03f48), 0 lines
Add range `sets:new/0` (0x7f5fddf03f48, 0x7f5fddf04008), 3 lines
sets.erl:170
sets.erl:171
sets.erl:177
Add range `sets:new/1-CodeInfoPrologue` (0x7f5fddf04008, 0x7f5fddf04038), 0 lines
Add range `sets:new/1` (0x7f5fddf04038, 0x7f5fddf04118), 2 lines
sets.erl:177
sets.erl:180
```
which shows that line 177 is included twice, despite only being for
sets:new/1. This can also be identified by using this GDB command:
`maintenance info line-table sets.erl`.
Move func_line back after the func label and emit the last error nop
before the label. This ensures that the line is within the (possibly
aligned) func range.
---
erts/emulator/beam/jit/arm/beam_asm.hpp | 4 +++
.../emulator/beam/jit/arm/beam_asm_module.cpp | 34 ++++++++++++-------
erts/emulator/beam/jit/arm/ops.tab | 11 +++---
erts/emulator/beam/jit/asm_load.c | 3 +-
erts/emulator/beam/jit/x86/beam_asm.hpp | 4 +++
.../emulator/beam/jit/x86/beam_asm_module.cpp | 34 ++++++++++++-------
erts/emulator/beam/jit/x86/ops.tab | 11 +++---
7 files changed, 66 insertions(+), 35 deletions(-)
diff --git a/erts/emulator/beam/jit/arm/beam_asm.hpp b/erts/emulator/beam/jit/arm/beam_asm.hpp
index 27e3affdac..26415a2ae6 100644
--- a/erts/emulator/beam/jit/arm/beam_asm.hpp
+++ b/erts/emulator/beam/jit/arm/beam_asm.hpp
@@ -1401,6 +1401,10 @@ protected:
* the current code position is unreachable. */
void flush_pending_labels();
+ /* Move past the `last_error_offset` if necessary for the next instruction
+ * to be properly aligned (e.g. for line mappings). */
+ void flush_last_error();
+
/* Calls the given shared fragment, ensuring that the redzone is unused and
* that the return address forms a valid CP. */
template<typename Any>
diff --git a/erts/emulator/beam/jit/arm/beam_asm_module.cpp b/erts/emulator/beam/jit/arm/beam_asm_module.cpp
index caec6c0eb8..f940783460 100644
--- a/erts/emulator/beam/jit/arm/beam_asm_module.cpp
+++ b/erts/emulator/beam/jit/arm/beam_asm_module.cpp
@@ -376,6 +376,11 @@ void BeamModuleAssembler::emit_aligned_label(const ArgLabel &Label,
emit_label(Label);
}
+void BeamModuleAssembler::emit_i_func_label(const ArgLabel &Label) {
+ flush_last_error();
+ emit_aligned_label(Label, ArgVal(ArgVal::Word, sizeof(UWord)));
+}
+
void BeamModuleAssembler::emit_on_load() {
on_load = current_label;
}
@@ -430,22 +435,12 @@ void BeamModuleAssembler::emit_int_code_end() {
void BeamModuleAssembler::emit_line(const ArgWord &Loc) {
/* There is no need to align the line instruction. In the loaded code, the
* type of the pointer will be void* and that pointer will only be used in
- * comparisons.
- *
- * We only need to do something when there's a possibility of raising an
- * exception at the very end of the preceding instruction (and thus
- * pointing at the start of this one). If we were to do nothing, the error
- * would erroneously refer to this instead of the preceding line.
- *
- * Since line addresses are taken _after_ line instructions we can avoid
- * this by adding a nop when we detect this condition. */
- if (a.offset() == last_error_offset) {
- a.nop();
- }
+ * comparisons. */
+
+ flush_last_error();
}
void BeamModuleAssembler::emit_func_line(const ArgWord &Loc) {
- emit_line(Loc);
}
void BeamModuleAssembler::emit_empty_func_line() {
@@ -823,3 +818,16 @@ void BeamModuleAssembler::emit_constant(const Constant &constant) {
}
}
}
+
+void BeamModuleAssembler::flush_last_error() {
+ /* When there's a possibility of raising an exception at the very end of the
+ * preceding instruction (and thus pointing at the start of this one) and
+ * this instruction has a new line registered, the error would erroneously
+ * refer to this instead of the preceding line.
+ *
+ * By adding a nop when we detect this condition, the error will correctly
+ * refer to the preceding line. */
+ if (a.offset() == last_error_offset) {
+ a.nop();
+ }
+}
diff --git a/erts/emulator/beam/jit/arm/ops.tab b/erts/emulator/beam/jit/arm/ops.tab
index 73266ec4a0..3b0333a83d 100644
--- a/erts/emulator/beam/jit/arm/ops.tab
+++ b/erts/emulator/beam/jit/arm/ops.tab
@@ -42,12 +42,15 @@ label L
# An label aligned to a certain boundary. This is used in two cases:
#
-# * When the label points to the start of a function, as the ErtsCodeInfo
-# struct must be word-aligned.
+# * When the label points to the start of a function. See `i_func_label`.
# * When the address is stored on the stack or otherwise needs to be properly
# tagged as a continuation pointer.
aligned_label L t
+# A label indicating the start of a function. The label is word-aligned as is
+# required by the ErtsCodeInfo struct.
+i_func_label L
+
i_func_info I a a I
int_code_end
nif_start
@@ -903,8 +906,8 @@ int_func_start Func_Label Func_Line M F A |
func_prologue Entry_Label Entry_Line |
is_mfa_bif(M, F, A) =>
i_flush_stubs |
+ i_func_label Func_Label |
func_line Func_Line |
- aligned_label Func_Label u=8 |
i_func_info Func_Label M F A |
aligned_label Entry_Label u=4 |
i_breakpoint_trampoline |
@@ -914,8 +917,8 @@ int_func_start Func_Label Func_Line M F A |
int_func_start Func_Label Func_Line M F A |
func_prologue Entry_Label Entry_Line =>
i_flush_stubs |
+ i_func_label Func_Label |
func_line Func_Line |
- aligned_label Func_Label u=8 |
i_func_info Func_Label M F A |
aligned_label Entry_Label u=4 |
i_breakpoint_trampoline |
diff --git a/erts/emulator/beam/jit/asm_load.c b/erts/emulator/beam/jit/asm_load.c
index dd337a5d34..91ce7285c2 100644
--- a/erts/emulator/beam/jit/asm_load.c
+++ b/erts/emulator/beam/jit/asm_load.c
@@ -505,7 +505,8 @@ int beam_load_emit_op(LoaderState *stp, BeamOp *tmp_op) {
break;
case 'L': /* Define label */
ASSERT(stp->specific_op == op_label_L ||
- stp->specific_op == op_aligned_label_Lt);
+ stp->specific_op == op_aligned_label_Lt ||
+ stp->specific_op == op_i_func_label_L);
BeamLoadVerifyTag(stp, tag, TAG_u);
stp->last_label = curr->val;
if (stp->last_label < 0 ||
diff --git a/erts/emulator/beam/jit/x86/beam_asm.hpp b/erts/emulator/beam/jit/x86/beam_asm.hpp
index 4a2c965b21..105c9aa41a 100644
--- a/erts/emulator/beam/jit/x86/beam_asm.hpp
+++ b/erts/emulator/beam/jit/x86/beam_asm.hpp
@@ -1424,6 +1424,10 @@ protected:
* appropriate address before jumping there. */
const Label &resolve_fragment(void (*fragment)());
+ /* Move past the `last_error_offset` if necessary for the next instruction
+ * to be properly aligned (e.g. for line mappings). */
+ void flush_last_error();
+
void safe_fragment_call(void (*fragment)()) {
emit_assert_redzone_unused();
a.call(resolve_fragment(fragment));
diff --git a/erts/emulator/beam/jit/x86/beam_asm_module.cpp b/erts/emulator/beam/jit/x86/beam_asm_module.cpp
index 5ad3672c18..36b20842a0 100644
--- a/erts/emulator/beam/jit/x86/beam_asm_module.cpp
+++ b/erts/emulator/beam/jit/x86/beam_asm_module.cpp
@@ -342,6 +342,11 @@ void BeamModuleAssembler::emit_aligned_label(const ArgLabel &Label,
emit_label(Label);
}
+void BeamModuleAssembler::emit_i_func_label(const ArgLabel &Label) {
+ flush_last_error();
+ emit_aligned_label(Label, ArgVal(ArgVal::Word, sizeof(UWord)));
+}
+
void BeamModuleAssembler::emit_on_load() {
on_load = current_label;
}
@@ -362,22 +367,12 @@ void BeamModuleAssembler::emit_int_code_end() {
void BeamModuleAssembler::emit_line(const ArgWord &Loc) {
/* There is no need to align the line instruction. In the loaded code, the
* type of the pointer will be void* and that pointer will only be used in
- * comparisons.
- *
- * We only need to do something when there's a possibility of raising an
- * exception at the very end of the preceding instruction (and thus
- * pointing at the start of this one). If we were to do nothing, the error
- * would erroneously refer to this instead of the preceding line.
- *
- * Since line addresses are taken _after_ line instructions we can avoid
- * this by adding a nop when we detect this condition. */
- if (a.offset() == last_error_offset) {
- a.nop();
- }
+ * comparisons. */
+
+ flush_last_error();
}
void BeamModuleAssembler::emit_func_line(const ArgWord &Loc) {
- emit_line(Loc);
}
void BeamModuleAssembler::emit_empty_func_line() {
@@ -416,3 +411,16 @@ const Label &BeamModuleAssembler::resolve_fragment(void (*fragment)()) {
return it->second;
}
+
+void BeamModuleAssembler::flush_last_error() {
+ /* When there's a possibility of raising an exception at the very end of the
+ * preceding instruction (and thus pointing at the start of this one) and
+ * this instruction has a new line registered, the error would erroneously
+ * refer to this instead of the preceding line.
+ *
+ * By adding a nop when we detect this condition, the error will correctly
+ * refer to the preceding line. */
+ if (a.offset() == last_error_offset) {
+ a.nop();
+ }
+}
diff --git a/erts/emulator/beam/jit/x86/ops.tab b/erts/emulator/beam/jit/x86/ops.tab
index f8a46e1453..5d1c3ff42c 100644
--- a/erts/emulator/beam/jit/x86/ops.tab
+++ b/erts/emulator/beam/jit/x86/ops.tab
@@ -42,12 +42,15 @@ label L
# An label aligned to a certain boundary. This is used in two cases:
#
-# * When the label points to the start of a function, as the ErtsCodeInfo
-# struct must be word-aligned.
+# * When the label points to the start of a function. See `i_func_label`.
# * When the address is stored on the stack or otherwise needs to be properly
# tagged as a continuation pointer.
aligned_label L t
+# A label indicating the start of a function. The label is word-aligned as is
+# required by the ErtsCodeInfo struct.
+i_func_label L
+
i_func_info I a a I
int_code_end
nif_start
@@ -827,8 +830,8 @@ int_func_start Func_Label Func_Line M F A |
int_func_start Func_Label Func_Line M F A |
func_prologue Entry_Label Entry_Line |
is_mfa_bif(M, F, A) =>
+ i_func_label Func_Label |
func_line Func_Line |
- aligned_label Func_Label u=8 |
i_func_info Func_Label M F A |
aligned_label Entry_Label u=4 |
i_breakpoint_trampoline |
@@ -837,8 +840,8 @@ int_func_start Func_Label Func_Line M F A |
int_func_start Func_Label Func_Line M F A |
func_prologue Entry_Label Entry_Line =>
+ i_func_label Func_Label |
func_line Func_Line |
- aligned_label Func_Label u=8 |
i_func_info Func_Label M F A |
aligned_label Entry_Label u=4 |
i_breakpoint_trampoline |
--
2.43.0
