inline assembly to AST in json export #7537

djudjuu · 2019-10-15T10:42:43Z

Description

resolves #2419

creates different kinds of Json nodes for each kind of AsmData-struct.
similar to the solidityAST, there is a "nodeType" field and a "src"-location.

Goal: It should be possible to recreate the AsmData from the Json.

TEST added: I added a contract with a bunch of inlineAssembly (assebly.sol from the syntaxTests) to /test/libsolidity/ASTJSON
Then I ran the testsuite and used its output from the error-message to create assembly.json and assembly_legacy.json.

Checklist

Code compiles correctly
All tests are passing
New tests have been created which fail without the change (if possible)
README / documentation was extended, if necessary
-> left a note on [DOCS] Add AST output #7387
Changelog entry (if change is visible to the user)
Used meaningful commit messages

libsolidity/ast/AsmJsonConverter.h

libsolidity/ast/AsmJsonConverter.cpp

djudjuu · 2019-10-24T14:05:48Z

HELP NEEDED: All test that fail are related to the fuzzer testing soljson.

I don't really know how to debug that or make sense of it. any pointers?

UPDATE: All solved

bshastry · 2019-10-28T09:33:12Z

HELP NEEDED: All test that fail are related to the fuzzer testing soljson.

I don't really know how to debug that or make sense of it. any pointers?

@djudjuu I didn't take a look at all failures. One failure that I spent some time on is from the t_ubu_release_cli that looks like so

Invalid error: "InternalCompilerError"
Fuzzer failed on test_254ca6c11f2f9744a828fdf899fa357a2bff321ec25630224b2ce8a4031b9a04_solidityendtoendtest_cpp.solInvalid error: "InternalCompilerError"
Fuzzer failed on test_5897c478c6ab519fb05b145b1fcbc940d8b3ebaa7e9b65b1fec7c2033e29d60b_solidityendtoendtest_cpp.solxargs: /root/project/build/test/tools/solfuzzer: terminated by signal 11
Exited with code 123

What I tried to do is the following

Located failing print to here

solidity/test/cmdlineTests.sh

Lines 435 to 446 in 302a51a

    
           printTask "Testing soljson via the fuzzer..." 
        
           SOLTMPDIR=$(mktemp -d) 
        
           ( 
        
               set -e 
        
               cd "$SOLTMPDIR" 
        
               "$REPO_ROOT"/scripts/isolate_tests.py "$REPO_ROOT"/test/ 
        
               "$REPO_ROOT"/scripts/isolate_tests.py "$REPO_ROOT"/docs/ docs 
        
               echo *.sol | xargs -P 4 -n 50 "$REPO_ROOT"/${SOLIDITY_BUILD_DIR}/test/tools/solfuzzer --quiet --input-files 
        
               echo *.sol | xargs -P 4 -n 50 "$REPO_ROOT"/${SOLIDITY_BUILD_DIR}/test/tools/solfuzzer --without-optimizer --quiet --input-files 
        
           ) 
        
           rm -rf "$SOLTMPDIR"

Since the failing test is obfuscated by a hash string, added a print statement in the isolate_scripts.py script here

solidity/scripts/isolate_tests.py

Line 74 in 302a51a

open('test_%s_%s.sol' % (hashlib.sha256(test).hexdigest(), cleaned_filename), 'wb').write(remainder)

Located the failing solidity test case (corresponding to the error output test_254ca6c11f2f9744a828fdf899fa357a2bff321ec25630224b2ce8a4031b9a04_solidityendtoendtest_cpp) to be this

solidity/test/libsolidity/SolidityEndToEndTest.cpp

Lines 9848 to 9860 in 302a51a

    
           		contract C { 
        
           			function f(uint a) public returns (uint b) { 
        
           				assembly { 
        
           					function fac(n) -> nf { 
        
           						switch n 
        
           						case 0 { nf := 1 } 
        
           						case 1 { nf := 1 } 
        
           						default { nf := mul(n, fac(sub(n, 1))) } 
        
           					} 
        
           					b := fac(a) 
        
           				} 
        
           			} 
        
           		}

One option to debug this specific failure further is to copy paste the highlighted solidity test case into a standalone file and run it against solfuzzer. My guess is that the run should failure with an internal compiler error, and then you can debug the issue further. Hope this helps you :)

libsolidity/ast/ASTJsonConverter.cpp

libsolidity/ast/AsmJsonConverter.cpp

chriseth · 2019-11-04T12:11:45Z

Please retarget to 0.6.0 - I think this might break some tools.

Marenz · 2019-11-11T17:27:55Z

I retargeted it, but I can't retrigger the tests

leonardoalt

Why does it sometimes use (*this)(...) and sometimes boost::apply_visitor(*this, ...);?
It would be nice to have some smaller tests also. It's impossible to review 1000 lines of JSON output.

leonardoalt · 2019-11-12T15:51:27Z

libsolidity/ast/ASTJsonConverter.cpp

@@ -24,6 +24,7 @@
 #include <libsolidity/ast/AST.h>
 #include <libyul/AsmData.h>
 #include <libyul/AsmPrinter.h>
+#include <libsolidity/ast/AsmJsonConverter.h>


This should be moved two lines up.

leonardoalt · 2019-11-12T15:52:27Z

libsolidity/ast/ASTJsonConverter.cpp

-	int sourceIndex{-1};
-	if (_location.source && m_sourceIndices.count(_location.source->name()))
-		sourceIndex = m_sourceIndices.at(_location.source->name());
+	size_t sourceIndex = sourceIndexFromLocation(_location);


Before, if the condition was false, sourceIndex == -1. Now it asserts. Why is the latter true?

I think we can be rather sure to have a source, but better to not assert, I think.

leonardoalt · 2019-11-12T15:53:59Z

libsolidity/ast/AsmJsonConverter.h

+ */
+
+#include <libyul/AsmDataForward.h>
+#include <liblangutil/SourceLocation.h>


This include should be moved up.

Same thing: I think liblangutil is more basic than libyul.

Yes, got confused

leonardoalt · 2019-11-12T15:54:40Z

libsolidity/ast/AsmJsonConverter.h

+using TypedNameList = std::vector<yul::TypedName>;
+
+/**
+ * Converter of the yul AST into JSON format


If this converts Yul to JSON, shouldn't it go into libyul instead of libsolidity?

leonardoalt · 2019-11-12T15:55:05Z

libsolidity/ast/AsmJsonConverter.h

+/**
+ * Converter of the yul AST into JSON format
+ */
+class AsmJsonConverter : public boost::static_visitor<Json::Value>


Suggested change

class AsmJsonConverter : public boost::static_visitor<Json::Value>

class AsmJsonConverter: public boost::static_visitor<Json::Value>

leonardoalt · 2019-11-12T15:55:40Z

libsolidity/ast/AsmJsonConverter.h

+private:
+	Json::Value createAstNode(langutil::SourceLocation const& _location, std::string _nodeType) const;
+	template <class T>
+	Json::Value vectorOfVariantsToJson(std::vector<T> const& vec ) const;


Suggested change

Json::Value vectorOfVariantsToJson(std::vector<T> const& vec ) const;

Json::Value vectorOfVariantsToJson(std::vector<T> const& vec) const;

leonardoalt · 2019-11-12T15:56:17Z

libsolidity/ast/AsmJsonConverter.cpp

+#include <libsolidity/ast/AsmJsonConverter.h>
+#include <libyul/AsmData.h>
+#include <liblangutil/Exceptions.h>
+


Extra blank line.

leonardoalt · 2019-11-12T15:56:46Z

libsolidity/ast/AsmJsonConverter.cpp

+
+#include <libsolidity/ast/AsmJsonConverter.h>
+#include <libyul/AsmData.h>
+#include <liblangutil/Exceptions.h>


This line should be moved one up. It should still come after AsmJsonConverter.h, since that's the header of this cpp.

Isn't liblangutil more basic than libyul?

ah sorry, that's correct

leonardoalt · 2019-11-12T15:57:38Z

libsolidity/ast/AsmJsonConverter.cpp

+namespace solidity
+{
+
+Json::Value AsmJsonConverter::createAstNode(langutil::SourceLocation const& _location, string _nodeType) const


I think this and the next function should go to the end of the file to follow the same order as the header.

chriseth · 2019-11-12T16:05:47Z

Why does it sometimes use (*this)(...) and sometimes boost::apply_visitor(*this, ...);?: Depends on whether the argument is a variant (then you have to use apply_visitor), or a specific type (then (*this)(...) has to be used).

leonardoalt · 2019-11-13T10:58:53Z

test/libsolidity/ASTJSON/assembly/call.json

+            "statements":
+            [
+              {
+                "AST":


Actually what's this AST key?

It's a node-type-specific key for the node type "InlineAssembly".

Actually - it would probably be better to prefix all the Yul AST node types by Yul wouldn't it?

It sounds quite generic. Why not call it something closer to InlineAssembly?

Actually - it would probably be better to prefix all the Yul AST node types by Yul wouldn't it?

Yea I agree

But the keys that depend on the node type are always "generic" because they are only seen in the context of the node type.

libsolidity/ast/ASTJsonConverter.cpp

libyul/AsmJsonConverter.cpp

chriseth reviewed Oct 15, 2019

View reviewed changes

libsolidity/ast/AsmJsonConverter.h Outdated Show resolved Hide resolved

chriseth reviewed Oct 15, 2019

View reviewed changes

libsolidity/ast/AsmJsonConverter.h Outdated Show resolved Hide resolved

chriseth reviewed Oct 15, 2019

View reviewed changes