Add --import-asm-json input mode. #12704

aarlt · 2022-02-22T02:12:11Z

Replaces #11807.
Replaced by #12799,#12809 & #12834.

chriseth · 2022-03-07T11:34:55Z

I assume the question above has been resolved, right?

solc/CommandLineParser.cpp

scripts/ImportExportTest.sh

aarlt · 2022-03-14T22:11:07Z

solc/CommandLineParser.cpp

@@ -451,6 +455,13 @@ void CommandLineParser::parseOutputSelection()
 			CompilerOutputs::componentName(&CompilerOutputs::ewasm),
 			CompilerOutputs::componentName(&CompilerOutputs::ewasmIR),
 		};
+		static set<string> const evmAssemblyJsonImportModeOutputs = {


@chriseth @ekpyron does the restriction of the output modes make sense for you too?

@cameel can you answer this?

Yeah, looks sensible. I think they all should be possible to generate from imported assembly.

My only doubt would be about binaryRuntime. If it comes from compiling a contract, we do have the construction/runtime code split. Do we always have it with arbitrary assembly though?

@cameel yeah good point. An initial version of this PR did not support any binaryRuntime stuff - because I also saw it like you seem to see it. If I recall correctly, @chriseth wanted to have support for binaryRuntime and also support for the srcmap & srcmap-runtime. I understood it like that it would be great to support as many output modes that are usually also working. To achieve this, of course, we need to have some presumption about the imported assembly json. E.g. structure related to creational code vs. runtime code. Right now not every (well-formed & correct) assembly json will lead be create correct output (e.g. missing "runtime section").

@chriseth @ekpyron This question was more about the high-level view on this PR: if someone uses the asm json import, what should be the supported output-modes. Are my current assumptions to limit and/or support specific output-modes correct?

The main use-case for this I understood like this: someone wrote in contract in solidity, this contract gets exported to asm json, the user may do some tweaks & optimisations on the given code, but e.g. there is no need to change the initial structure e.g. creational code vs runtime code. Of course the user could add more data sections - but the initial structure should not get modified (creational vs runtime). As of now, I also did not really check the behaviour of multiple data sections etc - I guess it should just work, but I'm not entirely sure about this.

The main question is: are my assumptions correct? Do we want to support these output modes for assembly json import? Also for #12704 (comment) - where my assumptions correct to limit and/or support these?

Also for #12704 (comment) - where my assumptions correct to limit and/or support these?

This second check is redundant. It's just the opposite of the supported input modes, isn't it?

aarlt · 2022-03-14T22:12:32Z

solc/CommandLineParser.cpp

@@ -941,9 +959,27 @@ void CommandLineParser::processArgs()
 	for (auto& option: conflictingWithStopAfter)
 		checkMutuallyExclusive({g_strStopAfter, option});

+	array<string, 11> const conflictingWithAsmJsonImport{


@chriseth @ekpyron and what about this? does it make sense to not allow those?

Again @cameel

Not completely sure about storageLayout. So we lose that info at assembly level?
And --gas - so the estimator must be working at the Solidity level I assume and not by looking at opcodes?

By the way, putting it here seems redundant to me. You already do this check in parseOutputSelection() so this will never fail. conflictingWithStopAfter was here because it's just a regular option, not an input mode. And conflictingWithStopAfter should eventually be integrated into validOptionInputModeCombinations above anyway - this was being refactored in #12166 but it's stalled for now.

aarlt · 2022-03-14T22:26:53Z

Regarding the mismatch in the optimised byte-code:

Export assembly json
solc/solc <constract> --asm-json > /tmp/asm.json
Equivalence needed for - * I was assuming this *
solc/solc <contract> —optimize --opcodes (A)
solc/solc --import-asm-json /tmp/asm.json --opcodes --optimize (B)

Basically for these compilations the used optimiser(-steps) are different. That's why we see a difference in the byte-code. (A) uses class CompilerStack, usingAssemblyStack::optimize(), and that uses OptimiserSuite, where (B) uses Assembly, Assembly::optimise(..), Assembly::optimiseInternal(..). The final optimisation steps differ in both cases.

I think it totally make sense to allow --import-asm-json with --optimize, but not to see it like that we expect that the resulting byte-code of (A) and (B) need to be the same.

@chriseth @ekpyron What do you think?

aarlt · 2022-03-14T22:27:54Z

libsolidity/interface/CompilerStack.cpp

+		string const evmSourceName = m_evmAssemblyJson.begin()->first;
+		Json::Value const evmJson = m_evmAssemblyJson.begin()->second;
+
+		evmasm::Assembly::OptimiserSettings optimiserSettings;


I think I saw somewhere a optimiser-settings transform function.

aarlt · 2022-03-14T22:28:23Z

libsolidity/interface/CompilerStack.cpp

+		m_contracts[evmSourceName].evmAssembly = make_shared<evmasm::Assembly>(true, evmSourceName);
+		m_contracts[evmSourceName].evmAssembly->loadFromAssemblyJSON(m_evmAssemblyJson[evmSourceName]);
+		if (m_optimiserSettings.enabled)
+			m_contracts[evmSourceName].evmAssembly->optimise(optimiserSettings);


@chriseth @ekpyron I do the optimisation here like this. Does it make sense?

aarlt · 2022-03-14T22:28:44Z

libsolidity/interface/CompilerStack.cpp

+		m_contracts[evmSourceName].evmRuntimeAssembly->setSources(m_contracts[evmSourceName].evmAssembly->sources());
+		m_contracts[evmSourceName].evmRuntimeAssembly->loadFromAssemblyJSON(m_evmAssemblyJson[evmSourceName][".data"]["0"], false);
+		if (m_optimiserSettings.enabled)
+			m_contracts[evmSourceName].evmRuntimeAssembly->optimise(optimiserSettings);


@chriseth @ekpyron Similar optimisation as above. But for the runtime part. Does it make sense?

aarlt · 2022-03-14T22:35:10Z

libsolidity/interface/CompilerStack.cpp

+
+		m_contracts[evmSourceName].evmRuntimeAssembly = make_shared<evmasm::Assembly>(false, evmSourceName);
+		m_contracts[evmSourceName].evmRuntimeAssembly->setSources(m_contracts[evmSourceName].evmAssembly->sources());
+		m_contracts[evmSourceName].evmRuntimeAssembly->loadFromAssemblyJSON(m_evmAssemblyJson[evmSourceName][".data"]["0"], false);


@chriseth @ekpyron Does it make sense to load the runtime assembly hardcoded from ".data"[0] here? Not sure whether we should do this dynamically - e.g. that the correct run time will be taken automatically. I guess for this I would need to analyse the "deploy" code. Not sure whether we really want this. What are your opinions?

libevmasm/Assembly.cpp

chriseth · 2022-03-15T15:18:32Z

libevmasm/Assembly.cpp

+		item.m_modifierDepth = modifierDepth;
+		if (!value.empty())
+			item.setJumpType(value);
+		result = item;


Suggested change

result = item;

return item;

chriseth · 2022-03-15T15:19:56Z

libevmasm/Assembly.cpp

+		{
+			if (!value.empty())
+				data = u256(value);
+			updateUsedTags(data);


Wouldn't it be better to compute that once at the end from the full list of items?

libevmasm/Assembly.cpp

chriseth · 2022-03-15T15:23:42Z

libevmasm/Assembly.cpp

+		else if (name == "PUSH [$]")
+		{
+			if (!value.empty())
+				data = u256("0x" + value);


Please check that this works properly when the value starts with a zero (i.e. leading zeros may be removed somewhere in the process).

libevmasm/Assembly.cpp

chriseth · 2022-03-15T15:28:33Z

libevmasm/Assembly.cpp

+
+vector<Json::Value> Assembly::assemblyItemAsJSON(AssemblyItem const& _item, int _sourceIndex) const
+{
+	vector<Json::Value> result;


Shouldn't this return a single Json::Value?

chriseth · 2022-03-15T15:28:49Z

libevmasm/Assembly.cpp

+			_item.m_modifierDepth,
+			_item.location().start,
+			_item.location().end,


Can this be simplified so that we don't have to pass all three values all the time?

libevmasm/Assembly.cpp

libevmasm/Assembly.h

aarlt · 2022-03-15T16:42:22Z

After talking to @chriseth, we decided to split-up this PR.

aarlt force-pushed the asm-json-import branch from 9bcc135 to 67a69aa Compare February 22, 2022 02:18

aarlt mentioned this pull request Feb 28, 2022

Add support for importing asm-json. #11807

Closed

15 tasks

aarlt force-pushed the asm-json-import branch from 67a69aa to c81c915 Compare March 1, 2022 04:43

aarlt mentioned this pull request Mar 1, 2022

Update buildpacks to include jq. #12726

Merged

This comment was marked as resolved.

Sign in to view

aarlt force-pushed the asm-json-import branch 2 times, most recently from 4128beb to 4b7d64c Compare March 2, 2022 20:25

aarlt changed the base branch from develop to update-ci-cmake-jq-buildpacks March 2, 2022 20:38

aarlt force-pushed the asm-json-import branch from 4b7d64c to 029751a Compare March 2, 2022 20:40

Base automatically changed from update-ci-cmake-jq-buildpacks to develop March 2, 2022 20:55

aarlt force-pushed the asm-json-import branch from 029751a to 4da7da4 Compare March 2, 2022 20:57

aarlt force-pushed the asm-json-import branch from 4da7da4 to 3981e40 Compare March 7, 2022 22:10

stackenbotten reviewed Mar 9, 2022

View reviewed changes

solc/CommandLineParser.cpp Outdated Show resolved Hide resolved

stackenbotten reviewed Mar 9, 2022

View reviewed changes

solc/CommandLineParser.cpp Outdated Show resolved Hide resolved

aarlt force-pushed the asm-json-import branch from 5b3c7c7 to d464ce1 Compare March 9, 2022 21:55

aarlt commented Mar 9, 2022

View reviewed changes

scripts/ImportExportTest.sh Outdated Show resolved Hide resolved

aarlt added 6 commits March 14, 2022 08:14

[solc] Add --import-asm-json input mode.

43fe553

[solc] Basic --import-asm-json import.

d71de24

scripts/ImportExportTest.sh: Add generic script for import export.

e25a1a3

scripts/ImportExportTest.sh: Add direct import/export tests.

00f0343

CLI: Add checks for invalid output-modes for --import-asm-json.

0c0853e

rebase

c097842

aarlt force-pushed the asm-json-import branch from d464ce1 to c097842 Compare March 14, 2022 14:24

aarlt added 3 commits March 14, 2022 10:22

scripts/ImportExportTest.sh: fix direct export via --asm-json.

39d56ee

Minor optimization.

43faf23

scripts/ImportExportTest.sh: adding test for --optimized

e8f76ff

aarlt commented Mar 14, 2022

View reviewed changes

scripts/ImportExportTest.sh: disable --optimize tests for evm-assembly.

6700a65