Merge pull request #1589 from bjorng/bjorn/erts/instruction-offsets

Point out the correct line number in stack traces

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};
     }