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

Issue 2609363004: [asm.js] [wasm] Store function start position for stack check (Closed)

Created:
3 years, 11 months ago by Clemens Hammacher
Modified:
3 years, 11 months ago
Reviewers:
titzer, ahaas
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[asm.js] [wasm] Store function start position for stack check We did not associate any position to the stack check in the wasm function prologue, hence a check failed later when trying to map the non-existent position to the asm.js source position. With this CL, we add a mapping to the source position table, mapping the stack check call to byte offset 0 (which is distinct from any valid instruction position). Also, we add another entry to the asm.js source position sidetable, mapping byte offset 0 to the start source position of the function body. R=titzer@chromium.org, ahaas@chromium.org BUG=chromium:677685 Review-Url: https://codereview.chromium.org/2609363004 Cr-Commit-Position: refs/heads/master@{#42130} Committed: https://chromium.googlesource.com/v8/v8/+/fc327e23087a88f6581d4bb2dc026b915b926589

Patch Set 1 #

Patch Set 2 : Fix error type #

Patch Set 3 : Weaken test condition because of unrelated bug #

Patch Set 4 : It's 2017 already :) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+156 lines, -41 lines) Patch
M src/asmjs/asm-wasm-builder.cc View 1 chunk +5 lines, -0 lines 0 comments Download
M src/compiler/wasm-compiler.cc View 1 chunk +5 lines, -8 lines 0 comments Download
M src/runtime/runtime.h View 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime/runtime-test.cc View 1 chunk +7 lines, -0 lines 0 comments Download
M src/wasm/function-body-decoder.cc View 1 chunk +3 lines, -1 line 0 comments Download
M src/wasm/module-decoder.cc View 1 chunk +6 lines, -2 lines 0 comments Download
M src/wasm/wasm-module-builder.h View 2 chunks +2 lines, -0 lines 0 comments Download
M src/wasm/wasm-module-builder.cc View 3 chunks +18 lines, -4 lines 0 comments Download
A test/mjsunit/regress/regress-677685.js View 1 2 3 1 chunk +32 lines, -0 lines 0 comments Download
M test/mjsunit/wasm/asm-wasm-stack.js View 1 2 6 chunks +69 lines, -26 lines 0 comments Download
M test/mjsunit/wasm/stack.js View 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 22 (17 generated)
Clemens Hammacher
3 years, 11 months ago (2017-01-04 12:41:58 UTC) #3
titzer
On 2017/01/04 12:41:58, Clemens Hammacher wrote: lgtm
3 years, 11 months ago (2017-01-09 08:56:32 UTC) #16
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/2609363004/60001
3 years, 11 months ago (2017-01-09 08:59:35 UTC) #18
ahaas
lgtm
3 years, 11 months ago (2017-01-09 09:03:51 UTC) #19
commit-bot: I haz the power
3 years, 11 months ago (2017-01-09 09:43:13 UTC) #22
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/fc327e23087a88f6581d4bb2dc026b915b9...

Powered by Google App Engine
This is Rietveld 408576698