solidity icon indicating copy to clipboard operation
solidity copied to clipboard

Fix inconsistent negative subassembly indices between different sizeof(size_t)

Open clonker opened this issue 8 months ago • 3 comments

When computing object ids for referencing subassemblies, these ids are currently determined as negative DFS enumeration based on size_t:

https://github.com/ethereum/solidity/blob/a65229279d769a7b2936aef2aa802d77abbdc343/libevmasm/Assembly.cpp#L1825

This PR changes this to uint64_t, so that assembly text - in particular PUSH #[$] instructions - is consistent over different type sizes of size_t.

Since these indices are stored in a map and later referenced for exporting and importing, as well as (currently) numeric_limits<size_t>::max() is used to indicate the root object or an empty state which is used in, eg, various asserts, I found it helpful to wrap the uint64_t into a struct so that these places become apparent and dealing with the sizes is explicit. This caused a whole avalanche of changes, as such object ids (or more generally: SubAssemblyIDs) are used in many places. In particular, also yul::Object::subIds are subject to the type.

We were lacking a cmdline test that has non-zero PUSH #[$]es, so I added one. Since emscripten builds do not execute them, we have no direct regression test by this but I have added one to the solc-js tests. While doing that I realized that it is currently not possible (to me at least :P) to request ASM json output via standard json interface if the language is Yul.

Fixes #15953

clonker avatar Mar 19 '25 09:03 clonker

While doing that I realized that it is currently not possible (to me at least :P) to request ASM json output via standard json interface if the language is Yul.

Isn't that the evm.assembly option?

{
	"language": "Yul",
	"sources":
	{
		"C":
		{
			"content": "{ let x := mload(0) sstore(add(x, 0), 0) }"
		}
	},
	"settings":
	{
		"outputSelection":
		{
			"*": { "*": ["evm.assembly"], "": [ "*" ] }
		}
	}
}

r0qs avatar Mar 19 '25 11:03 r0qs

Isn't that the evm.assembly option?

The equivalent that is showing the PUSH #[$]es would be evm.legacyAssembly

clonker avatar Mar 19 '25 11:03 clonker

This pull request is stale because it has been open for 14 days with no activity. It will be closed in 7 days unless the stale label is removed.

github-actions[bot] avatar Jun 04 '25 12:06 github-actions[bot]

This pull request is stale because it has been open for 14 days with no activity. It will be closed in 7 days unless the stale label is removed.

github-actions[bot] avatar Jun 19 '25 12:06 github-actions[bot]

This pull request is stale because it has been open for 14 days with no activity. It will be closed in 7 days unless the stale label is removed.

github-actions[bot] avatar Jul 04 '25 12:07 github-actions[bot]