Merge pull request #534 from ethereum/instruction_names

Get instruction names from instr::traits[]

lib/evmone/tracing.cpp

@@ -12,9 +12,10 @@ namespace evmone
 {
 namespace
 {
-std::string get_name(const char* const* names, uint8_t opcode)
+std::string get_name(uint8_t opcode)
 {
-    const auto name = names[opcode];
+    // TODO: Create constexpr tables of names (maybe even per revision).
+    const auto name = instr::traits[opcode].name;
     return (name != nullptr) ? name : "0x" + evmc::hex(opcode);
 }
 
@@ -25,21 +26,18 @@ class HistogramTracer : public Tracer
     {
         const int32_t depth;
         const uint8_t* const code;
-        const char* const* const opcode_names;
         uint32_t counts[256]{};
 
-        Context(int32_t _depth, const uint8_t* _code, const char* const* _opcode_names) noexcept
-          : depth{_depth}, code{_code}, opcode_names{_opcode_names}
-        {}
+        Context(int32_t _depth, const uint8_t* _code) noexcept : depth{_depth}, code{_code} {}
     };
 
     std::stack<Context> m_contexts;
     std::ostream& m_out;
 
     void on_execution_start(
-        evmc_revision rev, const evmc_message& msg, bytes_view code) noexcept override
+        evmc_revision /*rev*/, const evmc_message& msg, bytes_view code) noexcept override
     {
-        m_contexts.emplace(msg.depth, code.data(), evmc_get_instruction_names_table(rev));
+        m_contexts.emplace(msg.depth, code.data());
     }
 
     void on_instruction_start(uint32_t pc, const intx::uint256* /*stack_top*/, int /*stack_height*/,
@@ -52,13 +50,12 @@ class HistogramTracer : public Tracer
     void on_execution_end(const evmc_result& /*result*/) noexcept override
     {
         const auto& ctx = m_contexts.top();
-        const auto names = ctx.opcode_names;
 
         m_out << "--- # HISTOGRAM depth=" << ctx.depth << "\nopcode,count\n";
         for (size_t i = 0; i < std::size(ctx.counts); ++i)
         {
             if (ctx.counts[i] != 0)
-                m_out << get_name(names, static_cast<uint8_t>(i)) << ',' << ctx.counts[i] << '\n';
+                m_out << get_name(static_cast<uint8_t>(i)) << ',' << ctx.counts[i] << '\n';
         }
 
         m_contexts.pop();
@@ -80,7 +77,6 @@ class InstructionTracer : public Tracer
     };
 
     std::stack<Context> m_contexts;
-    const char* const* m_opcode_names = nullptr;
     std::ostream& m_out;  ///< Output stream.
 
     void output_stack(const intx::uint256* stack_top, int stack_height)
@@ -100,8 +96,6 @@ class InstructionTracer : public Tracer
     void on_execution_start(
         evmc_revision rev, const evmc_message& msg, bytes_view code) noexcept override
     {
-        if (m_contexts.empty())
-            m_opcode_names = evmc_get_instruction_names_table(rev);
         m_contexts.emplace(code.data(), msg.gas);
 
         m_out << "{";
@@ -120,7 +114,7 @@ class InstructionTracer : public Tracer
         m_out << "{";
         m_out << R"("pc":)" << pc;
         m_out << R"(,"op":)" << int{opcode};
-        m_out << R"(,"opName":")" << get_name(m_opcode_names, opcode) << '"';
+        m_out << R"(,"opName":")" << get_name(opcode) << '"';
         m_out << R"(,"gas":)" << state.gas_left;
         output_stack(stack_top, stack_height);
 
@@ -147,8 +141,6 @@ class InstructionTracer : public Tracer
         m_out << "}\n";
 
         m_contexts.pop();
-        if (m_contexts.empty())
-            m_opcode_names = nullptr;
     }
 
 public:

test/fuzzer/fuzzer.cpp

@@ -306,12 +306,12 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t data_size) noe
     auto ref_host = in.host;  // Copy Host.
     const auto& code = ref_host.accounts[in.msg.recipient].code;
 
-    if (print_input)
+    if (print_input != nullptr)
     {
         std::cout << "rev: " << int{in.rev} << "\n";
         std::cout << "depth: " << int{in.msg.depth} << "\n";
         std::cout << "code: " << hex(code) << "\n";
-        std::cout << "decoded: " << decode(code, in.rev) << "\n";
+        std::cout << "decoded: " << decode(code) << "\n";
         std::cout << "input: " << hex({in.msg.input_data, in.msg.input_size}) << "\n";
         std::cout << "account: " << hex(in.msg.recipient) << "\n";
         std::cout << "caller: " << hex(in.msg.sender) << "\n";

test/unittests/bytecode_test.cpp

@@ -80,32 +80,24 @@ TEST(bytecode, repeat)
     EXPECT_EQ(0 * OP_STOP, "");
 }
 
-TEST(bytecode, to_name)
-{
-    EXPECT_EQ(to_name(OP_SAR), "SAR");
-    EXPECT_EQ(to_name(OP_SAR, EVMC_HOMESTEAD), "UNDEFINED_INSTRUCTION:1d");
-}
-
 TEST(bytecode, decode)
 {
     const auto code = push(0x01e240) + OP_DUP1 + OP_GAS + "cc" + OP_REVERT;
-    EXPECT_EQ(decode(code, EVMC_FRONTIER),
-        "bytecode{} + OP_PUSH3 + \"01e240\" + OP_DUP1 + OP_GAS + \"cc\" + \"fd\"");
-    EXPECT_EQ(decode(code, EVMC_BYZANTIUM),
-        "bytecode{} + OP_PUSH3 + \"01e240\" + OP_DUP1 + OP_GAS + \"cc\" + OP_REVERT");
+    EXPECT_EQ(
+        decode(code), R"(bytecode{} + OP_PUSH3 + "01e240" + OP_DUP1 + OP_GAS + "cc" + OP_REVERT)");
 }
 
 TEST(bytecode, decode_push_trimmed_data)
 {
     const auto code1 = bytecode{} + OP_PUSH2 + "0000";
-    EXPECT_EQ(decode(code1, EVMC_FRONTIER), "bytecode{} + OP_PUSH2 + \"0000\"");
+    EXPECT_EQ(decode(code1), "bytecode{} + OP_PUSH2 + \"0000\"");
 
     const auto code2 = bytecode{} + OP_PUSH2 + "00";
-    EXPECT_EQ(decode(code2, EVMC_FRONTIER), "bytecode{} + OP_PUSH2 + \"00\"");
+    EXPECT_EQ(decode(code2), "bytecode{} + OP_PUSH2 + \"00\"");
 
     const auto code3 = bytecode{} + OP_PUSH2;
-    EXPECT_EQ(decode(code3, EVMC_FRONTIER), "bytecode{} + OP_PUSH2");
+    EXPECT_EQ(decode(code3), "bytecode{} + OP_PUSH2");
 
     const auto code4 = bytecode{} + OP_PUSH2 + "";
-    EXPECT_EQ(decode(code4, EVMC_FRONTIER), "bytecode{} + OP_PUSH2");
+    EXPECT_EQ(decode(code4), "bytecode{} + OP_PUSH2");
 }

test/unittests/evm_state_test.cpp

@@ -6,6 +6,7 @@
 /// about accounts, without storage.
 
 #include "evm_fixture.hpp"
+#include <evmone/instructions_traits.hpp>
 
 using namespace evmc::literals;
 using evmone::test::evm;
@@ -193,7 +194,7 @@ TEST_P(evm, log_data_cost)
         EXPECT_EQ(host.recorded_logs.size(), 0);
         execute(cost - 1, code);
         EXPECT_EQ(result.status_code, EVMC_OUT_OF_GAS);
-        EXPECT_EQ(host.recorded_logs.size(), 0) << to_name(op);
+        EXPECT_EQ(host.recorded_logs.size(), 0) << evmone::instr::traits[op].name;
         host.recorded_logs.clear();
     }
 }

test/unittests/evm_test.cpp

@@ -623,12 +623,10 @@ TEST_P(evm, undefined_instructions)
 {
     for (auto i = 0; i <= EVMC_MAX_REVISION; ++i)
     {
-        auto r = evmc_revision(i);
-        auto names = evmc_get_instruction_names_table(r);
-
+        const auto r = evmc_revision(i);
         for (uint8_t opcode = 0; opcode <= 0xfe; ++opcode)
         {
-            if (names[opcode] != nullptr)
+            if (evmone::instr::gas_costs[r][opcode] != evmone::instr::undefined)
                 continue;
 
             auto res = vm.execute(host, r, {}, &opcode, sizeof(opcode));

test/unittests/instructions_test.cpp

@@ -101,7 +101,7 @@ TEST(instructions, compare_with_evmc_instruction_tables)
 
             const auto case_descr = [rev](size_t opcode) {
                 auto case_descr_str = std::ostringstream{};
-                case_descr_str << "opcode " << to_name(evmc_opcode(opcode), rev);
+                case_descr_str << "opcode " << instr::traits[opcode].name;
                 case_descr_str << " on revision " << rev;
                 return case_descr_str.str();
             };

test/unittests/tracing_test.cpp

@@ -6,6 +6,7 @@
 #include <evmc/evmc.hpp>
 #include <evmc/mocked_host.hpp>
 #include <evmone/evmone.h>
+#include <evmone/instructions_traits.hpp>
 #include <evmone/tracing.hpp>
 #include <evmone/vm.hpp>
 #include <gmock/gmock.h>
@@ -58,8 +59,7 @@ class tracing : public Test
             int /*stack_height*/, const evmone::ExecutionState& /*state*/) noexcept override
         {
             const auto opcode = m_code[pc];
-            m_trace << m_name << pc << ":"
-                    << evmc_get_instruction_names_table(EVMC_MAX_REVISION)[opcode] << " ";
+            m_trace << m_name << pc << ":" << evmone::instr::traits[opcode].name << " ";
         }
 
     public:

test/utils/CMakeLists.txt

@@ -11,8 +11,4 @@ add_library(testutils STATIC
 )
 
 target_link_libraries(testutils PRIVATE evmc::instructions)
-target_include_directories(testutils PUBLIC ${PROJECT_SOURCE_DIR})
-
-add_library(testutils-dump STATIC dump.cpp dump.hpp)
-target_link_libraries(testutils-dump PRIVATE testutils evmone intx::intx)
-target_include_directories(testutils-dump PRIVATE ${evmone_private_include_dir})
+target_include_directories(testutils PUBLIC ${PROJECT_SOURCE_DIR} ${evmone_private_include_dir})

test/utils/bytecode.hpp

@@ -4,7 +4,7 @@
 #pragma once
 
 #include <evmc/evmc.hpp>
-#include <evmc/instructions.h>
+#include <evmone/instructions_traits.hpp>
 #include <intx/intx.hpp>
 #include <test/utils/utils.hpp>
 #include <algorithm>
@@ -414,23 +414,13 @@ inline std::string hex(evmc_opcode opcode) noexcept
     return hex(static_cast<uint8_t>(opcode));
 }
 
-inline std::string to_name(evmc_opcode opcode, evmc_revision rev = EVMC_MAX_REVISION) noexcept
-{
-    const auto names = evmc_get_instruction_names_table(rev);
-    if (const auto name = names[opcode]; name)
-        return name;
-
-    return "UNDEFINED_INSTRUCTION:" + hex(opcode);
-}
-
-inline std::string decode(bytes_view bytecode, evmc_revision rev)
+inline std::string decode(bytes_view bytecode)
 {
     auto s = std::string{"bytecode{}"};
-    const auto names = evmc_get_instruction_names_table(rev);
     for (auto it = bytecode.begin(); it != bytecode.end(); ++it)
     {
         const auto opcode = *it;
-        if (const auto name = names[opcode]; name)
+        if (const auto name = evmone::instr::traits[opcode].name; name)
         {
             s += std::string{" + OP_"} + name;