From 0cb5ba0ef0beadb0385a61c738286db4c949d3b8 Mon Sep 17 00:00:00 2001 From: Michael Starzinger Date: Mon, 3 Apr 2017 16:46:10 +0200 Subject: [PATCH] [asm.js] Fix function table call position tracking. This adds test coverage for the source position tracking of function table calls in asm.js and fixes the discovered issues. It also fixes function start positions (used by errors thrown at stack checks). R=clemensh@chromium.org TEST=mjsunit/wasm/asm-wasm-stack BUG=v8:6127,v8:6166 Change-Id: Id6ab6dc72bcedb0d838eed315e2a05fbc59039f4 Reviewed-on: https://chromium-review.googlesource.com/465949 Commit-Queue: Michael Starzinger Reviewed-by: Clemens Hammacher Cr-Commit-Position: refs/heads/master@{#44348} --- src/asmjs/asm-parser.cc | 15 ++++++--------- test/inspector/inspector.status | 1 - test/mjsunit/mjsunit.status | 4 +++- test/mjsunit/wasm/asm-wasm-stack.js | 23 +++++++++++++---------- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/asmjs/asm-parser.cc b/src/asmjs/asm-parser.cc index 9b5c21e2b9..12ba75e2d3 100644 --- a/src/asmjs/asm-parser.cc +++ b/src/asmjs/asm-parser.cc @@ -712,7 +712,6 @@ void AsmJsParser::ValidateFunctionTable() { // 6.4 ValidateFunction void AsmJsParser::ValidateFunction() { - int start_position = scanner_.GetPosition(); EXPECT_TOKEN(TOK(function)); if (!scanner_.IsGlobal()) { FAIL("Expected function name"); @@ -741,6 +740,7 @@ void AsmJsParser::ValidateFunction() { return_type_ = nullptr; // Record start of the function, used as position for the stack check. + int start_position = static_cast(scanner_.Position()); current_function_builder_->SetAsmFunctionStartPosition(start_position); std::vector params; @@ -2037,6 +2037,8 @@ AsmType* AsmJsParser::ValidateCall() { current_function_builder_->Emit(kExprI32Add); // We have to use a temporary for the correct order of evaluation. current_function_builder_->EmitSetLocal(tmp); + // The position of function table calls is after the table lookup. + pos = static_cast(scanner_.Position()); } std::vector param_types; ZoneVector param_specific_types(zone()); @@ -2122,10 +2124,8 @@ AsmType* AsmJsParser::ValidateCall() { function_info->type = function_type; if (function_info->kind == VarKind::kTable) { current_function_builder_->EmitGetLocal(tmp); - // TODO(bradnelson): Figure out the right debug scanner offset and - // re-enable. - // current_function_builder_->AddAsmWasmOffset(scanner_.GetPosition(), - // scanner_.GetPosition()); + // TODO(mstarzinger): Fix the {to_number_position} and test it. + current_function_builder_->AddAsmWasmOffset(pos, pos); current_function_builder_->Emit(kExprCallIndirect); current_function_builder_->EmitVarUint(signature_index); current_function_builder_->EmitVarUint(0); // table index @@ -2272,10 +2272,7 @@ AsmType* AsmJsParser::ValidateCall() { current_function_builder_->EmitVarUint(signature_index); current_function_builder_->EmitVarUint(0); // table index } else { - // TODO(bradnelson): Figure out the right debug scanner offset and - // re-enable. - // current_function_builder_->AddAsmWasmOffset(scanner_.GetPosition(), - // scanner_.GetPosition()); + current_function_builder_->AddAsmWasmOffset(pos, pos); current_function_builder_->Emit(kExprCallFunction); current_function_builder_->EmitDirectCallIndex(function_info->index); } diff --git a/test/inspector/inspector.status b/test/inspector/inspector.status index 54474a6338..cec99e0779 100644 --- a/test/inspector/inspector.status +++ b/test/inspector/inspector.status @@ -8,7 +8,6 @@ ['variant != default', { # Issue 6166. 'debugger/asm-js-breakpoint-during-exec': [PASS, FAIL], - 'debugger/asm-js-stack': [PASS, FAIL], # Issue 6167. 'debugger/eval-scopes': [PASS, FAIL], 'debugger/scope-skip-variables-with-empty-name': [PASS, FAIL], diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 6a6c355cfb..96b8f303f8 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -661,7 +661,9 @@ # need to re-enable these for the "new" validator. 'regress/regress-618608': [SKIP], 'wasm/asm-wasm-exception-in-tonumber': [SKIP], - 'wasm/asm-wasm-stack': [SKIP], + + # Issue 6127: Currently {StashCode} breaks the source position table. + 'wasm/asm-wasm-expr': [SKIP], }], # variant == asm_wasm ['variant == wasm_traps', { diff --git a/test/mjsunit/wasm/asm-wasm-stack.js b/test/mjsunit/wasm/asm-wasm-stack.js index 7246163e9c..968315cfcb 100644 --- a/test/mjsunit/wasm/asm-wasm-stack.js +++ b/test/mjsunit/wasm/asm-wasm-stack.js @@ -68,8 +68,10 @@ function generateWasmFromAsmJs(stdlib, foreign) { case 0: callThrow(); break; case 1: redirectFun(0); break; case 2: redirectFun(1); break; + case 3: funTable[i & 0](2); break; } } + var funTable = [ redirectFun ]; return redirectFun; } @@ -88,8 +90,8 @@ function generateWasmFromAsmJs(stdlib, foreign) { '^ *at throwException \\(' + filename + ':56:9\\)$', '^ *at callThrow \\(' + filename + ':63:5\\)$', '^ *at redirectFun \\(' + filename + ':68:15\\)$', - '^ *at PreformattedStackTraceFromJS \\(' + filename + ':81:5\\)$', - '^ *at ' + filename + ':94:3$' + '^ *at PreformattedStackTraceFromJS \\(' + filename + ':83:5\\)$', + '^ *at ' + filename + ':96:3$' ]); })(); @@ -103,7 +105,7 @@ Error.prepareStackTrace = function(error, frames) { assertTrue(%IsWasmCode(fun)); var e = null; try { - fun(2); + fun(3); } catch (ex) { e = ex; } @@ -114,8 +116,9 @@ Error.prepareStackTrace = function(error, frames) { ['redirectFun', 68, 15], // -- ['redirectFun', 69, 15], // -- ['redirectFun', 70, 15], // -- - ['CallsiteObjectsFromJS', 106, 5], // -- - [null, 120, 3] + ['redirectFun', 71, 30], // -- + ['CallsiteObjectsFromJS', 108, 5], // -- + [null, 123, 3] ]); })(); @@ -133,15 +136,15 @@ function generateOverflowWasmFromAsmJs() { assertTrue(%IsWasmCode(fun)); var e = null; try { - fun(2); + fun(23); } catch (ex) { e = ex; } assertInstanceof(e, RangeError, 'RangeError should have been thrown'); checkTopFunctionsOnCallsites(e, [ - ['f', 124, 13], // -- - ['f', 126, 12], // -- - ['f', 126, 12], // -- - ['f', 126, 12] // -- + ['f', 127, 13], // -- + ['f', 129, 12], // -- + ['f', 129, 12], // -- + ['f', 129, 12] // -- ]); })();