New issue
Advanced search Search tips

Issue 839800 link

Starred by 1 user
Status: Fixed
Owner:
clemensh@chromium.org
Closed: May 2018
Cc:
adamk@chromium.org
titzer@chromium.org
dschuff@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

[Liftoff] div/rem bug on ia32

Project Member Reported by clemensh@chromium.org, May 4 2018 
Found by fuzzing locally:

load('test/mjsunit/wasm/wasm-constants.js');
load('test/mjsunit/wasm/wasm-module-builder.js');

const builder = new WasmModuleBuilder();
builder.addFunction(undefined, kSig_i_v).addBody([
  kExprI32Const, 0,     // i32.const 0      [0]
  kExprI32Const, 0,     // i32.const 0      [0, 0]
  kExprI32Const, 1,     // i32.const 1      [0, 0, 1]
  kExprI32RemS,         // i32.rem_s -> 0   [0, 0]
  kExprI32ShrS,         // i32.shr_s -> 0   [0]
  kExprI32Const, 0,     // i32.const 0      [0, 0]
  kExprI32Const, 0x7f,  // i32.const -1     [0, 0, -1]
  kExprI32RemS,         // i32.rem_s -> 0   [0, 0]
  kExprI32Const, 0,     // i32.const 0      [0, 0, 0]
  kExprI32Const, 1,     // i32.const 1      [0, 0, 0, 1]
  kExprI32GeU,          // i32.ge_u -> 0    [0, 0, 0]
  kExprI32GeS,          // i32.ge_s -> 1    [0, 1]
  kExprI32GeU,          // i32.ge_u -> 0    [0]
]);
builder.addExport('main', 0);
const instance = builder.instantiate();
print(instance.exports.main(1, 2, 3));



Should produce 0, but produces one on Liftoff.
Project Member

Comment 1 by bugdroid1@chromium.org, May 4 2018 

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

commit a2dbb84ccd51fb7adb4ab4e1014c96024fe58408
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Fri May 04 12:29:39 2018

[Liftoff] Extend binop tests for div and rem

Add binop tests for div and rem of i32 and i64. The test is extended to
handle traps, and to check that the value of local variables is not
affected by the operation.

R=titzer@chromium.org

Bug: v8:6600,  chromium:839800 
Change-Id: I1a4cbc40bd399666d9831d021afb96e0c53a9f64
Reviewed-on: https://chromium-review.googlesource.com/1044166
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52989}
[modify] https://crrev.com/a2dbb84ccd51fb7adb4ab4e1014c96024fe58408/test/cctest/wasm/test-run-wasm.cc
Project Member

Comment 2 by bugdroid1@chromium.org, May 4 2018 

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

commit c20d7f660513bd24e594d311420573408ca0b923
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Fri May 04 13:36:39 2018

[Liftoff] Fix conditional spilling on div and rem

On div and rem on ia32 and x64, we sometimes need to spill. If this
spilling code happens inside of a branch, the cache state will reflect
that the value was spilled, even though the actual spilling code might
not have executed.

R=titzer@chromium.org

Bug: v8:6600,  chromium:839800 
Change-Id: I93b681a23119f903feb54235d6d44a7cbd5815fe
Reviewed-on: https://chromium-review.googlesource.com/1044185
Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52995}
[modify] https://crrev.com/c20d7f660513bd24e594d311420573408ca0b923/src/wasm/baseline/ia32/liftoff-assembler-ia32.h
[modify] https://crrev.com/c20d7f660513bd24e594d311420573408ca0b923/src/wasm/baseline/x64/liftoff-assembler-x64.h
[modify] https://crrev.com/c20d7f660513bd24e594d311420573408ca0b923/test/cctest/wasm/test-run-wasm.cc

Comment 3 by clemensh@chromium.org, May 4 2018

Status: Fixed (was: Started)
►

Sign in to add a comment