Skip to content

Commit

Permalink
jit: func_line alignment for line table
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
danielfinke committed Oct 29, 2024
1 parent 36fb786 commit e5a9522
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 35 deletions.
4 changes: 4 additions & 0 deletions erts/emulator/beam/jit/arm/beam_asm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1401,6 +1401,10 @@ class BeamModuleAssembler : public BeamAssembler,
* 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>
Expand Down
34 changes: 21 additions & 13 deletions erts/emulator/beam/jit/arm/beam_asm_module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
}
}
11 changes: 7 additions & 4 deletions erts/emulator/beam/jit/arm/ops.tab
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down
3 changes: 2 additions & 1 deletion erts/emulator/beam/jit/asm_load.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
4 changes: 4 additions & 0 deletions erts/emulator/beam/jit/x86/beam_asm.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1424,6 +1424,10 @@ class BeamModuleAssembler : public BeamAssembler,
* 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));
Expand Down
34 changes: 21 additions & 13 deletions erts/emulator/beam/jit/x86/beam_asm_module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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() {
Expand Down Expand Up @@ -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 exceptions 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();
}
}
11 changes: 7 additions & 4 deletions erts/emulator/beam/jit/x86/ops.tab
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down

0 comments on commit e5a9522

Please sign in to comment.