New issue
Advanced search Search tips

Issue 698587 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Crash in v8::internal::Invoke

Project Member Reported by ClusterFuzz, Mar 5 2017

Issue description

Comment 1 Deleted

Cc: titzer@chromium.org
Owner: clemensh@chromium.org
Status: Assigned (was: Untriaged)
CF points to https://codereview.chromium.org/2696143006. PTAL.
Status: Started (was: Assigned)
The test case imports an empty ArrayBuffer as heap memory in asm.js.
We get confused in code patching since there is a memory, but its length and start are both zero/nullptr.
Working an a fix.
Minimized test case:

var heap = new ArrayBuffer();
function asm(stdlib, ffi, heap) {
  "use asm";
  return {};
}
asm({}, {}, heap);

Cc: hablich@chromium.org
Labels: Merge-Request-58
CL: https://chromium-review.googlesource.com/450257

This will need backmerging.
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 6 2017

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

commit 7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Mon Mar 06 13:39:54 2017

[wasm] Fix code specialization for empty memory buffer

From asm.js code we might get an empty ArrayBuffer as heap memory. In
this case, both the old memory start and the new memory start will be
nullptr. The size however has to be patched from default_size to 0.

This CL changes code specialization to be able to either patch memory
references, or patch memory sizes or both.

R=titzer@chromium.org, ahaas@chromium.org
BUG= chromium:698587 

Change-Id: I4d9d811d75cb83842f23df317e8e7fc02aeb5146
Reviewed-on: https://chromium-review.googlesource.com/450257
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43613}
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/src/assembler.cc
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/src/assembler.h
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/src/wasm/wasm-code-specialization.cc
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/src/wasm/wasm-module.cc
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/test/cctest/compiler/test-run-wasm-machops.cc
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/test/cctest/test-run-wasm-relocation-arm.cc
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/test/cctest/test-run-wasm-relocation-arm64.cc
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/test/cctest/test-run-wasm-relocation-ia32.cc
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/test/cctest/test-run-wasm-relocation-x64.cc
[modify] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/test/cctest/test-run-wasm-relocation-x87.cc
[add] https://crrev.com/7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab/test/mjsunit/regress/wasm/regression-698587.js

Status: Fixed (was: Started)
Project Member

Comment 8 by ClusterFuzz, Mar 7 2017

ClusterFuzz has detected this issue as fixed in range 43612:43613.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4824254056235008

Fuzzer: mbarbella_js_mutation
Job Type: linux_asan_d8
Platform Id: linux

Crash Type: UNKNOWN READ
Crash Address: 0x000000000000
Crash State:
  v8::internal::Invoke
  v8::internal::CallInternal
  v8::internal::Execution::Call
  
Sanitizer: address (ASAN)

Regressed: V8: 43323:43324
Fixed: V8: 43612:43613

Reproducer Testcase: https://cluster-fuzz.appspot.com/download/AMIfv970kiW6zbrcKSAROOS3lIXqvCOUjQfbuw1ZIKx2jGDt1f5MxzHZvgsEYcYXQ74d8MsfInbU2LPrcml_OTIBgEmTV7quIjbjyrdMz-3G3bfIXARLy5K-w4yNft1DFKITR1doYoXeIBL0gvbo253nK5NJaUGhXJ-Z2gi8WMKsqf7-CL4AJszAOb9m5MqxQfJQZcztMdS_1k4W9QAfTZEniwTzY8WqKW3AeU_ApL6v1FyRzl-nwRQv1pUkWZTmaYMJwZu2R6E74rSeui646YEulhfDCXtNeRRwyqH99sgav-_htkdh5IrVWXd-XpNoEaD2rnay-FJdnKPp8jPGQLq1L9Ea_5GMfkBs4i1BdJsYISdWiJ_nKOk?testcase_id=4824254056235008


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 9 by sheriffbot@chromium.org, Mar 7 2017

Labels: -Merge-Request-58 Hotlist-Merge-Approved Merge-Approved-58
Your change meets the bar and is auto-approved for M58. Please go ahead and merge the CL to branch 3029 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), bhthompson@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by bugdroid1@chromium.org, Mar 7 2017

Labels: merge-merged-5.8
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786

commit c0b599d9ce16da8cad33c90dfdb5a687eb7ac786
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Tue Mar 07 15:05:10 2017

Merged: [wasm] Fix code specialization for empty memory buffer

Revision: 7d8a3028ddabbf3db5e3f5fb23d0fb1dadcc21ab

BUG= chromium:698587 
LOG=N
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=titzer@chromium.org

Review-Url: https://codereview.chromium.org/2735103003 .
Cr-Commit-Position: refs/branch-heads/5.8@{#13}
Cr-Branched-From: eda659cc5e307f20ac1ad542ba12ab32eaf4c7ef-refs/heads/5.8.283@{#1}
Cr-Branched-From: 4310cd02d2160b1457baed81a2f40063eb264a21-refs/heads/master@{#43429}

[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/src/assembler.cc
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/src/assembler.h
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/src/wasm/wasm-code-specialization.cc
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/src/wasm/wasm-module.cc
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/test/cctest/compiler/test-run-wasm-machops.cc
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/test/cctest/test-run-wasm-relocation-arm.cc
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/test/cctest/test-run-wasm-relocation-arm64.cc
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/test/cctest/test-run-wasm-relocation-ia32.cc
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/test/cctest/test-run-wasm-relocation-x64.cc
[modify] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/test/cctest/test-run-wasm-relocation-x87.cc
[add] https://crrev.com/c0b599d9ce16da8cad33c90dfdb5a687eb7ac786/test/mjsunit/regress/wasm/regression-698587.js

Thanks for the merge, if there is no pending work please remove 
Merge-Approved-58 label.
Labels: -Merge-Approved-58
Thanks for the reminder!
I am just curious: Is there a reason why the bot (that added merge-merged-5.8) does not remove it?
The bot does not understand non-Chromium branches.

Sign in to add a comment