Skip to content

eof: Pass _eofVersion down to libevmasm/Assembly class #15356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions libevmasm/Assembly.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ std::pair<std::shared_ptr<Assembly>, std::vector<std::string>> Assembly::fromJSO
"Member 'sourceList' may only be present in the root JSON object."
);

auto result = std::make_shared<Assembly>(EVMVersion{}, _level == 0 /* _creation */, "" /* _name */);
auto result = std::make_shared<Assembly>(EVMVersion{}, _level == 0 /* _creation */, std::nullopt, "" /* _name */);
std::vector<std::string> parsedSourceList;
if (_json.contains("sourceList"))
{
Expand Down Expand Up @@ -735,7 +735,7 @@ std::map<u256, u256> const& Assembly::optimiseInternal(
{
count = 0;

if (_settings.runInliner)
if (_settings.runInliner && !m_eofVersion.has_value())
Inliner{
m_items,
_tagsReferencedFromOutside,
Expand All @@ -762,7 +762,7 @@ std::map<u256, u256> const& Assembly::optimiseInternal(
}

// This only modifies PushTags, we have to run again to actually remove code.
if (_settings.runDeduplicate)
if (_settings.runDeduplicate && !m_eofVersion.has_value())
{
BlockDeduplicator deduplicator{m_items};
if (deduplicator.deduplicate())
Expand All @@ -787,7 +787,8 @@ std::map<u256, u256> const& Assembly::optimiseInternal(
}
}

if (_settings.runCSE)
// TODO: investigate for EOF
if (_settings.runCSE && !m_eofVersion.has_value())
{
// Control flow graph optimization has been here before but is disabled because it
// assumes we only jump to tags that are pushed. This is not the case anymore with
Expand Down Expand Up @@ -839,7 +840,8 @@ std::map<u256, u256> const& Assembly::optimiseInternal(
}
}

if (_settings.runConstantOptimiser)
// TODO: investigate for EOF
if (_settings.runConstantOptimiser && !m_eofVersion.has_value())
ConstantOptimisationMethod::optimiseConstants(
isCreation(),
isCreation() ? 1 : _settings.expectedExecutionsPerDeployment,
Expand All @@ -862,6 +864,9 @@ LinkerObject const& Assembly::assemble() const

LinkerObject& ret = m_assembledObject;

bool const eof = m_eofVersion.has_value();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool const eof = m_eofVersion.has_value();
bool const eof = m_eofVersion.has_value();
solAssert(!eof || m_eofVersion == 1, "Invalid EOF version.");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

solAssert(!eof || m_eofVersion == 1, "Invalid EOF version.");

size_t subTagSize = 1;
std::map<u256, std::pair<std::string, std::vector<size_t>>> immutableReferencesBySub;
for (auto const& sub: m_subs)
Expand Down Expand Up @@ -957,6 +962,7 @@ LinkerObject const& Assembly::assemble() const
}
case PushTag:
{
assertThrow(!eof, AssemblyException, "PushTag in EOF code");
ret.bytecode.push_back(tagPush);
tagRef[ret.bytecode.size()] = i.splitForeignPushTag();
ret.bytecode.resize(ret.bytecode.size() + bytesPerTag);
Expand All @@ -968,13 +974,15 @@ LinkerObject const& Assembly::assemble() const
ret.bytecode.resize(ret.bytecode.size() + bytesPerDataRef);
break;
case PushSub:
assertThrow(!eof, AssemblyException, "PushSub in EOF code");
assertThrow(i.data() <= std::numeric_limits<size_t>::max(), AssemblyException, "");
ret.bytecode.push_back(dataRefPush);
subRef.insert(std::make_pair(static_cast<size_t>(i.data()), ret.bytecode.size()));
ret.bytecode.resize(ret.bytecode.size() + bytesPerDataRef);
break;
case PushSubSize:
{
assertThrow(!eof, AssemblyException, "PushSubSize in EOF code");
assertThrow(i.data() <= std::numeric_limits<size_t>::max(), AssemblyException, "");
auto s = subAssemblyById(static_cast<size_t>(i.data()))->assemble().bytecode.size();
i.setPushedValue(u256(s));
Expand All @@ -987,6 +995,7 @@ LinkerObject const& Assembly::assemble() const
}
case PushProgramSize:
{
assertThrow(!eof, AssemblyException, "PushProgramSize in EOF code");
ret.bytecode.push_back(dataRefPush);
sizeRef.push_back(static_cast<unsigned>(ret.bytecode.size()));
ret.bytecode.resize(ret.bytecode.size() + bytesPerDataRef);
Expand All @@ -998,6 +1007,7 @@ LinkerObject const& Assembly::assemble() const
ret.bytecode.resize(ret.bytecode.size() + 20);
break;
case PushImmutable:
assertThrow(!eof, AssemblyException, "PushImmutable in EOF code");
ret.bytecode.push_back(static_cast<uint8_t>(Instruction::PUSH32));
// Maps keccak back to the "identifier" std::string of that immutable.
ret.immutableReferences[i.data()].first = m_immutables.at(i.data());
Expand All @@ -1011,6 +1021,7 @@ LinkerObject const& Assembly::assemble() const
break;
case AssignImmutable:
{
assertThrow(!eof, AssemblyException, "AssignImmutable in EOF code");
// Expect 2 elements on stack (source, dest_base)
auto const& offsets = immutableReferencesBySub[i.data()].second;
for (size_t i = 0; i < offsets.size(); ++i)
Expand Down Expand Up @@ -1063,7 +1074,7 @@ LinkerObject const& Assembly::assemble() const
"Some immutables were read from but never assigned, possibly because of optimization."
);

if (!m_subs.empty() || !m_data.empty() || !m_auxiliaryData.empty())
if (!eof && (!m_subs.empty() || !m_data.empty() || !m_auxiliaryData.empty()))
// Append an INVALID here to help tests find miscompilation.
ret.bytecode.push_back(static_cast<uint8_t>(Instruction::INVALID));

Expand Down
11 changes: 9 additions & 2 deletions libevmasm/Assembly.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,14 @@ using AssemblyPointer = std::shared_ptr<Assembly>;
class Assembly
{
public:
Assembly(langutil::EVMVersion _evmVersion, bool _creation, std::string _name): m_evmVersion(_evmVersion), m_creation(_creation), m_name(std::move(_name)) { }

Assembly(langutil::EVMVersion _evmVersion, bool _creation, std::optional<uint8_t> _eofVersion, std::string _name):
m_evmVersion(_evmVersion),
m_creation(_creation),
m_eofVersion(_eofVersion),
m_name(std::move(_name))
{}

std::optional<uint8_t> eofVersion() const { return m_eofVersion; }
AssemblyItem newTag() { assertThrow(m_usedTags < 0xffffffff, AssemblyException, ""); return AssemblyItem(Tag, m_usedTags++); }
AssemblyItem newPushTag() { assertThrow(m_usedTags < 0xffffffff, AssemblyException, ""); return AssemblyItem(PushTag, m_usedTags++); }
/// Returns a tag identified by the given name. Creates it if it does not yet exist.
Expand Down Expand Up @@ -242,6 +248,7 @@ class Assembly
int m_deposit = 0;
/// True, if the assembly contains contract creation code.
bool const m_creation = false;
std::optional<uint8_t> m_eofVersion;
/// Internal name of the assembly object, only used with the Yul backend
/// currently
std::string m_name;
Expand Down
2 changes: 1 addition & 1 deletion libsolidity/codegen/CompilerContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class CompilerContext
RevertStrings _revertStrings,
CompilerContext* _runtimeContext = nullptr
):
m_asm(std::make_shared<evmasm::Assembly>(_evmVersion, _runtimeContext != nullptr, std::string{})),
m_asm(std::make_shared<evmasm::Assembly>(_evmVersion, _runtimeContext != nullptr, std::nullopt, std::string{})),
m_evmVersion(_evmVersion),
m_revertStrings(_revertStrings),
m_reservedMemory{0},
Expand Down
8 changes: 8 additions & 0 deletions libsolidity/interface/CompilerStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,10 @@ std::string const* CompilerStack::sourceMapping(std::string const& _contractName
{
solAssert(m_stackState == CompilationSuccessful, "Compilation was not successful.");

// TODO
if (m_eofVersion.has_value())
return nullptr;

Contract const& c = contract(_contractName);
if (!c.sourceMapping)
{
Expand All @@ -877,6 +881,10 @@ std::string const* CompilerStack::runtimeSourceMapping(std::string const& _contr
{
solAssert(m_stackState == CompilationSuccessful, "Compilation was not successful.");

// TODO
if (m_eofVersion.has_value())
return nullptr;

Contract const& c = contract(_contractName);
if (!c.runtimeSourceMapping)
{
Expand Down
2 changes: 1 addition & 1 deletion libyul/YulStack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ YulStack::assembleEVMWithDeployed(std::optional<std::string_view> _deployName)
yulAssert(m_parserResult->hasCode(), "");
yulAssert(m_parserResult->analysisInfo, "");

evmasm::Assembly assembly(m_evmVersion, true, {});
evmasm::Assembly assembly(m_evmVersion, true, m_eofVersion, {});
EthAssemblyAdapter adapter(assembly);

// NOTE: We always need stack optimization when Yul optimizer is disabled (unless code contains
Expand Down
2 changes: 1 addition & 1 deletion libyul/backends/evm/EVMObjectCompiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ void EVMObjectCompiler::run(Object const& _object, bool _optimize)
yulAssert(_object.hasCode(), "No code.");
if (m_eofVersion.has_value())
yulAssert(
_optimize && (m_dialect.evmVersion() == langutil::EVMVersion()),
_optimize && (m_dialect.evmVersion() >= langutil::EVMVersion::prague()),
"Experimental EOF support is only available for optimized via-IR compilation and the most recent EVM version."
);
if (_optimize && m_dialect.evmVersion().canOverchargeGasForCall())
Expand Down
2 changes: 1 addition & 1 deletion libyul/backends/evm/EthAssemblyAdapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ void EthAssemblyAdapter::appendAssemblySize()

std::pair<std::shared_ptr<AbstractAssembly>, AbstractAssembly::SubID> EthAssemblyAdapter::createSubAssembly(bool _creation, std::string _name)
{
std::shared_ptr<evmasm::Assembly> assembly{std::make_shared<evmasm::Assembly>(m_assembly.evmVersion(), _creation, std::move(_name))};
std::shared_ptr<evmasm::Assembly> assembly{std::make_shared<evmasm::Assembly>(m_assembly.evmVersion(), _creation, m_assembly.eofVersion(), std::move(_name))};
auto sub = m_assembly.newSub(assembly);
return {std::make_shared<EthAssemblyAdapter>(*assembly), static_cast<size_t>(sub.data())};
}
Expand Down
21 changes: 11 additions & 10 deletions test/libevmasm/Assembler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <libevmasm/Disassemble.h>
#include <libyul/Exceptions.h>

#include <test/Common.h>
#include <boost/test/unit_test.hpp>

#include <algorithm>
Expand Down Expand Up @@ -61,15 +62,15 @@ BOOST_AUTO_TEST_CASE(all_assembly_items)
{ "verbatim.asm", 2 }
};
EVMVersion evmVersion = solidity::test::CommonOptions::get().evmVersion();
Assembly _assembly{evmVersion, false, {}};
Assembly _assembly{evmVersion, false, solidity::test::CommonOptions::get().eofVersion(), {}};
auto root_asm = std::make_shared<std::string>("root.asm");
_assembly.setSourceLocation({1, 3, root_asm});

Assembly _subAsm{evmVersion, false, {}};
Assembly _subAsm{evmVersion, false, solidity::test::CommonOptions::get().eofVersion(), {}};
auto sub_asm = std::make_shared<std::string>("sub.asm");
_subAsm.setSourceLocation({6, 8, sub_asm});

Assembly _verbatimAsm(evmVersion, true, "");
Assembly _verbatimAsm(evmVersion, true, solidity::test::CommonOptions::get().eofVersion(), "");
auto verbatim_asm = std::make_shared<std::string>("verbatim.asm");
_verbatimAsm.setSourceLocation({8, 18, verbatim_asm});

Expand Down Expand Up @@ -246,7 +247,7 @@ BOOST_AUTO_TEST_CASE(immutables_and_its_source_maps)
{ *subName, 1 }
};

auto subAsm = std::make_shared<Assembly>(evmVersion, false, std::string{});
auto subAsm = std::make_shared<Assembly>(evmVersion, false, solidity::test::CommonOptions::get().eofVersion(), std::string{});
for (char i = 0; i < numImmutables; ++i)
{
for (int r = 0; r < numActualRefs; ++r)
Expand All @@ -256,7 +257,7 @@ BOOST_AUTO_TEST_CASE(immutables_and_its_source_maps)
}
}

Assembly assembly{evmVersion, true, {}};
Assembly assembly{evmVersion, true, solidity::test::CommonOptions::get().eofVersion(), {}};
for (char i = 1; i <= numImmutables; ++i)
{
assembly.setSourceLocation({10*i, 10*i + 3+i, assemblyName});
Expand Down Expand Up @@ -306,11 +307,11 @@ BOOST_AUTO_TEST_CASE(immutable)
{ "sub.asm", 1 }
};
EVMVersion evmVersion = solidity::test::CommonOptions::get().evmVersion();
Assembly _assembly{evmVersion, true, {}};
Assembly _assembly{evmVersion, true, solidity::test::CommonOptions::get().eofVersion(), {}};
auto root_asm = std::make_shared<std::string>("root.asm");
_assembly.setSourceLocation({1, 3, root_asm});

Assembly _subAsm{evmVersion, false, {}};
Assembly _subAsm{evmVersion, false, solidity::test::CommonOptions::get().eofVersion(), {}};
auto sub_asm = std::make_shared<std::string>("sub.asm");
_subAsm.setSourceLocation({6, 8, sub_asm});
_subAsm.appendImmutable("someImmutable");
Expand Down Expand Up @@ -404,10 +405,10 @@ BOOST_AUTO_TEST_CASE(immutable)
BOOST_AUTO_TEST_CASE(subobject_encode_decode)
{
EVMVersion evmVersion = solidity::test::CommonOptions::get().evmVersion();
Assembly assembly{evmVersion, true, {}};
Assembly assembly{evmVersion, true, solidity::test::CommonOptions::get().eofVersion(), {}};

std::shared_ptr<Assembly> subAsmPtr = std::make_shared<Assembly>(evmVersion, false, std::string{});
std::shared_ptr<Assembly> subSubAsmPtr = std::make_shared<Assembly>(evmVersion, false, std::string{});
std::shared_ptr<Assembly> subAsmPtr = std::make_shared<Assembly>(evmVersion, false, solidity::test::CommonOptions::get().eofVersion(), std::string{});
std::shared_ptr<Assembly> subSubAsmPtr = std::make_shared<Assembly>(evmVersion, false, solidity::test::CommonOptions::get().eofVersion(), std::string{});

assembly.appendSubroutine(subAsmPtr);
subAsmPtr->appendSubroutine(subSubAsmPtr);
Expand Down
4 changes: 2 additions & 2 deletions test/libevmasm/Optimiser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1346,8 +1346,8 @@ BOOST_AUTO_TEST_CASE(jumpdest_removal_subassemblies)
settings.evmVersion = solidity::test::CommonOptions::get().evmVersion();
settings.expectedExecutionsPerDeployment = OptimiserSettings{}.expectedExecutionsPerDeployment;

Assembly main{settings.evmVersion, false, {}};
AssemblyPointer sub = std::make_shared<Assembly>(settings.evmVersion, true, std::string{});
Assembly main{settings.evmVersion, false, solidity::test::CommonOptions::get().eofVersion(), {}};
AssemblyPointer sub = std::make_shared<Assembly>(settings.evmVersion, true, solidity::test::CommonOptions::get().eofVersion(), std::string{});

sub->append(u256(1));
auto t1 = sub->newTag();
Expand Down
39 changes: 0 additions & 39 deletions test/libsolidity/Metadata.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,45 +225,6 @@ BOOST_AUTO_TEST_CASE(metadata_stamp_experimental)
}
}

BOOST_AUTO_TEST_CASE(metadata_eof_experimental)
{
// Check that setting an EOF version results in the experimental flag being set.
char const* sourceCode = R"(
pragma solidity >=0.0;
contract test {
function g(function(uint) external returns (uint) x) public {}
}
)";
for (auto metadataFormat: std::set<CompilerStack::MetadataFormat>{
CompilerStack::MetadataFormat::NoMetadata,
CompilerStack::MetadataFormat::WithReleaseVersionTag,
CompilerStack::MetadataFormat::WithPrereleaseVersionTag
})
{
CompilerStack compilerStack;
compilerStack.setMetadataFormat(metadataFormat);
compilerStack.setSources({{"", sourceCode}});
compilerStack.setEVMVersion({});
compilerStack.setViaIR(true);
compilerStack.setEOFVersion(1);
compilerStack.setOptimiserSettings(true);
BOOST_REQUIRE_MESSAGE(compilerStack.compile(), "Compiling contract failed");
bytes const& bytecode = compilerStack.runtimeObject("test").bytecode;
std::string const& metadata = compilerStack.metadata("test");
BOOST_CHECK(solidity::test::isValidMetadata(metadata));

auto const cborMetadata = requireParsedCBORMetadata(bytecode, metadataFormat);

if (metadataFormat == CompilerStack::MetadataFormat::NoMetadata)
BOOST_CHECK(cborMetadata.count("experimental") == 0);
else
{
BOOST_CHECK(cborMetadata.count("experimental") == 1);
BOOST_CHECK(cborMetadata.at("experimental") == "true");
}
}
}

BOOST_AUTO_TEST_CASE(metadata_relevant_sources)
{
CompilerStack compilerStack;
Expand Down
2 changes: 1 addition & 1 deletion test/libyul/EVMCodeTransformTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ TestCase::TestResult EVMCodeTransformTest::run(std::ostream& _stream, std::strin
return TestResult::FatalError;
}

evmasm::Assembly assembly{solidity::test::CommonOptions::get().evmVersion(), false, {}};
evmasm::Assembly assembly{solidity::test::CommonOptions::get().evmVersion(), false, std::nullopt, {}};
EthAssemblyAdapter adapter(assembly);
EVMObjectCompiler::compile(
*stack.parserResult(),
Expand Down
2 changes: 1 addition & 1 deletion test/tools/fuzzer_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ void FuzzerUtil::testConstantOptimizer(std::string const& _input, bool _quiet)

for (bool isCreation: {false, true})
{
Assembly assembly{langutil::EVMVersion{}, isCreation, {}};
Assembly assembly{langutil::EVMVersion{}, isCreation, std::nullopt, {}};
for (u256 const& n: numbers)
{
if (!_quiet)
Expand Down