From 063d01223b67681cfaf1f4b2a7fd858f44d73582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 26 Sep 2019 17:17:44 +0200 Subject: [PATCH 1/5] Add scratchpad to evmc_result --- include/evmc/evmc.h | 51 ++++++++++++++++++--------------- include/evmc/evmc.hpp | 6 +++- include/evmc/helpers.h | 4 +-- test/unittests/test_cpp.cpp | 2 +- test/unittests/test_helpers.cpp | 4 +-- test/vmtester/tests.cpp | 4 +-- 6 files changed, 40 insertions(+), 31 deletions(-) diff --git a/include/evmc/evmc.h b/include/evmc/evmc.h index fe950d379..8a5c08ead 100644 --- a/include/evmc/evmc.h +++ b/include/evmc/evmc.h @@ -336,6 +336,33 @@ struct evmc_result; */ typedef void (*evmc_release_result_fn)(const struct evmc_result* result); +union evmc_result_scratchpad +{ + /** + * The address of the contract created by create instructions. + * + * This field has valid value only if: + * - it is a result of the Host method evmc_host_interface::call + * - and the result describes successful contract creation + * (evmc_result::status_code is ::EVMC_SUCCESS). + * In all other cases the address MUST be null bytes. + */ + evmc_address create_address; + + /** + * Reserved data that MAY be used by a evmc_result object creator. + * + * This reserved 4 bytes together with 20 bytes from create_address form + * 24 bytes of memory called "optional data" within evmc_result struct + * to be optionally used by the evmc_result object creator. + * + * @see evmc_result_optional_data, evmc_get_optional_data(). + * + * Also extends the size of the evmc_result to 64 bytes (full cache line). + */ + uint8_t bytes[64]; +}; + /** The EVM code execution result. */ struct evmc_result { @@ -391,29 +418,7 @@ struct evmc_result */ evmc_release_result_fn release; - /** - * The address of the contract created by create instructions. - * - * This field has valid value only if: - * - it is a result of the Host method evmc_host_interface::call - * - and the result describes successful contract creation - * (evmc_result::status_code is ::EVMC_SUCCESS). - * In all other cases the address MUST be null bytes. - */ - evmc_address create_address; - - /** - * Reserved data that MAY be used by a evmc_result object creator. - * - * This reserved 4 bytes together with 20 bytes from create_address form - * 24 bytes of memory called "optional data" within evmc_result struct - * to be optionally used by the evmc_result object creator. - * - * @see evmc_result_optional_data, evmc_get_optional_data(). - * - * Also extends the size of the evmc_result to 64 bytes (full cache line). - */ - uint8_t padding[4]; + union evmc_result_scratchpad scratchpad; }; diff --git a/include/evmc/evmc.hpp b/include/evmc/evmc.hpp index e161c0324..658d0113c 100644 --- a/include/evmc/evmc.hpp +++ b/include/evmc/evmc.hpp @@ -266,7 +266,6 @@ constexpr auto make_result = evmc_make_result; class result : private evmc_result { public: - using evmc_result::create_address; using evmc_result::gas_left; using evmc_result::output_data; using evmc_result::output_size; @@ -332,6 +331,11 @@ class result : private evmc_result this->release = nullptr; // Disable releasing of this object. return out; } + + const evmc_address& get_create_address() noexcept + { + return scratchpad.create_address; + } }; /// @copybrief evmc_vm diff --git a/include/evmc/helpers.h b/include/evmc/helpers.h index 36febfd72..fcfec9df9 100644 --- a/include/evmc/helpers.h +++ b/include/evmc/helpers.h @@ -197,14 +197,14 @@ union evmc_result_optional_storage static inline union evmc_result_optional_storage* evmc_get_optional_storage( struct evmc_result* result) { - return (union evmc_result_optional_storage*)&result->create_address; + return (union evmc_result_optional_storage*)&result->scratchpad; } /** Provides read-only access to evmc_result "optional storage". */ static inline const union evmc_result_optional_storage* evmc_get_const_optional_storage( const struct evmc_result* result) { - return (const union evmc_result_optional_storage*)&result->create_address; + return (const union evmc_result_optional_storage*)&result->scratchpad; } /** @} */ diff --git a/test/unittests/test_cpp.cpp b/test/unittests/test_cpp.cpp index 7ac6a2dbc..062ae30b5 100644 --- a/test/unittests/test_cpp.cpp +++ b/test/unittests/test_cpp.cpp @@ -523,7 +523,7 @@ TEST(cpp, result_create) EXPECT_EQ(c.status_code, r.status_code); EXPECT_EQ(c.gas_left, r.gas_left); ASSERT_EQ(c.output_size, r.output_size); - EXPECT_EQ(evmc::address{c.create_address}, evmc::address{r.create_address}); + EXPECT_EQ(evmc::address{c.scratchpad.create_address}, evmc::address{r.get_create_address()}); ASSERT_TRUE(c.release); EXPECT_TRUE(std::memcmp(c.output_data, r.output_data, c.output_size) == 0); c.release(&c); diff --git a/test/unittests/test_helpers.cpp b/test/unittests/test_helpers.cpp index cc9f21ac5..40ac68fc0 100644 --- a/test/unittests/test_helpers.cpp +++ b/test/unittests/test_helpers.cpp @@ -10,7 +10,7 @@ static_assert(sizeof(evmc_bytes32) == 32, "evmc_bytes32 is too big"); static_assert(sizeof(evmc_address) == 20, "evmc_address is too big"); -static_assert(sizeof(evmc_result) <= 64, "evmc_result does not fit cache line"); +static_assert(sizeof(evmc_result) <= 128, "evmc_result does not fit 2 cache lines"); static_assert(sizeof(evmc_vm) <= 64, "evmc_vm does not fit cache line"); static_assert(offsetof(evmc_message, value) % sizeof(size_t) == 0, "evmc_message.value not aligned"); @@ -22,7 +22,7 @@ static_assert(sizeof(evmc_call_kind) == sizeof(int), static_assert(sizeof(evmc_revision) == sizeof(int), "Enum `evmc_revision` is not the size of int"); static constexpr size_t optionalDataSize = - sizeof(evmc_result) - offsetof(evmc_result, create_address); + sizeof(evmc_result) - offsetof(evmc_result, scratchpad); static_assert(optionalDataSize >= sizeof(evmc_result_optional_storage), "evmc_result's optional data space is too small"); diff --git a/test/vmtester/tests.cpp b/test/vmtester/tests.cpp index 12fd5b58e..002e81452 100644 --- a/test/vmtester/tests.cpp +++ b/test/vmtester/tests.cpp @@ -83,7 +83,7 @@ TEST_F(evmc_vm_test, execute_call) read_buffer(result.output_data, result.output_size); } - EXPECT_TRUE(evmc::is_zero(result.create_address)); + EXPECT_TRUE(evmc::is_zero(result.scratchpad.create_address)); if (result.release) result.release(&result); @@ -120,7 +120,7 @@ TEST_F(evmc_vm_test, execute_create) } // The VM will never provide the create address. - EXPECT_TRUE(evmc::is_zero(result.create_address)); + EXPECT_TRUE(evmc::is_zero(result.scratchpad.create_address)); if (result.release) result.release(&result); From f42601ffaa3737a3f08e51410a80c71c297b2e9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 26 Sep 2019 17:21:25 +0200 Subject: [PATCH 2/5] Drop "optional result data" --- include/evmc/evmc.h | 2 ++ include/evmc/helpers.h | 49 --------------------------------- test/unittests/test_cpp.cpp | 4 +-- test/unittests/test_helpers.cpp | 5 ---- 4 files changed, 4 insertions(+), 56 deletions(-) diff --git a/include/evmc/evmc.h b/include/evmc/evmc.h index 8a5c08ead..fe9662237 100644 --- a/include/evmc/evmc.h +++ b/include/evmc/evmc.h @@ -361,6 +361,8 @@ union evmc_result_scratchpad * Also extends the size of the evmc_result to 64 bytes (full cache line). */ uint8_t bytes[64]; + + void* pointer; }; /** The EVM code execution result. */ diff --git a/include/evmc/helpers.h b/include/evmc/helpers.h index fcfec9df9..d4c50b02e 100644 --- a/include/evmc/helpers.h +++ b/include/evmc/helpers.h @@ -160,53 +160,4 @@ static inline void evmc_release_result(struct evmc_result* result) result->release(result); } - -/** - * Helpers for optional storage of evmc_result. - * - * In some contexts (i.e. evmc_result::create_address is unused) objects of - * type evmc_result contains a memory storage that MAY be used by the object - * owner. This group defines helper types and functions for accessing - * the optional storage. - * - * @defgroup result_optional_storage Result Optional Storage - * @{ - */ - -/** - * The union representing evmc_result "optional storage". - * - * The evmc_result struct contains 24 bytes of optional storage that can be - * reused by the object creator if the object does not contain - * evmc_result::create_address. - * - * A VM implementation MAY use this memory to keep additional data - * when returning result from evmc_execute_fn(). - * The host application MAY use this memory to keep additional data - * when returning result of performed calls from evmc_call_fn(). - * - * @see evmc_get_optional_storage(), evmc_get_const_optional_storage(). - */ -union evmc_result_optional_storage -{ - uint8_t bytes[24]; /**< 24 bytes of optional storage. */ - void* pointer; /**< Optional pointer. */ -}; - -/** Provides read-write access to evmc_result "optional storage". */ -static inline union evmc_result_optional_storage* evmc_get_optional_storage( - struct evmc_result* result) -{ - return (union evmc_result_optional_storage*)&result->scratchpad; -} - -/** Provides read-only access to evmc_result "optional storage". */ -static inline const union evmc_result_optional_storage* evmc_get_const_optional_storage( - const struct evmc_result* result) -{ - return (const union evmc_result_optional_storage*)&result->scratchpad; -} - -/** @} */ - /** @} */ diff --git a/test/unittests/test_cpp.cpp b/test/unittests/test_cpp.cpp index 062ae30b5..eb0a78cea 100644 --- a/test/unittests/test_cpp.cpp +++ b/test/unittests/test_cpp.cpp @@ -236,13 +236,13 @@ TEST(cpp, result) int release_called = 0; { auto raw_result = evmc_result{}; - evmc_get_optional_storage(&raw_result)->pointer = &release_called; + raw_result.scratchpad.pointer = &release_called; EXPECT_EQ(release_called, 0); raw_result.output_data = &output; raw_result.release = [](const evmc_result* r) { EXPECT_EQ(r->output_data, &output); - ++*static_cast(evmc_get_const_optional_storage(r)->pointer); + ++*static_cast(r->scratchpad.pointer); }; EXPECT_EQ(release_called, 0); diff --git a/test/unittests/test_helpers.cpp b/test/unittests/test_helpers.cpp index 40ac68fc0..08f6b2db7 100644 --- a/test/unittests/test_helpers.cpp +++ b/test/unittests/test_helpers.cpp @@ -21,11 +21,6 @@ static_assert(sizeof(evmc_call_kind) == sizeof(int), "Enum `evmc_call_kind` is not the size of int"); static_assert(sizeof(evmc_revision) == sizeof(int), "Enum `evmc_revision` is not the size of int"); -static constexpr size_t optionalDataSize = - sizeof(evmc_result) - offsetof(evmc_result, scratchpad); -static_assert(optionalDataSize >= sizeof(evmc_result_optional_storage), - "evmc_result's optional data space is too small"); - TEST(helpers, release_result) { auto r1 = evmc_result{}; From 20fff46e4440c7d662dc3f28541180c8205a14b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Sat, 28 Sep 2019 11:55:05 +0200 Subject: [PATCH 3/5] Separate create_address from scratchpad --- include/evmc/evmc.h | 21 +++++++++++---------- include/evmc/evmc.hpp | 6 +----- test/unittests/test_cpp.cpp | 2 +- test/unittests/test_helpers.cpp | 4 ++++ test/vmtester/tests.cpp | 4 ++-- 5 files changed, 19 insertions(+), 18 deletions(-) diff --git a/include/evmc/evmc.h b/include/evmc/evmc.h index fe9662237..9bb4d7191 100644 --- a/include/evmc/evmc.h +++ b/include/evmc/evmc.h @@ -338,16 +338,6 @@ typedef void (*evmc_release_result_fn)(const struct evmc_result* result); union evmc_result_scratchpad { - /** - * The address of the contract created by create instructions. - * - * This field has valid value only if: - * - it is a result of the Host method evmc_host_interface::call - * - and the result describes successful contract creation - * (evmc_result::status_code is ::EVMC_SUCCESS). - * In all other cases the address MUST be null bytes. - */ - evmc_address create_address; /** * Reserved data that MAY be used by a evmc_result object creator. @@ -420,6 +410,17 @@ struct evmc_result */ evmc_release_result_fn release; + /** + * The address of the contract created by create instructions. + * + * This field has valid value only if: + * - it is a result of the Host method evmc_host_interface::call + * - and the result describes successful contract creation + * (evmc_result::status_code is ::EVMC_SUCCESS). + * In all other cases the address MUST be null bytes. + */ + evmc_address create_address; + union evmc_result_scratchpad scratchpad; }; diff --git a/include/evmc/evmc.hpp b/include/evmc/evmc.hpp index 658d0113c..6759b379a 100644 --- a/include/evmc/evmc.hpp +++ b/include/evmc/evmc.hpp @@ -270,6 +270,7 @@ class result : private evmc_result using evmc_result::output_data; using evmc_result::output_size; using evmc_result::status_code; + using evmc_result::create_address; /// Creates the result from the provided arguments. /// @@ -331,11 +332,6 @@ class result : private evmc_result this->release = nullptr; // Disable releasing of this object. return out; } - - const evmc_address& get_create_address() noexcept - { - return scratchpad.create_address; - } }; /// @copybrief evmc_vm diff --git a/test/unittests/test_cpp.cpp b/test/unittests/test_cpp.cpp index eb0a78cea..f390fffcb 100644 --- a/test/unittests/test_cpp.cpp +++ b/test/unittests/test_cpp.cpp @@ -523,7 +523,7 @@ TEST(cpp, result_create) EXPECT_EQ(c.status_code, r.status_code); EXPECT_EQ(c.gas_left, r.gas_left); ASSERT_EQ(c.output_size, r.output_size); - EXPECT_EQ(evmc::address{c.scratchpad.create_address}, evmc::address{r.get_create_address()}); + EXPECT_EQ(evmc::address{c.create_address}, evmc::address{r.create_address}); ASSERT_TRUE(c.release); EXPECT_TRUE(std::memcmp(c.output_data, r.output_data, c.output_size) == 0); c.release(&c); diff --git a/test/unittests/test_helpers.cpp b/test/unittests/test_helpers.cpp index 08f6b2db7..f0b71e66a 100644 --- a/test/unittests/test_helpers.cpp +++ b/test/unittests/test_helpers.cpp @@ -21,6 +21,10 @@ static_assert(sizeof(evmc_call_kind) == sizeof(int), "Enum `evmc_call_kind` is not the size of int"); static_assert(sizeof(evmc_revision) == sizeof(int), "Enum `evmc_revision` is not the size of int"); + +static_assert(offsetof(evmc_result, scratchpad) % alignof(void*) == 0, + "evmc_result's scratchpad not properly aligned"); + TEST(helpers, release_result) { auto r1 = evmc_result{}; diff --git a/test/vmtester/tests.cpp b/test/vmtester/tests.cpp index 002e81452..12fd5b58e 100644 --- a/test/vmtester/tests.cpp +++ b/test/vmtester/tests.cpp @@ -83,7 +83,7 @@ TEST_F(evmc_vm_test, execute_call) read_buffer(result.output_data, result.output_size); } - EXPECT_TRUE(evmc::is_zero(result.scratchpad.create_address)); + EXPECT_TRUE(evmc::is_zero(result.create_address)); if (result.release) result.release(&result); @@ -120,7 +120,7 @@ TEST_F(evmc_vm_test, execute_create) } // The VM will never provide the create address. - EXPECT_TRUE(evmc::is_zero(result.scratchpad.create_address)); + EXPECT_TRUE(evmc::is_zero(result.create_address)); if (result.release) result.release(&result); From f3d7a4b0a576adb6e155ac3dfbd7237c05b156c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Sat, 28 Sep 2019 12:17:04 +0200 Subject: [PATCH 4/5] Move scratchpad type definition to evmc_result --- include/evmc/evmc.h | 36 ++++++++++++++++----------------- test/unittests/test_helpers.cpp | 2 +- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/include/evmc/evmc.h b/include/evmc/evmc.h index 9bb4d7191..9583c8e62 100644 --- a/include/evmc/evmc.h +++ b/include/evmc/evmc.h @@ -336,24 +336,6 @@ struct evmc_result; */ typedef void (*evmc_release_result_fn)(const struct evmc_result* result); -union evmc_result_scratchpad -{ - - /** - * Reserved data that MAY be used by a evmc_result object creator. - * - * This reserved 4 bytes together with 20 bytes from create_address form - * 24 bytes of memory called "optional data" within evmc_result struct - * to be optionally used by the evmc_result object creator. - * - * @see evmc_result_optional_data, evmc_get_optional_data(). - * - * Also extends the size of the evmc_result to 64 bytes (full cache line). - */ - uint8_t bytes[64]; - - void* pointer; -}; /** The EVM code execution result. */ struct evmc_result @@ -421,7 +403,23 @@ struct evmc_result */ evmc_address create_address; - union evmc_result_scratchpad scratchpad; + union scratchpad + { + /** + * Reserved data that MAY be used by a evmc_result object creator. + * + * This reserved 4 bytes together with 20 bytes from create_address form + * 24 bytes of memory called "optional data" within evmc_result struct + * to be optionally used by the evmc_result object creator. + * + * @see evmc_result_optional_data, evmc_get_optional_data(). + * + * Also extends the size of the evmc_result to 64 bytes (full cache line). + */ + uint8_t bytes[32]; + + void* pointer; + } scratchpad; }; diff --git a/test/unittests/test_helpers.cpp b/test/unittests/test_helpers.cpp index f0b71e66a..e9a71086e 100644 --- a/test/unittests/test_helpers.cpp +++ b/test/unittests/test_helpers.cpp @@ -22,7 +22,7 @@ static_assert(sizeof(evmc_call_kind) == sizeof(int), static_assert(sizeof(evmc_revision) == sizeof(int), "Enum `evmc_revision` is not the size of int"); -static_assert(offsetof(evmc_result, scratchpad) % alignof(void*) == 0, +static_assert(offsetof(evmc_result, scratchpad) % 8 == 0, "evmc_result's scratchpad not properly aligned"); TEST(helpers, release_result) From cddddb7baeb63b760a25ddf6ee0233a5ddffadd4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Sat, 28 Sep 2019 12:26:49 +0200 Subject: [PATCH 5/5] Update rust --- bindings/rust/evmc-sys/src/lib.rs | 2 +- bindings/rust/evmc-vm/src/lib.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bindings/rust/evmc-sys/src/lib.rs b/bindings/rust/evmc-sys/src/lib.rs index 6a545ff80..edaa9568d 100644 --- a/bindings/rust/evmc-sys/src/lib.rs +++ b/bindings/rust/evmc-sys/src/lib.rs @@ -47,7 +47,7 @@ mod tests { // TODO: add other checks from test/unittests/test_helpers.cpp assert_eq!(size_of::(), 32); assert_eq!(size_of::(), 20); - assert!(size_of::() <= 64); + assert!(size_of::() <= 128); assert!(size_of::() <= 64); } } diff --git a/bindings/rust/evmc-vm/src/lib.rs b/bindings/rust/evmc-vm/src/lib.rs index 6f604e000..f660b16be 100644 --- a/bindings/rust/evmc-vm/src/lib.rs +++ b/bindings/rust/evmc-vm/src/lib.rs @@ -447,7 +447,7 @@ impl Into for ExecutionResult { } else { Address { bytes: [0u8; 20] } }, - padding: [0u8; 4], + scratchpad: ffi::evmc_result_scratchpad { bytes: [0u8; 32] }, } } } @@ -532,7 +532,7 @@ mod tests { output_size: 4, release: Some(test_result_dispose), create_address: Address { bytes: [0u8; 20] }, - padding: [0u8; 4], + scratchpad: ffi::evmc_result_scratchpad { bytes: [0u8; 32] }, }; let r: ExecutionResult = f.into(); @@ -778,7 +778,7 @@ mod tests { output_size: msg.input_size, release: None, create_address: ffi::evmc_address::default(), - padding: [0u8; 4], + scratchpad: ffi::evmc_result_scratchpad { bytes: [0u8; 32] }, } }