New issue
Advanced search Search tips

Issue 788441 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Security



Sign in to add a comment

DCHECK failure in non_compiled_functions.size() == idx in module-compiler.cc

Project Member Reported by ClusterFuzz, Nov 24 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=6216928837500928

Fuzzer: ochang_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  non_compiled_functions.size() == idx in module-compiler.cc
  v8::internal::wasm::LazyCompilationOrchestrator::CompileLazy
  v8::internal::WasmCompiledModule::CompileLazy
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=49425:49426

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6216928837500928

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Nov 24 2017

Labels: Test-Predator-Auto-Owner
Owner: clemensh@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/v8/v8/+/6802775efcff6c0b48dd9c7df922f38eff676d16 (Reland "[wasm] Fix importing wasm-lazy-compile stubs").

If this is incorrect, please remove the owner and apply the Test-Predator-Wrong-CLs label.
Project Member

Comment 2 by ClusterFuzz, Nov 24 2017

Components: Blink>JavaScript>WebAssembly
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Labels: Security_Impact-Head M-64
Project Member

Comment 4 by sheriffbot@chromium.org, Nov 25 2017

Labels: ReleaseBlock-Stable
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

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

Comment 5 by sheriffbot@chromium.org, Nov 25 2017

Labels: Pri-1
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 28 2017

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

commit 95d13c2ee3116b9e4f16465298ec563331c5c80f
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Tue Nov 28 14:26:50 2017

[wasm] Add tracing for lazy compilation

This CL adds the --trace-wasm-lazy-compilation flag, to track which
function is being compiled, and how many locations in the caller and in
function tables are actually being patched.
It seems that we currently don't patch the caller correctly when
calling through wasm-to-wasm stubs, and this tracing helps to find the
issue.

Drive-by: Fix order and location of macro undefs.

R=titzer@chromium.org

Bug:  chromium:788441 
Change-Id: I6091c0d490a729f8e3cb759cd661cf52129d2211
Reviewed-on: https://chromium-review.googlesource.com/793157
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49669}
[modify] https://crrev.com/95d13c2ee3116b9e4f16465298ec563331c5c80f/src/flag-definitions.h
[modify] https://crrev.com/95d13c2ee3116b9e4f16465298ec563331c5c80f/src/wasm/module-compiler.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 29 2017

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

commit efed6ba94a722c919d23241117fc4a44f2d06d8a
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Wed Nov 29 11:53:26 2017

[wasm] Lazy-compilation: Fix patching of wasm-to-wasm wrappers

Cross-instance calls call through a wasm-to-wasm stub, which
tail-calls and hence does not show up on the stack. It was not being
patched so far, leading to repeatedly calling through the
WasmCompileLazy stub. Even though this did not crash, it resulted in
significant overhead.
This CL fixes this and also adds checks to ensure that we patch at
least one call site whenever we execute the WasmCompileLazy stub.

R=titzer@chromium.org

Bug:  chromium:788441 ,  v8:5991 
Change-Id: I1c2cd52497c577252a64dbf1cfa92d2f2e60b06c
Reviewed-on: https://chromium-review.googlesource.com/794132
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49709}
[modify] https://crrev.com/efed6ba94a722c919d23241117fc4a44f2d06d8a/src/wasm/module-compiler.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 29 2017

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

commit 690ac5760cbabc6544c5bfa03a11eafd773bb8b6
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Wed Nov 29 13:20:34 2017

[wasm] Lazy-compilation: Support exporting an import

When exporting an imported wasm function, we generate a js-to-wasm
wrapper which calls the wasm-to-wasm wrapper (which then tail-calls
the WasmCompileLazy stub).
This wasm-to-wasm wrapper also needs to be patched.

R=titzer@chromium.org

Bug:  chromium:788441 ,  v8:5991 
Change-Id: Ibf27618a0511851cb55714b720fe7299a21c2959
Reviewed-on: https://chromium-review.googlesource.com/795990
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49715}
[modify] https://crrev.com/690ac5760cbabc6544c5bfa03a11eafd773bb8b6/src/wasm/module-compiler.cc
[modify] https://crrev.com/690ac5760cbabc6544c5bfa03a11eafd773bb8b6/test/mjsunit/wasm/lazy-compilation.js

Status: Fixed (was: Assigned)
Project Member

Comment 10 by sheriffbot@chromium.org, Nov 29 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 11 by ClusterFuzz, Nov 30 2017

ClusterFuzz has detected this issue as fixed in range 49714:49715.

Detailed report: https://clusterfuzz.com/testcase?key=6216928837500928

Fuzzer: ochang_js_fuzzer
Job Type: linux_asan_d8_dbg
Platform Id: linux

Crash Type: DCHECK failure
Crash Address: 
Crash State:
  non_compiled_functions.size() == idx in module-compiler.cc
  v8::internal::wasm::LazyCompilationOrchestrator::CompileLazy
  v8::internal::WasmCompiledModule::CompileLazy
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=49425:49426
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_d8_dbg&range=49714:49715

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=6216928837500928

See https://github.com/google/clusterfuzz-tools 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 12 by ClusterFuzz, Nov 30 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Fixed)
ClusterFuzz testcase 6216928837500928 is verified as fixed, so closing issue as verified.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 30 2017

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

commit bb64e34f0cece2c2231714749fc73a39bc0d8871
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Thu Nov 30 08:54:12 2017

[wasm] Add "freeze" flag to test lazy compilation

In order to test that we don't repeatedly go through the
WasmCompileLazy runtime function, add a flag to the
LazyCompilationOrchestrator to "freeze" it, i.e. disallow any further
lazy compilation.
In tests, use this flag to first call a method, then freeze lazy
compilation, then call the method again to assert that no further lazy
compilation is triggered.

This test currently fails with --wasm-jit-to-native, so disable it for
that variant.

R=titzer@chromium.org
CC=mtrofin@chromium.org

Bug:  v8:7140 ,  chromium:788441 ,  v8:5991 
Change-Id: I18a40d302c24041740d8a54351d06ed968f4beec
Reviewed-on: https://chromium-review.googlesource.com/796430
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49734}
[modify] https://crrev.com/bb64e34f0cece2c2231714749fc73a39bc0d8871/src/runtime/runtime-test.cc
[modify] https://crrev.com/bb64e34f0cece2c2231714749fc73a39bc0d8871/src/runtime/runtime.h
[modify] https://crrev.com/bb64e34f0cece2c2231714749fc73a39bc0d8871/src/wasm/module-compiler.cc
[modify] https://crrev.com/bb64e34f0cece2c2231714749fc73a39bc0d8871/src/wasm/module-compiler.h
[modify] https://crrev.com/bb64e34f0cece2c2231714749fc73a39bc0d8871/test/mjsunit/mjsunit.status
[modify] https://crrev.com/bb64e34f0cece2c2231714749fc73a39bc0d8871/test/mjsunit/wasm/lazy-compilation.js

Project Member

Comment 14 by sheriffbot@chromium.org, Mar 7 2018

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

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

Comment 15 by sheriffbot@chromium.org, Mar 27 2018

Labels: -Security_Impact-Head -M-64 M-65 Security_Impact-Stable
Labels: -ReleaseBlock-Stable

Sign in to add a comment