Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

solc: Stack overflow importing large AST from json file #13179

Closed
jaa2 opened this issue Jun 21, 2022 · 6 comments · Fixed by #13205
Closed

solc: Stack overflow importing large AST from json file #13179

jaa2 opened this issue Jun 21, 2022 · 6 comments · Fixed by #13205

Comments

@jaa2
Copy link
Contributor

jaa2 commented Jun 21, 2022

Description

Importing an AST with --import-ast using a large JSON file results in a stack overflow and segmentation fault with certain output options.

A quick run with memcheck shows that it stems from a regex match.

Environment

  • Compiler version: 0.8.16-develop.2022.6.21+commit.c3ea8661.mod.Linux.g++
  • Target EVM version (as per compiler settings): london
  • Operating system: Linux

Steps to Reproduce

Create a large AST as JSON. Here is Uniswap V3 as an example (20.2 MB) from the external tests: uniswap_ast.zip

Try generating IR output - currently segfaults:

solc --import-ast ./164dbdb3af44e9eea766cd449cf8864c.json --ir

Try generating assembly from IR - currently segfaults:

solc --import-ast ./164dbdb3af44e9eea766cd449cf8864c.json --via-ir --asm

Regular assembly with the --asm flag only works as expected for this file.

@Marenz
Copy link
Contributor

Marenz commented Jun 21, 2022

I believe this won't be too difficult to solve, so marking this as easy/good first issue, but in case I am wrong, feel free to comment/correct here

@matheusaaguiar matheusaaguiar self-assigned this Jun 21, 2022
@matheusaaguiar
Copy link
Collaborator

matheusaaguiar commented Jun 22, 2022

I have tracked down the stack overflow to a regex_search call.
Apparently when the size of the string_view is sufficiently large (39547 in the example of this issue), the regex_search produces the stack overflow. It seems to be a known problem, since I have found some bug reports about similar situations with regex_search.
Not sure how it could be fixed. I guess we can't split the string_view being searched. I thought of merging two or more match_results returned from successive "range-restricted" regex searches (search first n positions, then next n or less until the end), but it is not possible.

@matheusaaguiar
Copy link
Collaborator

matheusaaguiar commented Jun 23, 2022

So far here is the situation: IR is generated with comments next to instructions indicating the source location.

 /// @src 19:243:377  ",\"src\":\"45:22:19\"},{\"abstract\":false,\"baseContracts\":[],\"canonicalName\":\"FixedPoint96\",\"contractDependencies\":[],\"contractKind\":\"libra"

The comments may contain some "garbage" which come from debug info (as reported in issue #12168). The debug info comes from method dispenseLocationComment.
Looking at method parseSrcComment, the "garbage" which comes after the source location is not used and does not seem to affect the return value. Maybe, we could just filter out the debug info which in some cases is large enough to cause segfault. I tested that approach and then there is a stack too deep exception:

Uncaught exception:
/solidity/libyul/backends/evm/EVMObjectCompiler.cpp(115): Throw in function void solidity::yul::EVMObjectCompiler::run(solidity::yul::Object&, bool)
Dynamic exception type: boost::wrapexcept<solidity::yul::StackTooDeepError>
std::exception::what: Variable ret_0 is 1 slot(s) too deep inside the stack.
[solidity::util::tag_comment*] = Variable ret_0 is 1 slot(s) too deep inside the stack.

I guess that is another problem with too long sequences of chars. I will continue to investigate from this point now, until we can decided the best solution.

@jaa2
Copy link
Contributor Author

jaa2 commented Jun 23, 2022

stack too deep exception

This is actually the expected output without --optimize

@matheusaaguiar
Copy link
Collaborator

matheusaaguiar commented Jun 24, 2022

Yes, just checked it and with --optimize there is no error.

@Marenz
Copy link
Contributor

Marenz commented Sep 13, 2022

Probably related: #13496

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants