Also prevent too much recursion in the assembly parser.

[chriseth]

No description provided.

[axic]

Should not have types here.

[axic]

Why are the tests passing if this has types?

[chriseth]

Because it requires there to be an error. I guess the parser just continues after the :.

[chriseth]

Ah right, it does not get until that point.

[axic]

Yep, 20000 is over the limit.

[axic]

Include in changelog please.

[chriseth]

Fixed.

[axic]

I get this:

unknown location:0: fatal error: in "SolidityInlineAssembly/Parsing/recursion_depth": memory access violation at address: 0x7fff5079ad60: invalid permissions
/Users/alex/Projects/solidity/test/libsolidity/InlineAssembly.cpp:403: last checkpoint: "recursion_depth" entry.

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x7fff5f3ffc80)
    frame #0: 0x0000000101860c99 libsolidity.dylib`dev::solidity::Scanner::scanToken() + 25
libsolidity.dylib`dev::solidity::Scanner::scanToken:
->  0x101860c99 <+25>: movq   %rdi, -0x8e0(%rbp)
    0x101860ca0 <+32>: movq   -0x8e0(%rbp), %rax
    0x101860ca7 <+39>: movq   %rax, %rdi
    0x101860caa <+42>: addq   $0xc0, %rdi
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=2, address=0x7fff5f3ffc80)
  * frame #0: 0x0000000101860c99 libsolidity.dylib`dev::solidity::Scanner::scanToken() + 25
    frame #1: 0x0000000101862587 libsolidity.dylib`dev::solidity::Scanner::next() + 87
    frame #2: 0x000000010185fffa libsolidity.dylib`dev::solidity::ParserBase::expectToken(dev::solidity::Token::Value) + 14378
    frame #3: 0x00000001018a00de libsolidity.dylib`dev::solidity::assembly::Parser::parseBlock() + 158
    frame #4: 0x00000001018a098b libsolidity.dylib`dev::solidity::assembly::Parser::parseStatement() + 523
    frame #5: 0x00000001018a0133 libsolidity.dylib`dev::solidity::assembly::Parser::parseBlock() + 243
...
    frame #3454: 0x00000001018a098b libsolidity.dylib`dev::solidity::assembly::Parser::parseStatement() + 523
    frame #3455: 0x00000001018a0133 libsolidity.dylib`dev::solidity::assembly::Parser::parseBlock() + 243
    frame #3456: 0x00000001018a098b libsolidity.dylib`dev::solidity::assembly::Parser::parseStatement() + 523
    frame #3457: 0x00000001018a0133 libsolidity.dylib`dev::solidity::assembly::Parser::parseBlock() + 243
    frame #3458: 0x000000010189fec3 libsolidity.dylib`dev::solidity::assembly::Parser::parse(std::__1::shared_ptr<dev::solidity::Scanner> const&) + 483
    frame #3459: 0x00000001017413ba libsolidity.dylib`dev::solidity::AssemblyStack::parseAndAnalyze(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 1034
    frame #3460: 0x000000010027e93f soltest`dev::solidity::test::(anonymous namespace)::parseAndReturnFirstError(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool, bool, dev::solidity::AssemblyStack::Machine) + 319
    frame #3461: 0x00000001002444c8 soltest`dev::solidity::test::(anonymous namespace)::expectError(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, bool, bool) + 136
    frame #3462: 0x000000010026551a soltest`dev::solidity::test::SolidityInlineAssembly::Parsing::recursion_depth::test_method() + 586
    frame #3463: 0x00000001002650f5 soltest`dev::solidity::test::SolidityInlineAssembly::Parsing::recursion_depth_invoker() + 677
...

[chriseth]

Interesting! Yes, macos has a much smaller stack size. So I guess we have to reduce the limit or reduce the size of stack frames.

[chriseth]

Reduced the max depth to 3000.

[axic]

There must be a different problem to stack size. The test for the Solidity parser works (worked even with the old limit), but the inline assembly one still fails even after reducing the stack limit.

[chriseth]

I think this is because the inline assembly parser just has larger stack frames. The inline assembly AST is built on the stack while the Solidity AST is built on the heap.

[chriseth]

Of course the proper way is to not have any recursion at all.

[axic]

Ran it again and it works. Probably there was some caching issue before.

[axic]

I guess before merging we should run the test in solc-js.

[chriseth]

Ok, will run the tests.

[chriseth]

Tests pass.

[axic]

I guess before merging we should run the test in solc-js.

With this I meant add a test case to solc-js and run it, since we don't know what kind of stack size / stack depth Emscripten / JS allows.

[chriseth]

Are you talking about a test that checks whether the recursion limit is hit before the stack height limit is hit?

[axic]

Yes, the same test we have here, but for solc-js (to validate Emscripten).