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

fix: Make IR set type of reference to popped element correctly #43

Closed
wants to merge 46 commits into from

Conversation

kevinclancy
Copy link

@kevinclancy kevinclancy commented May 18, 2023

Notes

The IR generated for calls to pop (for dynamic array types and bytes) would obtain a reference to the last element of the array and then delete it. But the type of this reference was always uint256 even though it should have been the type of the array element.

Now, we set the reference's type to the array element type. For bytes, we set the reference's type to bytes1.

Trail o' Bits developed a fix for this, but I think they used the wrong element type for bytes. It should be bytes1 but they use bytes. I will file an issue with them about this.

Testing

  • Run make test and verify all tests succeed.
  • Run ./evaluate.sh run 100 in tool-eval and verify all projects succeed
  • Run pipenv run slither test.sol --print slithir, where test.sol contains the following:
pragma solidity ^0.8.13;

contract Poptest {
    struct S {
        int x;
        int y;
    }
    S[] a;
    function poptest() internal {
        a.pop();
    }
}

Examine the IR and verify that the deleted reference variable has the correct type. Also, try changing the array a's type to bytes.

Related Issue

https://github.com/CertiKProject/slither-task/issues/501)

Additional Comments

I will file an issue in the Slither repo about handling bytes incorrectly.

Duckki Oe and others added 30 commits March 4, 2022 12:07
* populate a boolean flag in the Contract AST during solc AST parsing
* create two parsers and compilation units: one using the new certik ir and the other using slithir

https://github.com/CertiKProject/slither-task/issues/217
* only generate default values for variables with 'memory' and 'default' location
check that the type of variable is LocalVariable instead of using a _from_statement variable

https://github.com/CertiKProject/slither-task/issues/200
…21)

* add implicit initialization for return variables
* only assign default return value if location is memory (or default)
* add implicit return statement if they are missing
Previously, all detectors were incorrectly receiving certik ir because the uses_certik_ir property was being read from the detector class as a "truthy" function. This PR fixes the issue by changing uses_certik_ir to a static method.
…ecting condition node to false-statement node (#23)

- fixes the issue with wrong if-statement CFG when the then-clause is empty.

https://github.com/CertiKProject/slither-task/issues/241
…e array and generate SlithIR accordingly (#26)

CertiKProject/slither-task#212
Duckki Oe and others added 16 commits February 10, 2023 13:37
https://github.com/CertiKProject/slither-task/issues/327

* feat: add constant folding to IR
* added certik ir flag to SlitherCompilationUnit
- If `has_receiver_arg` is true, the call was resolved via using-for and has a receiver object expression as its first argument.
@kevinclancy kevinclancy requested a review from duckki May 18, 2023 00:29
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