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

Add support for top level using for directive #1352

Closed
CodeSandwich opened this issue Aug 17, 2022 · 12 comments
Closed

Add support for top level using for directive #1352

CodeSandwich opened this issue Aug 17, 2022 · 12 comments
Labels
enhancement New feature or request High Priority

Comments

@CodeSandwich
Copy link
Contributor

Describe the issue:

I'm trying to run slither . on a project and I'm getting the error:

'forge build --extra-output abi --extra-output userdoc --extra-output devdoc --extra-output evm.methodIdentifiers --force' running
Compiling 36 files with 0.8.15
Solc 0.8.15 finished in 5.35s
Compiler run successful

Traceback (most recent call last):
  File "/home/zuczek/.local/lib/python3.10/site-packages/slither/__main__.py", line 744, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/zuczek/.local/lib/python3.10/site-packages/slither/__main__.py", line 87, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/zuczek/.local/lib/python3.10/site-packages/slither/__main__.py", line 70, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/zuczek/.local/lib/python3.10/site-packages/slither/slither.py", line 95, in __init__
    parser.parse_top_level_from_loaded_json(ast, path)
  File "/home/zuczek/.local/lib/python3.10/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 320, in parse_top_level_from_loaded_json
    raise SlitherException(f"Top level {top_level_data[self.get_key()]} not supported")
slither.exceptions.SlitherException: Top level UsingForDirective not supported
Error:
Top level UsingForDirective not supported
Please report an issue to https://github.com/crytic/slither/issues

The project uses Forge (yesterday version) and Solidity 0.8.15. This is the first Slither run ever, all I did before was running pip3 install slither-analyzer. I haven't installed solc manually, Slither seems to be using Forge and whatever compiler it has inside. (If that's the case, congratulations on that integration, I love it!). I'm running Manjaro Linux. Both python --version and python3 --version yield Python 3.10.5.

Code example to reproduce the issue:

https://github.com/radicle-dev/drips-contracts

Version:

0.8.3

Relevant log output:

I don't know where can I find the logs, if you tell me, I'll add them
@CodeSandwich CodeSandwich added the bug-candidate Bugs reports that are not yet confirmed label Aug 17, 2022
@plotchy
Copy link
Contributor

plotchy commented Aug 17, 2022

Files with using A for B or using {x, y, x} for A in a top-level scope (outside of contracts) isn't supported atm.

Error also manifests as InvalidCompilation

Contract taken from Solidity docs:
https://docs.soliditylang.org/en/latest/contracts.html?highlight=using%20for#using-for

// SPDX-License-Identifier: GPL-3.0
pragma solidity ^0.8.3;

struct Data { mapping(uint => bool) flags; }
using {insert, remove, contains} for Data; // problem line

function insert(Data storage self, uint value)
    returns (bool)
{
    if (self.flags[value])
        return false; // already there
    self.flags[value] = true;
    return true;
}

function remove(Data storage self, uint value)
    returns (bool)
{
    if (!self.flags[value])
        return false; // not there
    self.flags[value] = false;
    return true;
}

function contains(Data storage self, uint value)
    view
    returns (bool)
{
    return self.flags[value];
}


contract C {
    Data knownValues;

    function register(uint value) public {
        // Here, all variables of type Data have
        // corresponding member functions.
        // The following function call is identical to
        // `Set.insert(knownValues, value)`
        require(knownValues.insert(value));
    }
}
crytic_compile.platform.exceptions.InvalidCompilation: Invalid solc compilation Error: Expected pragma, import directive or contract/interface/library/struct/enum/constant/function definition.
  --> using_for.sol:12:1:
   |
12 | using {insert, remove, contains} for Data;
   | ^^^^^

@CodeSandwich
Copy link
Contributor Author

Oh, that's right, the project does have one top-level using ... for ... global;: https://github.com/radicle-dev/drips-contracts/blob/master/src/Drips.sol#L24.

Should I open an issue in crytic-compile? IIUC crytic-compile is not a compiler, but a wrapper using whatever is used in the project. After a quick skimming it seems like dapptools is supported, but not Foundry (hence crytic/crytic-compile#230). Does the problem come from falling back from using Foundry to a raw, obsolete version of solc? Or is it Slither surprised by an unexpected AST node?

@CodeSandwich CodeSandwich changed the title [Bug-Candidate]: Error: Top level UsingForDirective not supported [Bug-Candidate]: Error when using ... for is used outside of a contract Aug 17, 2022
@0xalpharush
Copy link
Contributor

This solidity feature hasn't been added to slither yet, so this is the correct place to have opened an issue.

@0xalpharush 0xalpharush added enhancement New feature or request and removed bug-candidate Bugs reports that are not yet confirmed labels Aug 17, 2022
@0xalpharush 0xalpharush changed the title [Bug-Candidate]: Error when using ... for is used outside of a contract Add support for top level using for directive Aug 17, 2022
@CodeSandwich
Copy link
Contributor Author

I've tried running Slither using branch dev-fix-usingfor from PR #1378 and indeed it doesn't throw the Top level UsingForDirective not supported anymore. Unfortunately it throws this strange exception:

'forge build --extra-output abi --extra-output userdoc --extra-output devdoc --extra-output evm.methodIdentifiers --force' running
Compiling 52 files with 0.8.17
Solc 0.8.17 finished in 7.19s
Compiler run successful

Traceback (most recent call last):
  File "/home/zuczek/workspace/slither/slither/core/cfg/node.py", line 911, in _find_read_write_call
    self._high_level_calls.append((ir.destination.type.type, ir.function))
AttributeError: 'NoneType' object has no attribute 'type'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/zuczek/workspace/slither/slither/__main__.py", line 826, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/zuczek/workspace/slither/slither/__main__.py", line 97, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/zuczek/workspace/slither/slither/__main__.py", line 75, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/zuczek/workspace/slither/slither/slither.py", line 130, in __init__
    parser.analyze_contracts()
  File "/home/zuczek/workspace/slither/slither/solc_parsing/slither_compilation_unit_solc.py", line 519, in analyze_contracts
    self._convert_to_slithir()
  File "/home/zuczek/workspace/slither/slither/solc_parsing/slither_compilation_unit_solc.py", line 713, in _convert_to_slithir
    func.generate_slithir_and_analyze()
  File "/home/zuczek/workspace/slither/slither/core/declarations/function.py", line 1705, in generate_slithir_and_analyze
    node.slithir_generation()
  File "/home/zuczek/workspace/slither/slither/core/cfg/node.py", line 721, in slithir_generation
    self._find_read_write_call()
  File "/home/zuczek/workspace/slither/slither/core/cfg/node.py", line 914, in _find_read_write_call
    raise SlitherException(
slither.exceptions.SlitherException: Function not found on IR: TMP_320(None) = HIGH_LEVEL_CALL, dest:TMP_319(None), function:sig, arguments:['1889567281']  .
Node: EXPRESSION stdstore.target(token).sig(0x70a08231).with_key(to).checked_write(give) (lib/forge-std/src/Test.sol#104-108)
Function: tip
Please try compiling with a recent Solidity version. 'NoneType' object has no attribute 'type'
Error:
Function not found on IR: TMP_320(None) = HIGH_LEVEL_CALL, dest:TMP_319(None), function:sig, arguments:['1889567281']  .
Node: EXPRESSION stdstore.target(token).sig(0x70a08231).with_key(to).checked_write(give) (lib/forge-std/src/Test.sol#104-108)
Function: tip
Please try compiling with a recent Solidity version. 'NoneType' object has no attribute 'type'
Please report an issue to https://github.com/crytic/slither/issues

I have no idea what this exception means. Is it another issue or is the fix incomplete? The most similar exception to this one is in #1295, but it's hard to tell if it's the same problem.

@smonicas
Copy link
Contributor

Hi @CodeSandwich, i can't understand from only this exception if it's related to the fix or not. Could you share the codebase?

@CodeSandwich
Copy link
Contributor Author

Thank you for looking into this! The codebase is here: https://github.com/radicle-dev/drips-contracts

@CodeSandwich
Copy link
Contributor Author

CodeSandwich commented Oct 13, 2022

I've found the root issue. It turns out that Slither has a problem with the Foundry's test utils. Even the exception says that the problem was found in lib/forge-std/src/Test.sol. The solution is to temporarily hide the test directory from Slither:

FOUNDRY_TEST=nonexisten slither .

It's clearly a separate issue, so I've opened a new one: #1422.

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Jan 6, 2023

I've just wanted to use Slither for the first time in a while, and I bumped into this immediately as soon as I run it.

My math library PRBMath makes extensive use of user-defined value types and top-level using for directives - so all of the library users will bump into the same issue.

Is there any ETA for when this will be implemented in Slither?

We would love to be able to run the static analysis at least once before going to production.

@montyly
Copy link
Member

montyly commented Jan 7, 2023

Hi @PaulRBerg . We actually worked on the support this week (#1378, #1563). We encountered a couple of edge cases that slowed us down, but we are aiming for a release at the beginning of next week.

We will try slither on your repo to make sure it passes before the release

@PaulRBerg
Copy link
Contributor

Great news, thanks very much, @montyly!

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Jan 16, 2023

Thanks for implementing this feature, guys!

Unfortunately, we might have to re-open this issue. Slither doesn't work with chained function calls on value types:

pragma solidity >=0.8.17;

import { UD60x18 } from "prb-math/UD60x18.sol";

contract Foo {
    function tryUnwrap(UD60x18 a, UD60x18 b) external pure returns (uint256 c, uint256 d) {
        // This is ok
        c = a.unwrap();
        // This triggers a bug
        d = a.mul(b).unwrap();
    }
}

Defining c works, but d doesn't. That is, I am getting the following error:

Error:
Function not found on IR: TMP_3575(None) = HIGH_LEVEL_CALL, dest:TMP_3574(None), function:unwrap, arguments:[]  .
Node: EXPRESSION d = a.mul(b).unwrap() (src/Foo.sol#9)
Function: tryUnwrap
Please try compiling with a recent Solidity version. 'NoneType' object has no attribute 'type'

I can attach the full stack trace but you should be able to replicate this by spinning up a Foundry project and installing PRBMath (the latest commit from main).

@0xalpharush
Copy link
Contributor

@PaulRBerg I've opened an issue, and we'll get this fixed soon. Thanks for taking the time to report the bug and provide an example as it helps a ton!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request High Priority
Projects
None yet
Development

No branches or pull requests

6 participants