Merge pull request #648 from ethereum/hex_refactor

Refactor hex decoding utilities

include/evmc/hex.hpp

@@ -35,7 +35,7 @@ inline std::string hex(bytes_view bs)
     return str;
 }
 
-namespace internal_hex
+namespace internal
 {
 /// Extracts the nibble value out of a hex digit.
 /// Returns -1 in case of invalid hex digit.
@@ -50,46 +50,40 @@ inline constexpr int from_hex_digit(char h) noexcept
     else
         return -1;
 }
+}  // namespace internal
 
-/// The constexpr variant of std::isspace().
-inline constexpr bool isspace(char ch) noexcept
-{
-    // Implementation taken from LLVM's libc.
-    return ch == ' ' || (static_cast<unsigned>(ch) - '\t') < 5;
-}
-
-template <typename OutputIt>
-inline constexpr bool from_hex(std::string_view hex, OutputIt result) noexcept
+/// Decodes hex-encoded sequence of characters.
+///
+/// It is guaranteed that the output will not be longer than half of the input length.
+///
+/// @param begin  The input begin iterator. It only must satisfy input iterator concept.
+/// @param end    The input end iterator. It only must satisfy input iterator concept.
+/// @param out    The output iterator. It must satisfy output iterator concept.
+/// @return       True if successful, false if input is invalid hex.
+template <typename InputIt, typename OutputIt>
+inline constexpr bool from_hex(InputIt begin, InputIt end, OutputIt out) noexcept
 {
-    // Omit the optional 0x prefix.
-    if (hex.size() >= 2 && hex[0] == '0' && hex[1] == 'x')
-        hex.remove_prefix(2);
-
-    constexpr int empty_mark = -1;
-    int hi_nibble = empty_mark;
-    for (const auto h : hex)
+    int hi_nibble = -1;  // Init with invalid value, should never be used.
+    size_t i = 0;
+    for (auto it = begin; it != end; ++it, ++i)
     {
-        if (isspace(h))
-            continue;
-
-        const int v = from_hex_digit(h);
+        const auto h = *it;
+        const int v = evmc::internal::from_hex_digit(h);
         if (v < 0)
+        {
+            if (i == 1 && hi_nibble == 0 && h == 'x')  // 0x prefix
+                continue;
             return false;
+        }
 
-        if (hi_nibble == empty_mark)
-        {
+        if (i % 2 == 0)
             hi_nibble = v << 4;
-        }
         else
-        {
-            *result++ = static_cast<uint8_t>(hi_nibble | v);
-            hi_nibble = empty_mark;
-        }
+            *out++ = static_cast<uint8_t>(hi_nibble | v);
     }
 
-    return hi_nibble == empty_mark;
+    return i % 2 == 0;
 }
-}  // namespace internal_hex
 
 /// Validates hex encoded string.
 ///
@@ -103,7 +97,7 @@ inline bool validate_hex(std::string_view hex) noexcept
         noop_output_iterator operator++(int) noexcept { return *this; }  // NOLINT(cert-dcl21-cpp)
     };
 
-    return internal_hex::from_hex(hex, noop_output_iterator{});
+    return from_hex(hex.begin(), hex.end(), noop_output_iterator{});
 }
 
 /// Decodes hex encoded string to bytes.
@@ -115,7 +109,7 @@ inline std::optional<bytes> from_hex(std::string_view hex)
 {
     bytes bs;
     bs.reserve(hex.size() / 2);
-    if (!internal_hex::from_hex(hex, std::back_inserter(bs)))
+    if (!from_hex(hex.begin(), hex.end(), std::back_inserter(bs)))
         return {};
     return bs;
 }

test/unittests/CMakeLists.txt

@@ -23,6 +23,7 @@ add_executable(
     loader_mock.h
     loader_test.cpp
     mocked_host_test.cpp
+    filter_iterator_test.cpp
     tooling_test.cpp
     hex_test.cpp
 )

test/unittests/filter_iterator_test.cpp

@@ -0,0 +1,107 @@
+// EVMC: Ethereum Client-VM Connector API.
+// Copyright 2022 The EVMC Authors.
+// Licensed under the Apache License, Version 2.0.
+
+#include <tools/evmc/filter_iterator.hpp>
+#include <gtest/gtest.h>
+#include <cctype>
+
+using evmc::skip_space_iterator;
+
+namespace
+{
+std::string remove_space(std::string_view in)
+{
+    // Copy input to additional buffer. This helps with out-of-buffer reads detection by sanitizers.
+    const std::vector<char> in_buffer(in.begin(), in.end());
+
+    // Filter the input.
+    std::string out;
+    std::copy(skip_space_iterator{in_buffer.begin(), in_buffer.end()},
+              skip_space_iterator{in_buffer.end(), in_buffer.end()}, std::back_inserter(out));
+    return out;
+}
+
+bool is_positive(int x) noexcept
+{
+    return x > 0;
+}
+}  // namespace
+
+
+TEST(filter_iterator, filter_positive_integers)
+{
+    std::vector<int> in{1, 0, 0, 2, -3, 3, 4, 5, 0, 6, 7, -1, -2, 0, 8, 9, -10};
+    std::vector<int> out;
+
+    using iter = evmc::filter_iterator<std::vector<int>::const_iterator, is_positive>;
+    std::copy(iter{in.begin(), in.end()}, iter{in.end(), in.end()}, std::back_inserter(out));
+    ASSERT_EQ(out.size(), 9u);
+    EXPECT_EQ(out[0], 1);
+    EXPECT_EQ(out[1], 2);
+    EXPECT_EQ(out[2], 3);
+    EXPECT_EQ(out[3], 4);
+    EXPECT_EQ(out[4], 5);
+    EXPECT_EQ(out[5], 6);
+    EXPECT_EQ(out[6], 7);
+    EXPECT_EQ(out[7], 8);
+    EXPECT_EQ(out[8], 9);
+}
+
+
+TEST(skip_space_iterator, empty)
+{
+    EXPECT_EQ(remove_space(""), "");
+    EXPECT_EQ(remove_space(" "), "");
+    EXPECT_EQ(remove_space("  "), "");
+}
+
+TEST(skip_space_iterator, filter_middle)
+{
+    EXPECT_EQ(remove_space("x y"), "xy");
+    EXPECT_EQ(remove_space("x  y"), "xy");
+}
+
+TEST(skip_space_iterator, filter_front)
+{
+    EXPECT_EQ(remove_space(" x"), "x");
+    EXPECT_EQ(remove_space("  x"), "x");
+}
+
+TEST(skip_space_iterator, filter_back)
+{
+    EXPECT_EQ(remove_space("x "), "x");
+    EXPECT_EQ(remove_space("x  "), "x");
+}
+
+TEST(skip_space_iterator, filter_mixed)
+{
+    EXPECT_EQ(remove_space(" x y z "), "xyz");
+    EXPECT_EQ(remove_space("  x  y  z  "), "xyz");
+}
+
+TEST(skip_space_iterator, isspace)
+{
+    // Test internal isspace() compliance with std::isspace().
+    // The https://en.cppreference.com/w/cpp/string/byte/isspace has the list of "space" characters.
+
+    for (int i = int{std::numeric_limits<char>::min()}; i <= std::numeric_limits<char>::max(); ++i)
+    {
+        const auto c = static_cast<char>(i);
+        EXPECT_EQ(evmc::isspace(c), (std::isspace(c) != 0));
+        switch (c)
+        {
+        case ' ':
+        case '\f':
+        case '\n':
+        case '\r':
+        case '\t':
+        case '\v':
+            EXPECT_TRUE(evmc::isspace(c));
+            break;
+        default:
+            EXPECT_FALSE(evmc::isspace(c));
+            break;
+        }
+    }
+}

test/unittests/hex_test.cpp

@@ -3,8 +3,8 @@
 // Licensed under the Apache License, Version 2.0.
 
 #include <evmc/hex.hpp>
+#include <tools/evmc/filter_iterator.hpp>
 #include <gtest/gtest.h>
-#include <cctype>
 
 using namespace evmc;
 
@@ -65,13 +65,6 @@ TEST(hex, from_hex_0x_prefix)
     EXPECT_EQ(from_hex("0x001y"), std::nullopt);
 }
 
-TEST(hex, from_hex_skip_whitespace)
-{
-    EXPECT_EQ(from_hex("0x "), bytes{});
-    EXPECT_EQ(from_hex(" \n\t"), bytes{});
-    EXPECT_EQ(from_hex(" \n\tab\r"), bytes{0xab});
-}
-
 TEST(hex, validate_hex)
 {
     EXPECT_TRUE(validate_hex(""));
@@ -81,28 +74,19 @@ TEST(hex, validate_hex)
     EXPECT_FALSE(validate_hex("WXYZ"));
 }
 
-TEST(hex, isspace)
+TEST(hex, from_hex_skip_space)
 {
-    // Test internal isspace() compliance with std::isspace().
-    // The https://en.cppreference.com/w/cpp/string/byte/isspace has the list of "space" characters.
-
-    for (int i = int{std::numeric_limits<char>::min()}; i <= std::numeric_limits<char>::max(); ++i)
-    {
-        const auto c = static_cast<char>(i);
-        EXPECT_EQ(evmc::internal_hex::isspace(c), (std::isspace(c) != 0));
-        switch (c)
-        {
-        case ' ':
-        case '\f':
-        case '\n':
-        case '\r':
-        case '\t':
-        case '\v':
-            EXPECT_TRUE(evmc::internal_hex::isspace(c));
-            break;
-        default:
-            EXPECT_FALSE(evmc::internal_hex::isspace(c));
-            break;
-        }
-    }
+    // Combine from_hex with skip_space_iterator.
+    static constexpr auto from_hex_skip_space = [](std::string_view hex) {
+        bytes out;
+        const auto status =
+            from_hex(skip_space_iterator{hex.begin(), hex.end()},
+                     skip_space_iterator{hex.end(), hex.end()}, std::back_inserter(out));
+        EXPECT_TRUE(status);
+        return out;
+    };
+    EXPECT_EQ(from_hex_skip_space("0x010203"), (bytes{0x01, 0x02, 0x03}));
+    EXPECT_EQ(from_hex_skip_space("0x 010203 "), (bytes{0x01, 0x02, 0x03}));
+    EXPECT_EQ(from_hex_skip_space(" 0 x 0 1 0 2 0 3 "), (bytes{0x01, 0x02, 0x03}));
+    EXPECT_EQ(from_hex_skip_space("\f 0\r x  0  1\t 0  2 \v0  3 \n"), (bytes{0x01, 0x02, 0x03}));
 }

test/unittests/tooling_test.cpp

@@ -133,9 +133,8 @@ TEST(tool_commands, create_preserve_storage)
     std::ostringstream out;
 
     const auto exit_code =
-        run(vm, EVMC_BERLIN, 200,
-            *from_hex("60bb 6000 55 6a6000546000526001601ff3 6000 52 600b 6015 f3"), {}, true,
-            false, out);
+        run(vm, EVMC_BERLIN, 200, *from_hex("60bb6000556a6000546000526001601ff3600052600b6015f3"),
+            {}, true, false, out);
     EXPECT_EQ(exit_code, 0);
     EXPECT_EQ(out.str(), out_pattern("Berlin", 200, "success", 7, "bb", true));
 }
@@ -145,7 +144,7 @@ TEST(tool_commands, bench_add)
     auto vm = evmc::VM{evmc_create_example_vm()};
     std::ostringstream out;
 
-    const auto exit_code = run(vm, EVMC_LONDON, 200, *from_hex("6002 80 01"), {}, false, true, out);
+    const auto exit_code = run(vm, EVMC_LONDON, 200, *from_hex("60028001"), {}, false, true, out);
     EXPECT_EQ(exit_code, 0);
 
     const auto o = out.str();
@@ -160,7 +159,7 @@ TEST(tool_commands, bench_inconsistent_output)
     auto vm = evmc::VM{evmc_create_example_vm()};
     std::ostringstream out;
 
-    const auto code = *from_hex("6000 54 6001 6000 55 6000 52 6001 601f f3");
+    const auto code = *from_hex("60005460016000556000526001601ff3");
     const auto exit_code = run(vm, EVMC_BYZANTIUM, 200, code, {}, false, true, out);
     EXPECT_EQ(exit_code, 0);
 

tools/evmc/CMakeLists.txt

@@ -5,7 +5,7 @@
 hunter_add_package(CLI11)
 find_package(CLI11 REQUIRED)
 
-add_executable(evmc-tool main.cpp)
+add_executable(evmc-tool main.cpp filter_iterator.hpp)
 add_executable(evmc::tool ALIAS evmc-tool)
 set_target_properties(evmc-tool PROPERTIES OUTPUT_NAME evmc)
 set_source_files_properties(main.cpp PROPERTIES