Skip to content

Commit

Permalink
validation fixes
Browse files Browse the repository at this point in the history 
Pass all reference tests, update unit tests to match
* code section zero must be non-returning
* returning code sections must contain RETF or JUMPF into returning
* no RETF in non-returning sections

Signed-off-by: Danno Ferrin <[email protected]>
  • Loading branch information
@shemnon
shemnon committed Jan 31, 2024
1 parent c546bf7 commit d850b62
Show file tree
Hide file tree
Showing 10 changed files with 250 additions and 162 deletions.
11 changes: 7 additions & 4 deletions ethereum/evmtool/src/main/java/org/hyperledger/besu/evmtool/EOFTestSubCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void run() {
final File file = new File(fileName);
if (file.isFile()) {
final Map<String, EOFTestCaseSpec> eofTests = eofTestMapper.readValue(file, javaType);
executeEOFTest(eofTests);
executeEOFTest(file.toString(), eofTests);
} else {
parentCommand.out.println("File not found: " + fileName);
}
Expand All @@ -112,7 +112,7 @@ public void run() {
} else {
eofTests = eofTestMapper.readValue(eofTestFile.toFile(), javaType);
}
executeEOFTest(eofTests);
executeEOFTest(eofTestFile.toString(), eofTests);
}
}
} catch (final JsonProcessingException jpe) {
Expand All @@ -124,14 +124,15 @@ public void run() {
}

record TestExecutionResult(
String fileName,
String group,
String name,
String fork,
boolean passed,
boolean pass,
String expectedError,
String actualError) {}

private void executeEOFTest(final Map<String, EOFTestCaseSpec> eofTests) {
private void executeEOFTest(final String fileName, final Map<String, EOFTestCaseSpec> eofTests) {
List<TestExecutionResult> results = new ArrayList<>();

for (var testGroup : eofTests.entrySet()) {
Expand All @@ -152,6 +153,7 @@ private void executeEOFTest(final Map<String, EOFTestCaseSpec> eofTests) {
if (evmVersion == null) {
results.add(
new TestExecutionResult(
fileName,
groupName,
testName,
expectedForkName,
Expand All @@ -169,6 +171,7 @@ private void executeEOFTest(final Map<String, EOFTestCaseSpec> eofTests) {
}
results.add(
new TestExecutionResult(
fileName,
groupName,
testName,
expectedForkName,
Expand Down
2 changes: 1 addition & 1 deletion ethereum/referencetests/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ tasks.register('validateReferenceTestSubmodule') {
description = "Checks that the reference tests submodule is not accidentally changed"
doLast {
def result = new ByteArrayOutputStream()
def expectedHash = '8ea5bc6dcc99d0c391a51b82330b17dbfe381e5c'
def expectedHash = '36ccce5f94dbebe476ebd12d81501d9ac11b1049'
def submodulePath = java.nio.file.Path.of("${rootProject.projectDir}", "ethereum/referencetests/src/reference-test/external-resources").toAbsolutePath()
try {
exec {
Expand Down
2 changes: 1 addition & 1 deletion evm/src/main/java/org/hyperledger/besu/evm/code/CodeInvalid.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,6 @@ public int getCodeSectionCount() {

@Override
public int getEofVersion() {
return -1;
return Integer.MAX_VALUE;
}
}
19 changes: 18 additions & 1 deletion evm/src/main/java/org/hyperledger/besu/evm/code/CodeSection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ public final class CodeSection {
final int maxStackHeight;
/** The byte offset from the beginning of the container that the section starts at */
final int entryPoint;
/** Is this a returing code section (i.e. contains RETF or JUMPF into a returning section)? */
final boolean returning;

/**
* Instantiates a new Code section.
Expand All @@ -51,7 +53,13 @@ public CodeSection(
final int entryPoint) {
this.length = length;
this.inputs = inputs;
this.outputs = outputs;
if (outputs == 0x80) {
this.outputs = 0;
returning = false;
} else {
this.outputs = outputs;
returning = true;
}
this.maxStackHeight = maxStackHeight;
this.entryPoint = entryPoint;
}
Expand Down Expand Up @@ -83,6 +91,15 @@ public int getOutputs() {
return outputs;
}

/**
* Does this code seciton have a RETF return anywhere?
*
* @return returning
*/
public boolean isReturning() {
return returning;
}

/**
* Gets max stack height.
*
Expand Down
70 changes: 50 additions & 20 deletions evm/src/main/java/org/hyperledger/besu/evm/code/CodeV1Validation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.hyperledger.besu.evm.internal.Words.readBigEndianU16;

import org.hyperledger.besu.evm.operation.CallFOperation;
import org.hyperledger.besu.evm.operation.JumpFOperation;
import org.hyperledger.besu.evm.operation.PushOperation;
import org.hyperledger.besu.evm.operation.RelativeJumpIfOperation;
import org.hyperledger.besu.evm.operation.RelativeJumpOperation;
Expand Down Expand Up @@ -290,10 +291,10 @@ private CodeV1Validation() {
OpcodeInfo.unallocatedOpcode(0xcd),
OpcodeInfo.unallocatedOpcode(0xce),
OpcodeInfo.unallocatedOpcode(0xcf),
OpcodeInfo.unallocatedOpcode(0xd0),
OpcodeInfo.unallocatedOpcode(0xd1),
OpcodeInfo.unallocatedOpcode(0xd2),
OpcodeInfo.unallocatedOpcode(0xd3),
OpcodeInfo.validOpcode("DATALOAD", 0xd0, 1, 1, 1),
OpcodeInfo.validOpcode("DATALOADN", 0xd1, 0, 1, 3),
OpcodeInfo.validOpcode("DATASIZE", 0xd2, 0, 1, 1),
OpcodeInfo.validOpcode("DATACOPY", 0xd3, 3, 0, 1),
OpcodeInfo.unallocatedOpcode(0xd4),
OpcodeInfo.unallocatedOpcode(0xd5),
OpcodeInfo.unallocatedOpcode(0xd6),
Expand All @@ -311,13 +312,13 @@ private CodeV1Validation() {
OpcodeInfo.validOpcode("RJUMPV", 0xe2, 1, 0, 2),
OpcodeInfo.validOpcode("CALLF", 0xe3, 0, 0, 3),
OpcodeInfo.terminalOpcode("RETF", 0xe4, 0, 0, -1),
OpcodeInfo.terminalOpcode("JUMPF", 0xe5, 1, 0, -1),
OpcodeInfo.validOpcode("DUPN", 0xe6, 1, 1, 1),
OpcodeInfo.validOpcode("SWAPN", 0xe7, 1, 0, 1),
OpcodeInfo.validOpcode("DATALOAD", 0xe8, 1, 1, 1),
OpcodeInfo.validOpcode("DATALOADN", 0xe9, 0, 1, 1),
OpcodeInfo.validOpcode("DATACOPY", 0xea, 3, 0, 1),
OpcodeInfo.validOpcode("DATALOADN", 0xeb, 0, 1, 3),
OpcodeInfo.terminalOpcode("JUMPF", 0xe5, 0, 0, -3),
OpcodeInfo.validOpcode("DUPN", 0xe6, 0, 1, 2),
OpcodeInfo.validOpcode("SWAPN", 0xe7, 0, 0, 2),
OpcodeInfo.validOpcode("EXCHANGE", 0xe8, 0, 0, 2),
OpcodeInfo.unallocatedOpcode(0xe9),
OpcodeInfo.unallocatedOpcode(0xea),
OpcodeInfo.unallocatedOpcode(0xeb),
OpcodeInfo.unallocatedOpcode(0xec),
OpcodeInfo.unallocatedOpcode(0xed),
OpcodeInfo.unallocatedOpcode(0xee),
Expand Down Expand Up @@ -347,12 +348,15 @@ private CodeV1Validation() {
* @return validation code, null otherwise.
*/
public static String validateCode(final EOFLayout eofLayout) {
int sectionCount = eofLayout.getCodeSectionCount();
for (int i = 0; i < sectionCount; i++) {
CodeSection cs = eofLayout.getCodeSection(i);
if (!eofLayout.isValid()) {
return "Invalid EOF container - " + eofLayout.invalidReason();
}
for (CodeSection cs : eofLayout.codeSections()) {
var validation =
CodeV1Validation.validateCode(
eofLayout.container().slice(cs.getEntryPoint(), cs.getLength()), sectionCount);
eofLayout.container().slice(cs.getEntryPoint(), cs.getLength()),
cs.isReturning(),
eofLayout.codeSections());
if (validation != null) {
return validation;
}
Expand All @@ -366,13 +370,15 @@ public static String validateCode(final EOFLayout eofLayout) {
* @param code the code section code
* @return null if valid, otherwise a string containing an error reason.
*/
static String validateCode(final Bytes code, final int sectionCount) {
static String validateCode(
final Bytes code, final boolean returning, final CodeSection... codeSections) {
final int size = code.size();
final BitSet rjumpdests = new BitSet(size);
final BitSet immediates = new BitSet(size);
final byte[] rawCode = code.toArrayUnsafe();
OpcodeInfo opcodeInfo = OPCODE_INFO[0xfe];
int pos = 0;
boolean hasReturningOpcode = false;
while (pos < size) {
final int operationNum = rawCode[pos] & 0xff;
opcodeInfo = OPCODE_INFO[operationNum];
Expand Down Expand Up @@ -421,14 +427,31 @@ static String validateCode(final Bytes code, final int sectionCount) {
return "Truncated CALLF";
}
int section = readBigEndianU16(pos, rawCode);
if (section >= sectionCount) {
if (section >= codeSections.length) {
return "CALLF to non-existent section - " + Integer.toHexString(section);
}
pcPostInstruction += 2;
} else if (operationNum == JumpFOperation.OPCODE) {
if (pos + 2 > size) {
return "Truncated JUMPF";
}
int section = readBigEndianU16(pos, rawCode);
if (section >= codeSections.length) {
return "JUMPF to non-existent section - " + Integer.toHexString(section);
}
hasReturningOpcode |= codeSections[section].isReturning();
pcPostInstruction += 2;
} else if (operationNum == RetFOperation.OPCODE) {
hasReturningOpcode = true;
}
immediates.set(pos, pcPostInstruction);
pos = pcPostInstruction;
}
if (returning != hasReturningOpcode) {
return returning
? "No RETF or qualifying JUMPF"
: "Non-returing section has RETF or JUMPF into returning section";
}
if (!opcodeInfo.terminal) {
return "No terminating instruction";
}
Expand Down Expand Up @@ -459,6 +482,9 @@ static String validateStack(final EOFLayout eofLayout) {
* @return null if valid, otherwise an error string providing the validation error.
*/
public static String validateStack(final int codeSectionToValidate, final EOFLayout eofLayout) {
if (!eofLayout.isValid()) {
return "EOF Layout invalid - " + eofLayout.invalidReason();
}
try {
CodeSection toValidate = eofLayout.getCodeSection(codeSectionToValidate);
byte[] code =
Expand Down Expand Up @@ -500,14 +526,18 @@ public static String validateStack(final int codeSectionToValidate, final EOFLay
OpcodeInfo opcodeInfo = OPCODE_INFO[thisOp];
int stackInputs;
int stackOutputs;
int sectionStackUsed;
int pcAdvance = opcodeInfo.pcAdvance();
if (thisOp == CallFOperation.OPCODE) {
int section = readBigEndianU16(currentPC + 1, code);
stackInputs = eofLayout.getCodeSection(section).getInputs();
stackOutputs = eofLayout.getCodeSection(section).getOutputs();
CodeSection codeSection = eofLayout.getCodeSection(section);
stackInputs = codeSection.getInputs();
stackOutputs = codeSection.getOutputs();
sectionStackUsed = codeSection.getMaxStackHeight();
} else {
stackInputs = opcodeInfo.inputs();
stackOutputs = opcodeInfo.outputs();
sectionStackUsed = 0;
}

if (stackInputs > currentStackHeight) {
Expand All @@ -517,7 +547,7 @@ public static String validateStack(final int codeSectionToValidate, final EOFLay
}

currentStackHeight = currentStackHeight - stackInputs + stackOutputs;
if (currentStackHeight > MAX_STACK_HEIGHT) {
if (currentStackHeight + sectionStackUsed - stackOutputs > MAX_STACK_HEIGHT) {
return "Stack height exceeds 1024";
}

Expand Down
7 changes: 7 additions & 0 deletions evm/src/main/java/org/hyperledger/besu/evm/code/EOFLayout.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,13 @@ public static EOFLayout parseEOF(final Bytes container) {
}
codeSections[i] =
new CodeSection(codeSectionSize, typeData[i][0], typeData[i][1], typeData[i][2], pos);
if (i == 0 && typeData[0][1] != 0x80) {
return invalidLayout(
container,
version,
"Code section at zero expected non-returning flag, but had return stack of "
+ typeData[0][1]);
}
pos += codeSectionSize;
}

Expand Down
Loading

0 comments on commit d850b62

Please sign in to comment.