Commit d50ebde7 authored by Clemens Hammacher's avatar Clemens Hammacher Committed by Commit Bot

[wasm] Refactor wasm::Result type

- Store std::string instead of std::unique_ptr<char[]> for the error
  message.
- Remove ErrorCode, which was just kSuccess and kError anyway. Error is
  now detected on whether error_msg_ is empty or not.
- Refactor constructors for perfect forwarding; this will allow us to
  implement Result<std::unique_ptr<X*>>.
- Refactor Decoder::toResult for perfect forwarding.
- Remove output operators (operator<<) for Result; it was only used in
  the error case anyway. Print error message directly instead.
  The operator was problematic since it assumed the existence of an
  output operator for every T which is used in Result<T>.
- Remove ModuleError and FunctionError, introduce general static
  Result<T>::Error method instead.

R=ahaas@chromium.org

Change-Id: I1e0f602a61ee9780fee2a3ed33147d431fb092ba
Reviewed-on: https://chromium-review.googlesource.com/472748
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: 's avatarAndreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#44518}
parent beb4089e
......@@ -3858,7 +3858,8 @@ SourcePositionTable* WasmCompilationUnit::BuildGraphForWasmFunction(
if (graph_construction_result_.failed()) {
if (FLAG_trace_wasm_compiler) {
OFStream os(stdout);
os << "Compilation failed: " << graph_construction_result_ << std::endl;
os << "Compilation failed: " << graph_construction_result_.error_msg
<< std::endl;
}
return nullptr;
}
......
......@@ -173,13 +173,14 @@ class Decoder {
base::OS::DebugBreak();
}
#endif
const int kMaxErrorMsg = 256;
char* buffer = new char[kMaxErrorMsg];
constexpr int kMaxErrorMsg = 256;
EmbeddedVector<char, kMaxErrorMsg> buffer;
va_list arguments;
va_start(arguments, format);
base::OS::VSNPrintF(buffer, kMaxErrorMsg - 1, format, arguments);
int len = VSNPrintF(buffer, format, arguments);
CHECK_LT(0, len);
va_end(arguments);
error_msg_.reset(buffer);
error_msg_.assign(buffer.start(), len);
error_pc_ = pc;
onFirstError();
}
......@@ -200,20 +201,18 @@ class Decoder {
}
// Converts the given value to a {Result}, copying the error if necessary.
template <typename T>
Result<T> toResult(T val) {
Result<T> result;
template <typename T, typename U = typename std::remove_reference<T>::type>
Result<U> toResult(T&& val) {
Result<U> result(std::forward<T>(val));
if (failed()) {
TRACE("Result error: %s\n", error_msg_.get());
result.error_code = kError;
// The error message must not be empty, otherwise Result::failed() will be
// false.
DCHECK(!error_msg_.empty());
TRACE("Result error: %s\n", error_msg_.c_str());
DCHECK_GE(error_pc_, start_);
result.error_offset = static_cast<uint32_t>(error_pc_ - start_);
// transfer ownership of the error to the result.
result.error_msg.reset(error_msg_.release());
} else {
result.error_code = kSuccess;
result.error_msg = std::move(error_msg_);
}
result.val = std::move(val);
return result;
}
......@@ -223,10 +222,10 @@ class Decoder {
pc_ = start;
end_ = end;
error_pc_ = nullptr;
error_msg_.reset();
error_msg_.clear();
}
bool ok() const { return error_msg_ == nullptr; }
bool ok() const { return error_msg_.empty(); }
bool failed() const { return !ok(); }
bool more() const { return pc_ < end_; }
......@@ -240,7 +239,7 @@ class Decoder {
const byte* pc_;
const byte* end_;
const byte* error_pc_;
std::unique_ptr<char[]> error_msg_;
std::string error_msg_;
private:
template <typename IntType, bool checked>
......
......@@ -572,7 +572,7 @@ class WasmFullDecoder : public WasmDecoder {
bool TraceFailed() {
TRACE("wasm-error module+%-6d func+%d: %s\n\n", baserel(error_pc_),
startrel(error_pc_), error_msg_.get());
startrel(error_pc_), error_msg_.c_str());
return false;
}
......@@ -1950,7 +1950,7 @@ class WasmFullDecoder : public WasmDecoder {
virtual void onFirstError() {
end_ = start_; // Terminate decoding loop.
builder_ = nullptr; // Don't build any more nodes.
TRACE(" !%s\n", error_msg_.get());
TRACE(" !%s\n", error_msg_.c_str());
}
inline wasm::WasmCodePosition position() {
......
......@@ -213,7 +213,7 @@ class ModuleDecoder : public Decoder {
ModuleDecoder(Zone* zone, const byte* module_start, const byte* module_end,
ModuleOrigin origin)
: Decoder(module_start, module_end),
module_zone(zone),
module_zone_(zone),
origin_(FLAG_assume_asmjs_origin ? kAsmJsOrigin : origin) {
if (end_ < start_) {
error(start_, "end is less than start");
......@@ -252,7 +252,7 @@ class ModuleDecoder : public Decoder {
// Decodes an entire module.
ModuleResult DecodeModule(bool verify_functions = true) {
pc_ = start_;
WasmModule* module = new WasmModule(module_zone);
WasmModule* module = new WasmModule(module_zone_);
module->min_mem_pages = 0;
module->max_mem_pages = 0;
module->mem_export = false;
......@@ -697,7 +697,8 @@ class ModuleDecoder : public Decoder {
const WasmModule* finished_module = module;
ModuleResult result = toResult(finished_module);
if (verify_functions && result.ok()) {
result.MoveFrom(result_); // Copy error code and location.
// Copy error code and location.
result.MoveErrorFrom(intermediate_result_);
}
if (FLAG_dump_wasm_module) DumpModule(result);
return result;
......@@ -716,7 +717,8 @@ class ModuleDecoder : public Decoder {
if (ok()) VerifyFunctionBody(0, module_env, function);
FunctionResult result;
result.MoveFrom(result_); // Copy error code and location.
// Copy error code and location.
result.MoveErrorFrom(intermediate_result_);
result.val = function;
return result;
}
......@@ -734,8 +736,8 @@ class ModuleDecoder : public Decoder {
}
private:
Zone* module_zone;
ModuleResult result_;
Zone* module_zone_;
Result<bool> intermediate_result_;
ModuleOrigin origin_;
uint32_t off(const byte* ptr) { return static_cast<uint32_t>(ptr - start_); }
......@@ -847,23 +849,17 @@ class ModuleDecoder : public Decoder {
start_ + function->code_start_offset,
start_ + function->code_end_offset};
DecodeResult result = VerifyWasmCode(
module_zone->allocator(),
module_zone_->allocator(),
menv == nullptr ? nullptr : menv->module_env.module, body);
if (result.failed()) {
// Wrap the error message from the function decoder.
std::ostringstream str;
str << "in function " << func_name << ": ";
str << result;
std::string strval = str.str();
const char* raw = strval.c_str();
size_t len = strlen(raw);
char* buffer = new char[len];
strncpy(buffer, raw, len);
buffer[len - 1] = 0;
str << "in function " << func_name << ": " << result.error_msg;
// Copy error code and location.
result_.MoveFrom(result);
result_.error_msg.reset(buffer);
// Set error code and location, if this is the first error.
if (intermediate_result_.ok()) {
intermediate_result_.MoveErrorFrom(result);
}
}
}
......@@ -1123,38 +1119,12 @@ class ModuleDecoder : public Decoder {
// FunctionSig stores the return types first.
ValueType* buffer =
module_zone->NewArray<ValueType>(param_count + return_count);
module_zone_->NewArray<ValueType>(param_count + return_count);
uint32_t b = 0;
for (uint32_t i = 0; i < return_count; ++i) buffer[b++] = returns[i];
for (uint32_t i = 0; i < param_count; ++i) buffer[b++] = params[i];
return new (module_zone) FunctionSig(return_count, param_count, buffer);
}
};
// Helpers for nice error messages.
class ModuleError : public ModuleResult {
public:
explicit ModuleError(const char* msg) {
error_code = kError;
size_t len = strlen(msg) + 1;
char* result = new char[len];
strncpy(result, msg, len);
result[len - 1] = 0;
error_msg.reset(result);
}
};
// Helpers for nice error messages.
class FunctionError : public FunctionResult {
public:
explicit FunctionError(const char* msg) {
error_code = kError;
size_t len = strlen(msg) + 1;
char* result = new char[len];
strncpy(result, msg, len);
result[len - 1] = 0;
error_msg.reset(result);
return new (module_zone_) FunctionSig(return_count, param_count, buffer);
}
};
......@@ -1167,9 +1137,9 @@ ModuleResult DecodeWasmModule(Isolate* isolate, const byte* module_start,
IsWasm(origin) ? isolate->counters()->wasm_decode_wasm_module_time()
: isolate->counters()->wasm_decode_asm_module_time());
size_t size = module_end - module_start;
if (module_start > module_end) return ModuleError("start > end");
if (module_start > module_end) return ModuleResult::Error("start > end");
if (size >= kV8MaxWasmModuleSize)
return ModuleError("size > maximum module size");
return ModuleResult::Error("size > maximum module size: %zu", size);
// TODO(bradnelson): Improve histogram handling of size_t.
(IsWasm(origin) ? isolate->counters()->wasm_wasm_module_size_bytes()
: isolate->counters()->wasm_asm_module_size_bytes())
......@@ -1212,9 +1182,10 @@ FunctionResult DecodeWasmFunction(Isolate* isolate, Zone* zone,
is_wasm ? isolate->counters()->wasm_decode_wasm_function_time()
: isolate->counters()->wasm_decode_asm_function_time());
size_t size = function_end - function_start;
if (function_start > function_end) return FunctionError("start > end");
if (function_start > function_end)
return FunctionResult::Error("start > end");
if (size > kV8MaxWasmFunctionSize)
return FunctionError("size > maximum function size");
return FunctionResult::Error("size > maximum function size: %zu", size);
(is_wasm ? isolate->counters()->wasm_wasm_function_size_bytes()
: isolate->counters()->wasm_asm_function_size_bytes())
->AddSample(static_cast<int>(size));
......
......@@ -900,24 +900,6 @@ void wasm::UnpackAndRegisterProtectedInstructions(
}
}
std::ostream& wasm::operator<<(std::ostream& os, const WasmModule& module) {
os << "WASM module with ";
os << (module.min_mem_pages * module.kPageSize) << " min mem";
os << (module.max_mem_pages * module.kPageSize) << " max mem";
os << module.functions.size() << " functions";
os << module.functions.size() << " globals";
os << module.functions.size() << " data segments";
return os;
}
std::ostream& wasm::operator<<(std::ostream& os, const WasmFunction& function) {
os << "WASM function with signature " << *function.sig;
os << " code bytes: "
<< (function.code_end_offset - function.code_start_offset);
return os;
}
std::ostream& wasm::operator<<(std::ostream& os, const WasmFunctionName& name) {
os << "#" << name.function_->func_index;
if (name.function_->name_offset > 0) {
......
......@@ -391,8 +391,6 @@ struct WasmFunctionName {
WasmName name_;
};
std::ostream& operator<<(std::ostream& os, const WasmModule& module);
std::ostream& operator<<(std::ostream& os, const WasmFunction& function);
std::ostream& operator<<(std::ostream& os, const WasmFunctionName& name);
// Get the debug info associated with the given wasm object.
......
......@@ -15,34 +15,29 @@ namespace v8 {
namespace internal {
namespace wasm {
std::ostream& operator<<(std::ostream& os, const ErrorCode& error_code) {
switch (error_code) {
case kSuccess:
os << "Success";
break;
default: // TODO(titzer): render error codes
os << "Error";
break;
}
return os;
}
void ErrorThrower::Format(i::Handle<i::JSFunction> constructor,
const char* format, va_list args) {
// Only report the first error.
if (error()) return;
char buffer[256];
base::OS::VSNPrintF(buffer, 255, format, args);
constexpr int kMaxErrorMessageLength = 256;
EmbeddedVector<char, kMaxErrorMessageLength> buffer;
std::ostringstream str;
if (context_ != nullptr) {
str << context_ << ": ";
int context_len = 0;
if (context_) {
context_len = SNPrintF(buffer, "%s: ", context_);
CHECK_LE(0, context_len); // check for overflow.
}
str << buffer;
int message_len =
VSNPrintF(buffer.SubVector(context_len, buffer.length()), format, args);
CHECK_LE(0, message_len); // check for overflow.
Vector<char> whole_message = buffer.SubVector(0, context_len + message_len);
i::Handle<i::String> message =
isolate_->factory()->NewStringFromAsciiChecked(str.str().c_str());
isolate_->factory()
->NewStringFromOneByte(Vector<uint8_t>::cast(whole_message))
.ToHandleChecked();
exception_ = isolate_->factory()->NewError(constructor, message);
}
......
......@@ -8,6 +8,7 @@
#include <memory>
#include "src/base/compiler-specific.h"
#include "src/utils.h"
#include "src/handles.h"
#include "src/globals.h"
......@@ -19,62 +20,74 @@ class Isolate;
namespace wasm {
// Error codes for programmatic checking of the decoder's verification.
enum ErrorCode {
kSuccess,
kError, // TODO(titzer): introduce real error codes
};
// The overall result of decoding a function or a module.
template <typename T>
struct Result {
Result() : val(), error_code(kSuccess), error_offset(0) {}
Result(Result&& other) { *this = std::move(other); }
Result& operator=(Result&& other) {
MoveFrom(other);
val = other.val;
return *this;
}
class Result {
public:
Result() = default;
template <typename S>
explicit Result(S&& value) : val(value) {}
T val;
ErrorCode error_code;
uint32_t error_offset;
std::unique_ptr<char[]> error_msg;
template <typename S>
Result(Result<S>&& other)
: val(std::move(other.val)),
error_offset(other.error_offset),
error_msg(std::move(other.error_msg)) {}
bool ok() const { return error_code == kSuccess; }
bool failed() const { return error_code != kSuccess; }
Result& operator=(Result&& other) = default;
T val = T{};
uint32_t error_offset = 0;
std::string error_msg;
bool ok() const { return error_msg.empty(); }
bool failed() const { return !ok(); }
template <typename V>
void MoveFrom(Result<V>& that) {
error_code = that.error_code;
void MoveErrorFrom(Result<V>& that) {
error_offset = that.error_offset;
error_msg = std::move(that.error_msg);
// Use {swap()} + {clear()} instead of move assign, as {that} might still be
// used afterwards.
error_msg.swap(that.error_msg);
that.error_msg.clear();
}
private:
DISALLOW_COPY_AND_ASSIGN(Result);
};
void PRINTF_FORMAT(2, 3) error(const char* format, ...) {
va_list args;
va_start(args, format);
verror(format, args);
va_end(args);
}
template <typename T>
std::ostream& operator<<(std::ostream& os, const Result<T>& result) {
os << "Result = ";
if (result.ok()) {
if (result.val != nullptr) {
os << *result.val;
} else {
os << "success (no value)";
void PRINTF_FORMAT(2, 0) verror(const char* format, va_list args) {
size_t len = base::bits::RoundUpToPowerOfTwo32(
static_cast<uint32_t>(strlen(format)));
// Allocate increasingly large buffers until the message fits.
for (;; len *= 2) {
DCHECK_GE(kMaxInt, len);
error_msg.resize(len);
int written =
VSNPrintF(Vector<char>(&error_msg.front(), static_cast<int>(len)),
format, args);
if (written < 0) continue; // not enough space.
if (written == 0) error_msg = "Error"; // assign default message.
return;
}
} else if (result.error_msg.get() != nullptr) {
os << result.error_msg.get() << " @" << result.error_offset;
} else {
os << result.error_code;
}
os << std::endl;
return os;
}
V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream& os,
const ErrorCode& error_code);
static Result<T> PRINTF_FORMAT(1, 2) Error(const char* format, ...) {
va_list args;
va_start(args, format);
Result<T> result;
result.verror(format, args);
va_end(args);
return result;
}
private:
DISALLOW_COPY_AND_ASSIGN(Result);
};
// A helper for generating error messages that bubble up to JS exceptions.
class V8_EXPORT_PRIVATE ErrorThrower {
......@@ -91,9 +104,8 @@ class V8_EXPORT_PRIVATE ErrorThrower {
template <typename T>
void CompileFailed(const char* error, Result<T>& result) {
std::ostringstream str;
str << error << result;
CompileError("%s", str.str().c_str());
DCHECK(result.failed());
CompileError("%s", result.error_msg.c_str());
}
i::Handle<i::Object> Reify() {
......@@ -113,6 +125,7 @@ class V8_EXPORT_PRIVATE ErrorThrower {
i::Handle<i::Object> exception_;
bool wasm_error_ = false;
};
} // namespace wasm
} // namespace internal
} // namespace v8
......
......@@ -391,8 +391,8 @@ inline void TestBuildingGraph(Zone* zone, JSGraph* jsgraph, ModuleEnv* module,
uint32_t pc = result.error_offset;
std::ostringstream str;
str << "Verification failed: " << result.error_code << " pc = +" << pc
<< ", msg = " << result.error_msg.get();
str << "Verification failed; pc = +" << pc
<< ", msg = " << result.error_msg.c_str();
FATAL(str.str().c_str());
}
builder.Int64LoweringForTesting();
......
......@@ -36,7 +36,7 @@ const WasmModule* DecodeWasmModuleForTesting(
if (decoding_result.failed()) {
// Module verification failed. throw.
thrower->CompileError("WASM.compileRun() failed: %s",
decoding_result.error_msg.get());
decoding_result.error_msg.c_str());
}
if (thrower->error()) {
......
......@@ -51,37 +51,36 @@ static const WasmOpcode kInt32BinopOpcodes[] = {
#define WASM_BRV_IF_ZERO(depth, val) \
val, WASM_ZERO, kExprBrIf, static_cast<byte>(depth)
#define EXPECT_VERIFIES_C(sig, x) \
Verify(kSuccess, sigs.sig(), x, x + arraysize(x))
#define EXPECT_VERIFIES_C(sig, x) Verify(true, sigs.sig(), x, x + arraysize(x))
#define EXPECT_FAILURE_C(sig, x) Verify(kError, sigs.sig(), x, x + arraysize(x))
#define EXPECT_FAILURE_C(sig, x) Verify(false, sigs.sig(), x, x + arraysize(x))
#define EXPECT_VERIFIES_SC(sig, x) Verify(kSuccess, sig, x, x + arraysize(x))
#define EXPECT_VERIFIES_SC(sig, x) Verify(true, sig, x, x + arraysize(x))
#define EXPECT_FAILURE_SC(sig, x) Verify(kError, sig, x, x + arraysize(x))
#define EXPECT_FAILURE_SC(sig, x) Verify(false, sig, x, x + arraysize(x))
#define EXPECT_VERIFIES_S(env, ...) \
do { \
static byte code[] = {__VA_ARGS__}; \
Verify(kSuccess, env, code, code + arraysize(code)); \
#define EXPECT_VERIFIES_S(env, ...) \
do { \
static byte code[] = {__VA_ARGS__}; \
Verify(true, env, code, code + arraysize(code)); \
} while (false)
#define EXPECT_FAILURE_S(env, ...) \
do { \
static byte code[] = {__VA_ARGS__}; \
Verify(kError, env, code, code + arraysize(code)); \
#define EXPECT_FAILURE_S(env, ...) \
do { \
static byte code[] = {__VA_ARGS__}; \
Verify(false, env, code, code + arraysize(code)); \
} while (false)
#define EXPECT_VERIFIES(sig, ...) \
do { \
static const byte code[] = {__VA_ARGS__}; \
Verify(kSuccess, sigs.sig(), code, code + sizeof(code)); \
#define EXPECT_VERIFIES(sig, ...) \
do { \
static const byte code[] = {__VA_ARGS__}; \
Verify(true, sigs.sig(), code, code + sizeof(code)); \
} while (false)
#define EXPECT_FAILURE(sig, ...) \
do { \
static const byte code[] = {__VA_ARGS__}; \
Verify(kError, sigs.sig(), code, code + sizeof(code)); \
#define EXPECT_FAILURE(sig, ...) \
do { \
static const byte code[] = {__VA_ARGS__}; \
Verify(false, sigs.sig(), code, code + sizeof(code)); \
} while (false)
static bool old_eh_flag;
......@@ -126,7 +125,7 @@ class FunctionBodyDecoderTest : public TestWithZone {
// Prepends local variable declarations and renders nice error messages for
// verification failures.
void Verify(ErrorCode expected, FunctionSig* sig, const byte* start,
void Verify(bool expected_success, FunctionSig* sig, const byte* start,
const byte* end) {
PrepareBytecode(&start, &end);
......@@ -135,20 +134,16 @@ class FunctionBodyDecoderTest : public TestWithZone {
zone()->allocator(), module == nullptr ? nullptr : module->module, sig,
start, end);
if (result.error_code != expected) {
if (result.ok() != expected_success) {
uint32_t pc = result.error_offset;
std::ostringstream str;
if (expected == kSuccess) {
str << "Verification failed: " << result.error_code << " pc = +" << pc
<< ", msg = " << result.error_msg.get();
if (expected_success) {
str << "Verification failed: pc = +" << pc
<< ", msg = " << result.error_msg;
} else {
str << "Verification expected: " << expected << ", but got "
<< result.error_code;
if (result.error_code != kSuccess) {
str << " pc = +" << pc;
}
str << "Verification successed, expected failure; pc = +" << pc;
}
EXPECT_TRUE(false) << str.str().c_str();
EXPECT_TRUE(false) << str.str();
}
}
......@@ -265,8 +260,8 @@ TEST_F(FunctionBodyDecoderTest, Int32Const1) {
TEST_F(FunctionBodyDecoderTest, EmptyFunction) {
byte code[] = {0};
Verify(kSuccess, sigs.v_v(), code, code);
Verify(kError, sigs.i_i(), code, code);
Verify(true, sigs.v_v(), code, code);
Verify(false, sigs.i_i(), code, code);
}
TEST_F(FunctionBodyDecoderTest, IncompleteIf1) {
......@@ -322,7 +317,7 @@ TEST_F(FunctionBodyDecoderTest, Int32Const_off_end) {
byte code[] = {kExprI32Const, 0xaa, 0xbb, 0xcc, 0x44};
for (int size = 1; size <= 4; size++) {
Verify(kError, sigs.i_i(), code, code + size);
Verify(false, sigs.i_i(), code, code + size);
}
}
......@@ -508,7 +503,7 @@ TEST_F(FunctionBodyDecoderTest, BlockN) {
buffer[0] = kExprBlock;
buffer[1] = kLocalVoid;
buffer[i + 2] = kExprEnd;
Verify(kSuccess, sigs.v_i(), buffer, buffer + i + 3);
Verify(true, sigs.v_i(), buffer, buffer + i + 3);
}
}
......@@ -653,7 +648,7 @@ TEST_F(FunctionBodyDecoderTest, BlockN_off_end) {
byte code[] = {WASM_BLOCK(kExprNop, kExprNop, kExprNop, kExprNop)};
EXPECT_VERIFIES_C(v_v, code);
for (size_t i = 1; i < arraysize(code); i++) {
Verify(kError, sigs.v_v(), code, code + i);
Verify(false, sigs.v_v(), code, code + i);
}
}
......@@ -983,7 +978,7 @@ TEST_F(FunctionBodyDecoderTest, If_off_end) {
static const byte kCode[] = {
WASM_IF_ELSE(WASM_GET_LOCAL(0), WASM_GET_LOCAL(0), WASM_GET_LOCAL(0))};
for (size_t len = 3; len < arraysize(kCode); len++) {
Verify(kError, sigs.i_i(), kCode, kCode + len);
Verify(false, sigs.i_i(), kCode, kCode + len);
}
}
......@@ -2131,7 +2126,7 @@ TEST_F(FunctionBodyDecoderTest, BrTable2b) {
TEST_F(FunctionBodyDecoderTest, BrTable_off_end) {
static byte code[] = {B1(WASM_BR_TABLE(WASM_GET_LOCAL(0), 0, BR_TARGET(0)))};
for (size_t len = 1; len < sizeof(code); len++) {
Verify(kError, sigs.i_i(), code, code + len);
Verify(false, sigs.i_i(), code, code + len);
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment