Merge pull request #2897 from larryk85/fix/eos2818

Fix for eos#2818

contracts/test_api/test_api.hpp

@@ -87,6 +87,8 @@ struct test_db {
    static void idx_double_nan_create_fail(uint64_t receiver, uint64_t code, uint64_t action);
    static void idx_double_nan_modify_fail(uint64_t receiver, uint64_t code, uint64_t action);
    static void idx_double_nan_lookup_fail(uint64_t receiver, uint64_t code, uint64_t action);
+
+   static void misaligned_secondary_key256_tests(uint64_t, uint64_t, uint64_t);
 };
 
 struct test_multi_index {

contracts/test_api_db/test_api_db.cpp

@@ -20,6 +20,7 @@ extern "C" {
       WASM_TEST_HANDLER_EX(test_db, idx_double_nan_create_fail);
       WASM_TEST_HANDLER_EX(test_db, idx_double_nan_modify_fail);
       WASM_TEST_HANDLER_EX(test_db, idx_double_nan_lookup_fail);
+      WASM_TEST_HANDLER_EX(test_db, misaligned_secondary_key256_tests);
 
       //unhandled test call
       eosio_assert(false, "Unknown Test");

contracts/test_api_db/test_db.cpp

@@ -4,6 +4,7 @@
 #include <eosiolib/datastream.hpp>
 #include <eosiolib/db.h>
 #include <eosiolib/memory.hpp>
+#include <eosiolib/fixed_key.hpp>
 #include "../test_api/test_api.hpp"
 
 int primary[11]      = {0,1,2,3,4,5,6,7,8,9,10};
@@ -534,3 +535,13 @@ void test_db::idx_double_nan_lookup_fail(uint64_t receiver, uint64_t, uint64_t)
          eosio_assert( false, "idx_double_nan_lookup_fail: unexpected lookup_type" );
    }
 }
+
+void test_db::misaligned_secondary_key256_tests(uint64_t receiver, uint64_t, uint64_t) {
+   auto key = eosio::key256::make_from_word_sequence<uint64_t>(0ULL, 0ULL, 0ULL, 42ULL);
+   char* ptr = (char*)(&key);
+   ptr += 1;
+   // test that store doesn't crash on unaligned data
+   db_idx256_store( N(testapi), N(testtable), N(testapi), 1, (eosio::key256*)(ptr), 2 );
+   // test that find_primary doesn't crash on unaligned data
+   db_idx256_find_primary( N(testapi), N(testtable), N(testapi), (eosio::key256*)(ptr), 2, 0);
+}

libraries/chain/block_header_state.cpp

@@ -163,7 +163,6 @@ namespace eosio { namespace chain {
     FC_ASSERT( result.header.producer == h.producer, "wrong producer specified" );
     FC_ASSERT( result.header.schedule_version == h.schedule_version, "schedule_version in signed block is corrupted" );
 
-    //idump((h.producer)(h.block_num()-h.confirmed)(h.block_num()));
     auto itr = producer_to_last_produced.find(h.producer);
     if( itr != producer_to_last_produced.end() ) {
        FC_ASSERT( itr->second <= result.block_num - h.confirmed, "producer double-confirming known range" );

libraries/chain/include/eosio/chain/exceptions.hpp

@@ -191,6 +191,10 @@ namespace eosio { namespace chain {
       FC_DECLARE_DERIVED_EXCEPTION( wasm_serialization_error, wasm_exception,
                                     3070003, "Serialization Error Processing WASM" )
 
+      FC_DECLARE_DERIVED_EXCEPTION( overlapping_memory_error, wasm_exception,
+                                    3070004, "memcpy with overlapping memory" )
+
+
 
    FC_DECLARE_DERIVED_EXCEPTION( resource_exhausted_exception, chain_exception,
                                  3080000, "resource exhausted exception" )

libraries/chain/include/eosio/chain/webassembly/binaryen.hpp

@@ -76,12 +76,12 @@ struct interpreter_interface : ModuleInstance::ExternalInterface {
       FC_THROW_EXCEPTION(wasm_execution_error, why);
    }
 
-   void assert_memory_is_accessible(uint32_t offset, size_t size) {
-      if (offset + size > current_memory_size || offset + size < offset)
-         FC_THROW_EXCEPTION(wasm_execution_error, "access violation");
+   void assert_memory_is_accessible(uint32_t offset, uint32_t size) {
+      EOS_ASSERT(offset + size <= current_memory_size && offset + size >= offset,
+         wasm_execution_error, "access violation");
    }
 
-   char* get_validated_pointer(uint32_t offset, size_t size) {
+   char* get_validated_pointer(uint32_t offset, uint32_t size) {
       assert_memory_is_accessible(offset, size);
       return memory.data + offset;
    }
@@ -155,9 +155,9 @@ class binaryen_runtime : public eosio::chain::wasm_runtime_interface {
  * @tparam T
  */
 template<typename T>
-inline array_ptr<T> array_ptr_impl (interpreter_interface* interface, uint32_t ptr, size_t length)
+inline array_ptr<T> array_ptr_impl (interpreter_interface* interface, uint32_t ptr, uint32_t length)
 {
-   return array_ptr<T>((T*)(interface->get_validated_pointer(ptr, length * sizeof(T))));
+   return array_ptr<T>((T*)(interface->get_validated_pointer(ptr, length * (uint32_t)sizeof(T))));
 }
 
 /**
@@ -348,13 +348,40 @@ struct intrinsic_invoker_impl<Ret, std::tuple<array_ptr<T>, size_t, Inputs...>>
    using next_step = intrinsic_invoker_impl<Ret, std::tuple<Inputs...>>;
    using then_type = Ret(*)(interpreter_interface*, array_ptr<T>, size_t, Inputs..., LiteralList&, int);
 
-   template<then_type Then>
-   static Ret translate_one(interpreter_interface* interface, Inputs... rest, LiteralList& args, int offset) {
+   template<then_type Then, typename U=T>
+   static auto translate_one(interpreter_interface* interface, Inputs... rest, LiteralList& args, int offset) -> std::enable_if_t<std::is_const<U>::value, Ret> {
+      static_assert(!std::is_pointer<U>::value, "Currently don't support array of pointers");
       uint32_t ptr = args.at(offset - 1).geti32();
       size_t length = args.at(offset).geti32();
-      return Then(interface, array_ptr_impl<T>(interface, ptr, length), length, rest..., args, offset - 2);
+      T* base = array_ptr_impl<T>(interface, ptr, length);
+      if ( reinterpret_cast<uintptr_t>(base) % alignof(T) != 0 ) {
+         wlog( "misaligned array of const values" );
+         std::remove_const_t<T> copy[length];
+         T* copy_ptr = &copy[0];
+         memcpy( (void*)copy_ptr, (void*)base, length * sizeof(T) );
+         return Then(interface, static_cast<array_ptr<T>>(copy_ptr), length, rest..., args, offset - 2);
+      }
+      return Then(interface, static_cast<array_ptr<T>>(base), length, rest..., args, offset - 2);
    };
 
+   template<then_type Then, typename U=T>
+   static auto translate_one(interpreter_interface* interface, Inputs... rest, LiteralList& args, int offset) -> std::enable_if_t<!std::is_const<U>::value, Ret> {
+      static_assert(!std::is_pointer<U>::value, "Currently don't support array of pointers");
+      uint32_t ptr = args.at(offset - 1).geti32();
+      size_t length = args.at(offset).geti32();
+      T* base = array_ptr_impl<T>(interface, ptr, length);
+      if ( reinterpret_cast<uintptr_t>(base) % alignof(T) != 0 ) {
+         wlog( "misaligned array of values" );
+         std::remove_const_t<T> copy[length];
+         T* copy_ptr = &copy[0];
+         memcpy( (void*)copy_ptr, (void*)base, length * sizeof(T) );
+         Ret ret = Then(interface, static_cast<array_ptr<T>>(copy_ptr), length, rest..., args, offset - 2);  
+         memcpy( (void*)base, (void*)copy_ptr, length * sizeof(T) );
+         return ret;
+      }
+      return Then(interface, static_cast<array_ptr<T>>(base), length, rest..., args, offset - 2);
+   };
+   
    template<then_type Then>
    static const auto fn() {
       return next_step::template fn<translate_one<Then>>();
@@ -404,7 +431,7 @@ struct intrinsic_invoker_impl<Ret, std::tuple<array_ptr<T>, array_ptr<U>, size_t
       uint32_t ptr_t = args.at(offset - 2).geti32();
       uint32_t ptr_u = args.at(offset - 1).geti32();
       size_t length = args.at(offset).geti32();
-      assert(sizeof(T) == sizeof(U));
+      static_assert(std::is_same<std::remove_const_t<T>, char>::value && std::is_same<std::remove_const_t<U>, char>::value, "Currently only support array of (const)chars");
       return Then(interface, array_ptr_impl<T>(interface, ptr_t, length), array_ptr_impl<U>(interface, ptr_u, length), length, args, offset - 3);
    };
 

libraries/chain/include/eosio/chain/webassembly/common.hpp

@@ -60,7 +60,7 @@ namespace eosio { namespace chain {
       }
 
       T *value;
-   };
+   }; 
 
    /**
     * class to represent an in-wasm-memory char array that must be null terminated

libraries/chain/include/eosio/chain/webassembly/wavm.hpp

@@ -53,6 +53,8 @@ inline array_ptr<T> array_ptr_impl (running_instance_context& ctx, U32 ptr, size
    size_t mem_total = IR::numBytesPerPage * Runtime::getMemoryNumPages(mem);
    if (ptr >= mem_total || length > (mem_total - ptr) / sizeof(T))
       Runtime::causeException(Exception::Cause::accessViolation);
+   
+   T* ret_ptr = (T*)(getMemoryBaseAddress(mem) + ptr);
 
    return array_ptr<T>((T*)(getMemoryBaseAddress(mem) + ptr));
 }
@@ -373,10 +375,36 @@ struct intrinsic_invoker_impl<Ret, std::tuple<array_ptr<T>, size_t, Inputs...>,
    using next_step = intrinsic_invoker_impl<Ret, std::tuple<Inputs...>, std::tuple<Translated..., I32, I32>>;
    using then_type = Ret(*)(running_instance_context&, array_ptr<T>, size_t, Inputs..., Translated...);
 
-   template<then_type Then>
-   static Ret translate_one(running_instance_context& ctx, Inputs... rest, Translated... translated, I32 ptr, I32 size) {
+   template<then_type Then, typename U=T>
+   static auto translate_one(running_instance_context& ctx, Inputs... rest, Translated... translated, I32 ptr, I32 size) -> std::enable_if_t<std::is_const<U>::value, Ret> {
+      static_assert(!std::is_pointer<U>::value, "Currently don't support array of pointers");
+      const auto length = size_t(size);
+      T* base = array_ptr_impl<T>(ctx, ptr, length);
+      if ( reinterpret_cast<uintptr_t>(base) % alignof(T) != 0 ) {
+         wlog( "misaligned array of const values" );
+         std::remove_const_t<T> copy[length];
+         T* copy_ptr = &copy[0];
+         memcpy( (void*)copy_ptr, (void*)base, length * sizeof(T) );
+         return Then(ctx, static_cast<array_ptr<T>>(copy_ptr), length, rest..., translated...);
+      }
+      return Then(ctx, static_cast<array_ptr<T>>(base), length, rest..., translated...);
+   };
+
+   template<then_type Then, typename U=T>
+   static auto translate_one(running_instance_context& ctx, Inputs... rest, Translated... translated, I32 ptr, I32 size) -> std::enable_if_t<!std::is_const<U>::value, Ret> {
+      static_assert(!std::is_pointer<U>::value, "Currently don't support array of pointers");
       const auto length = size_t(size);
-      return Then(ctx, array_ptr_impl<T>(ctx, ptr, length), length, rest..., translated...);
+      T* base = array_ptr_impl<T>(ctx, ptr, length);
+      if ( reinterpret_cast<uintptr_t>(base) % alignof(T) != 0 ) {
+         wlog( "misaligned array of values" );
+         std::remove_const_t<T> copy[length];
+         T* copy_ptr = &copy[0];
+         memcpy( (void*)copy_ptr, (void*)base, length * sizeof(T) );
+         Ret ret = Then(ctx, static_cast<array_ptr<T>>(copy_ptr), length, rest..., translated...);  
+         memcpy( (void*)base, (void*)copy_ptr, length * sizeof(T) );
+         return ret;
+      }
+      return Then(ctx, static_cast<array_ptr<T>>(base), length, rest..., translated...);
    };
 
    template<then_type Then>
@@ -426,8 +454,8 @@ struct intrinsic_invoker_impl<Ret, std::tuple<array_ptr<T>, array_ptr<U>, size_t
 
    template<then_type Then>
    static Ret translate_one(running_instance_context& ctx, Inputs... rest, Translated... translated, I32 ptr_t, I32 ptr_u, I32 size) {
+      static_assert(std::is_same<std::remove_const_t<T>, char>::value && std::is_same<std::remove_const_t<U>, char>::value, "Currently only support array of (const)chars");
       const auto length = size_t(size);
-      assert(sizeof(T)==sizeof(U));
       return Then(ctx, array_ptr_impl<T>(ctx, ptr_t, length), array_ptr_impl<U>(ctx, ptr_u, length), length, rest..., translated...);
    };
 
@@ -535,32 +563,6 @@ struct intrinsic_invoker_impl<Ret, std::tuple<const name&, Inputs...>, std::tupl
    }
 };
 
-/**
- * Specialization for transcribing a reference to a fc::time_point_sec which can be passed as a native value
- *    This type transcribes into a native type which is loaded by value into a
- *    variable on the stack and then passed by reference to the intrinsic.
- *
- * @tparam Ret - the return type of the native method
- * @tparam Inputs - the remaining native parameters to transcribe
- * @tparam Translated - the list of transcribed wasm parameters
- */
-template<typename Ret, typename... Inputs, typename ...Translated>
-struct intrinsic_invoker_impl<Ret, std::tuple<const fc::time_point_sec&, Inputs...>, std::tuple<Translated...>> {
-   using next_step = intrinsic_invoker_impl<Ret, std::tuple<Inputs...>, std::tuple<Translated..., native_to_wasm_t<const fc::time_point_sec&> >>;
-   using then_type = Ret (*)(running_instance_context&, const fc::time_point_sec&, Inputs..., Translated...);
-
-   template<then_type Then>
-   static Ret translate_one(running_instance_context& ctx, Inputs... rest, Translated... translated, native_to_wasm_t<const fc::time_point_sec&> wasm_value) {
-      auto value = fc::time_point_sec(wasm_value);
-      return Then(ctx, value, rest..., translated...);
-   }
-
-   template<then_type Then>
-   static const auto fn() {
-      return next_step::template fn<translate_one<Then>>();
-   }
-};
-
 /**
  * Specialization for transcribing  a reference type in the native method signature
  *    This type transcribes into an int32  pointer checks the validity of that memory

libraries/chain/wasm_interface.cpp

@@ -681,7 +681,7 @@ class producer_api : public context_aware_api {
 
          auto copy_size = std::min( buffer_size, s );
          memcpy( producers, active_producers.data(), copy_size );
-
+         
          return copy_size;
       }
 };
@@ -1208,6 +1208,8 @@ class memory_api : public context_aware_api {
       :context_aware_api(ctx,true){}
 
       char* memcpy( array_ptr<char> dest, array_ptr<const char> src, size_t length) {
+         EOS_ASSERT((abs((ptrdiff_t)dest.value - (ptrdiff_t)src.value)) >= length,
+               overlapping_memory_error, "memcpy can only accept non-aliasing pointers");
          return (char *)::memcpy(dest, src, length);
       }
 

unittests/api_tests.cpp

@@ -1105,6 +1105,7 @@ BOOST_FIXTURE_TEST_CASE(db_tests, TESTER) { try {
    CALL_TEST_FUNCTION_AND_CHECK_EXCEPTION( *this, "test_db", "idx_double_nan_lookup_fail", fc::raw::pack(lookup_type),
                                            transaction_exception, "NaN is not an allowed value for a secondary key");
 
+   CALL_TEST_FUNCTION( *this, "test_db", "misaligned_secondary_key256_tests", {});
    BOOST_REQUIRE_EQUAL( validate(), true );
 } FC_LOG_AND_RETHROW() }
 
@@ -1299,9 +1300,9 @@ BOOST_FIXTURE_TEST_CASE(memory_tests, TESTER) { try {
 #endif
    CALL_TEST_FUNCTION( *this, "test_memory", "test_memset_memcpy", {} );
    produce_blocks(1000);
-   CALL_TEST_FUNCTION( *this, "test_memory", "test_memcpy_overlap_start", {} );
+   BOOST_CHECK_THROW( CALL_TEST_FUNCTION( *this, "test_memory", "test_memcpy_overlap_start", {} ), overlapping_memory_error );
    produce_blocks(1000);
-   CALL_TEST_FUNCTION( *this, "test_memory", "test_memcpy_overlap_end", {} );
+   BOOST_CHECK_THROW( CALL_TEST_FUNCTION( *this, "test_memory", "test_memcpy_overlap_end", {} ), overlapping_memory_error );
    produce_blocks(1000);
    CALL_TEST_FUNCTION( *this, "test_memory", "test_memcmp", {} );
    produce_blocks(1000);

unittests/contracts/test_wasts.hpp

@@ -79,7 +79,7 @@ static const char misaligned_const_ref_wast[] = R"=====(
 (module
  (import "env" "sha256" (func $sha256 (param i32 i32 i32)))
  (import "env" "assert_sha256" (func $assert_sha256 (param i32 i32 i32)))
- (import "env" "memcpy" (func $memcpy (param i32 i32 i32) (result i32)))
+ (import "env" "memmove" (func $memmove (param i32 i32 i32) (result i32)))
  (table 0 anyfunc)
  (memory $0 32)
  (data (i32.const 4) "hello")
@@ -92,7 +92,7 @@ static const char misaligned_const_ref_wast[] = R"=====(
    (i32.const 16)
   )
   (set_local $3
-   (call $memcpy
+   (call $memmove
     (i32.const 17)
     (i32.const 16)
     (i32.const 64)