From 1a6b44d67e077eb5904ab30255454693d6a1edac Mon Sep 17 00:00:00 2001 From: ludamad Date: Thu, 7 Dec 2023 16:38:41 -0500 Subject: [PATCH] fix: broken uint256_t implicit copy (#3625) Look at the diff, then consider this code ``` #include #include class Uint256_t { public: // Conversion operator (explicit) template explicit operator T() const { std::cout << "Uint256_t conversion operator called\n"; return static_cast(data[0]); } int data[4] = {0,0,0,0}; }; class Field { public: Field() {} // Constructor that takes a Uint256_t Field(int x) { std::cout << "This is silly!!!!" << std::endl; } Field(const Uint256_t& input) { (void)input; std::cout << "Field constructor called\n"; // Additional initialization code here } }; class Toot { public: Toot(Field f) { (void)f; } }; int main() { Uint256_t uint256_value; Field x = uint256_value; Toot t{uint256_value}; return 0; } ``` With std::integral concept on the cast: ``` Field constructor called Field constructor called ``` but without! which was caught in practice passing a uint256 to FieldSponge ``` Field constructor called Uint256_t conversion operator called This is silly!!!! ``` This truncated the field mysteriously! Open-ended implicit conversions are dangerous --------- Co-authored-by: ludamad --- barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp | 3 ++- barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp | 3 ++- .../stdlib/primitives/bigfield/bigfield.fuzzer.hpp | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp b/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp index 3dc320abe2d..e229f9b7d23 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uint128/uint128.hpp @@ -5,6 +5,7 @@ #ifdef __i386__ #include "barretenberg/common/serialize.hpp" +#include namespace numeric { @@ -37,7 +38,7 @@ class alignas(32) uint128_t { constexpr ~uint128_t() = default; explicit constexpr operator bool() const { return static_cast(data[0]); }; - template explicit constexpr operator T() const { return static_cast(data[0]); }; + template explicit constexpr operator T() const { return static_cast(data[0]); }; [[nodiscard]] constexpr bool get_bit(uint64_t bit_index) const; [[nodiscard]] constexpr uint64_t get_msb() const; diff --git a/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp b/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp index d8cd9ef2f18..5ddaa9713ae 100644 --- a/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp +++ b/barretenberg/cpp/src/barretenberg/numeric/uint256/uint256.hpp @@ -14,6 +14,7 @@ #include "../uint128/uint128.hpp" #include "barretenberg/common/serialize.hpp" #include "barretenberg/common/throw_or_abort.hpp" +#include #include #include #include @@ -91,7 +92,7 @@ class alignas(32) uint256_t { explicit constexpr operator bool() const { return static_cast(data[0]); }; - template explicit constexpr operator T() const { return static_cast(data[0]); }; + template explicit constexpr operator T() const { return static_cast(data[0]); }; [[nodiscard]] constexpr bool get_bit(uint64_t bit_index) const; [[nodiscard]] constexpr uint64_t get_msb() const; diff --git a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.fuzzer.hpp b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.fuzzer.hpp index 18e2cbe129e..5111ea71a54 100644 --- a/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.fuzzer.hpp +++ b/barretenberg/cpp/src/barretenberg/stdlib/primitives/bigfield/bigfield.fuzzer.hpp @@ -266,7 +266,7 @@ template class BigFieldBase { mask = (uint256_t(1) << mask_size) - 1; // Choose the bit range // Return instruction - return { .id = instruction_opcode, .arguments.element = Element(temp & mask) }; + return { .id = instruction_opcode, .arguments.element = Element(static_cast(temp & mask)) }; break; case OPCODE::RANDOMSEED: