Skip to content

Error when instrumenting an "if" in an assembly block #287

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

Closed
chapati23 opened this issue Sep 7, 2018 · 7 comments
Closed

Error when instrumenting an "if" in an assembly block #287

chapati23 opened this issue Sep 7, 2018 · 7 comments

Comments

@chapati23
Copy link

We're getting an error when trying to instrument one of our contracts that has some assembly:

There was a problem instrumenting ./coverageEnv/contracts/MyContract.sol: SyntaxError: Expected "=:", "for", "function", "hex", "let", "return", "switch", "{", "}", comment, end of line, number, string, or whitespace but "i" found. Line: 29, Column: 7

It fails in the if statement at the bottom.

Any idea why?

  /// @notice Checks if a given address is whitelisted
  /// @return true if address is whitelisted, false if not
  function isWhitelisted
  (
    address _address
  )
    public
    view
    returns (bool _isWhitelisted)
  {
    bytes4 _signature = bytes4(keccak256("whitelisted(address)"));
    address _whitelistContract = getContractAddress("Whitelist");

    assembly {
      let _pointer := mload(0x40)  // Set _pointer to free memory pointer
      mstore(_pointer, _signature) // Store _signature at _pointer
      mstore(add(_pointer, 0x04), _address) // Store _address at _pointer. Offset by 4 bytes for previously stored _signature

      // staticcall(g, a, in, insize, out, outsize) => returns 0 on error, 1 on success
      let result := staticcall(
        gas,                // g = gas: whatever was passed already
        _whitelistContract, // a = address: _whitelist address assigned from getContractAddress()
        _pointer,           // in = mem in  mem[in..(in+insize): set to _pointer pointer
        0x24,               // insize = mem insize  mem[in..(in+insize): size of signature (bytes4) + bytes32 = 0x24
        _pointer,           // out = mem out  mem[out..(out+outsize): output assigned to this storage address
        0x20                // outsize = mem outsize  mem[out..(out+outsize): output should be 32byte slot (bool size = 0x01 < slot size 0x20)
      )

      // Revert if not successful
      if iszero(result) {
        revert(0, 0)
      }

      _isWhitelisted := mload(_pointer) // Assign result to returned value
      mstore(0x40, add(_pointer, 0x24)) // Advance free memory pointer by largest _pointer size
    }
  }
@cgewecke
Copy link
Member

cgewecke commented Sep 7, 2018

@chapati23 It looks like the parser needs to be updated to handle some of the newer assembly syntax. Will fix this weekend, thanks for reporting.

@thekevinbrown
Copy link

Hey @cgewecke I'm hitting this issue too, wondering about how the fix is going. Any news? Anything I can do to help?

@0age
Copy link

0age commented Oct 10, 2018

If you have the flexibility to change the inline assembly a bit, a workaround is to use a switch statement instead. So:

      if iszero(result) {
        revert(0, 0)
      }

becomes:

      switch iszero(result)
      case 1 {
        revert(0, 0)
      }

@patitonar
Copy link

is there any plan to address this issue?

chancehudson added a commit to common-theory/contracts that referenced this issue Nov 13, 2018
@leanthebean
Copy link

Ran into this on the latest version also

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Jan 15, 2019

Any updates on this? Or a fork that addresses the issue?

@cgewecke
Copy link
Member

This is finally fixed in 0.6.0

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

No branches or pull requests

7 participants