New issue
Advanced search Search tips

Issue 660379 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 661510



Sign in to add a comment

Difference between default and ignition with natives

Project Member Reported by machenb...@chromium.org, Oct 28 2016

Issue description

The following js code has different behavior with and without --ignition. Is that expected? If yes, we might need to disable native syntax entirely.

  function __f_5() {
    try {
      __f_4();
    } catch (e) {
    }
  }
  function __f_4() {
    %DeoptimizeFunction(__f_5);
    throw "boom!";
  }
  %OptimizeFunctionOnNextCall(__f_5);
 __f_5();


Correctness fuzzer output:

machenbach@malumi:~/v8/clusterfuzz/src/tools/minimizer master $ /usr/bin/python /usr/local/google/home/machenbach/v8/clusterfuzz-data/workbench/fuzzme.py --random-seed 430512599 --d8 /usr/local/google/home/machenbach/v8/v8/out.gn/x64.release/d8 --v8-root /usr/local/google/home/machenbach/v8/v8 --flags "--gc-interval=413 --turbo --turbo-escape" default ignition /usr/local/google/home/machenbach/v8/clusterfuzz-data/workbench/out3/fuzz-03869.minimized.js 
# Result: Differed!

# CHECK

# Compared default with ignition

# Flags of default:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --gc-interval=413 --turbo --turbo-escape --random-seed 430512599
# Flags of ignition:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --gc-interval=413 --turbo --turbo-escape --random-seed 430512599 --ignition

Difference:
Different total output lines: 0 vs. 4

### Start of configuration default:

### End of configuration default

### Start of configuration ignition:
/usr/local/google/home/machenbach/v8/clusterfuzz-data/workbench/out3/fuzz-03869.minimized.js:9: boom!
    throw "boom!";
    ^


### End of configuration ignition


 
Labels: Project-Ignition
Owner: mstarzinger@chromium.org
Hmm, this seems like a bug :/. It looks like we are not catching the exception with Ignition where we should be. It looks like it might be related to turbofan inlining - when I pass --no-turbo-inlining we correctly catch the exception.

Michi - could you have a look?
Labels: -Project-Ignition Proj-Ignition
Cc: bmeu...@chromium.org jarin@chromium.org
Status: Assigned (was: Untriaged)
Yep, will take a look. Thanks!
This is an interesting issue. When the deoptimizer (lazily) constructs an interpreter frame the bytecode-offset points to after the call instruction. This makes the frame look like as if the bytecode-offset has already been bumped and the Builtins::kInterpreterEnterBytecodeDispatch just performs the dispatch without bumping the bytecode-offset.

Normally this approach works fine. However when the stack unwinding logic inspects such a frame, the bytecode-offset might fall outside of the try-block when the call is the very last instruction within the try-block. I am not yet entirely sure how to fix this, more thinking required.

The following is a shortened version of the necessary debug trace to demonstrate the issue. Note that the try-block of the {__f_5} function spans the (4,14) range right-hand side exclusive. The deoptimizer constructs a frame for {__f_5} with the bytecode-offset being 14. Normal interpretation of the function would keep a bytecode-offset of 9 until after the callee returns.

[generating bytecode for function: __f_5]
Parameter count 1
Frame size 32
  263 E> 0xa587acafb05 @    0 : 77                StackCheck 
         0xa587acafb06 @    1 : 22 ff f9          Mov <context>, r1
  280 S> 0xa587acafb09 @    4 : 0a f7             LdrUndefined r3
         0xa587acafb0b @    6 : 0c 04 f8          LdrGlobal [4], r2
  280 E> 0xa587acafb0e @    9 : 43 f8 f7 01 02    Call r2, r3-r3, [2]
         0xa587acafb13 @   14 : 62 17             Jump [23] (0xa587acafb2a @ 37)
         0xa587acafb15 @   16 : 21 f8             Star r2
         0xa587acafb17 @   18 : 20 fe             Ldar <closure>
         0xa587acafb19 @   20 : 5c f8 00 01       CreateCatchContext r2, [0], [1]
         0xa587acafb1d @   24 : 21 f9             Star r1
         0xa587acafb1f @   26 : 46 b6 00 fa 00    CallRuntime [182], r0-r0
         0xa587acafb24 @   31 : 20 f9             Ldar r1
         0xa587acafb26 @   33 : 10 fa             PushContext r0
         0xa587acafb28 @   35 : 11 fa             PopContext r0
         0xa587acafb2a @   37 : 04                LdaUndefined 
  309 S> 0xa587acafb2b @   38 : 7a                Return 
Constant pool (size = 2)
0xa587acafab1: [FixedArray]
 - length: 2
  [0]: 0x3b09d1888791 <String[1]: e>
  [1]: 0xa587acaf9a9 <FixedArray[7]>
Handler Table (size = 48)
   from   to       hdlr
  (   4,  14)  ->    16 (prediction=1, data=1)

[deoptimizing (DEOPT lazy): begin 0xa587acaf859 <JS Function __f_5 (SharedFunctionInfo 0xa587acaf529)> (opt #0) @0, FP to SP delta: 32, caller sp: 0x7fffecb46898]
  reading input frame __f_5 => bytecode_offset=14, args=1, height=5; inputs:
      0: 0xa587acaf859 ; [fp - 16] 0xa587acaf859 <JS Function __f_5 (SharedFunctionInfo 0xa587acaf529)>
      1: 0x171180a03209 ; [fp + 16] 0x171180a03209 <JS Global Object>
      2: 0xa587ac83951 ; [fp - 8] 0xa587ac83951 <FixedArray[235]>
      3: 0x3b09d1884c11 ; (literal 3) 0x3b09d1884c11 <Odd Oddball: optimized_out>
      4: 0xa587ac83951 ; [fp - 8] 0xa587ac83951 <FixedArray[235]>
      5: 0x3b09d1884c11 ; (literal 3) 0x3b09d1884c11 <Odd Oddball: optimized_out>
      6: 0x3b09d1884c11 ; (literal 3) 0x3b09d1884c11 <Odd Oddball: optimized_out>
      7: 0x3b09d1884c11 ; (literal 3) 0x3b09d1884c11 <Odd Oddball: optimized_out>
  reading input frame __f_4 => bytecode_offset=9, args=1, height=2; inputs:
      0: 0xa587acaf8d9 ; (literal 4) 0xa587acaf8d9 <JS Function __f_4 (SharedFunctionInfo 0xa587acaf5f9)>
      1: 0x171180a03209 ; (literal 5) 0x171180a03209 <JS Global Object>
      2: 0xa587ac83951 ; (literal 6) 0xa587ac83951 <FixedArray[235]>
      3: 0x3b09d1884c11 ; (literal 3) 0x3b09d1884c11 <Odd Oddball: optimized_out>
      4: 0x3b09d1882311 ; rax 0x3b09d1882311 <undefined>
  translating interpreted frame __f_5 => bytecode_offset=14, height=32
    0x7fffecb46890: [top + 88] <- 0x171180a03209 ;  0x171180a03209 <JS Global Object>  (input #1)
    -------------------------
    0x7fffecb46888: [top + 80] <- 0x22aabecfd29b ;  caller's pc
    0x7fffecb46880: [top + 72] <- 0x7fffecb468f0 ;  caller's fp
    0x7fffecb46878: [top + 64] <- 0xa587ac83951 ;  context    0xa587ac83951 <FixedArray[235]>  (input #2)
    0x7fffecb46870: [top + 56] <- 0xa587acaf859 ;  function    0xa587acaf859 <JS Function __f_5 (SharedFunctionInfo 0xa587acaf529)>  (input #0)
    0x7fffecb46868: [top + 48] <- 0x3b09d1882311 ;  new_target  0x3b09d1882311 <undefined>  (input #0)
    0x7fffecb46860: [top + 40] <- 0xa587acafad1 ;  bytecode array 0xa587acafad1 <BytecodeArray[39]>  (input #0)
    0x7fffecb46858: [top + 32] <- 0x4200000000 ;  bytecode offset 66  (input #0)
    -------------------------
    0x7fffecb46850: [top + 24] <- 0x3b09d1884c11 ;  0x3b09d1884c11 <Odd Oddball: optimized_out>  (input #3)
    0x7fffecb46848: [top + 16] <- 0xa587ac83951 ;  0xa587ac83951 <FixedArray[235]>  (input #4)
    0x7fffecb46840: [top + 8] <- 0x3b09d1884c11 ;  0x3b09d1884c11 <Odd Oddball: optimized_out>  (input #5)
    0x7fffecb46838: [top + 0] <- 0x3b09d1884c11 ;  0x3b09d1884c11 <Odd Oddball: optimized_out>  (input #6)
  translating interpreted frame __f_4 => bytecode_offset=9, height=16
    0x7fffecb46830: [top + 72] <- 0x171180a03209 ;  0x171180a03209 <JS Global Object>  (input #1)
    -------------------------
    0x7fffecb46828: [top + 64] <- 0x22aabecebe80 ;  caller's pc
    0x7fffecb46820: [top + 56] <- 0x7fffecb46880 ;  caller's fp
    0x7fffecb46818: [top + 48] <- 0xa587ac83951 ;  context    0xa587ac83951 <FixedArray[235]>  (input #2)
    0x7fffecb46810: [top + 40] <- 0xa587acaf8d9 ;  function    0xa587acaf8d9 <JS Function __f_4 (SharedFunctionInfo 0xa587acaf5f9)>  (input #0)
    0x7fffecb46808: [top + 32] <- 0x3b09d1882311 ;  new_target  0x3b09d1882311 <undefined>  (input #0)
    0x7fffecb46800: [top + 24] <- 0xa587acafd49 ;  bytecode array 0xa587acafd49 <BytecodeArray[12]>  (input #0)
    0x7fffecb467f8: [top + 16] <- 0x3d00000000 ;  bytecode offset 61  (input #0)
    -------------------------
    0x7fffecb467f0: [top + 8] <- 0x3b09d1884c11 ;  0x3b09d1884c11 <Odd Oddball: optimized_out>  (input #3)
    0x7fffecb467e8: [top + 0] <- 0x3b09d1882311 ;  accumulator 0x3b09d1882311 <undefined>  (input #4)
[deoptimizing (lazy): end 0xa587acaf859 <JS Function __f_5 (SharedFunctionInfo 0xa587acaf529)> @0 => node=9, pc=0x22aabecebe80, caller sp=0x7fffecb46898, state=TOS_REGISTER, took 0.576 ms]

Just for reference for thinking about how to do clustering of similar issues. Fuzzer re-reported the error as (here, the first __f_3() call is needed to repro):

function __f_3() {
  try {
    __f_2();
  } catch (e) {
  }
}
function __f_2() {
  %DeoptimizeFunction(__f_3);
  throw 42;
}
__f_3();
 %OptimizeFunctionOnNextCall(__f_3);
__f_3();


# Flags of default:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --gc-interval=473 --turbo-splitting --random-seed 1068817050
# Flags of ignition:
--abort_on_stack_overflow --expose-gc --allow-natives-syntax --invoke-weak-callbacks --omit-quit --gc-interval=473 --turbo-splitting --random-seed 1068817050 --ignition

Blocking: -650214 661510

Comment 7 by neis@chromium.org, Nov 7 2016

Cc: neis@chromium.org
 Issue 662415  has been merged into this issue.
Cc: jgruber@chromium.org
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/93c65952009a611b840142e63237c58c7267bfd1

commit 93c65952009a611b840142e63237c58c7267bfd1
Author: mstarzinger <mstarzinger@chromium.org>
Date: Thu Nov 10 11:34:46 2016

[turbofan] Advance bytecode offset after lazy deopt.

This changes {FrameState} nodes modeling "after" states to use bytecode
offsets pointing to the deoptimizing bytecode. This is in sync with the
normal execution, as the bytecode offset is advanced after operations
complete in regular bytecode handlers.

The change is necessary to ensure lazy deoptimized frames contain an
accurate bytecode offset while they are on the stack. Such frames can be
inspected by various stack walks. The continuation builtin will advance
the bytecode offset upon return.

R=jarin@chromium.org
TEST=mjsunit/regress/regress-crbug-660379
BUG= chromium:660379 

Review-Url: https://codereview.chromium.org/2487173002
Cr-Commit-Position: refs/heads/master@{#40887}

[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/builtins/arm/builtins-arm.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/builtins/arm64/builtins-arm64.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/builtins/builtins.h
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/builtins/ia32/builtins-ia32.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/builtins/mips/builtins-mips.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/builtins/mips64/builtins-mips64.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/builtins/x64/builtins-x64.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/compiler/bytecode-graph-builder.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/deoptimizer.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/frames.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/objects-inl.h
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/runtime/runtime-interpreter.cc
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/src/runtime/runtime.h
[modify] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/test/debugger/debugger.status
[add] https://crrev.com/93c65952009a611b840142e63237c58c7267bfd1/test/mjsunit/regress/regress-crbug-660379.js

Status: Fixed (was: Assigned)
 Issue 662915  has been merged into this issue.
Labels: -Restrict-View-Google -Needs-Feedback v8-foozzie-failure

Sign in to add a comment