New issue
Advanced search Search tips

Issue 840757 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Segfault for --code-comments with --print-wasm-code

Project Member Reported by clemensh@chromium.org, May 8 2018

Issue description

Reproducer:

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

// Flags: --code-comments --predictable --print-wasm-code

const builder = new WasmModuleBuilder();
// Add a call instruction, because the segfault happens when processing source
// positions.
builder.addFunction('foo', kSig_v_v).addBody([]);
builder.addFunction('test', kSig_v_v).addBody([kExprCallFunction, 0]);

builder.instantiate();

 
Cc: herhut@chromium.org
+Stefan: Ben said you had a similar issue.
Project Member

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

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

commit e084eea6284949ec0dd448e019c2c583edcd6c82
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Tue May 08 18:23:04 2018

Fix SourcePositionInfo for wasm

In wasm we often don't have a SharedFunctionInfo associated with a
compilation job, so we can't get a Script. Just print "unknown" in
these cases (instead of crashing).

R=titzer@chromium.org
CC=​herhut@chromium.org

Bug:  chromium:840757 ,  v8:7738 
Change-Id: I850c6adfd9e07c9a0f6dd018f1a9314feb89d887
Reviewed-on: https://chromium-review.googlesource.com/1049632
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53080}
[modify] https://crrev.com/e084eea6284949ec0dd448e019c2c583edcd6c82/src/perf-jit.cc
[modify] https://crrev.com/e084eea6284949ec0dd448e019c2c583edcd6c82/src/profiler/profiler-listener.cc
[modify] https://crrev.com/e084eea6284949ec0dd448e019c2c583edcd6c82/src/source-position.cc
[modify] https://crrev.com/e084eea6284949ec0dd448e019c2c583edcd6c82/src/source-position.h
[modify] https://crrev.com/e084eea6284949ec0dd448e019c2c583edcd6c82/test/mjsunit/mjsunit.status
[add] https://crrev.com/e084eea6284949ec0dd448e019c2c583edcd6c82/test/mjsunit/regress/wasm/regress-840757.js

Status: Fixed (was: Started)
Project Member

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

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

commit 2b6fb352a6a54f2177b71578c1ea70ddc981d650
Author: Michael Achenbach <machenbach@chromium.org>
Date: Wed May 09 07:11:03 2018

Revert "Fix SourcePositionInfo for wasm"

This reverts commit e084eea6284949ec0dd448e019c2c583edcd6c82.

Reason for revert:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20UBSanVptr/builds/3163

Original change's description:
> Fix SourcePositionInfo for wasm
> 
> In wasm we often don't have a SharedFunctionInfo associated with a
> compilation job, so we can't get a Script. Just print "unknown" in
> these cases (instead of crashing).
> 
> R=​titzer@chromium.org
> CC=​​herhut@chromium.org
> 
> Bug:  chromium:840757 ,  v8:7738 
> Change-Id: I850c6adfd9e07c9a0f6dd018f1a9314feb89d887
> Reviewed-on: https://chromium-review.googlesource.com/1049632
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Reviewed-by: Ben Titzer <titzer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#53080}

TBR=titzer@chromium.org,clemensh@chromium.org

Change-Id: Ib2020ea3f2b778df9fe50ccbe803938f2f4fd709
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  chromium:840757 ,  v8:7738 
Reviewed-on: https://chromium-review.googlesource.com/1051265
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53082}
[modify] https://crrev.com/2b6fb352a6a54f2177b71578c1ea70ddc981d650/src/perf-jit.cc
[modify] https://crrev.com/2b6fb352a6a54f2177b71578c1ea70ddc981d650/src/profiler/profiler-listener.cc
[modify] https://crrev.com/2b6fb352a6a54f2177b71578c1ea70ddc981d650/src/source-position.cc
[modify] https://crrev.com/2b6fb352a6a54f2177b71578c1ea70ddc981d650/src/source-position.h
[modify] https://crrev.com/2b6fb352a6a54f2177b71578c1ea70ddc981d650/test/mjsunit/mjsunit.status
[delete] https://crrev.com/7ff35bd54269132ddb49825806934465195b79b0/test/mjsunit/regress/wasm/regress-840757.js

Status: Started (was: Fixed)
Project Member

Comment 6 by bugdroid1@chromium.org, May 9 2018

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

commit aae0732c72b131b38d8c829417554154d099b143
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Wed May 09 16:39:55 2018

Reland "Fix SourcePositionInfo for wasm"

This is a reland of e084eea6284949ec0dd448e019c2c583edcd6c82.
Undefined behavious was fixed in https://crrev.com/c/1051235.

Original change's description:
> Fix SourcePositionInfo for wasm
>
> In wasm we often don't have a SharedFunctionInfo associated with a
> compilation job, so we can't get a Script. Just print "unknown" in
> these cases (instead of crashing).
>
> R=titzer@chromium.org
> CC=​herhut@chromium.org
>
> Bug:  chromium:840757 ,  v8:7738 
> Change-Id: I850c6adfd9e07c9a0f6dd018f1a9314feb89d887
> Reviewed-on: https://chromium-review.googlesource.com/1049632
> Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
> Reviewed-by: Ben Titzer <titzer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#53080}

TBR=titzer@chromium.org

Bug:  chromium:840757 ,  v8:7738 
Change-Id: If04040a33766955cfed78e7c27226dd04c3f9b9f
Reviewed-on: https://chromium-review.googlesource.com/1051266
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53111}
[modify] https://crrev.com/aae0732c72b131b38d8c829417554154d099b143/src/perf-jit.cc
[modify] https://crrev.com/aae0732c72b131b38d8c829417554154d099b143/src/profiler/profiler-listener.cc
[modify] https://crrev.com/aae0732c72b131b38d8c829417554154d099b143/src/source-position.cc
[modify] https://crrev.com/aae0732c72b131b38d8c829417554154d099b143/src/source-position.h
[add] https://crrev.com/aae0732c72b131b38d8c829417554154d099b143/test/mjsunit/regress/wasm/regress-840757.js

Status: Fixed (was: Started)

Sign in to add a comment