Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Implement subroutines for yul functions. #8809

Closed
wants to merge 2 commits into from
Closed

Implement subroutines for yul functions. #8809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc04085
Implement subroutines for yul functions.
chriseth Apr 30, 2020
fbad843
Also test berlin.
chriseth May 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/soltest_all.sh
View file
Open in desktop
Expand Up @@ -31,7 +31,7 @@ REPODIR="$(realpath $(dirname $0)/..)"
EVM=istanbul OPTIMIZE=1 ABI_ENCODER_V2=1 ${REPODIR}/.circleci/soltest.sh

for OPTIMIZE in 0 1; do
for EVM in homestead byzantium constantinople petersburg istanbul; do
for EVM in homestead byzantium constantinople petersburg istanbul berlin; do
EVM=$EVM OPTIMIZE=$OPTIMIZE BOOST_TEST_ARGS="-t !@nooptions" ${REPODIR}/.circleci/soltest.sh
done
done
29 changes: 20 additions & 9 deletions libevmasm/Assembly.cpp
View file
Open in desktop
Expand Up @@ -110,7 +110,11 @@ class Functionalizer
))
{
flush();
m_out << m_prefix << (_item.type() == Tag ? "" : " ") << _item.toAssemblyText() << endl;
m_out <<
m_prefix <<
((_item.type() == Tag || _item.type() == Subtag) ? "" : " ") <<
_item.toAssemblyText() <<
endl;
return;
}
string expression = _item.toAssemblyText();
Expand Down Expand Up @@ -173,7 +177,7 @@ void Assembly::assemblyStream(ostream& _out, string const& _prefix, StringMap co

if (!m_data.empty() || !m_subs.empty())
{
_out << _prefix << "stop" << endl;
_out << _prefix << "beginsub" << endl;
Copy link
Member

Choose a reason for hiding this comment

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

This has to be versioned (evmVersion >= berlin ? beginsub : stop)

for (auto const& i: m_data)
if (u256(i.first) >= m_subs.size())
_out << _prefix << "data_" << toHex(u256(i.first)) << " " << toHex(i.second) << endl;
Expand Down Expand Up @@ -302,10 +306,13 @@ Json::Value Assembly::assemblyJSON(map<string, unsigned> const& _sourceIndices)
));
break;
case Tag:
case Subtag:
collection.append(
createJsonValue("tag", sourceIndex, i.location().start, i.location().end, toString(i.data())));
createJsonValue("tag", sourceIndex, i.location().start, i.location().end, toString(i.data()))
);
collection.append(
createJsonValue("JUMPDEST", sourceIndex, i.location().start, i.location().end));
createJsonValue(i.type() == Tag ? "JUMPDEST" : "BEGINSUB", sourceIndex, i.location().start, i.location().end)
);
break;
case PushData:
collection.append(createJsonValue("PUSH data", sourceIndex, i.location().start, i.location().end, toStringInHex(i.data())));
Expand Down Expand Up @@ -682,12 +689,16 @@ LinkerObject const& Assembly::assemble() const
ret.bytecode.resize(ret.bytecode.size() + 20);
break;
case Tag:
case Subtag:
assertThrow(i.data() != 0, AssemblyException, "Invalid tag position.");
assertThrow(i.splitForeignPushTag().first == numeric_limits<size_t>::max(), AssemblyException, "Foreign tag.");
assertThrow(ret.bytecode.size() < 0xffffffffL, AssemblyException, "Tag too large.");
assertThrow(m_tagPositionsInBytecode[static_cast<size_t>(i.data())] == numeric_limits<size_t>::max(), AssemblyException, "Duplicate tag position.");
m_tagPositionsInBytecode[static_cast<size_t>(i.data())] = ret.bytecode.size();
ret.bytecode.push_back((uint8_t)Instruction::JUMPDEST);
if (i.type() == Tag)
ret.bytecode.push_back((uint8_t)Instruction::JUMPDEST);
else
ret.bytecode.push_back((uint8_t)Instruction::BEGINSUB);
break;
default:
assertThrow(false, InvalidOpcode, "Unexpected opcode while assembling.");
Expand All @@ -702,8 +713,8 @@ LinkerObject const& Assembly::assemble() const


if (!m_subs.empty() || !m_data.empty() || !m_auxiliaryData.empty())
// Append an INVALID here to help tests find miscompilation.
ret.bytecode.push_back(uint8_t(Instruction::INVALID));
// Append a BEGINSUB here to help tests find miscompilation.
ret.bytecode.push_back(uint8_t(Instruction::BEGINSUB));
Copy link
Member

@axic axic May 8, 2020 •
edited

Choose a reason for hiding this comment

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

This should be versioned (evmVersion >= berlin ? beginsub : invalid)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought so, too, but BEGINSUB is an invalid opcode prior to berlin...

Copy link
Member

Choose a reason for hiding this comment

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

That is true, but one of the outstanding question is whether any deployed contract contains a BEGINSUB opcode between a JUMP/JUMPDEST which would render that JUMP broken.

Iff this PR were to be merged and people would deploy contracts with this change, that would increase the number of contracts to be analyzed.

In short, we'd be nicer citizens keeping INVALID there prior to Berlin :)


for (size_t i = 0; i < m_subs.size(); ++i)
{
Expand All @@ -729,8 +740,8 @@ LinkerObject const& Assembly::assemble() const
m_subs[subId]->m_tagPositionsInBytecode;
assertThrow(tagId < tagPositions.size(), AssemblyException, "Reference to non-existing tag.");
size_t pos = tagPositions[tagId];
assertThrow(pos != numeric_limits<size_t>::max(), AssemblyException, "Reference to tag without position.");
assertThrow(util::bytesRequired(pos) <= bytesPerTag, AssemblyException, "Tag too large for reserved space.");
//assertThrow(pos != numeric_limits<size_t>::max(), AssemblyException, "Reference to tag without position.");
//assertThrow(util::bytesRequired(pos) <= bytesPerTag, AssemblyException, "Tag too large for reserved space.");
bytesRef r(ret.bytecode.data() + i.first, bytesPerTag);
toBigEndian(pos, r);
}
Expand Down
18 changes: 14 additions & 4 deletions libevmasm/AssemblyItem.cpp
View file
Open in desktop
Expand Up @@ -33,7 +33,7 @@ static_assert(sizeof(size_t) <= 8, "size_t must be at most 64-bits wide");
AssemblyItem AssemblyItem::toSubAssemblyTag(size_t _subId) const
{
assertThrow(data() < (u256(1) << 64), util::Exception, "Tag already has subassembly set.");
assertThrow(m_type == PushTag || m_type == Tag, util::Exception, "");
assertThrow(m_type == PushTag || m_type == Tag || m_type == Subtag, util::Exception, "");
auto tag = static_cast<size_t>(u256(data()) & 0xffffffffffffffffULL);
AssemblyItem r = *this;
r.m_type = PushTag;
Expand All @@ -43,7 +43,7 @@ AssemblyItem AssemblyItem::toSubAssemblyTag(size_t _subId) const

pair<size_t, size_t> AssemblyItem::splitForeignPushTag() const
{
assertThrow(m_type == PushTag || m_type == Tag, util::Exception, "");
assertThrow(m_type == PushTag || m_type == Tag || m_type == Subtag, util::Exception, "");
u256 combined = u256(data());
size_t subId = static_cast<size_t>((combined >> 64) - 1);
size_t tag = static_cast<size_t>(combined & 0xffffffffffffffffULL);
Expand All @@ -52,7 +52,7 @@ pair<size_t, size_t> AssemblyItem::splitForeignPushTag() const

void AssemblyItem::setPushTagSubIdAndTag(size_t _subId, size_t _tag)
{
assertThrow(m_type == PushTag || m_type == Tag, util::Exception, "");
assertThrow(m_type == PushTag || m_type == Tag || m_type == Subtag, util::Exception, "");
u256 data = _tag;
if (_subId != numeric_limits<size_t>::max())
data |= (u256(_subId) + 1) << 64;
Expand All @@ -64,7 +64,8 @@ size_t AssemblyItem::bytesRequired(size_t _addressLength) const
switch (m_type)
{
case Operation:
case Tag: // 1 byte for the JUMPDEST
case Subtag:
case Tag: // 1 byte for the JUMPDEST / BEGINSUB
return 1;
case PushString:
return 1 + 32;
Expand Down Expand Up @@ -121,6 +122,7 @@ size_t AssemblyItem::returnValues() const
case PushDeployTimeAddress:
return 1;
case Tag:
case Subtag:
return 0;
default:
break;
Expand Down Expand Up @@ -148,6 +150,7 @@ bool AssemblyItem::canBeFunctional() const
case PushImmutable:
return true;
case Tag:
case Subtag:
return false;
default:
break;
Expand Down Expand Up @@ -203,6 +206,10 @@ string AssemblyItem::toAssemblyText() const
assertThrow(data() < 0x10000, AssemblyException, "Declaration of sub-assembly tag.");
text = string("tag_") + to_string(static_cast<size_t>(data())) + ":";
break;
case Subtag:
assertThrow(data() < 0x10000, AssemblyException, "Declaration of sub-assembly tag.");
text = string("beginsubtag_") + to_string(size_t(data())) + ":";
break;
case PushData:
text = string("data_") + util::toHex(data());
break;
Expand Down Expand Up @@ -271,6 +278,9 @@ ostream& solidity::evmasm::operator<<(ostream& _out, AssemblyItem const& _item)
case Tag:
_out << " Tag " << _item.data();
break;
case Subtag:
_out << " Beginsub " << _item.data();
break;
case PushData:
_out << " PushData " << hex << static_cast<unsigned>(_item.data()) << dec;
break;
Expand Down
1 change: 1 addition & 0 deletions libevmasm/AssemblyItem.h
View file
Open in desktop
Expand Up @@ -42,6 +42,7 @@ enum AssemblyItemType {
PushSubSize,
PushProgramSize,
Tag,
Subtag,
PushData,
PushLibraryAddress, ///< Push a currently unknown address of another (library) contract.
PushDeployTimeAddress, ///< Push an address to be filled at deploy time. Should not be touched by the optimizer.
Expand Down
1 change: 1 addition & 0 deletions libevmasm/BlockDeduplicator.cpp
View file
Open in desktop
Expand Up @@ -104,6 +104,7 @@ bool BlockDeduplicator::applyTagReplacement(
size_t _subId
)
{
// TODO this might be problematic for jumpsub
bool changed = false;
for (AssemblyItem& item: _items)
if (item.type() == PushTag)
Expand Down
1 change: 1 addition & 0 deletions libevmasm/GasMeter.cpp
View file
Open in desktop
Expand Up @@ -55,6 +55,7 @@ GasMeter::GasConsumption GasMeter::estimateMax(AssemblyItem const& _item, bool _
gas = runGas(Instruction::PUSH1);
break;
case Tag:
// TODO Subtag
gas = runGas(Instruction::JUMPDEST);
break;
case Operation:
Expand Down
6 changes: 6 additions & 0 deletions libevmasm/Instruction.cpp
View file
Open in desktop
Expand Up @@ -165,6 +165,9 @@ std::map<std::string, Instruction> const solidity::evmasm::c_instructions =
{ "LOG2", Instruction::LOG2 },
{ "LOG3", Instruction::LOG3 },
{ "LOG4", Instruction::LOG4 },
{ "JUMPSUB", Instruction::JUMPSUB },
{ "BEGINSUB", Instruction::BEGINSUB },
{ "RETURNSUB", Instruction::RETURNSUB },
{ "CREATE", Instruction::CREATE },
{ "CALL", Instruction::CALL },
{ "CALLCODE", Instruction::CALLCODE },
Expand Down Expand Up @@ -311,6 +314,9 @@ static std::map<Instruction, InstructionInfo> const c_instructionInfo =
{ Instruction::LOG2, { "LOG2", 0, 4, 0, true, Tier::Special } },
{ Instruction::LOG3, { "LOG3", 0, 5, 0, true, Tier::Special } },
{ Instruction::LOG4, { "LOG4", 0, 6, 0, true, Tier::Special } },
{ Instruction::JUMPSUB, { "JUMPSUB", 0, 1, 0, true, Tier::Special } },
{ Instruction::BEGINSUB, { "BEGINSUB", 0, 0, 0, true, Tier::Special } },
{ Instruction::RETURNSUB, { "RETURNSUB", 0, 0, 0, true, Tier::Special } },
{ Instruction::CREATE, { "CREATE", 0, 3, 1, true, Tier::Special } },
{ Instruction::CALL, { "CALL", 0, 7, 1, true, Tier::Special } },
{ Instruction::CALLCODE, { "CALLCODE", 0, 7, 1, true, Tier::Special } },
Expand Down
4 changes: 4 additions & 0 deletions libevmasm/Instruction.h
View file
Open in desktop
Expand Up @@ -185,6 +185,10 @@ enum class Instruction: uint8_t
EIP615_PUTLOCAL, ///< pop top of stack to local variable -- not part of Instructions.cpp
EIP615_GETLOCAL, ///< push local variable to top of stack -- not part of Instructions.cpp

BEGINSUB = 0x5c, ///< set a potential jumpsub destination
RETURNSUB = 0x5d, ///< return to subroutine jumped from
JUMPSUB = 0x5e, ///< alter the program counter to a beginsub

CREATE = 0xf0, ///< create a new account with associated code
CALL, ///< message-call into an account
CALLCODE, ///< message-call with another account's code only
Expand Down
2 changes: 1 addition & 1 deletion libevmasm/JumpdestRemover.cpp
View file
Open in desktop
Expand Up @@ -40,7 +40,7 @@ bool JumpdestRemover::optimise(set<size_t> const& _tagsReferencedFromOutside)
m_items.end(),
[&](AssemblyItem const& _item)
{
if (_item.type() != Tag)
if (_item.type() != Tag && _item.type() != Subtag)
return false;
auto asmIdAndTag = _item.splitForeignPushTag();
assertThrow(asmIdAndTag.first == numeric_limits<size_t>::max(), OptimizerException, "Sub-assembly tag used as label.");
Expand Down
2 changes: 1 addition & 1 deletion libevmasm/KnownState.cpp
View file
Open in desktop
Expand Up @@ -87,7 +87,7 @@ ostream& KnownState::stream(ostream& _out) const
KnownState::StoreOperation KnownState::feedItem(AssemblyItem const& _item, bool _copyItem)
{
StoreOperation op;
if (_item.type() == Tag)
if (_item.type() == Tag || _item.type() == Subtag)
{
// can be ignored
}
Expand Down
10 changes: 9 additions & 1 deletion libevmasm/SemanticInformation.cpp
View file
Open in desktop
Expand Up @@ -35,6 +35,7 @@ bool SemanticInformation::breaksCSEAnalysisBlock(AssemblyItem const& _item, bool
default:
case UndefinedItem:
case Tag:
case Subtag:
case PushDeployTimeAddress:
case AssignImmutable:
return true;
Expand Down Expand Up @@ -110,7 +111,12 @@ bool SemanticInformation::isSwapInstruction(AssemblyItem const& _item)

bool SemanticInformation::isJumpInstruction(AssemblyItem const& _item)
{
return _item == Instruction::JUMP || _item == Instruction::JUMPI;
// TODO check the usages of this function
return
_item == Instruction::JUMP ||
_item == Instruction::JUMPI ||
_item == Instruction::JUMPSUB ||
_item == Instruction::RETURNSUB;
}

bool SemanticInformation::altersControlFlow(AssemblyItem const& _item)
Expand All @@ -124,6 +130,8 @@ bool SemanticInformation::altersControlFlow(AssemblyItem const& _item)
case Instruction::JUMP:
case Instruction::JUMPI:
case Instruction::RETURN:
case Instruction::JUMPSUB:
case Instruction::RETURNSUB:
case Instruction::SELFDESTRUCT:
case Instruction::STOP:
case Instruction::INVALID:
Expand Down
4 changes: 4 additions & 0 deletions liblangutil/EVMVersion.cpp
View file
Open in desktop
Expand Up @@ -45,6 +45,10 @@ bool EVMVersion::hasOpcode(Instruction _opcode) const
return hasChainID();
case Instruction::SELFBALANCE:
return hasSelfBalance();
case Instruction::JUMPSUB:
case Instruction::BEGINSUB:
case Instruction::RETURNSUB:
return supportsSubroutines();
default:
return true;
}
Expand Down
1 change: 1 addition & 0 deletions liblangutil/EVMVersion.h
View file
Open in desktop
Expand Up @@ -86,6 +86,7 @@ class EVMVersion:
bool hasExtCodeHash() const { return *this >= constantinople(); }
bool hasChainID() const { return *this >= istanbul(); }
bool hasSelfBalance() const { return *this >= istanbul(); }
bool supportsSubroutines() const { return *this >= berlin(); }

bool hasOpcode(evmasm::Instruction _opcode) const;

Expand Down
1 change: 1 addition & 0 deletions libsolidity/codegen/CompilerContext.cpp
View file
Open in desktop
Expand Up @@ -132,6 +132,7 @@ void CompilerContext::callLowLevelFunction(

*this << lowLevelFunctionTag(_name, _inArgs, _outArgs, _generator);

// TODO could use jumpsub?
appendJump(evmasm::AssemblyItem::JumpType::IntoFunction);
adjustStackOffset(static_cast<int>(_outArgs) - 1 - static_cast<int>(_inArgs));
*this << retTag.tag();
Expand Down
1 change: 1 addition & 0 deletions libsolidity/codegen/ContractCompiler.cpp
View file
Open in desktop
Expand Up @@ -505,6 +505,7 @@ void ContractCompiler::appendFunctionSelector(ContractDefinition const& _contrac
m_context << Instruction::DUP1 << Instruction::CALLDATASIZE << Instruction::SUB;
CompilerUtils(m_context).abiDecode(functionType->parameterTypes());
}
// TODO could use JUMPSUB?
m_context.appendJumpTo(
m_context.functionEntryLabel(functionType->declaration()),
evmasm::AssemblyItem::JumpType::IntoFunction
Expand Down
1 change: 1 addition & 0 deletions libsolidity/codegen/ExpressionCompiler.cpp
View file
Open in desktop
Expand Up @@ -614,6 +614,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
// Extract the runtime part.
m_context << ((u256(1) << 32) - 1) << Instruction::AND;

// TODO could use jumpsub
m_context.appendJump(evmasm::AssemblyItem::JumpType::IntoFunction);
m_context << returnLabel;

Expand Down
30 changes: 15 additions & 15 deletions libsolidity/interface/CompilerStack.cpp
View file
Open in desktop
Expand Up @@ -1060,25 +1060,25 @@ void CompilerStack::compileContract(
solAssert(false, "Optimizer exception during compilation");
}

try
{
// try
// {
// Assemble deployment (incl. runtime) object.
compiledContract.object = compiler->assembledObject();
}
catch(evmasm::AssemblyException const&)
{
solAssert(false, "Assembly exception for bytecode");
}

try
{
// }
// catch(evmasm::AssemblyException const&)
// {
// solAssert(false, "Assembly exception for bytecode");
// }

// try
// {
// Assemble runtime object.
compiledContract.runtimeObject = compiler->runtimeObject();
}
catch(evmasm::AssemblyException const&)
{
solAssert(false, "Assembly exception for deployed bytecode");
}
// }
// catch(evmasm::AssemblyException const&)
// {
// solAssert(false, "Assembly exception for deployed bytecode");
// }

// Throw a warning if EIP-170 limits are exceeded:
// If contract creation initialization returns data with length of more than 0x6000 (214 + 213) bytes,
Expand Down
5 changes: 4 additions & 1 deletion libyul/AsmAnalysis.cpp
View file
Open in desktop
Expand Up @@ -573,7 +573,10 @@ bool AsmAnalyzer::validateInstructions(evmasm::Instruction _instr, SourceLocatio
yulAssert(
_instr != evmasm::Instruction::JUMP &&
_instr != evmasm::Instruction::JUMPI &&
_instr != evmasm::Instruction::JUMPDEST,
_instr != evmasm::Instruction::JUMPDEST &&
_instr != evmasm::Instruction::JUMPSUB &&
_instr != evmasm::Instruction::RETURNSUB &&
_instr != evmasm::Instruction::BEGINSUB
"");

auto errorForVM = [&](ErrorId _errorId, string const& vmKindMessage) {
Expand Down
3 changes: 3 additions & 0 deletions libyul/AsmParser.cpp
View file
Open in desktop
Expand Up @@ -70,6 +70,9 @@ std::map<string, evmasm::Instruction> const& Parser::instructions()
{
if (
instruction.second == evmasm::Instruction::JUMPDEST ||
instruction.second == evmasm::Instruction::BEGINSUB ||
instruction.second == evmasm::Instruction::JUMPSUB ||
instruction.second == evmasm::Instruction::RETURNSUB ||
evmasm::isPushInstruction(instruction.second)
)
continue;
Expand Down