aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBjörn Gustavsson <[email protected]>2017-09-30 07:48:34 +0200
committerBjörn Gustavsson <[email protected]>2017-09-30 09:43:33 +0200
commit9a50a5d5fc1c74d91efad74e1cb3e78f38e2b75c (patch)
tree5999ee43220e0c7823f4651b1f3b5731825804b3
parentd0875a87fc59bea515294455b2047040118f3a7e (diff)
downloadotp-9a50a5d5fc1c74d91efad74e1cb3e78f38e2b75c.tar.gz
otp-9a50a5d5fc1c74d91efad74e1cb3e78f38e2b75c.tar.bz2
otp-9a50a5d5fc1c74d91efad74e1cb3e78f38e2b75c.zip
Point out the correct line number in stack traces
Sometimes the line number in a stack trace could be wrong, for example for this code: t() -> Res = id(x), %<== Wrong line number. Res + 1. id(I) -> I. The line number pointed out in the stack trace would be the line before the line where the exception occurred. The reason is the way the increment instruction instruction is implemented: OpCase(i_increment_rWtd): { increment_reg_val = r(0); } I -= 1; goto increment__execute; OpCase(i_increment_xWtd): { increment_reg_val = xb(I[1]); } goto increment__execute; increment__execute: /* Common code for increment */ . . . (The implementation in OTP 20 is similar, but hand-coded directly in beam_emu.c instead of generated.) The instruction i_increment_rWtd decrements the instruction pointer (I) before jumping to the common code. That means that I points *before* the 'increment' instruction. If there is a 'line' instruction directly before the 'increment' instruction (as there is in this example), the instruction pointer will point before that line. Thus the previous line will be picked up instead. To eliminate this bug, we must never decrement the instruction pointer. Instead, we can increment the other (longer) instructions in the same group of combined instructions: OpCase(i_increment_rWtd): { increment_reg_val = r(0); } goto increment__execute; OpCase(i_increment_xWtd): { increment_reg_val = xb(I[1]); } I += 1; goto increment__execute; increment__execute: /* Common code for increment */ . . . Also fix a bug that was only a potential bug when ddaed7774eb0a introduced relative jumps, but is now a real bug. See the added comment for SET_I_REL() in macros.tab.
-rw-r--r--erts/emulator/beam/macros.tab11
-rw-r--r--erts/emulator/test/exception_SUITE.erl21
-rwxr-xr-xerts/emulator/utils/beam_makeops25
3 files changed, 45 insertions, 12 deletions
diff --git a/erts/emulator/beam/macros.tab b/erts/emulator/beam/macros.tab
index 6f9b78af6f..0d175a7ec6 100644
--- a/erts/emulator/beam/macros.tab
+++ b/erts/emulator/beam/macros.tab
@@ -28,9 +28,18 @@ REFRESH_GEN_DEST() {
dst_ptr = REG_TARGET_PTR(dst);
}
+// $Offset is relative to the start of the instruction (not to the
+// location of the failure label reference). Since combined
+// instructions may increment the instruction pointer (e.g. in
+// 'increment') for some of the instructions in the group, we actually
+// use a virtual start position common to all instructions in the
+// group. To calculate the correct virtual position, we will need to
+// add $IP_ADJUSTMENT to the offset. ($IP_ADJUSTMENT will usually be
+// zero, except in a few bit syntax instructions.)
+
SET_I_REL(Offset) {
ASSERT(VALID_INSTR(*(I + ($Offset))));
- I += $Offset;
+ I += $Offset + $IP_ADJUSTMENT;
}
SET_CP_I_ABS(Target) {
diff --git a/erts/emulator/test/exception_SUITE.erl b/erts/emulator/test/exception_SUITE.erl
index e473a10be7..0f27251fcb 100644
--- a/erts/emulator/test/exception_SUITE.erl
+++ b/erts/emulator/test/exception_SUITE.erl
@@ -662,6 +662,15 @@ line_numbers(Config) when is_list(Config) ->
{?MODULE,line_numbers,1,_}|_]}} =
(catch applied_bif_2()),
+ {'EXIT',{badarith,
+ [{?MODULE,increment1,1,[{file,"increment.erl"},{line,45}]},
+ {?MODULE,line_numbers,1,_}|_]}} =
+ (catch increment1(x)),
+ {'EXIT',{badarith,
+ [{?MODULE,increment2,1,[{file,"increment.erl"},{line,48}]},
+ {?MODULE,line_numbers,1,_}|_]}} =
+ (catch increment2(x)),
+
ok.
id(I) -> I.
@@ -762,3 +771,15 @@ applied_bif_2() -> %Line 8
R = process_info(self(), current_location), %Line 9
fail = R, %Line 10
ok. %Line 11
+
+%% The increment instruction used to decrement the instruction
+%% pointer, which would cause the line number in a stack trace to
+%% be the previous line number.
+
+-file("increment.erl", 42).
+increment1(Arg) -> %Line 43
+ Res = id(Arg), %Line 44
+ Res + 1. %Line 45
+increment2(Arg) -> %Line 46
+ _ = id(Arg), %Line 47
+ Arg + 1. %Line 48
diff --git a/erts/emulator/utils/beam_makeops b/erts/emulator/utils/beam_makeops
index a9b2c8861c..e4c91f1685 100755
--- a/erts/emulator/utils/beam_makeops
+++ b/erts/emulator/utils/beam_makeops
@@ -1111,7 +1111,7 @@ sub combine_instruction_group {
my %offsets;
my @instrs;
my %num_references;
- my $group_size = 0;
+ my $group_size = 999;
# Do basic error checking. Associate operands of instructions
# with the correct micro instructions. Calculate offsets for micro
@@ -1140,7 +1140,7 @@ sub combine_instruction_group {
}
my $size = cg_combined_size($s, 1, @first);
$offsets{$s} = $offset
- unless defined $offsets{$s} and $offsets{$s} >= $offset;
+ unless defined $offsets{$s} and $offsets{$s} < $offset;
$offset += $size - 1;
my $label = micro_label($s);
$num_references{$label} = 0;
@@ -1148,7 +1148,7 @@ sub combine_instruction_group {
$opcase = '';
}
$spec_op_info{$print_name}->{'size'} = $offset + 1;
- $group_size = $offset if $group_size < $offset;
+ $group_size = $offset if $group_size >= $offset;
push @instrs, [$specific_key,@new_subs];
}
}
@@ -1218,19 +1218,19 @@ sub combine_instruction_group {
my $flags = '';
my $transfer_to_next = '';
- my $dec = 0;
+ my $inc = 0;
unless ($i == $#slots) {
$flags = "-no_next";
my $next_offset = $label_to_offset{$next};
- $dec = $next_offset - ($offset + $size);
- $transfer_to_next = "I -= $dec;\n" if $dec;
+ $inc = ($offset + $size) - $next_offset;
+ $transfer_to_next = "I += $inc;\n" if $inc;
$transfer_to_next .= "goto $next;\n\n";
}
my($gen_code,$down,$up) =
cg_combined_code($s, 1, $flags, $offset,
- $group_size-$offset-$dec, @first);
+ $group_size-$offset, $inc, @first);
my $spec_label = "$opcase$label";
$down{$spec_label} = $down;
$up{$spec_label} = $up;
@@ -1280,7 +1280,7 @@ sub micro_label {
sub cg_basic {
my($name,@args) = @_;
- my($size,$code,$pack_spec) = code_gen($name, 1, '', 0, undef, @args);
+ my($size,$code,$pack_spec) = code_gen($name, 1, '', 0, undef, undef, @args);
$pack_spec = build_pack_spec($pack_spec);
($size,$code,$pack_spec);
}
@@ -1291,7 +1291,7 @@ sub cg_basic {
sub cg_combined_size {
my($name,$pack,@args) = @_;
- my($size) = code_gen($name, $pack, '', 0, undef, @args);
+ my($size) = code_gen($name, $pack, '', 0, undef, undef, @args);
$size;
}
@@ -1300,6 +1300,7 @@ sub cg_combined_size {
#
sub cg_combined_code {
+ my($name,$pack,$extra_comments,$offset,$comp_size,$inc,@args) = @_;
my($size,$code,$pack_spec) = code_gen(@_);
if ($pack_spec eq '') {
($code,'','');
@@ -1310,7 +1311,8 @@ sub cg_combined_code {
}
sub code_gen {
- my($name,$pack,$extra_comments,$offset,$group_size,@args) = @_;
+ my($name,$pack,$extra_comments,$offset,$comp_size,$inc,@args) = @_;
+ my $group_size = defined $comp_size ? $comp_size + $inc : undef;
my $size = 0;
my $flags = '';
my @f;
@@ -1416,6 +1418,7 @@ sub code_gen {
$bindings{$var} = $f[$i];
}
$bindings{'NEXT_INSTRUCTION'} = "I+" . ($group_size+$offset+1);
+ $bindings{'IP_ADJUSTMENT'} = defined $inc ? $inc : 0;
$c_code = eval { expand_all($c_code, \%bindings) };
unless (defined $c_code) {
warn $@;
@@ -1539,7 +1542,7 @@ sub expand_macro {
my %new_bindings;
# Keep the special, pre-defined bindings.
- foreach my $key (qw(NEXT_INSTRUCTION)) {
+ foreach my $key (qw(NEXT_INSTRUCTION IP_ADJUSTMENT)) {
$new_bindings{$key} = $bindings{$key};
}