diff --git a/bindings/go/evmc/evmc.go b/bindings/go/evmc/evmc.go index a456869..dd3e3a0 100644 --- a/bindings/go/evmc/evmc.go +++ b/bindings/go/evmc/evmc.go @@ -58,7 +58,6 @@ static struct evmc_result execute_wrapper(struct evmc_instance* instance, import "C" import ( - "errors" "fmt" "sync" "unsafe" @@ -154,28 +153,18 @@ func Load(filename string) (instance *Instance, err error) { var loaderErr C.enum_evmc_loader_error_code handle := C.evmc_load_and_create(cfilename, &loaderErr) C.free(unsafe.Pointer(cfilename)) - switch loaderErr { - case C.EVMC_LOADER_SUCCESS: + + if loaderErr == C.EVMC_LOADER_SUCCESS { instance = &Instance{handle} - case C.EVMC_LOADER_CANNOT_OPEN: - optionalErrMsg := C.evmc_last_error_msg() - if optionalErrMsg != nil { - msg := C.GoString(optionalErrMsg) - err = fmt.Errorf("evmc loader: %s", msg) + } else { + errMsg := C.evmc_last_error_msg() + if errMsg != nil { + err = fmt.Errorf("EVMC loading error: %s", C.GoString(errMsg)) } else { - err = fmt.Errorf("evmc loader: cannot open %s", filename) + err = fmt.Errorf("EVMC loading error %d", int(loaderErr)) } - case C.EVMC_LOADER_SYMBOL_NOT_FOUND: - err = fmt.Errorf("evmc loader: the EVMC create function not found in %s", filename) - case C.EVMC_LOADER_INVALID_ARGUMENT: - panic("evmc loader: filename argument is invalid") - case C.EVMC_LOADER_INSTANCE_CREATION_FAILURE: - err = errors.New("evmc loader: VM instance creation failure") - case C.EVMC_LOADER_ABI_VERSION_MISMATCH: - err = errors.New("evmc loader: ABI version mismatch") - default: - panic(fmt.Sprintf("evmc loader: unexpected error (%d)", int(loaderErr))) } + return instance, err } diff --git a/include/evmc/loader.h b/include/evmc/loader.h index 8b7378b..bcded63 100644 --- a/include/evmc/loader.h +++ b/include/evmc/loader.h @@ -105,10 +105,12 @@ struct evmc_instance* evmc_load_and_create(const char* filename, /** * Returns the human-readable message describing the most recent error - * that occurred in EVMC loading. + * that occurred in EVMC loading since the last call to this function. * * In case any loading function returned ::EVMC_LOADER_SUCCESS this function always returns NULL. * In case of error code other than success returned, this function MAY return the error message. + * Calling this function "consumes" the error message and the function will return NULL + * from subsequent invocations. * This function is not thread-safe. * * @return Error message or NULL if no additional information is available. diff --git a/lib/loader/loader.c b/lib/loader/loader.c index c92438c..b2078ad 100644 --- a/lib/loader/loader.c +++ b/lib/loader/loader.c @@ -8,7 +8,9 @@ #include #include +#include #include +#include #include #if defined(EVMC_LOADER_MOCK) @@ -19,15 +21,26 @@ #define DLL_OPEN(filename) LoadLibrary(filename) #define DLL_CLOSE(handle) FreeLibrary(handle) #define DLL_GET_CREATE_FN(handle, name) (evmc_create_fn)(uintptr_t) GetProcAddress(handle, name) +#define DLL_GET_ERROR_MSG() NULL #else #include #define DLL_HANDLE void* #define DLL_OPEN(filename) dlopen(filename, RTLD_LAZY) #define DLL_CLOSE(handle) dlclose(handle) #define DLL_GET_CREATE_FN(handle, name) (evmc_create_fn)(uintptr_t) dlsym(handle, name) +#define DLL_GET_ERROR_MSG() dlerror() #endif -#define PATH_MAX_LENGTH 4096 +#ifdef __has_attribute +#if __has_attribute(format) +#define ATTR_FORMAT(archetype, string_index, first_to_check) \ + __attribute__((format(archetype, string_index, first_to_check))) +#endif +#endif + +#ifndef ATTR_FORMAT +#define ATTR_FORMAT(...) +#endif #if !_WIN32 /* @@ -44,8 +57,30 @@ static void strcpy_s(char* dest, size_t destsz, const char* src) } #endif +#define PATH_MAX_LENGTH 4096 + static const char* last_error_msg = NULL; +#define LAST_ERROR_MSG_BUFFER_SIZE 511 + +// Buffer for formatted error messages. +// It has one null byte extra to avoid buffer read overflow during concurrent access. +static char last_error_msg_buffer[LAST_ERROR_MSG_BUFFER_SIZE + 1]; + +ATTR_FORMAT(printf, 2, 3) +static enum evmc_loader_error_code set_error(enum evmc_loader_error_code error_code, + const char* format, + ...) +{ + va_list args; + va_start(args, format); + if (vsnprintf(last_error_msg_buffer, LAST_ERROR_MSG_BUFFER_SIZE, format, args) < + LAST_ERROR_MSG_BUFFER_SIZE) + last_error_msg = last_error_msg_buffer; + va_end(args); + return error_code; +} + evmc_create_fn evmc_load(const char* filename, enum evmc_loader_error_code* error_code) { @@ -55,26 +90,33 @@ evmc_create_fn evmc_load(const char* filename, enum evmc_loader_error_code* erro if (!filename) { - ec = EVMC_LOADER_INVALID_ARGUMENT; + ec = set_error(EVMC_LOADER_INVALID_ARGUMENT, "invalid argument: file name cannot be null"); goto exit; } const size_t length = strlen(filename); - if (length == 0 || length > PATH_MAX_LENGTH) + if (length == 0) { - ec = EVMC_LOADER_INVALID_ARGUMENT; + ec = set_error(EVMC_LOADER_INVALID_ARGUMENT, "invalid argument: file name cannot be empty"); + goto exit; + } + else if (length > PATH_MAX_LENGTH) + { + ec = set_error(EVMC_LOADER_INVALID_ARGUMENT, + "invalid argument: file name is too long (%d, maximum allowed length is %d)", + (int)length, PATH_MAX_LENGTH); goto exit; } DLL_HANDLE handle = DLL_OPEN(filename); if (!handle) { -#if !defined(EVMC_LOADER_MOCK) && !_WIN32 - // If available, get the error message from dlerror(). - last_error_msg = dlerror(); -#endif - - ec = EVMC_LOADER_CANNOT_OPEN; + // Get error message if available. + last_error_msg = DLL_GET_ERROR_MSG(); + if (last_error_msg) + ec = EVMC_LOADER_CANNOT_OPEN; + else + ec = set_error(EVMC_LOADER_CANNOT_OPEN, "cannot open %s", filename); goto exit; } @@ -129,7 +171,8 @@ evmc_create_fn evmc_load(const char* filename, enum evmc_loader_error_code* erro if (!create_fn) { DLL_CLOSE(handle); - ec = EVMC_LOADER_SYMBOL_NOT_FOUND; + ec = set_error(EVMC_LOADER_SYMBOL_NOT_FOUND, "EVMC create function not found in %s", + filename); } exit: @@ -140,7 +183,9 @@ exit: const char* evmc_last_error_msg() { - return last_error_msg; + const char* m = last_error_msg; + last_error_msg = NULL; + return m; } struct evmc_instance* evmc_load_and_create(const char* filename, @@ -155,13 +200,16 @@ struct evmc_instance* evmc_load_and_create(const char* filename, struct evmc_instance* instance = create_fn(); if (!instance) { - *error_code = EVMC_LOADER_INSTANCE_CREATION_FAILURE; + *error_code = set_error(EVMC_LOADER_INSTANCE_CREATION_FAILURE, + "creating EVMC instance of %s has failed", filename); return NULL; } if (!evmc_is_abi_compatible(instance)) { - *error_code = EVMC_LOADER_ABI_VERSION_MISMATCH; + *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); return NULL; } diff --git a/test/unittests/loader_mock.h b/test/unittests/loader_mock.h index a012c8f..bcac4ca 100644 --- a/test/unittests/loader_mock.h +++ b/test/unittests/loader_mock.h @@ -14,10 +14,14 @@ const char* evmc_test_library_path = NULL; const char* evmc_test_library_symbol = NULL; evmc_create_fn evmc_test_create_fn = NULL; +static const char* evmc_test_last_error_msg = NULL; + static int evmc_test_load_library(const char* filename) { + evmc_test_last_error_msg = NULL; if (filename && evmc_test_library_path && strcmp(filename, evmc_test_library_path) == 0) return magic_handle; + evmc_test_last_error_msg = "cannot load library"; return 0; } @@ -36,7 +40,16 @@ static evmc_create_fn evmc_test_get_symbol_address(int handle, const char* symbo return NULL; } +static const char* evmc_test_get_last_error_msg() +{ + // Return the last error message only once. + const char* m = evmc_test_last_error_msg; + evmc_test_last_error_msg = NULL; + return m; +} + #define DLL_HANDLE int #define DLL_OPEN(filename) evmc_test_load_library(filename) #define DLL_CLOSE(handle) evmc_test_free_library(handle) #define DLL_GET_CREATE_FN(handle, name) evmc_test_get_symbol_address(handle, name) +#define DLL_GET_ERROR_MSG() evmc_test_get_last_error_msg() diff --git a/test/unittests/test_loader.cpp b/test/unittests/test_loader.cpp index 38e3440..b3af7ee 100644 --- a/test/unittests/test_loader.cpp +++ b/test/unittests/test_loader.cpp @@ -66,8 +66,14 @@ TEST_F(loader, load_long_path) const std::string path(5000, 'a'); evmc_loader_error_code ec; EXPECT_EQ(evmc_load(path.c_str(), &ec), nullptr); + EXPECT_STREQ(evmc_last_error_msg(), + "invalid argument: file name is too long (5000, maximum allowed length is 4096)"); + EXPECT_EQ(evmc_last_error_msg(), nullptr); EXPECT_EQ(ec, EVMC_LOADER_INVALID_ARGUMENT); EXPECT_EQ(evmc_load(path.c_str(), nullptr), nullptr); + EXPECT_STREQ(evmc_last_error_msg(), + "invalid argument: file name is too long (5000, maximum allowed length is 4096)"); + EXPECT_EQ(evmc_last_error_msg(), nullptr); } TEST_F(loader, load_null_path) @@ -75,15 +81,23 @@ TEST_F(loader, load_null_path) evmc_loader_error_code ec; EXPECT_EQ(evmc_load(nullptr, &ec), nullptr); EXPECT_EQ(ec, EVMC_LOADER_INVALID_ARGUMENT); + EXPECT_STREQ(evmc_last_error_msg(), "invalid argument: file name cannot be null"); + EXPECT_EQ(evmc_last_error_msg(), nullptr); EXPECT_EQ(evmc_load(nullptr, nullptr), nullptr); + EXPECT_STREQ(evmc_last_error_msg(), "invalid argument: file name cannot be null"); + EXPECT_EQ(evmc_last_error_msg(), nullptr); } TEST_F(loader, load_empty_path) { evmc_loader_error_code ec; EXPECT_EQ(evmc_load("", &ec), nullptr); + EXPECT_STREQ(evmc_last_error_msg(), "invalid argument: file name cannot be empty"); + EXPECT_EQ(evmc_last_error_msg(), nullptr); EXPECT_EQ(ec, EVMC_LOADER_INVALID_ARGUMENT); EXPECT_EQ(evmc_load("", nullptr), nullptr); + EXPECT_STREQ(evmc_last_error_msg(), "invalid argument: file name cannot be empty"); + EXPECT_EQ(evmc_last_error_msg(), nullptr); } TEST_F(loader, load_prefix_aaa) @@ -104,6 +118,7 @@ TEST_F(loader, load_prefix_aaa) EXPECT_EQ(ec, EVMC_LOADER_SUCCESS); ASSERT_NE(fn, nullptr); EXPECT_EQ((uintptr_t)fn(), 0xaaa); + EXPECT_EQ(evmc_last_error_msg(), nullptr); } } @@ -115,6 +130,7 @@ TEST_F(loader, load_eee_bbb) ASSERT_NE(fn, nullptr); EXPECT_EQ(ec, EVMC_LOADER_SUCCESS); EXPECT_EQ((uintptr_t)fn(), 0xeeebbb); + EXPECT_EQ(evmc_last_error_msg(), nullptr); } @@ -137,7 +153,17 @@ TEST_F(loader, load_windows_path) evmc_loader_error_code ec; evmc_load(path, &ec); - EXPECT_EQ(ec, should_open ? EVMC_LOADER_SUCCESS : EVMC_LOADER_CANNOT_OPEN); + if (should_open) + { + EXPECT_EQ(ec, EVMC_LOADER_SUCCESS); + EXPECT_EQ(evmc_last_error_msg(), nullptr); + } + else + { + EXPECT_EQ(ec, EVMC_LOADER_CANNOT_OPEN); + EXPECT_STREQ(evmc_last_error_msg(), "cannot load library"); + EXPECT_EQ(evmc_last_error_msg(), nullptr); + } } } @@ -152,6 +178,8 @@ TEST_F(loader, load_symbol_not_found) evmc_loader_error_code ec; EXPECT_EQ(evmc_load(evmc_test_library_path, &ec), nullptr); EXPECT_EQ(ec, EVMC_LOADER_SYMBOL_NOT_FOUND); + EXPECT_EQ(evmc_last_error_msg(), "EVMC create function not found in " + std::string(path)); + EXPECT_EQ(evmc_last_error_msg(), nullptr); EXPECT_EQ(evmc_load(evmc_test_library_path, nullptr), nullptr); } } @@ -177,6 +205,7 @@ TEST_F(loader, load_and_create_failure) auto vm = evmc_load_and_create(evmc_test_library_path, &ec); EXPECT_EQ(vm, nullptr); EXPECT_EQ(ec, EVMC_LOADER_INSTANCE_CREATION_FAILURE); + EXPECT_STREQ(evmc_last_error_msg(), "creating EVMC instance of failure.vm has failed"); } TEST_F(loader, load_and_create_abi_mismatch) @@ -187,4 +216,6 @@ TEST_F(loader, load_and_create_abi_mismatch) auto vm = evmc_load_and_create(evmc_test_library_path, &ec); EXPECT_EQ(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"); } diff --git a/test/vmtester/vmtester.cpp b/test/vmtester/vmtester.cpp index ae3b1e8..94aea68 100644 --- a/test/vmtester/vmtester.cpp +++ b/test/vmtester/vmtester.cpp @@ -137,27 +137,14 @@ int main(int argc, char* argv[]) std::cout << "Testing " << evmc_module << "\n"; evmc_loader_error_code ec; create_fn = evmc_load(evmc_module.c_str(), &ec); - switch (ec) - { - case EVMC_LOADER_SUCCESS: - break; - case EVMC_LOADER_CANNOT_OPEN: + + if (ec != EVMC_LOADER_SUCCESS) { const auto error = evmc_last_error_msg(); if (error) std::cerr << error << "\n"; else - std::cerr << "Cannot open " << evmc_module << "\n"; - return static_cast(ec); - } - case EVMC_LOADER_SYMBOL_NOT_FOUND: - std::cerr << "EVMC create function not found in " << evmc_module << "\n"; - return static_cast(ec); - case EVMC_LOADER_INVALID_ARGUMENT: - std::cerr << "Invalid argument: \"" << evmc_module << "\"\n"; - return static_cast(ec); - default: - std::cerr << "Unexpected error in evmc_load(): " << ec << "\n"; + std::cerr << "Loading error " << ec << "\n"; return static_cast(ec); }