Merge pull request #9029 from ethereum/fewerInternalSourceRefs

Do not set source locations for small internal assembly routines.
This commit is contained in:
chriseth 2020-05-28 11:14:20 +02:00 committed by GitHub
commit f608e50bad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 46 additions and 65 deletions

View File

@ -6,6 +6,7 @@ Language Features:
Compiler Features:
* Code Generator: Do not introduce new source references for small internal routines.
* Build system: Update the soljson.js build to emscripten 1.39.15 and boost 1.73.0 and include Z3 for integrated SMTChecker support without the callback mechanism.
* SMTChecker: Support array ``length``.
* SMTChecker: Support array ``push`` and ``pop``.

View File

@ -97,6 +97,7 @@ public:
/// Changes the source location used for each appended item.
void setSourceLocation(langutil::SourceLocation const& _location) { m_currentSourceLocation = _location; }
langutil::SourceLocation const& currentSourceLocation() const { return m_currentSourceLocation; }
/// Assembles the assembly into bytecode. The assembly should not be modified after this call, since the assembled version is cached.
LinkerObject const& assemble() const;

View File

@ -64,7 +64,7 @@ protected:
};
/// Location of the current token
SourceLocation currentLocation() const;
virtual SourceLocation currentLocation() const;
///@{
///@name Helper functions

View File

@ -422,7 +422,12 @@ void CompilerContext::appendInlineAssembly(
ErrorReporter errorReporter(errors);
auto scanner = make_shared<langutil::Scanner>(langutil::CharStream(_assembly, "--CODEGEN--"));
yul::EVMDialect const& dialect = yul::EVMDialect::strictAssemblyForEVM(m_evmVersion);
shared_ptr<yul::Block> parserResult = yul::Parser(errorReporter, dialect).parse(scanner, false);
optional<langutil::SourceLocation> locationOverride;
if (!_system)
locationOverride = m_asm->currentSourceLocation();
shared_ptr<yul::Block> parserResult =
yul::Parser(errorReporter, dialect, std::move(locationOverride))
.parse(scanner, false);
#ifdef SOL_OUTPUT_ASM
cout << yul::AsmPrinter(&dialect)(*parserResult) << endl;
#endif

View File

@ -46,8 +46,15 @@ public:
None, ForLoopPre, ForLoopPost, ForLoopBody
};
explicit Parser(langutil::ErrorReporter& _errorReporter, Dialect const& _dialect):
ParserBase(_errorReporter), m_dialect(_dialect) {}
explicit Parser(
langutil::ErrorReporter& _errorReporter,
Dialect const& _dialect,
std::optional<langutil::SourceLocation> _locationOverride = {}
):
ParserBase(_errorReporter),
m_dialect(_dialect),
m_locationOverride(std::move(_locationOverride))
{}
/// Parses an inline assembly block starting with `{` and ending with `}`.
/// @param _reuseScanner if true, do check for end of input after the `}`.
@ -60,6 +67,11 @@ public:
protected:
using ElementaryOperation = std::variant<Literal, Identifier, FunctionCall>;
langutil::SourceLocation currentLocation() const override
{
return m_locationOverride ? *m_locationOverride : ParserBase::currentLocation();
}
/// Creates an inline assembly node with the current source location.
template <class T> T createWithLocation() const
{
@ -91,6 +103,7 @@ protected:
private:
Dialect const& m_dialect;
std::optional<langutil::SourceLocation> m_locationOverride;
ForLoopComponent m_currentForLoopComponent = ForLoopComponent::None;
bool m_insideFunction = false;
};

View File

@ -19,20 +19,14 @@ EVM assembly:
sstore
/* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */
callvalue
/* "--CODEGEN--":2:4 */
dup1
iszero
tag_5
jumpi
/* "--CODEGEN--":27:28 */
0x00
/* "--CODEGEN--":24:25 */
dup1
/* "--CODEGEN--":17:29 */
revert
/* "--CODEGEN--":2:4 */
tag_5:
/* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */
pop
jump(tag_6)
/* "optimizer_BlockDeDuplicator/input.sol":77:103 function fun_x() public {} */
@ -53,21 +47,14 @@ sub_0: assembly {
/* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */
mstore(0x40, 0x80)
callvalue
/* "--CODEGEN--":5:14 */
dup1
/* "--CODEGEN--":2:4 */
iszero
tag_1
jumpi
/* "--CODEGEN--":27:28 */
0x00
/* "--CODEGEN--":24:25 */
dup1
/* "--CODEGEN--":17:29 */
revert
/* "--CODEGEN--":2:4 */
tag_1:
/* "optimizer_BlockDeDuplicator/input.sol":60:213 contract C {... */
pop
jumpi(tag_2, lt(calldatasize, 0x04))
shr(0xe0, calldataload(0x00))
@ -87,11 +74,8 @@ sub_0: assembly {
tag_3
jumpi
tag_2:
/* "--CODEGEN--":12:13 */
0x00
/* "--CODEGEN--":9:10 */
dup1
/* "--CODEGEN--":2:14 */
revert
/* "optimizer_BlockDeDuplicator/input.sol":138:174 function f() public { true ? 1 : 3;} */
tag_3:

View File

@ -76,11 +76,8 @@ stop
sub_0: assembly {
/* "optimizer_user_yul/input.sol":60:525 contract C... */
mstore(0x40, 0x80)
/* "--CODEGEN--":12:13 */
0x00
/* "--CODEGEN--":9:10 */
dup1
/* "--CODEGEN--":2:14 */
revert
auxdata: AUXDATA REMOVED

View File

@ -1,2 +1,2 @@
{"contracts":{"a.sol":{"A":{"evm":{"deployedBytecode":{"immutableReferences":{"3":[{"length":32,"start":77}]},"linkReferences":{},"object":"bytecode removed","opcodes":"opcodes removed","sourceMap":"36:96:0:-:0;;;;5:9:-1;2:2;;;27:1;24;17:12;2:2;36:96:0;;;;;;;;;;;;;;;;12:1:-1;9;2:12;74:56:0;;;:::i;:::-;;;;;;;;;;;;;;;;;;;;108:7;126:1;119:8;;74:56;:::o"}}}}},"errors":[{"component":"general","formattedMessage":"a.sol: Warning: Source file does not specify required compiler version!
{"contracts":{"a.sol":{"A":{"evm":{"deployedBytecode":{"immutableReferences":{"3":[{"length":32,"start":77}]},"linkReferences":{},"object":"bytecode removed","opcodes":"opcodes removed","sourceMap":"36:96:0:-:0;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;74:56;;;:::i;:::-;;;;;;;;;;;;;;;;;;;;108:7;126:1;119:8;;74:56;:::o"}}}}},"errors":[{"component":"general","formattedMessage":"a.sol: Warning: Source file does not specify required compiler version!
","message":"Source file does not specify required compiler version!","severity":"warning","sourceLocation":{"end":-1,"file":"a.sol","start":-1},"type":"Warning"}],"sources":{"a.sol":{"id":0}}}

View File

@ -107,13 +107,13 @@ void printAssemblyLocations(AssemblyItems const& _items)
cout <<
"\t\tvector<SourceLocation>(" <<
_repetitions <<
", SourceLocation(" <<
", SourceLocation{" <<
_loc.start <<
", " <<
_loc.end <<
", make_shared<string>(\"" <<
_loc.source->name() <<
"\"))) +" << endl;
"\")}) +" << endl;
};
vector<SourceLocation> locations;
@ -175,33 +175,13 @@ BOOST_AUTO_TEST_CASE(location_test)
vector<SourceLocation> locations;
if (solidity::test::CommonOptions::get().optimize)
locations =
vector<SourceLocation>(4, SourceLocation{2, 82, sourceCode}) +
vector<SourceLocation>(1, SourceLocation{5, 14, codegenCharStream}) +
vector<SourceLocation>(3, SourceLocation{2, 4, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{27, 28, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{24, 25, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{17, 29, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{2, 4, codegenCharStream}) +
vector<SourceLocation>(16, SourceLocation{2, 82, sourceCode}) +
vector<SourceLocation>(1, SourceLocation{12, 13, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{9, 10, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{2, 14, codegenCharStream}) +
vector<SourceLocation>(31, SourceLocation{2, 82, sourceCode}) +
vector<SourceLocation>(21, SourceLocation{20, 79, sourceCode}) +
vector<SourceLocation>(1, SourceLocation{72, 74, sourceCode}) +
vector<SourceLocation>(2, SourceLocation{20, 79, sourceCode});
else
locations =
vector<SourceLocation>(4, SourceLocation{2, 82, sourceCode}) +
vector<SourceLocation>(1, SourceLocation{5, 14, codegenCharStream}) +
vector<SourceLocation>(3, SourceLocation{2, 4, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{27, 28, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{24, 25, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{17, 29, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{2, 4, codegenCharStream}) +
vector<SourceLocation>(hasShifts ? 16 : 17, SourceLocation{2, 82, sourceCode}) +
vector<SourceLocation>(1, SourceLocation{12, 13, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{9, 10, codegenCharStream}) +
vector<SourceLocation>(1, SourceLocation{2, 14, codegenCharStream}) +
vector<SourceLocation>(hasShifts ? 31 : 32, SourceLocation{2, 82, sourceCode}) +
vector<SourceLocation>(24, SourceLocation{20, 79, sourceCode}) +
vector<SourceLocation>(1, SourceLocation{49, 58, sourceCode}) +
vector<SourceLocation>(1, SourceLocation{72, 74, sourceCode}) +

View File

@ -370,15 +370,15 @@ BOOST_AUTO_TEST_CASE(basic_compilation)
BOOST_CHECK(contract["evm"]["assembly"].isString());
BOOST_CHECK(contract["evm"]["assembly"].asString().find(
" /* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n "
"callvalue\n /* \"--CODEGEN--\":5:14 */\n dup1\n "
"/* \"--CODEGEN--\":2:4 */\n iszero\n tag_1\n jumpi\n "
"/* \"--CODEGEN--\":27:28 */\n 0x00\n /* \"--CODEGEN--\":24:25 */\n "
"dup1\n /* \"--CODEGEN--\":17:29 */\n revert\n /* \"--CODEGEN--\":2:4 */\n"
"tag_1:\n /* \"fileA\":0:14 contract A { } */\n pop\n dataSize(sub_0)\n dup1\n "
"callvalue\n dup1\n "
"iszero\n tag_1\n jumpi\n "
"0x00\n "
"dup1\n revert\n"
"tag_1:\n pop\n dataSize(sub_0)\n dup1\n "
"dataOffset(sub_0)\n 0x00\n codecopy\n 0x00\n return\nstop\n\nsub_0: assembly {\n "
"/* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n "
"/* \"--CODEGEN--\":12:13 */\n 0x00\n /* \"--CODEGEN--\":9:10 */\n "
"dup1\n /* \"--CODEGEN--\":2:14 */\n revert\n\n auxdata: 0xa26469706673582212"
"/* \"fileA\":0:14 contract A { } */\n mstore(0x40, 0x80)\n "
"0x00\n "
"dup1\n revert\n\n auxdata: 0xa26469706673582212"
) == 0);
BOOST_CHECK(contract["evm"]["gasEstimates"].isObject());
BOOST_CHECK_EQUAL(contract["evm"]["gasEstimates"].size(), 1);
@ -402,15 +402,15 @@ BOOST_AUTO_TEST_CASE(basic_compilation)
"{\"begin\":0,\"end\":14,\"name\":\"PUSH\",\"source\":0,\"value\":\"40\"},"
"{\"begin\":0,\"end\":14,\"name\":\"MSTORE\",\"source\":0},"
"{\"begin\":0,\"end\":14,\"name\":\"CALLVALUE\",\"source\":0},"
"{\"begin\":5,\"end\":14,\"name\":\"DUP1\",\"source\":-1},"
"{\"begin\":2,\"end\":4,\"name\":\"ISZERO\",\"source\":-1},"
"{\"begin\":2,\"end\":4,\"name\":\"PUSH [tag]\",\"source\":-1,\"value\":\"1\"},"
"{\"begin\":2,\"end\":4,\"name\":\"JUMPI\",\"source\":-1},"
"{\"begin\":27,\"end\":28,\"name\":\"PUSH\",\"source\":-1,\"value\":\"0\"},"
"{\"begin\":24,\"end\":25,\"name\":\"DUP1\",\"source\":-1},"
"{\"begin\":17,\"end\":29,\"name\":\"REVERT\",\"source\":-1},"
"{\"begin\":2,\"end\":4,\"name\":\"tag\",\"source\":-1,\"value\":\"1\"},"
"{\"begin\":2,\"end\":4,\"name\":\"JUMPDEST\",\"source\":-1},"
"{\"begin\":0,\"end\":14,\"name\":\"DUP1\",\"source\":0},"
"{\"begin\":0,\"end\":14,\"name\":\"ISZERO\",\"source\":0},"
"{\"begin\":0,\"end\":14,\"name\":\"PUSH [tag]\",\"source\":0,\"value\":\"1\"},"
"{\"begin\":0,\"end\":14,\"name\":\"JUMPI\",\"source\":0},"
"{\"begin\":0,\"end\":14,\"name\":\"PUSH\",\"source\":0,\"value\":\"0\"},"
"{\"begin\":0,\"end\":14,\"name\":\"DUP1\",\"source\":0},"
"{\"begin\":0,\"end\":14,\"name\":\"REVERT\",\"source\":0},"
"{\"begin\":0,\"end\":14,\"name\":\"tag\",\"source\":0,\"value\":\"1\"},"
"{\"begin\":0,\"end\":14,\"name\":\"JUMPDEST\",\"source\":0},"
"{\"begin\":0,\"end\":14,\"name\":\"POP\",\"source\":0},"
"{\"begin\":0,\"end\":14,\"name\":\"PUSH #[$]\",\"source\":0,\"value\":\"0000000000000000000000000000000000000000000000000000000000000000\"},"
"{\"begin\":0,\"end\":14,\"name\":\"DUP1\",\"source\":0},"