Support ethdebug source locations under EOF #15994

clonker · 2025-04-10T09:52:56Z

Adds an EthdebugSchema header with the relevant part of the schema mapped to structs with corresponding to_json methods and validations
Skips the context for instructions with source locations that are not valid (ie, (-1, -1))
Adds source location info in evmasm assembly for assembleEOF
Refactors legacy assemble to use RAII-style instruction location gathering

Fixes the unoptimized part of #15978.
Fixes #15998.

matheusaaguiar

lgtm

cameel

I want to take a closer look at this, especially the Assembly.cpp part, but for now just a few small annoyances I found while doing a quick initial pass.

cameel · 2025-04-22T21:07:21Z

test/cmdlineTests/standard_output_debuginfo_ethdebug_compatible_eof/output.json

The output here is quite long and seems to have little to do with ethdebug itself (the ethdebug JSON gets stripped). Do we need it all? What's the point?

I think I just added it because there is the same test for non-eof. And looking at it now, I agree. There doesn't seem to be much value to keep it (or: them) around, especially with #16009 around the corner. I have removed this one.

test/cmdlineTests/standard_output_debuginfo_ethdebug_compatible_eof/input.json

test/cmdlineTests/ethdebug_eof_container_osaka/output

libevmasm/Assembly.cpp

libevmasm/EthdebugSchema.h

aarlt · 2025-05-07T05:38:14Z

test/cmdlineTests/standard_output_debuginfo_ethdebug_with_interfaces_and_abstracts/output.json

+            "C": {
+                "evm": {
+                    "bytecode": {
+                        "ethdebug": null


ok, I thought those would get automatically removed.

it's even better if they don't. if you request ethdebug output there should be an ethdebug output artifact there that indicates that there is no ethdebug output :)

aarlt · 2025-05-07T05:39:17Z

ok.. at least on the cli I just saw that ethdebug returns null for the interface (e.g. using test/libsolidity/semanticTests/interfaceID/homer.sol) - I guess we want an empty json object there. I think it would be nice to add a small test for that simple interface case.

regarding the null stuff I changed my mind - lets fix this later if its really needed.

libevmasm/Assembly.cpp

Changelog.md

nikola-matic · 2025-05-14T13:58:10Z

libevmasm/Ethdebug.cpp

+	schema::program::Instruction::Operation operation;
+	operation.mnemonic = instructionInfo(static_cast<Instruction>(_linkerObject.bytecode[_start]), _assembly.evmVersion()).name;
+	static size_t constexpr instructionSize = 1;
+	if (_start + instructionSize < _end)


Might even make sense to replace this with an assert as this could end up with an empty operation, which is not desired?

there is an assert that start < end. which leaves start + 1== end as possibility, ie, an op with no argument data - which is desired and possible I'd think

libevmasm/Ethdebug.cpp

cameel

I only looked at the Assembly changes so far, but they look good. We could merge that if you extract it into a separate PR.

cameel · 2025-05-14T18:20:35Z

Changelog.md

@@ -4,7 +4,7 @@ Language Features:


 Compiler Features:
-
+* ethdebug: Experimental support for instructions and source locations under EOF.


Suggested change

* ethdebug: Experimental support for instructions and source locations under EOF.

* ethdebug: Experimental support for instructions and source locations under EOF.

Uhh. Happy to fix this - but why? It's not like there's a blank line between bullet point list and headline anywhere else.

Ah, my bad, should have been the other way around - an extra empty line below :)

but why?

Two empty lines between sections.

Maybe we should just change the convention to a single line? No one seems to be able to keep it straight or see it in the diff and it gets changed by PRs back and forth. I usually ignore it but I got annoyed enough seeing it again to suggest a correction :)

Haha yeah, that explains it. Might've also been my pre-coffee confusion that I didn't realize you meant the blank line below. I am absolutely fine with changing it to one line in any case! :)

libevmasm/Assembly.cpp

clonker · 2025-05-21T08:21:38Z

The assembler stuff has been merged with #16052, which leaves this PR in a state in which it solely adds ethdebug features

clonker force-pushed the eof_source_locations_unoptimized branch 2 times, most recently from 20d6a64 to fb559fb Compare April 11, 2025 09:12

clonker marked this pull request as ready for review April 11, 2025 09:38

clonker force-pushed the eof_source_locations_unoptimized branch 9 times, most recently from e6a1055 to 2a91773 Compare April 12, 2025 06:49

clonker requested review from cameel and aarlt April 12, 2025 09:12

clonker force-pushed the eof_source_locations_unoptimized branch 5 times, most recently from d82b70c to 0fd1232 Compare April 15, 2025 07:16

clonker mentioned this pull request Apr 17, 2025

Test ethdebug program output against corresponding schema #16009

Open

clonker force-pushed the eof_source_locations_unoptimized branch from 0fd1232 to cf6aa71 Compare April 17, 2025 08:18

clonker requested a review from matheusaaguiar April 17, 2025 09:11

clonker added the ethdebug label Apr 17, 2025

matheusaaguiar previously approved these changes Apr 22, 2025

View reviewed changes

cameel reviewed Apr 22, 2025

View reviewed changes

clonker dismissed matheusaaguiar’s stale review via 19a6b42 April 23, 2025 06:16

clonker force-pushed the eof_source_locations_unoptimized branch 3 times, most recently from 3c02df0 to f9a0985 Compare April 23, 2025 06:51

clonker requested a review from cameel April 28, 2025 15:44

clonker force-pushed the eof_source_locations_unoptimized branch from 42d5d8f to 7eecf23 Compare May 6, 2025 12:35

aarlt reviewed May 7, 2025

View reviewed changes

clonker force-pushed the eof_source_locations_unoptimized branch from 7eecf23 to e696889 Compare May 7, 2025 09:27

aarlt previously approved these changes May 9, 2025

View reviewed changes

clonker dismissed aarlt’s stale review via 667d2e3 May 9, 2025 10:37

clonker force-pushed the eof_source_locations_unoptimized branch from e696889 to 667d2e3 Compare May 9, 2025 10:37

aarlt previously approved these changes May 9, 2025

View reviewed changes

nikola-matic reviewed May 14, 2025

View reviewed changes

clonker dismissed aarlt’s stale review via bcf1c34 May 14, 2025 15:16

clonker force-pushed the eof_source_locations_unoptimized branch from 667d2e3 to bcf1c34 Compare May 14, 2025 15:16

cameel reviewed May 14, 2025

View reviewed changes

clonker force-pushed the eof_source_locations_unoptimized branch 3 times, most recently from af84571 to aad6fca Compare May 15, 2025 09:08

clonker mentioned this pull request May 15, 2025

Collect instruction location info for EOF assembly #16052

Merged

clonker force-pushed the eof_source_locations_unoptimized branch from aad6fca to 739761a Compare May 15, 2025 14:04

clonker added 8 commits May 21, 2025 10:19

Ethdebug requires assembly instance

43bc751

Ethdebug instructions output over multiple code sections

5a9a6bf

Add ethdebug schema

a33bc20

Ethdebug uses schema for serialization

d43371b

Update test expectations with eof-enabled ethdebug

2b45e5b

Test ethdebug output when requested in standard json under EOF

1243cca

Add tests for ethdebug output on abstract contracts and interfaces

925e532

Changelog uses single blank line between headlines

0ca8800

clonker force-pushed the eof_source_locations_unoptimized branch from 2f4f25b to 0ca8800 Compare May 21, 2025 08:19

clonker requested review from nikola-matic and aarlt May 21, 2025 08:20

aarlt approved these changes May 22, 2025

View reviewed changes

		@@ -4,7 +4,7 @@ Language Features:


		Compiler Features:

		* ethdebug: Experimental support for instructions and source locations under EOF.

Support ethdebug source locations under EOF #15994

Are you sure you want to change the base?

Support ethdebug source locations under EOF #15994

Conversation

clonker commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matheusaaguiar left a comment

Choose a reason for hiding this comment

Uh oh!

cameel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clonker Apr 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aarlt commented May 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cameel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cameel May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

clonker commented May 21, 2025

Uh oh!

Uh oh!

clonker commented Apr 10, 2025 •

edited

Loading

clonker Apr 23, 2025 •

edited

Loading

cameel May 15, 2025 •

edited

Loading