Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Add support for PUSH0 instruction to debugger & disassembler #5898

Merged
merged 3 commits into from
Feb 16, 2023
+13 −12
Merged

Add support for PUSH0 instruction to debugger & disassembler #5898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aeb7a05
Add PUSH0 opcode
haltman-at Feb 15, 2023
a84012e
Display just "PUSH0", not "PUSH0 0x"
haltman-at Feb 15, 2023
740700b
Rewrite some calculations in terms of PUSH0
haltman-at Feb 15, 2023
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
2 changes: 1 addition & 1 deletion packages/code-utils/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function parseCode(
name: parseOpcode(code[pc])
};
if (opcode.name.slice(0, 4) === "PUSH") {
const length = code[pc] - 0x60 + 1; //0x60 is code for PUSH1
const length = code[pc] - 0x5f; //0x5f is code for PUSH0
let pushData = code.subarray(pc + 1, pc + length + 1);
if (pushData.length < length) {
//if we run out of bytes for our pushdata, fill the rest
Expand Down
1 change: 1 addition & 0 deletions packages/code-utils/src/opcodes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ const codes = {
0x5b: "JUMPDEST",

// 0x60 & 0x70 range - pushes
0x5f: "PUSH0",
Copy link
Member

Choose a reason for hiding this comment

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

The comment above is confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, didn't really know what to change it to though... (thoughts?)

Copy link
Member

Choose a reason for hiding this comment

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

Beginning of push op codes maybe?

0x60: "PUSH1",
0x61: "PUSH2",
0x62: "PUSH3",
Expand Down
4 changes: 1 addition & 3 deletions packages/codec/lib/abi-data/allocate/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,9 +1014,7 @@ function constructorOutputAllocation(
if (contractKind === "library") {
//note: I am relying on this being present!
//(also this part is a bit HACKy)
const pushAddressInstruction = (0x60 + Evm.Utils.ADDRESS_SIZE - 1).toString(
16
); //"73"
const pushAddressInstruction = (0x5f + Evm.Utils.ADDRESS_SIZE).toString(16); //"73"
Copy link
Member

Choose a reason for hiding this comment

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

Note, this calculation is used in three places across two modules, adding error-prone maintenance complexity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's not really any maintenance complexity here? Like there's nothing here that needs to be updated. I just changed the base of the calculation from PUSH1 to PUSH0 now that the latter exists... what would change it further in the future?

Note of course I could have just used a literal "73" but I figured that doing the calculation made the intent clearer.

Copy link
Member

@cds-amal cds-amal Feb 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I meant that if a new opcode is updated to be base, future maintainers would have three places to update. However, what can come before PUSH0, as you pointed out? :)

const delegateCallGuardString =
"0x" + pushAddressInstruction + "..".repeat(Evm.Utils.ADDRESS_SIZE);
if (binary.startsWith(delegateCallGuardString)) {
Expand Down
8 changes: 2 additions & 6 deletions packages/codec/lib/contexts/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,7 @@ export function normalizeContexts(contexts: Contexts): Contexts {
//now we must handle the delegatecall guard -- libraries' deployedBytecode will include
//0s in place of their own address instead of a link reference at the
//beginning, so we need to account for that too
const pushAddressInstruction = (0x60 + Evm.Utils.ADDRESS_SIZE - 1).toString(
16
); //"73"
const pushAddressInstruction = (0x5f + Evm.Utils.ADDRESS_SIZE).toString(16); //"73"
for (let context of Object.values(newContexts)) {
if (context.contractKind === "library" && !context.isConstructor) {
context.binary = context.binary.replace(
Expand Down Expand Up @@ -401,9 +399,7 @@ function contractKind(
const deployedBytecode = Shims.NewToLegacy.forBytecode(
contract.deployedBytecode
);
const pushAddressInstruction = (0x60 + Evm.Utils.ADDRESS_SIZE - 1).toString(
16
); //"73"
const pushAddressInstruction = (0x5f + Evm.Utils.ADDRESS_SIZE).toString(16); //"73"
const libraryString =
"0x" + pushAddressInstruction + "00".repeat(Evm.Utils.ADDRESS_SIZE);
return deployedBytecode.startsWith(libraryString) ? "library" : "contract";
Expand Down
4 changes: 3 additions & 1 deletion packages/core/lib/commands/opcode/run.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ module.exports = async function (options) {
console.log(
Conversion.toHexString(opcode.pc, lastPCByteLength) + ":",
opcode.name,
opcode.pushData || ""
opcode.pushData !== undefined && opcode.pushData !== "0x"
? opcode.pushData
: "" //display just "PUSH0", not "PUSH0 0x"
);
});
};
6 changes: 5 additions & 1 deletion packages/debug-utils/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,11 @@ var DebugUtils = {

formatInstruction: function (instruction) {
return truffleColors.mint(
instruction.name + " " + (instruction.pushData || "")
instruction.name +
" " +
(instruction.pushData !== undefined && instruction.pushData !== "0x"
? instruction.pushData
: "") //display just "PUSH0", not "PUSH0 0x"
);
},

Expand Down