Merge pull request #2491 from EOSIO/2460-disallow-nan

Disallow NaN floating point values in secondary tables

contracts/eosiolib/db.h

@@ -88,15 +88,15 @@ int32_t db_idx256_lowerbound(account_name code, account_name scope, table_name t
 int32_t db_idx256_upperbound(account_name code, account_name scope, table_name table, void* data, uint32_t data_len, uint64_t* primary);
 int32_t db_idx256_end(account_name code, account_name scope, table_name table);
 
-int32_t db_idx_double_store(account_name scope, table_name table, account_name payer, uint64_t id, const uint64_t* secondary);
-void db_idx_double_update(int32_t iterator, account_name payer, const uint64_t* secondary);
+int32_t db_idx_double_store(account_name scope, table_name table, account_name payer, uint64_t id, const double* secondary);
+void db_idx_double_update(int32_t iterator, account_name payer, const double* secondary);
 void db_idx_double_remove(int32_t iterator);
 int32_t db_idx_double_next(int32_t iterator, uint64_t* primary);
 int32_t db_idx_double_previous(int32_t iterator, uint64_t* primary);
-int32_t db_idx_double_find_primary(account_name code, account_name scope, table_name table, uint64_t* secondary, uint64_t primary);
-int32_t db_idx_double_find_secondary(account_name code, account_name scope, table_name table, const uint64_t* secondary, uint64_t* primary);
-int32_t db_idx_double_lowerbound(account_name code, account_name scope, table_name table, uint64_t* secondary, uint64_t* primary);
-int32_t db_idx_double_upperbound(account_name code, account_name scope, table_name table, uint64_t* secondary, uint64_t* primary);
+int32_t db_idx_double_find_primary(account_name code, account_name scope, table_name table, double* secondary, uint64_t primary);
+int32_t db_idx_double_find_secondary(account_name code, account_name scope, table_name table, const double* secondary, uint64_t* primary);
+int32_t db_idx_double_lowerbound(account_name code, account_name scope, table_name table, double* secondary, uint64_t* primary);
+int32_t db_idx_double_upperbound(account_name code, account_name scope, table_name table, double* secondary, uint64_t* primary);
 int32_t db_idx_double_end(account_name code, account_name scope, table_name table);
 
 }

contracts/eosiolib/multi_index.hpp

@@ -90,47 +90,7 @@ struct secondary_index_db_functions<TYPE> {\
 WRAP_SECONDARY_SIMPLE_TYPE(idx64,  uint64_t)
 WRAP_SECONDARY_SIMPLE_TYPE(idx128, uint128_t)
 WRAP_SECONDARY_ARRAY_TYPE(idx256, key256)
-
-template<>
-struct secondary_index_db_functions<double> {
-   static int32_t db_idx_next( int32_t iterator, uint64_t* primary )          { return db_idx_double_next( iterator, primary ); }
-   static int32_t db_idx_previous( int32_t iterator, uint64_t* primary )      { return db_idx_double_previous( iterator, primary ); }
-   static void    db_idx_remove( int32_t iterator  )                          { db_idx_double_remove( iterator ); }
-   static int32_t db_idx_end( uint64_t code, uint64_t scope, uint64_t table ) { return db_idx_double_end( code, scope, table ); }
-   static int32_t db_idx_store( uint64_t scope, uint64_t table, uint64_t payer, uint64_t id, double secondary ) {
-      uint64_t val = *(uint64_t*)(&secondary); // Get uint64_t representation of double secondary
-      return db_idx_double_store( scope, table, payer, id, &val );
-   }
-   static void    db_idx_update( int32_t iterator, uint64_t payer, double secondary ) {
-      uint64_t val = *(uint64_t*)(&secondary); // Get uint64_t representation of double secondary
-      db_idx_double_update( iterator, payer, &val );
-   }
-   static int32_t db_idx_find_primary( uint64_t code, uint64_t scope, uint64_t table, uint64_t primary, double& secondary ) {
-      uint64_t val = 0;
-      auto itr = db_idx_double_find_primary( code, scope, table, &val, primary );
-      if( itr >= 0 )
-         secondary = *(double*)(&val); // Store double secondary from uint64_t representation stored in val
-      return itr;
-   }
-   static int32_t db_idx_find_secondary( uint64_t code, uint64_t scope, uint64_t table, double secondary, uint64_t& primary ) {
-      uint64_t val = *(uint64_t*)(&secondary);
-      return db_idx_double_find_secondary( code, scope, table, &val, &primary );
-   }
-   static int32_t db_idx_lowerbound( uint64_t code, uint64_t scope, uint64_t table, double& secondary, uint64_t& primary ) {
-      uint64_t val = *(uint64_t*)(&secondary); // Get uint64_t representation of double secondary
-      auto itr = db_idx_double_lowerbound( code, scope, table, &val, &primary );
-      if( itr >= 0 )
-         secondary = *(double*)(&val); // Store double secondary from uint64_t representation stored in val
-      return itr;
-   }
-   static int32_t db_idx_upperbound( uint64_t code, uint64_t scope, uint64_t table, double& secondary, uint64_t& primary ) {
-      uint64_t val = *(uint64_t*)(&secondary); // Get uint64_t representation of double secondary
-      auto itr = db_idx_double_upperbound( code, scope, table, &val, &primary );
-      if( itr >= 0 )
-         secondary = *(double*)(&val); // Store double secondary from uint64_t representation stored in val
-      return itr;
-   }
-};
+WRAP_SECONDARY_SIMPLE_TYPE(idx_double, double)
 
 template<uint64_t TableName, typename T, typename... Indices>
 class multi_index;

contracts/test_api/test_api.hpp

@@ -90,6 +90,10 @@ struct test_db {
    static void idx64_upperbound(uint64_t receiver, uint64_t code, uint64_t action);
 
    static void test_invalid_access(uint64_t receiver, uint64_t code, uint64_t action);
+
+   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);
 };
 
 struct test_multi_index {

contracts/test_api_db/test_api_db.cpp

@@ -17,6 +17,9 @@ extern "C" {
       WASM_TEST_HANDLER_EX(test_db, idx64_lowerbound);
       WASM_TEST_HANDLER_EX(test_db, idx64_upperbound);
       WASM_TEST_HANDLER_EX(test_db, test_invalid_access);
+      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);
 
       //unhandled test call
       eosio_assert(false, "Unknown Test");

contracts/test_api_db/test_db.cpp

@@ -495,3 +495,41 @@ void test_db::test_invalid_access(uint64_t receiver, uint64_t code, uint64_t act
       eosio_assert( value == ia.val, "test_invalid_access: value did not match" );
    }
 }
+
+void test_db::idx_double_nan_create_fail(uint64_t receiver, uint64_t, uint64_t) {
+   double x = 0.0;
+   x = x / x; // create a NaN
+   db_idx_double_store( N(nan), N(nan), receiver, 0, &x); // should fail
+}
+
+void test_db::idx_double_nan_modify_fail(uint64_t receiver, uint64_t, uint64_t) {
+   double x = 0.0;
+   db_idx_double_store( N(nan), N(nan), receiver, 0, &x);
+   auto itr = db_idx_double_find_primary(receiver, N(nan), N(nan), &x, 0);
+   x = 0.0;
+   x = x / x; // create a NaN
+   db_idx_double_update(itr, 0, &x); // should fail
+}
+
+void test_db::idx_double_nan_lookup_fail(uint64_t receiver, uint64_t, uint64_t) {
+   auto act = eosio::get_action(1, 0);
+   auto lookup_type = eosio::unpack<uint32_t>(act.data);
+
+   uint64_t pk;
+   double x = 0.0;
+   db_idx_double_store( N(nan), N(nan), receiver, 0, &x);
+   x = x / x; // create a NaN
+   switch( lookup_type ) {
+      case 0: // find
+         db_idx_double_find_secondary(receiver, N(nan), N(nan), &x, &pk);
+      break;
+      case 1: // lower bound
+         db_idx_double_lowerbound(receiver, N(nan), N(nan), &x, &pk);
+      break;
+      case 2: // upper bound
+         db_idx_double_upperbound(receiver, N(nan), N(nan), &x, &pk);
+      break;
+      default:
+         eosio_assert( false, "idx_double_nan_lookup_fail: unexpected lookup_type" );
+   }
+}

libraries/chain/include/eosio/chain/contracts/contract_table_objects.hpp

@@ -133,10 +133,8 @@ namespace eosio { namespace chain { namespace contracts {
    typedef secondary_index<key256_t,index256_object_type>::index_index  index256_index;
 
    struct soft_double_less {
-      bool operator()( uint64_t a, uint64_t b )const {
-         float64_t x; x.v = a;
-         float64_t y; y.v = b;
-         return f64_lt(x, y);
+      bool operator()( const float64_t& lhs, const float64_t& rhs )const {
+         return f64_lt(lhs, rhs);
       }
    };
 
@@ -145,8 +143,8 @@ namespace eosio { namespace chain { namespace contracts {
     *
     *  The software double implementation is using the Berkeley softfloat library (release 3).
     */
-   typedef secondary_index<uint64_t,index_double_object_type,soft_double_less>::index_object  index_double_object;
-   typedef secondary_index<uint64_t,index_double_object_type,soft_double_less>::index_index   index_double_index;
+   typedef secondary_index<float64_t,index_double_object_type,soft_double_less>::index_object  index_double_object;
+   typedef secondary_index<float64_t,index_double_object_type,soft_double_less>::index_index   index_double_index;
 
 } // ::contracts
 

libraries/chain/wasm_interface.cpp

@@ -668,7 +668,8 @@ class softfloat_api : public context_aware_api {
 
       static bool sign_bit( float32_t f ) { return f.v >> 31; }
       static bool sign_bit( float64_t f ) { return f.v >> 63; }
-      
+
+
 
 };
 class producer_api : public context_aware_api {
@@ -976,6 +977,42 @@ class console_api : public context_aware_api {
          return context.IDX.previous_secondary(iterator, primary);\
       }
 
+#define DB_API_METHOD_WRAPPERS_FLOAT_SECONDARY(IDX, TYPE)\
+      int db_##IDX##_store( uint64_t scope, uint64_t table, uint64_t payer, uint64_t id, const TYPE& secondary ) {\
+         EOS_ASSERT( !softfloat_api::is_nan( secondary ), transaction_exception, "NaN is not an allowed value for a secondary key" );\
+         return context.IDX.store( scope, table, payer, id, secondary );\
+      }\
+      void db_##IDX##_update( int iterator, uint64_t payer, const TYPE& secondary ) {\
+         EOS_ASSERT( !softfloat_api::is_nan( secondary ), transaction_exception, "NaN is not an allowed value for a secondary key" );\
+         return context.IDX.update( iterator, payer, secondary );\
+      }\
+      void db_##IDX##_remove( int iterator ) {\
+         return context.IDX.remove( iterator );\
+      }\
+      int db_##IDX##_find_secondary( uint64_t code, uint64_t scope, uint64_t table, const TYPE& secondary, uint64_t& primary ) {\
+         EOS_ASSERT( !softfloat_api::is_nan( secondary ), transaction_exception, "NaN is not an allowed value for a secondary key" );\
+         return context.IDX.find_secondary(code, scope, table, secondary, primary);\
+      }\
+      int db_##IDX##_find_primary( uint64_t code, uint64_t scope, uint64_t table, TYPE& secondary, uint64_t primary ) {\
+         return context.IDX.find_primary(code, scope, table, secondary, primary);\
+      }\
+      int db_##IDX##_lowerbound( uint64_t code, uint64_t scope, uint64_t table,  TYPE& secondary, uint64_t& primary ) {\
+         EOS_ASSERT( !softfloat_api::is_nan( secondary ), transaction_exception, "NaN is not an allowed value for a secondary key" );\
+         return context.IDX.lowerbound_secondary(code, scope, table, secondary, primary);\
+      }\
+      int db_##IDX##_upperbound( uint64_t code, uint64_t scope, uint64_t table,  TYPE& secondary, uint64_t& primary ) {\
+         EOS_ASSERT( !softfloat_api::is_nan( secondary ), transaction_exception, "NaN is not an allowed value for a secondary key" );\
+         return context.IDX.upperbound_secondary(code, scope, table, secondary, primary);\
+      }\
+      int db_##IDX##_end( uint64_t code, uint64_t scope, uint64_t table ) {\
+         return context.IDX.end_secondary(code, scope, table);\
+      }\
+      int db_##IDX##_next( int iterator, uint64_t& primary  ) {\
+         return context.IDX.next_secondary(iterator, primary);\
+      }\
+      int db_##IDX##_previous( int iterator, uint64_t& primary ) {\
+         return context.IDX.previous_secondary(iterator, primary);\
+      }
 
 class database_api : public context_aware_api {
    public:
@@ -1015,7 +1052,7 @@ class database_api : public context_aware_api {
       DB_API_METHOD_WRAPPERS_SIMPLE_SECONDARY(idx64,  uint64_t)
       DB_API_METHOD_WRAPPERS_SIMPLE_SECONDARY(idx128, uint128_t)
       DB_API_METHOD_WRAPPERS_ARRAY_SECONDARY(idx256, 2, uint128_t)
-      DB_API_METHOD_WRAPPERS_SIMPLE_SECONDARY(idx_double, uint64_t)
+      DB_API_METHOD_WRAPPERS_FLOAT_SECONDARY(idx_double, float64_t)
 };
 
 class memory_api : public context_aware_api {
@@ -1250,37 +1287,37 @@ class compiler_builtins : public context_aware_api {
       }
       int __eqtf2( uint64_t la, uint64_t ha, uint64_t lb, uint64_t hb ) {
          float128_t a = {{ la, ha }};
-         float128_t b = {{ la, ha }};
+         float128_t b = {{ lb, hb }};
          return f128_eq( a, b );
       }
       int __netf2( uint64_t la, uint64_t ha, uint64_t lb, uint64_t hb ) {
          float128_t a = {{ la, ha }};
-         float128_t b = {{ la, ha }};
+         float128_t b = {{ lb, hb }};
          return !f128_eq( a, b );
       }
       int __getf2( uint64_t la, uint64_t ha, uint64_t lb, uint64_t hb ) {
          float128_t a = {{ la, ha }};
-         float128_t b = {{ la, ha }};
+         float128_t b = {{ lb, hb }};
          return !f128_lt( a, b );
       }
       int __gttf2( uint64_t la, uint64_t ha, uint64_t lb, uint64_t hb ) {
          float128_t a = {{ la, ha }};
-         float128_t b = {{ la, ha }};
+         float128_t b = {{ lb, hb }};
          return !f128_lt( a, b ) && !f128_eq( a, b );
       }
       int __letf2( uint64_t la, uint64_t ha, uint64_t lb, uint64_t hb ) {
          float128_t a = {{ la, ha }};
-         float128_t b = {{ la, ha }};
+         float128_t b = {{ lb, hb }};
          return f128_le( a, b );
       }
       int __lttf2( uint64_t la, uint64_t ha, uint64_t lb, uint64_t hb ) {
          float128_t a = {{ la, ha }};
-         float128_t b = {{ la, ha }};
+         float128_t b = {{ lb, hb }};
          return f128_lt( a, b );
       }
       int __cmptf2( uint64_t la, uint64_t ha, uint64_t lb, uint64_t hb ) {
          float128_t a = {{ la, ha }};
-         float128_t b = {{ la, ha }};
+         float128_t b = {{ lb, hb }};
          if ( f128_lt( a, b ) )
             return -1;
          if ( f128_eq( a, b ) )
@@ -1289,7 +1326,7 @@ class compiler_builtins : public context_aware_api {
       }
       int __unordtf2( uint64_t la, uint64_t ha, uint64_t lb, uint64_t hb ) {
          float128_t a = {{ la, ha }};
-         float128_t b = {{ la, ha }};
+         float128_t b = {{ lb, hb }};
          if ( f128_isSignalingNaN( a ) || f128_isSignalingNaN( b ) )
             return 1;
          return 0;

tests/api_tests/api_tests.cpp

@@ -945,6 +945,21 @@ BOOST_FIXTURE_TEST_CASE(db_tests, TESTER) { try {
                       N(testapi) );
    BOOST_CHECK_EQUAL( res, success() );
 
+   CALL_TEST_FUNCTION_AND_CHECK_EXCEPTION( *this, "test_db", "idx_double_nan_create_fail", {},
+                                           transaction_exception, "NaN is not an allowed value for a secondary key");
+   CALL_TEST_FUNCTION_AND_CHECK_EXCEPTION( *this, "test_db", "idx_double_nan_modify_fail", {},
+                                           transaction_exception, "NaN is not an allowed value for a secondary key");
+
+   uint32_t lookup_type = 0; // 0 for find, 1 for lower bound, and 2 for upper bound;
+   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");
+   lookup_type = 1;
+   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");
+   lookup_type = 2;
+   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");
+
    BOOST_REQUIRE_EQUAL( validate(), true );
 } FC_LOG_AND_RETHROW() }