Merge pull request #4061 from trufflesuite/stop-the-pop

Bug fix: Prevent function argument corruption after bare block

packages/debugger/lib/data/sagas/index.js

@@ -363,6 +363,15 @@ function* variablesAndMappingsSaga() {
       //the rest hopefully will be caught by the modifier preamble
       //(in fact they won't all be, but...)
 
+      //HACK: prevent parameter allocation while popping
+      //sometimes Solidity's sourcemapping will jump back to the function
+      //definition after a bare block while it pops the stack a bit.
+      //we don't want to allocate then, so we'll break out if the current
+      //instruction is a POP.
+      if (yield select(data.current.isPop)) {
+        break;
+      }
+
       //HACK: filter out some garbage
       //this filters out the case where we're really in an invocation of a
       //modifier or base constructor, but have temporarily hit the definition

packages/debugger/lib/data/selectors/index.js

@@ -1507,7 +1507,12 @@ const data = createSelectorTree({
     callContext: createLeaf(
       [evm.current.step.callContext],
       debuggerContextToDecoderContext
-    )
+    ),
+
+    /**
+     * data.current.isPop
+     */
+    isPop: createLeaf([evm.current.step.isPop], identity)
   },
 
   /**

packages/debugger/lib/evm/selectors/index.js

@@ -157,7 +157,13 @@ function createStepSelectors(step, state = null) {
     touchesStorage: createLeaf(
       ["./isStore", "isLoad"],
       (stores, loads) => stores || loads
-    )
+    ),
+
+    /*
+     * .isPop
+     * used by data
+     */
+    isPop: createLeaf(["./trace"], step => step.op === "POP")
   };
 
   if (state) {

packages/debugger/test/data/ids.js

@@ -148,13 +148,31 @@ contract ModifierTest {
 }
 `;
 
+const __POPQUIZ = `
+pragma solidity ^0.8.0;
+
+contract PopTest {
+
+  event Here(uint);
+
+  function run(uint x) public {
+    {
+      uint something = 3;
+      emit Here(something);
+    }
+    emit Here(x); //BREAK
+  }
+}
+`;
+
 const __MIGRATION = `
 let Intervening = artifacts.require("Intervening");
 let Inner = artifacts.require("Inner");
 let AddressTest = artifacts.require("AddressTest");
 let FactorialTest = artifacts.require("FactorialTest");
 let InterveningLib = artifacts.require("InterveningLib");
 let ModifierTest = artifacts.require("ModifierTest");
+let PopTest = artifacts.require("PopTest");
 
 module.exports = async function(deployer) {
   await deployer.deploy(InterveningLib);
@@ -165,6 +183,7 @@ module.exports = async function(deployer) {
   await deployer.deploy(AddressTest);
   await deployer.deploy(FactorialTest);
   await deployer.deploy(ModifierTest);
+  await deployer.deploy(PopTest);
 };
 `;
 
@@ -173,7 +192,8 @@ let sources = {
   "AddressTest.sol": __ADDRESS,
   "Intervening.sol": __INTERVENING,
   "InterveningLib.sol": __INTERVENINGLIB,
-  "ModifierTest.sol": __MODIFIERS
+  "ModifierTest.sol": __MODIFIERS,
+  "PopTest.sol": __POPQUIZ
 };
 
 let migrations = {
@@ -310,4 +330,27 @@ describe("Variable IDs", function () {
     await bugger.continueUntilBreakpoint();
     assert.property(await bugger.variables(), "flag");
   });
+
+  it("Is not thrown off by bare blocks", async function () {
+    this.timeout(3000);
+    let instance = await abstractions.PopTest.deployed();
+    let receipt = await instance.run(107);
+    let txHash = receipt.tx;
+
+    let bugger = await Debugger.forTx(txHash, { provider, compilations });
+
+    debug("sourceId %d", bugger.view(solidity.current.source).id);
+
+    let sourceId = bugger.view(solidity.current.source).id;
+    let source = bugger.view(solidity.current.source).source;
+    await bugger.addBreakpoint({
+      sourceId,
+      line: lineOf("BREAK", source)
+    });
+    await bugger.continueUntilBreakpoint();
+    assert.strictEqual(
+      Codec.Format.Utils.Inspect.unsafeNativize(await bugger.variable("x")),
+      107
+    );
+  });
 });