From 9ae7f7b68d5ec2eddfafbf233cce50405cd88f94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 19 Oct 2020 19:51:05 +0200 Subject: [PATCH 1/3] Add FIZZY_UNREACHABLE Add macro FIZZY_UNREACHABLE to mark unreachable code. It fails a standard assert in build with asserts enabled. It fails in UBSan. It is an optimization hint for optimized builds. --- lib/fizzy/CMakeLists.txt | 6 ++++++ lib/fizzy/asserts.cpp | 20 ++++++++++++++++++++ lib/fizzy/asserts.hpp | 28 ++++++++++++++++++++++++++++ test/unittests/CMakeLists.txt | 1 + test/unittests/asserts_test.cpp | 14 ++++++++++++++ 5 files changed, 69 insertions(+) create mode 100644 lib/fizzy/asserts.cpp create mode 100644 lib/fizzy/asserts.hpp create mode 100644 test/unittests/asserts_test.cpp diff --git a/lib/fizzy/CMakeLists.txt b/lib/fizzy/CMakeLists.txt index e77ac1cbb..c25f92408 100644 --- a/lib/fizzy/CMakeLists.txt +++ b/lib/fizzy/CMakeLists.txt @@ -10,6 +10,8 @@ target_include_directories(fizzy PUBLIC ${FIZZY_INCLUDE_DIR}) target_sources( fizzy PRIVATE ${FIZZY_INCLUDE_DIR}/fizzy/fizzy.h + asserts.cpp + asserts.hpp cxx20/bit.hpp cxx20/span.hpp bytes.hpp @@ -37,6 +39,10 @@ target_sources( value.hpp ) +if(CMAKE_BUILD_TYPE STREQUAL Coverage AND CMAKE_CXX_COMPILER_ID MATCHES GNU) + set_source_files_properties(asserts.cpp PROPERTIES COMPILE_DEFINITIONS GCOV) +endif() + # The fizzy::fizzy-internal links fizzy::fizzy library with access to internal headers. add_library(fizzy-internal INTERFACE) add_library(fizzy::fizzy-internal ALIAS fizzy-internal) diff --git a/lib/fizzy/asserts.cpp b/lib/fizzy/asserts.cpp new file mode 100644 index 000000000..ab91bdbce --- /dev/null +++ b/lib/fizzy/asserts.cpp @@ -0,0 +1,20 @@ +// Fizzy: A fast WebAssembly interpreter +// Copyright 2020 The Fizzy Authors. +// SPDX-License-Identifier: Apache-2.0 + +#include "asserts.hpp" + +#ifdef GCOV +extern "C" void __gcov_flush(); +#endif + +namespace fizzy +{ +bool unreachable() noexcept +{ +#ifdef GCOV + __gcov_flush(); +#endif + return false; // LCOV_EXCL_LINE +} +} // namespace fizzy diff --git a/lib/fizzy/asserts.hpp b/lib/fizzy/asserts.hpp new file mode 100644 index 000000000..bc2012435 --- /dev/null +++ b/lib/fizzy/asserts.hpp @@ -0,0 +1,28 @@ +// Fizzy: A fast WebAssembly interpreter +// Copyright 2020 The Fizzy Authors. +// SPDX-License-Identifier: Apache-2.0 + +#include + +#ifndef __has_builtin +#define __has_builtin(name) 0 +#endif + +#ifndef __has_feature +#define __has_feature(name) 0 +#endif + + +namespace fizzy +{ +/// A helper for assert(unreachable()). Always returns false. Also flushes coverage data. +bool unreachable() noexcept; +} // namespace fizzy + +#ifndef NDEBUG +#define FIZZY_UNREACHABLE() assert(fizzy::unreachable()) +#elif __has_builtin(__builtin_unreachable) +#define FIZZY_UNREACHABLE() __builtin_unreachable() +#else +#define FIZZY_UNREACHABLE() (void)0 +#endif diff --git a/test/unittests/CMakeLists.txt b/test/unittests/CMakeLists.txt index 94347c894..71474dd05 100644 --- a/test/unittests/CMakeLists.txt +++ b/test/unittests/CMakeLists.txt @@ -10,6 +10,7 @@ target_link_libraries(fizzy-unittests PRIVATE fizzy::fizzy-internal fizzy::test- target_sources( fizzy-unittests PRIVATE api_test.cpp + asserts_test.cpp capi_test.cpp constexpr_vector_test.cpp cxx20_bit_test.cpp diff --git a/test/unittests/asserts_test.cpp b/test/unittests/asserts_test.cpp new file mode 100644 index 000000000..1eaacd15f --- /dev/null +++ b/test/unittests/asserts_test.cpp @@ -0,0 +1,14 @@ +// Fizzy: A fast WebAssembly interpreter +// Copyright 2020 The Fizzy Authors. +// SPDX-License-Identifier: Apache-2.0 + +#include "asserts.hpp" +#include + +#if !defined(NDEBUG) || __has_feature(undefined_behavior_sanitizer) +TEST(asserts_death, unreachable) +{ + static constexpr auto unreachable = []() noexcept { FIZZY_UNREACHABLE(); }; + EXPECT_DEATH(unreachable(), "unreachable"); +} +#endif From ec905dcab5bccb91cc6f9e6a2af152d3202802bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 19 Oct 2020 19:56:56 +0200 Subject: [PATCH 2/3] Replace assert(false) with FIZZY_UNREACHABLE --- lib/fizzy/execute.cpp | 4 ++-- lib/fizzy/parser.cpp | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp index 379abcb74..c3a5e63af 100644 --- a/lib/fizzy/execute.cpp +++ b/lib/fizzy/execute.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "execute.hpp" +#include "asserts.hpp" #include "cxx20/bit.hpp" #include "stack.hpp" #include "trunc_boundaries.hpp" @@ -1554,8 +1555,7 @@ ExecutionResult execute(Instance& instance, FuncIdx func_idx, const Value* args, } default: - assert(false); - break; + FIZZY_UNREACHABLE(); } } diff --git a/lib/fizzy/parser.cpp b/lib/fizzy/parser.cpp index f8d42fc5f..478773dec 100644 --- a/lib/fizzy/parser.cpp +++ b/lib/fizzy/parser.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 #include "parser.hpp" +#include "asserts.hpp" #include "leb128.hpp" #include "limits.hpp" #include "types.hpp" @@ -532,8 +533,8 @@ std::unique_ptr parse(bytes_view input) case ExternalKind::Global: module->imported_global_types.emplace_back(import.desc.global); break; - default: - assert(false); + default: // LCOV_EXCL_LINE + FIZZY_UNREACHABLE(); // LCOV_EXCL_LINE } } @@ -635,8 +636,8 @@ std::unique_ptr parse(bytes_view input) if (export_.index >= total_global_count) throw validation_error{"invalid index of an exported global"}; break; - default: - assert(false); + default: // LCOV_EXCL_LINE + FIZZY_UNREACHABLE(); // LCOV_EXCL_LINE } if (!export_names.emplace(export_.name).second) throw validation_error{"duplicate export name " + export_.name}; From cbec087dcfdffc52f533ef9dc0e11297fc7b6558 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Mon, 19 Oct 2020 20:50:34 +0200 Subject: [PATCH 3/3] test: Add execution death test --- test/unittests/CMakeLists.txt | 1 + test/unittests/execute_death_test.cpp | 28 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 test/unittests/execute_death_test.cpp diff --git a/test/unittests/CMakeLists.txt b/test/unittests/CMakeLists.txt index 71474dd05..6c2ea9951 100644 --- a/test/unittests/CMakeLists.txt +++ b/test/unittests/CMakeLists.txt @@ -18,6 +18,7 @@ target_sources( end_to_end_test.cpp execute_call_test.cpp execute_control_test.cpp + execute_death_test.cpp execute_floating_point_test.cpp execute_numeric_test.cpp execute_test.cpp diff --git a/test/unittests/execute_death_test.cpp b/test/unittests/execute_death_test.cpp new file mode 100644 index 000000000..b94610a5c --- /dev/null +++ b/test/unittests/execute_death_test.cpp @@ -0,0 +1,28 @@ +// Fizzy: A fast WebAssembly interpreter +// Copyright 2020 The Fizzy Authors. +// SPDX-License-Identifier: Apache-2.0 + +#include "asserts.hpp" +#include "execute.hpp" +#include + +using namespace fizzy; + +#if !defined(NDEBUG) || __has_feature(undefined_behavior_sanitizer) +TEST(execute_death, malformed_instruction_opcode) +{ + constexpr auto malformed_opcode = static_cast(6); + + Code code; + code.instructions.emplace_back(malformed_opcode); + code.instructions.emplace_back(Instr::end); + + auto module = std::make_unique(); + module->typesec.emplace_back(FuncType{}); + module->funcsec.emplace_back(TypeIdx{0}); + module->codesec.emplace_back(std::move(code)); + + auto instance = instantiate(std::move(module)); + EXPECT_DEATH(execute(*instance, 0, nullptr), "unreachable"); +} +#endif