New issue
Advanced search Search tips

Issue 722445 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

CHECK failure: AllowHeapAllocation::IsAllowed() in heap.cc

Project Member Reported by ClusterFuzz, May 15 2017

Issue description

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

Fuzzer: libfuzzer_v8_wasm_call_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  AllowHeapAllocation::IsAllowed() in heap.cc
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=470025:470101

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


Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Components: Blink>JavaScript
Labels: Test-Predator-Wrong-CLs M-60
Owner: clemensh@chromium.org
Status: Started (was: Untriaged)
Error in wasm compiler, will take a look.
Cc: bmeu...@chromium.org ahaas@chromium.org
CL is here: https://chromium-review.googlesource.com/505496

Background: This check fails on a switch node: CHECK(input->op()->ControlOutputCount() > 0 || input->opcode() == IrOpcode::kDead)

input->op()->ControlOutputCount() is -805306368 (0xd0000000).

The problem is that we generate a switch node with control_output_count=0xd0000000.
This is passed to CommonOperatorBuilder::Switch as size_t.
Operator::Operator then does a CheckRange<uint32_t>(control_out) and stores the value as uint32_t, which is still fine.
Operator::ControlOutputCount() then returns the uint32_t as int, producing the negative value.

I fixed this by just not producing the switch with the huge control output count in the first place.
Additionally, I strengthened the range check in the Operator constructor to check that all given counts are in the integer range.

+bmeurer for the latter change (in Operator::Operator).
Project Member

Comment 4 by bugdroid1@chromium.org, May 17 2017

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

commit 74519c43fd7c29ca5e2e37525151bde5a8e31d34
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Wed May 17 09:46:46 2017

[wasm] Check for illegal br table count

The underlying issue is that TF Nodes cannot handle input counts
outside the integer range. On an illegal br_table instruction, we
generated a switch node with a control output count >kMaxInt.
Operator::ControlOutputCount turned this into a negative integer later,
leading to a failing DCHECK.
Since such large numbers cannot occur in any valid wasm function anyway,
we just add an additional check to the br table count. There is already
a TODO in the code to change Operator::ControlOutputCount to size_t.

R=ahaas@chromium.org
BUG= chromium:722445 

Change-Id: I1975072226e073dee6c8da3b9fa9a050a4695917
Reviewed-on: https://chromium-review.googlesource.com/505496
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45365}
[modify] https://crrev.com/74519c43fd7c29ca5e2e37525151bde5a8e31d34/src/compiler/operator.cc
[modify] https://crrev.com/74519c43fd7c29ca5e2e37525151bde5a8e31d34/src/wasm/function-body-decoder.cc
[add] https://crrev.com/74519c43fd7c29ca5e2e37525151bde5a8e31d34/test/mjsunit/regress/wasm/regression-722445.js

Status: Fixed (was: Started)
Project Member

Comment 6 by ClusterFuzz, May 19 2017

ClusterFuzz has detected this issue as fixed in range 472682:472738.

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

Fuzzer: libfuzzer_v8_wasm_call_fuzzer
Job Type: libfuzzer_chrome_asan_debug
Platform Id: linux

Crash Type: CHECK failure
Crash Address: 
Crash State:
  AllowHeapAllocation::IsAllowed() in heap.cc
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=470025:470101
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_asan_debug&range=472682:472738

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


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

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

Sign in to add a comment