Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spectest-interp: assert_malformed must error in reader alone #2252

Merged
merged 1 commit into from
Jun 12, 2023
Merged

spectest-interp: assert_malformed must error in reader alone #2252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 92 additions & 11 deletions src/tools/spectest-interp.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <vector>

#include "wabt/binary-reader-ir.h"
#include "wabt/binary-reader-nop.h"
#include "wabt/binary-reader.h"
#include "wabt/cast.h"
#include "wabt/common.h"
Expand Down Expand Up @@ -275,7 +276,7 @@ TypedValue GetLane(const TypedValue& tv, Type lane_type, int lane) {
return result;
}

bool ValidIR(const std::string& filename) {
bool CheckIR(const std::string& filename, bool validate) {
std::vector<uint8_t> file_data;

if (Failed(ReadFile(filename, &file_data))) {
Expand All @@ -295,10 +296,22 @@ bool ValidIR(const std::string& filename) {
return false;
}

if (!validate) {
return true;
}

return Succeeded(
ValidateModule(&module, &errors, ValidateOptions{s_features}));
}

bool WellformedIR(const std::string& filename) {
return CheckIR(filename, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is "well formed" something that exists in the spec, or something we a creating there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be the spec's term (e.g. "A byte sequence is a well-formed encoding of a module if and only if it is generated by the grammar." from 5.1). If you think it's better to match the script language, we could call these functions "InvalidIR" and "MalformedIR" (instead of "ValidIR" and "WellformedIR") and just negate the callsites?

}

bool ValidIR(const std::string& filename) {
return CheckIR(filename, true);
}

class AssertReturnCommand : public CommandMixin<CommandType::AssertReturn> {
public:
Action action;
Expand Down Expand Up @@ -1224,8 +1237,15 @@ class CommandRunner {

void TallyCommand(wabt::Result);

wabt::Result ReadInvalidTextModule(std::string_view module_filename,
const std::string& header);
wabt::Result ReadTextModule(std::string_view module_filename,
const std::string& header,
bool validate);
wabt::Result ReadMalformedBinaryModule(std::string_view module_filename,
Errors* errors);
wabt::Result ReadMalformedModule(int line_number,
std::string_view module_filename,
ModuleType module_type,
const char* desc);
wabt::Result ReadInvalidModule(int line_number,
std::string_view module_filename,
ModuleType module_type,
Expand Down Expand Up @@ -1407,9 +1427,9 @@ ActionResult CommandRunner::RunAction(int line_number,
return result;
}

wabt::Result CommandRunner::ReadInvalidTextModule(
std::string_view module_filename,
const std::string& header) {
wabt::Result CommandRunner::ReadTextModule(std::string_view module_filename,
const std::string& header,
bool validate) {
std::vector<uint8_t> file_data;
wabt::Result result = ReadFile(module_filename, &file_data);
Errors errors;
Expand All @@ -1419,6 +1439,11 @@ wabt::Result CommandRunner::ReadInvalidTextModule(
std::unique_ptr<wabt::Module> module;
WastParseOptions options(s_features);
result = ParseWatModule(lexer.get(), &module, &errors, &options);

if (validate && Succeeded(result)) {
result =
ValidateModule(module.get(), &errors, ValidateOptions{s_features});
}
}

auto line_finder = lexer->MakeLineFinder();
Expand Down Expand Up @@ -1463,7 +1488,7 @@ wabt::Result CommandRunner::ReadInvalidModule(int line_number,

switch (module_type) {
case ModuleType::Text: {
return ReadInvalidTextModule(module_filename, header);
return ReadTextModule(module_filename, header, true);
}

case ModuleType::Binary: {
Expand All @@ -1482,6 +1507,61 @@ wabt::Result CommandRunner::ReadInvalidModule(int line_number,
WABT_UNREACHABLE;
}

wabt::Result CommandRunner::ReadMalformedBinaryModule(
std::string_view module_filename,
Errors* errors) {
std::vector<uint8_t> file_data;

CHECK_RESULT(ReadFile(module_filename, &file_data));

const bool kReadDebugNames = true;
const bool kStopOnFirstError = true;
const bool kFailOnCustomSectionError = true;
ReadBinaryOptions options(s_features, s_log_stream.get(), kReadDebugNames,
kStopOnFirstError, kFailOnCustomSectionError);

class BinaryReaderErrorLogging : public BinaryReaderNop {
Errors* errors_;

public:
BinaryReaderErrorLogging(Errors* errors) : errors_(errors) {}

bool OnError(const Error& error) override {
errors_->push_back(error);
return true;
}
};

BinaryReaderErrorLogging reader_delegate{errors};
return ReadBinary(file_data.data(), file_data.size(), &reader_delegate,
options);
}

wabt::Result CommandRunner::ReadMalformedModule(
int line_number,
std::string_view module_filename,
ModuleType module_type,
const char* desc) {
std::string header = StringPrintf(
"%s:%d: %s passed", source_filename_.c_str(), line_number, desc);

switch (module_type) {
case ModuleType::Text: {
return ReadTextModule(module_filename, header, false);
}

case ModuleType::Binary: {
Errors errors;
wabt::Result result = ReadMalformedBinaryModule(module_filename, &errors);
FormatErrorsToFile(errors, Location::Type::Binary, {}, stdout, header,
PrintHeader::Once);
return result;
}
}

WABT_UNREACHABLE;
}

Extern::Ptr CommandRunner::GetImport(const std::string& module,
const std::string& name) {
auto mod_iter = registry_.find(module);
Expand Down Expand Up @@ -1564,16 +1644,17 @@ wabt::Result CommandRunner::OnActionCommand(const ActionCommand* command) {

wabt::Result CommandRunner::OnAssertMalformedCommand(
const AssertMalformedCommand* command) {
wabt::Result result = ReadInvalidModule(command->line, command->filename,
command->type, "assert_malformed");
wabt::Result result = ReadMalformedModule(command->line, command->filename,
command->type, "assert_malformed");
if (Succeeded(result)) {
PrintError(command->line, "expected module to be malformed: \"%s\"",
command->filename.c_str());
return wabt::Result::Error;
}

if (ValidIR(command->filename)) {
PrintError(command->line, "IR Validator thinks module is valid: \"%s\"",
if (WellformedIR(command->filename)) {
PrintError(command->line,
"BinaryReaderIR thinks module is well-formed: \"%s\"",
command->filename.c_str());
return wabt::Result::Error;
}
Expand Down
3 changes: 1 addition & 2 deletions test/spec/binary.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,7 @@ out/test/spec/binary.wast:1772: assert_malformed passed:
out/test/spec/binary.wast:1786: assert_malformed passed:
000001a: error: unfinished section (expected end: 0x1b)
out/test/spec/binary.wast:1817: assert_malformed passed:
out/test/spec/binary/binary.174.wasm:0000025: error: function type variable out of range: 11 (max 1)
0000025: error: OnBlockExpr callback failed
0000027: error: function body must end with END opcode
out/test/spec/binary.wast:1852: assert_malformed passed:
0000017: error: multiple Start sections
177/177 tests passed.
Expand Down
3 changes: 1 addition & 2 deletions test/spec/exception-handling/binary.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,7 @@ out/test/spec/exception-handling/binary.wast:1772: assert_malformed passed:
out/test/spec/exception-handling/binary.wast:1786: assert_malformed passed:
000001a: error: unfinished section (expected end: 0x1b)
out/test/spec/exception-handling/binary.wast:1817: assert_malformed passed:
out/test/spec/exception-handling/binary/binary.174.wasm:0000025: error: function type variable out of range: 11 (max 1)
0000025: error: OnBlockExpr callback failed
0000027: error: function body must end with END opcode
out/test/spec/exception-handling/binary.wast:1852: assert_malformed passed:
0000017: error: multiple Start sections
177/177 tests passed.
Expand Down
3 changes: 1 addition & 2 deletions test/spec/memory64/binary.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,7 @@ out/test/spec/memory64/binary.wast:900: assert_malformed passed:
out/test/spec/memory64/binary.wast:914: assert_malformed passed:
000001a: error: unfinished section (expected end: 0x1b)
out/test/spec/memory64/binary.wast:945: assert_malformed passed:
out/test/spec/memory64/binary/binary.102.wasm:0000025: error: function type variable out of range: 11 (max 1)
0000025: error: OnBlockExpr callback failed
0000026: error: unable to read uint8_t: opcode
out/test/spec/memory64/binary.wast:980: assert_malformed passed:
0000017: error: multiple Start sections
105/105 tests passed.
Expand Down
3 changes: 1 addition & 2 deletions test/spec/multi-memory/binary.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,7 @@ out/test/spec/multi-memory/binary.wast:1581: assert_malformed passed:
out/test/spec/multi-memory/binary.wast:1595: assert_malformed passed:
000001a: error: unfinished section (expected end: 0x1b)
out/test/spec/multi-memory/binary.wast:1626: assert_malformed passed:
out/test/spec/multi-memory/binary/binary.164.wasm:0000025: error: function type variable out of range: 11 (max 1)
0000025: error: OnBlockExpr callback failed
0000027: error: function body must end with END opcode
out/test/spec/multi-memory/binary.wast:1661: assert_malformed passed:
0000017: error: multiple Start sections
167/167 tests passed.
Expand Down