Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(21)

Issue 2563613003: Revert of [wasm] Fix location for error in asm.js ToNumber conversion (Closed)

Created:
4 years ago by Clemens Hammacher
Modified:
4 years ago
CC:
jgruber, v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Revert of [wasm] Fix location for error in asm.js ToNumber conversion (patchset #5 id:80001 of https://codereview.chromium.org/2555243002/ ) Reason for revert: gc-stress failures Original issue's description: > [wasm] Fix location for error in asm.js ToNumber conversion > > In the asm.js code translated to wasm, we call imported functions via a > WASM_TO_JS stub, which first calls the function and then calls ToNumber > on the return value. Exceptions can happen in both calls. > We were only ever reporting the location of the function call, whereas > asm.js code executed via turbofan reported the location of the type > coercion operator ("+" on "+foo()" or "|" on "foo()|0"). > > This CL implements the same behaviour for asm.js code translated to > wasm. The following is changed: > - the AsmWasmBuilder records the parent node when descending on a binary > operator (also "+foo()" is represented by a binary operation). > - it stores not one location per call in the source position side > table, but two (one for the call, one for the parent which does the > type coercion). > - the wasm compiler annotates the source positions "0" and "1" to the > two calls in the WASM_TO_JS wrapper (only if the module origin is > asm.js). > - during stack trace generation (in the StackTraceIterator), when we > move from the WASM_TO_JS frame to the WASM frame, we remember at which > call inside the WASM_TO_JS wrapper we are, and encode this information > in the generated caller state, used for the WASM frame. > - the same information is also stored in the FrameArray which is used > to reconstruct the stack trace later. > > R=titzer@chromium.org, bradnelson@chromium.org > CC=jgruber@chromium.org > BUG=v8:4203, v8:5724 > > Committed: https://crrev.com/94cd46b55e24fa2bb7b06b3da4d5ba7f029bc262 > Cr-Commit-Position: refs/heads/master@{#41599} TBR=bradnelson@chromium.org,mstarzinger@chromium.org,titzer@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:4203, v8:5724 Committed: https://crrev.com/d3d125417db0c278a2d27e4a965ad4e82c66e2af Cr-Commit-Position: refs/heads/master@{#41601}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -281 lines) Patch
M src/asmjs/asm-wasm-builder.cc View 6 chunks +4 lines, -21 lines 0 comments Download
M src/compiler/pipeline.h View 1 chunk +4 lines, -4 lines 0 comments Download
M src/compiler/pipeline.cc View 3 chunks +8 lines, -12 lines 0 comments Download
M src/compiler/wasm-compiler.h View 1 chunk +1 line, -2 lines 0 comments Download
M src/compiler/wasm-compiler.cc View 5 chunks +3 lines, -12 lines 0 comments Download
M src/frames.h View 2 chunks +0 lines, -3 lines 0 comments Download
M src/frames.cc View 2 chunks +1 line, -24 lines 0 comments Download
M src/isolate.cc View 2 chunks +4 lines, -12 lines 0 comments Download
M src/messages.h View 2 chunks +0 lines, -7 lines 0 comments Download
M src/messages.cc View 2 chunks +1 line, -11 lines 0 comments Download
M src/objects.h View 1 chunk +0 lines, -1 line 0 comments Download
M src/wasm/module-decoder.h View 1 chunk +1 line, -6 lines 0 comments Download
M src/wasm/module-decoder.cc View 2 chunks +4 lines, -9 lines 0 comments Download
M src/wasm/wasm-module.cc View 3 chunks +3 lines, -5 lines 0 comments Download
M src/wasm/wasm-module-builder.h View 1 chunk +1 line, -1 line 0 comments Download
M src/wasm/wasm-module-builder.cc View 2 chunks +4 lines, -8 lines 0 comments Download
M src/wasm/wasm-objects.h View 1 chunk +1 line, -2 lines 0 comments Download
M src/wasm/wasm-objects.cc View 6 chunks +19 lines, -33 lines 0 comments Download
M test/cctest/wasm/wasm-run-utils.h View 1 chunk +3 lines, -3 lines 0 comments Download
D test/mjsunit/wasm/asm-wasm-exception-in-tonumber.js View 1 chunk +0 lines, -105 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
Clemens Hammacher
Created Revert of [wasm] Fix location for error in asm.js ToNumber conversion
4 years ago (2016-12-08 17:34:00 UTC) #1
titzer
On 2016/12/08 17:34:00, Clemens Hammacher wrote: > Created Revert of [wasm] Fix location for error ...
4 years ago (2016-12-08 17:35:15 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2563613003/1
4 years ago (2016-12-08 17:35:45 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years ago (2016-12-08 17:36:05 UTC) #6
commit-bot: I haz the power
4 years ago (2016-12-08 17:36:25 UTC) #8
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/d3d125417db0c278a2d27e4a965ad4e82c66e2af
Cr-Commit-Position: refs/heads/master@{#41601}

Powered by Google App Engine
This is Rietveld 408576698