ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.01k stars 5.7k forks source link

Emscripten binaries for version 0.3.6 in solc-bin were not built from the commit tagged as v0.3.6 #10846

Open cameel opened 3 years ago

cameel commented 3 years ago

In solc-bin we have soljson-v0.3.6+commit.3fc68da5.js and soljson-v0.3.6+commit.3fc68da5.js, both built from 3fc68da5. Tag v0.3.6 points at commit 988fe5e (i.e. one commit after 3fc68da5).

This is a problem because they will produce different bytecode than releases from other platforms built on v0.3.6 (e.g. the recent macOS rebuilds).

The binaries in solc-bin are not necessarily the files that were released - release v0.3.6 has no binaries attached at all - so I think we could rebuild on the tag and put both in the repo.

cameel commented 3 years ago

@chriseth I have manually compared the Linux and wasm reports for 0.3.6 and bytecode differs only in a handful of cases. All of these contain libraries and the only difference is this small fragment:

< 03063fc68da5
---
> 0306cbf55226

I investigated this and turns out it's the version stamp for libraries added by injectVersionStampIntoSub():

void CompilerContext::injectVersionStampIntoSub(size_t _subIndex)
{
    eth::Assembly& sub = m_asm.sub(_subIndex);
    sub.injectStart(Instruction::POP);
    sub.injectStart(fromBigEndian<u256>(binaryVersion()));
}

Note that the stamp in each case consists of 03 06 (corresponding to 0.3.6) followed by the first 8 digits of the commit hash.

Surprisingly, cbf55226 is not a valid commit and definitely not the commit v0.3.6 points at (988fe5e). It's probably the commit ID that was created after the script cherry-picked necessary fixes. It should not happen though because I did set commit_hash.txt to get the same commit ID as in the emscripten case. This method worked for >= 0.4.0 (bytecode reports match emscripten) so I wonder if 0.3.6 maybe just did not not support commit_hash.txt? I need to check that.

Another unexpected thing is that I did not see for >= 0.4.0 is that there are a few files for which one of the binaries errored out and the other did not.

Full bytecode diff

Here are the whole reports if you want to take a look:

For convenience, here's one of the test cases that have this difference in the bytecode:

1c1
< test_06bdc56c19e023d03ff89762979800ef24c770577d9138159f8b62b30650da2b_solidityendtoendtest_cpp.sol:D 6060604052607c8060106000396000f36503063fc68da55060606040526000357c010000000000000000000000000000000000000000000000000000000090048063eee9720614604157603d565b6007565b60556004808035906020019091905050606b565b6040518082815260200191505060405180910390f35b60008160020290506077565b91905056
---
> test_06bdc56c19e023d03ff89762979800ef24c770577d9138159f8b62b30650da2b_solidityendtoendtest_cpp.sol:D 6060604052607c8060106000396000f3650306cbf552265060606040526000357c010000000000000000000000000000000000000000000000000000000090048063eee9720614604157603d565b6007565b60556004808035906020019091905050606b565b6040518082815260200191505060405180910390f35b60008160020290506077565b91905056

Full bytecode diff for a test case where one of the binaries errors out

< test_17921e1d1ec89741ca84ac79266675a6e3afb8a468111859ace1e73eaf606c30_gasmeter_cpp.sol: <ERROR>
---
> test_17921e1d1ec89741ca84ac79266675a6e3afb8a468111859ace1e73eaf606c30_gasmeter_cpp.sol:test 606060405260c68060106000396000f360606040526000357c010000000000000000000000000000000000000000000000000000000090048063b3de648b146041578063e420264a14605757603f565b005b605560048080359060200190919050506081565b005b606b600480803590602001909190505060b3565b6040518082815260200191505060405180910390f35b600781111560a357600160956008830a60b3565b0160016000508190555060af565b60016000600050819055505b5b50565b6000600160005054905060c1565b91905056
> test_17921e1d1ec89741ca84ac79266675a6e3afb8a468111859ace1e73eaf606c30_gasmeter_cpp.sol:test <NO METADATA>
axic commented 3 years ago

I think it would be useful documenting all these in a section in the documentation (instead of issues). By "documenting all these" I mean what people should expect from which distributed version. It may be a better option than trying to resolve problems for ancient releases. Of course documenting it would not be possible without the meticulous investigative work you have done with these.

cameel commented 3 years ago

Yeah, documenting them sounds like a good idea. But I think we still should fix at least those problems that are easy to fix (like for example renaming the Linux 0.4.10) :)

Do you think it should be documented in the main docs or is the solc-bin README (or separate READMEs in the directories containing the binaries) a better place?

axic commented 3 years ago

But I think we still should fix at least those problems that are easy to fix (like for example renaming the Linux 0.4.10) :)

Absolutely, where it is easy to fix, go ahead. But probably it is not worth spending days/weeks on these :)

Do you think it should be documented in the main docs or is the solc-bin README (or separate READMEs in the directories containing the binaries) a better place?

I think the main documentation is better, nobody reads those readmes too much.

cameel commented 3 years ago

OK. So I'll add a section the docs on that.