From 591ac7c3a8d215c941f2b5a3c2c94c893d7c2fd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 30 May 2019 14:10:00 +0200 Subject: [PATCH 1/4] loader: Add more interesting VM mocks for unit tests --- test/unittests/test_loader.cpp | 76 +++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/test/unittests/test_loader.cpp b/test/unittests/test_loader.cpp index 06a92ac..8ae7ada 100644 --- a/test/unittests/test_loader.cpp +++ b/test/unittests/test_loader.cpp @@ -2,11 +2,12 @@ // Copyright 2018-2019 The EVMC Authors. // Licensed under the Apache License, Version 2.0. +#include #include - #include - #include +#include +#include #if _WIN32 static constexpr bool is_windows = true; @@ -15,22 +16,93 @@ static constexpr bool is_windows = false; #endif extern "C" { +/// The library path expected by mocked evmc_test_load_library(). extern const char* evmc_test_library_path; + +/// The symbol name expected by mocked evmc_test_get_symbol_address(). extern const char* evmc_test_library_symbol; + +/// The pointer to function returned by evmc_test_get_symbol_address(). extern evmc_create_fn evmc_test_create_fn; } class loader : public ::testing::Test { protected: + static int create_count; + static int destroy_count; + static std::unordered_map supported_options; + static std::vector> recorded_options; + + loader() noexcept + { + create_count = 0; + destroy_count = 0; + supported_options.clear(); + recorded_options.clear(); + } + void setup(const char* path, const char* symbol, evmc_create_fn fn) noexcept { evmc_test_library_path = path; evmc_test_library_symbol = symbol; evmc_test_create_fn = fn; } + + static void destroy(evmc_instance*) noexcept { ++destroy_count; } + + static evmc_set_option_result set_option(evmc_instance*, + const char* name, + const char* value) noexcept + { + recorded_options.push_back({name, value ? value : ""}); // NOLINT + + auto it = supported_options.find(name); + if (it == supported_options.end()) + return EVMC_SET_OPTION_INVALID_NAME; + if (it->second == nullptr) + return value == nullptr ? EVMC_SET_OPTION_SUCCESS : EVMC_SET_OPTION_INVALID_VALUE; + if (value == nullptr) + return EVMC_SET_OPTION_INVALID_VALUE; + return std::string{value} == it->second ? EVMC_SET_OPTION_SUCCESS : + EVMC_SET_OPTION_INVALID_VALUE; + } + + /// Creates a VM mock with only destroy() method. + static evmc_instance* create_vm_barebone() + { + static auto instance = + evmc_instance{EVMC_ABI_VERSION, "", "", destroy, nullptr, nullptr, nullptr, nullptr}; + ++create_count; + return &instance; + } + + /// Creates a VM mock with ABI version different than in this project. + static evmc_instance* create_vm_with_wrong_abi() + { + constexpr auto wrong_abi_version = 1985; + static_assert(wrong_abi_version != EVMC_ABI_VERSION, ""); + static auto instance = + evmc_instance{wrong_abi_version, "", "", destroy, nullptr, nullptr, nullptr, nullptr}; + ++create_count; + return &instance; + } + + /// Creates a VM mock with optional set_option() method. + static evmc_instance* create_vm_with_set_option() noexcept + { + static auto instance = + evmc_instance{EVMC_ABI_VERSION, "", "", destroy, nullptr, nullptr, nullptr, set_option}; + ++create_count; + return &instance; + } }; +int loader::create_count = 0; +int loader::destroy_count = 0; +std::unordered_map loader::supported_options; +std::vector> loader::recorded_options; + static evmc_instance* create_aaa() { return (evmc_instance*)0xaaa; From ba10da07d002c6a9aa30a828e4759fa7fbebe884 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 30 May 2019 14:15:20 +0200 Subject: [PATCH 2/4] loader: Use new VM mock with wrong ABI version --- test/unittests/test_loader.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/test/unittests/test_loader.cpp b/test/unittests/test_loader.cpp index 8ae7ada..8083be6 100644 --- a/test/unittests/test_loader.cpp +++ b/test/unittests/test_loader.cpp @@ -118,12 +118,6 @@ static evmc_instance* create_failure() return nullptr; } -static evmc_instance* create_abi42() -{ - static int abi_version = 42; - return reinterpret_cast(&abi_version); -} - TEST_F(loader, load_nonexistent) { constexpr auto path = "nonexistent"; @@ -282,12 +276,14 @@ TEST_F(loader, load_and_create_failure) TEST_F(loader, load_and_create_abi_mismatch) { - setup("abi42.vm", "evmc_create", create_abi42); + setup("abi1985.vm", "evmc_create", create_vm_with_wrong_abi); evmc_loader_error_code ec; auto vm = evmc_load_and_create(evmc_test_library_path, &ec); EXPECT_TRUE(vm == nullptr); EXPECT_EQ(ec, EVMC_LOADER_ABI_VERSION_MISMATCH); - EXPECT_STREQ(evmc_last_error_msg(), - "EVMC ABI version 42 of abi42.vm mismatches the expected version 6"); + const auto expected_error_msg = + "EVMC ABI version 1985 of abi1985.vm mismatches the expected version " + + std::to_string(EVMC_ABI_VERSION); + EXPECT_EQ(evmc_last_error_msg(), expected_error_msg); } From 7b02081f2e5d2f3b078fcd6492c5a64006e829dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 30 May 2019 14:21:04 +0200 Subject: [PATCH 3/4] loader: Destroy VM with incompatible ABI When a VM is loaded but turns out is has incompatible ABI version it should be destroyed before reporting the loading error and returning a null handle. Fixes https://github.com/ethereum/evmc/issues/305 --- lib/loader/loader.c | 1 + test/unittests/test_loader.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/loader/loader.c b/lib/loader/loader.c index 94a6c88..935d30c 100644 --- a/lib/loader/loader.c +++ b/lib/loader/loader.c @@ -207,6 +207,7 @@ struct evmc_instance* evmc_load_and_create(const char* filename, if (!evmc_is_abi_compatible(instance)) { + evmc_destroy(instance); *error_code = set_error(EVMC_LOADER_ABI_VERSION_MISMATCH, "EVMC ABI version %d of %s mismatches the expected version %d", instance->abi_version, filename, EVMC_ABI_VERSION); diff --git a/test/unittests/test_loader.cpp b/test/unittests/test_loader.cpp index 8083be6..bdde22e 100644 --- a/test/unittests/test_loader.cpp +++ b/test/unittests/test_loader.cpp @@ -286,4 +286,5 @@ TEST_F(loader, load_and_create_abi_mismatch) "EVMC ABI version 1985 of abi1985.vm mismatches the expected version " + std::to_string(EVMC_ABI_VERSION); EXPECT_EQ(evmc_last_error_msg(), expected_error_msg); + EXPECT_EQ(destroy_count, create_count); } From b0133730be6f18baf38a9cd5387b927e1cf3c7e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 30 May 2019 14:25:55 +0200 Subject: [PATCH 4/4] Add CHANGELOG entry about the fix in loader --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 968fa6d..e058137 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ [[#261](https://github.com/ethereum/evmc/issues/261), [#263](https://github.com/ethereum/evmc/pull/263)] The `vmtester` tool now builds with MSVC with `/std:c++17`. +- Fixed: + [[#305](https://github.com/ethereum/evmc/issues/305), + [#306](https://github.com/ethereum/evmc/pull/306)] + A loaded VM with incompatible ABI version is not properly destroyed. ## [6.2.2] - 2019-05-16