Skip to content

Support multiple assignment in inline assembly#2924

Merged
chriseth merged 3 commits intodevelopfrom
inlineasm-assign-multi
Sep 20, 2017
Merged

Support multiple assignment in inline assembly#2924
chriseth merged 3 commits intodevelopfrom
inlineasm-assign-multi

Conversation

@chriseth
Copy link
Contributor

@chriseth chriseth commented Sep 18, 2017

Fixes #2923.

@chriseth chriseth requested a review from axic September 18, 2017 14:07
@axic axic force-pushed the inlineasm-assign-multi branch from c5810bf to c417846 Compare September 19, 2017 07:33
@chriseth chriseth force-pushed the inlineasm-assign-multi branch from c417846 to 1670b98 Compare September 19, 2017 08:26
@axic axic force-pushed the inlineasm-assign-multi branch from 1670b98 to 042fe35 Compare September 19, 2017 09:59
@axic
Copy link
Contributor

axic commented Sep 19, 2017

Added changelog and fixed travis.

Copy link
Contributor Author

@chriseth chriseth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful, thanks!

@axic axic force-pushed the inlineasm-assign-multi branch from 042fe35 to 47bca53 Compare September 19, 2017 10:44
@axic
Copy link
Contributor

axic commented Sep 19, 2017

Added tests for the parser errors and managed to crash:

unknown location:0: fatal error: in "JuliaParser/recursion_depth": memory access violation at address: 0x7fff54b11f50: invalid permissions
/Users/alex/Projects/solidity/test/libjulia/Parser.cpp:240: last checkpoint: "recursion_depth" entry.

@chriseth
Copy link
Contributor Author

This probably means we incorrectly estimated the stack frame size on one of the architectures.

@axic
Copy link
Contributor

axic commented Sep 19, 2017

It seems like going to a vector was enough to tip it over:

 -			assignment.variableName = identifier;
 +			assignment.variableNames.emplace_back(identifier);

@axic axic force-pushed the inlineasm-assign-multi branch from 47bca53 to 0cbd6ad Compare September 19, 2017 11:04
@axic
Copy link
Contributor

axic commented Sep 19, 2017

Added more tests and fixed the stack limit for Mac.

@axic axic mentioned this pull request Sep 19, 2017
4 tasks
@chriseth
Copy link
Contributor Author

Good to merge!

@axic
Copy link
Contributor

axic commented Sep 19, 2017

This crashes on travis:

/home/travis/build/ethereum/solidity/libsolidity/codegen/ContractCompiler.cpp(562): fatal error in "auto dev::solidity::ContractCompiler::visit(const dev::solidity::InlineAssembly &)::(anonymous class)::operator()(const assembly::Identifier &, julia::IdentifierContext, julia::AbstractAssembly &) const": std::exception: 
/home/travis/build/ethereum/solidity/test/RPCSession.cpp(324): last checkpoint

@axic
Copy link
Contributor

axic commented Sep 19, 2017

This code is triggering it:

               contract C {
                        function f() pure returns (bytes32 ret) {
                                assembly {
                                        ret := keccak256(0, 0)
                                }
                        }
                        function g() pure returns (bytes32 ret) {
                                assembly {
                                        0
                                        0
                                        keccak256
                                        =: ret
                                }
                        }
                        function h() pure returns (bytes32 ret) {
                                assembly {
                                        ret := sha3(0, 0)
                                }
                        }
                        function i() pure returns (bytes32 ret) {
                                assembly {
                                        0
                                        0
                                        sha3
                                        =: ret
                                }
                        }
                }

Out of this, the stack assignment (=: ret) parts fail.

@chriseth chriseth force-pushed the inlineasm-assign-multi branch from 0cbd6ad to e14ab95 Compare September 20, 2017 09:17
@chriseth
Copy link
Contributor Author

Ok, should be working now.

@axic
Copy link
Contributor

axic commented Sep 20, 2017

@chriseth please merge if tests pass

@chriseth chriseth merged commit 2adeb26 into develop Sep 20, 2017
@axic axic deleted the inlineasm-assign-multi branch September 20, 2017 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants