Merge pull request #650 from ethereum/fixStaticAnalysis

Static Analysis: Fix bug when passing function as parameter

remix-solidity/src/analysis/modules/staticAnalysisCommon.js

@@ -278,7 +278,22 @@ function getFunctionOrModifierDefinitionReturnParameterPart (funcNode) {
  * @return {string} parameter signature
  */
 function getFunctionCallTypeParameterType (func) {
-  return new RegExp(basicRegex.FUNCTIONSIGNATURE).exec(getFunctionCallType(func))[1]
+  var type = getFunctionCallType(func)
+  if (type.startsWith('function (')) {
+    var paramTypes = ''
+    var openPar = 1
+    for (var x = 10; x < type.length; x++) {
+      var c = type.charAt(x)
+      if (c === '(') openPar++
+      else if (c === ')') openPar--
+
+      if (openPar === 0) return paramTypes
+
+      paramTypes += c
+    }
+  } else {
+    throw new Error('staticAnalysisCommon.js: cannot extract parameter types from function call')
+  }
 }
 
 /**

remix-solidity/test/analysis/staticAnalysisCommon-test.js

@@ -1912,3 +1912,185 @@ test('staticAnalysisCommon.isLowLevelCall', function (t) {
   t.notOk(common.isLowLevelCallcodeInst(callAst), 'call is not callcode')
   t.ok(common.isLowLevelDelegatecallInst(delegatecallAst) && common.isLowLevelCall(delegatecallAst), 'delegatecall is llc should work')
 })
+
+test('staticAnalysisCommon: Call of parameter function', function (t) {
+  t.plan(7)
+  var node1 = {
+    'attributes': {
+      'argumentTypes': null,
+      'isConstant': false,
+      'isLValue': false,
+      'isPure': false,
+      'isStructConstructorCall': false,
+      'lValueRequested': false,
+      'names': [
+        null
+      ],
+      'type': 'uint256',
+      'type_conversion': false
+    },
+    'children': [
+      {
+        'attributes': {
+          'argumentTypes': [
+            {
+              'typeIdentifier': 't_uint256',
+              'typeString': 'uint256'
+            },
+            {
+              'typeIdentifier': 't_uint256',
+              'typeString': 'uint256'
+            }
+          ],
+          'overloadedDeclarations': [
+            null
+          ],
+          'referencedDeclaration': 25,
+          'type': 'function (uint256,uint256) pure returns (uint256)',
+          'value': 'f'
+        },
+        'id': 34,
+        'name': 'Identifier',
+        'src': '267:1:0'
+      },
+      {
+        'attributes': {
+          'argumentTypes': null,
+          'overloadedDeclarations': [
+            null
+          ],
+          'referencedDeclaration': 27,
+          'type': 'uint256',
+          'value': 'x'
+        },
+        'id': 35,
+        'name': 'Identifier',
+        'src': '269:1:0'
+      },
+      {
+        'attributes': {
+          'argumentTypes': null,
+          'overloadedDeclarations': [
+            null
+          ],
+          'referencedDeclaration': 29,
+          'type': 'uint256',
+          'value': 'y'
+        },
+        'id': 36,
+        'name': 'Identifier',
+        'src': '272:1:0'
+      }
+    ],
+    'id': 37,
+    'name': 'FunctionCall',
+    'src': '267:7:0'
+  }
+
+  t.ok(common.isLocalCall(node1), 'is not LocalCall')
+  t.notOk(common.isThisLocalCall(node1), 'is not this local call')
+  t.notOk(common.isSuperLocalCall(node1), 'is not super local call')
+  t.notOk(common.isExternalDirectCall(node1), 'is not ExternalDirectCall')
+  t.notOk(common.isLibraryCall(node1), 'is not LibraryCall')
+
+  t.equals(common.getFunctionCallType(node1), 'function (uint256,uint256) pure returns (uint256)', 'Extracts right type')
+
+  t.equals(common.getFunctionCallTypeParameterType(node1), 'uint256,uint256', 'Extracts param right type')
+})
+
+test('staticAnalysisCommon: function call with of function with function parameter', function (t) {
+  t.plan(2)
+  var node1 = {
+    'attributes': {
+      'argumentTypes': null,
+      'isConstant': false,
+      'isLValue': false,
+      'isPure': false,
+      'isStructConstructorCall': false,
+      'lValueRequested': false,
+      'names': [
+        null
+      ],
+      'type': 'uint256',
+      'type_conversion': false
+    },
+    'children': [
+      {
+        'attributes': {
+          'argumentTypes': [
+            {
+              'typeIdentifier': 't_function_internal_pure$_t_uint256_$_t_uint256_$returns$_t_uint256_$',
+              'typeString': 'function (uint256,uint256) pure returns (uint256)'
+            },
+            {
+              'typeIdentifier': 't_uint256',
+              'typeString': 'uint256'
+            },
+            {
+              'typeIdentifier': 't_uint256',
+              'typeString': 'uint256'
+            }
+          ],
+          'overloadedDeclarations': [
+            null
+          ],
+          'referencedDeclaration': 40,
+          'type': 'function (function (uint256,uint256) pure returns (uint256),uint256,uint256) pure returns (uint256)',
+          'value': 'eval'
+        },
+        'id': 49,
+        'name': 'Identifier',
+        'src': '361:4:0'
+      },
+      {
+        'attributes': {
+          'argumentTypes': null,
+          'overloadedDeclarations': [
+            null
+          ],
+          'referencedDeclaration': 15,
+          'type': 'function (uint256,uint256) pure returns (uint256)',
+          'value': 'plus'
+        },
+        'id': 50,
+        'name': 'Identifier',
+        'src': '366:4:0'
+      },
+      {
+        'attributes': {
+          'argumentTypes': null,
+          'overloadedDeclarations': [
+            null
+          ],
+          'referencedDeclaration': 42,
+          'type': 'uint256',
+          'value': 'x'
+        },
+        'id': 51,
+        'name': 'Identifier',
+        'src': '372:1:0'
+      },
+      {
+        'attributes': {
+          'argumentTypes': null,
+          'overloadedDeclarations': [
+            null
+          ],
+          'referencedDeclaration': 44,
+          'type': 'uint256',
+          'value': 'y'
+        },
+        'id': 52,
+        'name': 'Identifier',
+        'src': '375:1:0'
+      }
+    ],
+    'id': 53,
+    'name': 'FunctionCall',
+    'src': '361:16:0'
+  }
+
+  t.equals(common.getFunctionCallType(node1), 'function (function (uint256,uint256) pure returns (uint256),uint256,uint256) pure returns (uint256)', 'Extracts right type')
+
+  t.equals(common.getFunctionCallTypeParameterType(node1), 'function (uint256,uint256) pure returns (uint256),uint256,uint256', 'Extracts param right type')
+})

remix-solidity/test/analysis/staticAnalysisIssues-test.js

@@ -0,0 +1,35 @@
+var test = require('tape')
+var remixLib = require('remix-lib')
+
+var StatRunner = require('../../src/analysis/staticAnalysisRunner')
+var compilerInput = remixLib.helpers.compiler.compilerInput
+
+var solc = require('solc/wrapper')
+var compiler = solc(require('../../soljson'))
+
+var fs = require('fs')
+var path = require('path')
+
+function compile (fileName) {
+  var content = fs.readFileSync(path.join(__dirname, 'test-contracts', fileName), 'utf8')
+  return JSON.parse(compiler.compileStandardWrapper(compilerInput(content)))
+}
+
+test('staticAnalysisIssues.functionParameterPassingError', function (t) {
+  // https://github.com/ethereum/browser-solidity/issues/889#issuecomment-351746474
+  t.plan(2)
+  var res = compile('functionParameters.sol')
+
+  var module = require('../../src/analysis/modules/checksEffectsInteraction')
+
+  var statRunner = new StatRunner()
+
+  t.doesNotThrow(() => {
+    statRunner.runWithModuleList(res, [{ name: module.name, mod: new module.Module() }], (reports) => {
+    })
+  }, true, 'Analysis should not throw')
+
+  statRunner.runWithModuleList(res, [{ name: module.name, mod: new module.Module() }], (reports) => {
+    t.ok(!reports.some((mod) => mod.report.some((rep) => rep.warning.includes('INTERNAL ERROR')), 'Should not have internal errors'))
+  })
+})

remix-solidity/test/analysis/test-contracts/functionParameters.sol

@@ -0,0 +1,16 @@
+pragma solidity ^0.4.18;
+
+contract B {
+    function plus(uint a, uint b) pure internal returns (uint) {
+        return a + b;
+    }
+
+    function eval(function (uint, uint) pure internal returns (uint) f, uint x, uint y) pure internal returns (uint) {
+        return f(x, y);
+    }
+
+    function calc(uint x, uint y) pure public returns (uint) {
+        return eval(plus, x, y);
+        // return plus(x, y);
+    }
+}

remix-solidity/test/decoder/localDecoder.js

@@ -32,7 +32,9 @@ function test (st, vm, privateKey) {
       misc2LocalTest(st, vm, privateKey, output.contracts['test.sol']['miscLocal2'].evm.bytecode.object, output, function () {
         output = compiler.compileStandardWrapper(compilerInput(structArrayLocal.contract))
         output = JSON.parse(output)
-        structArrayLocalTest(st, vm, privateKey, output.contracts['test.sol']['structArrayLocal'].evm.bytecode.object, output, function () {})
+        structArrayLocalTest(st, vm, privateKey, output.contracts['test.sol']['structArrayLocal'].evm.bytecode.object, output, function () {
+          st.end()
+        })
       })
     })
   })

remix-solidity/test/decoder/stateTests/mapping.js

@@ -17,12 +17,10 @@ module.exports = function testMappingStorage (st, cb) {
     } else {
       remixLib.global.web3.eth.getTransaction(txHash, (error, tx) => {
         if (error) {
+          console.log(error)
           st.end(error)
         } else {
-          testMapping(st, vm, privateKey, tx.contractAddress, output, (error) => {
+          testMapping(st, vm, privateKey, tx.contractAddress, output, cb)
-            st.end(error)
-            cb()
-          })
         }
       })
     }
@@ -39,6 +37,7 @@ function testMapping (st, vm, privateKey, contractAddress, output, cb) {
             console.log(txHash)
             remixLib.global.web3.eth.getTransaction(txHash, (error, tx) => {
               if (error) {
+                console.log(error)
                 st.end(error)
               } else {
                 var TraceManager = require('remix-core').trace.TraceManager
@@ -62,7 +61,7 @@ function testMapping (st, vm, privateKey, contractAddress, output, cb) {
                     st.equal(result['_iBreakSolidityStateInt'].type, 'mapping(uint256 => uint256)')
                     st.equal(result['_iBreakSolidityStateInt'].value['0000000000000000000000000000000000000000000000000000000000000001'].value, '1')
                     st.equal(result['_iBreakSolidityStateInt'].value['0000000000000000000000000000000000000000000000000000000000000001'].type, 'uint256')
-                    // st.end()
+                    cb()
                   }, (reason) => {
                     console.log('fail')
                     st.end(reason)

remix-solidity/test/decoder/vmCall.js

@@ -3,6 +3,7 @@ var utileth = require('ethereumjs-util')
 var Tx = require('ethereumjs-tx')
 var Block = require('ethereumjs-block')
 var BN = require('ethereumjs-util').BN
+var remixLib = require('remix-lib')
 
 function sendTx (vm, from, to, value, data, cb) {
   var tx = new Tx({
@@ -33,7 +34,6 @@ function sendTx (vm, from, to, value, data, cb) {
   Init VM / Send Transaction
 */
 function initVM (st, privateKey) {
-  var remixLib = require('remix-lib')
   var utileth = require('ethereumjs-util')
   var VM = require('ethereumjs-vm')
   var Web3Providers = remixLib.vm.Web3Providers
@@ -52,7 +52,6 @@ function initVM (st, privateKey) {
       st.fail(mes)
     } else {
       remixLib.global.web3 = obj
-      st.end()
     }
   })
   return vm

remix-solidity/test/tests.js

@@ -1,7 +1,8 @@
-// require('./decoder/decodeInfo.js')
+require('./decoder/decodeInfo.js')
-// require('./decoder/storageLocation.js')
+require('./decoder/storageLocation.js')
 require('./decoder/storageDecoder.js')
-// require('./decoder/localDecoder.js')
+require('./decoder/localDecoder.js')
 
-// require('./analysis/staticAnalysisCommon-test.js')
+require('./analysis/staticAnalysisCommon-test.js')
-// require('./analysis/staticAnalysisIntegration-test.js')
+require('./analysis/staticAnalysisIntegration-test.js')
+require('./analysis/staticAnalysisIssues-test.js')