CFGWalker with Single-thread failed

[XMadrid]

When use CFGWalker to walk module with single thread, get a double-free fault.

I found we introduced two varaible: BasicBlock* exit = nullptr; and bool hasSyntheticExit = false; in src/cfg/cfg-traversal.h. And did not reset them in doWalkFunction.
In this case, the two variables from the previous function will still be used when we walk the subsequent function.

[kripken]

In general we do not reset such global state in those classes. Typically they are used with isFunctionParallel = true in which case a new instance is created each time (even in a single-threaded build, we should be creating new instances even if they do not actually run in parallel, for compatibility).

If you use them in a non-parallel way then I can see how there would be a problem, and this is in cfg-walker.h so it does imply that all users should be aware of this, which is kind of annoying, sorry about that. To improve this perhaps we could mark the class as function-parallel, though we'd need to check if we have any other uses in the codebase.

Alternatively we could reset those fields, but that would just add code size and work for a situation that function-parallel code is meant to avoid, so I'd lean against that.

[XMadrid]

Before this PR #6079, we can use CFGWalker with isFunctionParallel = False. I think we should keep the forward compatibility.

In fact, it may not be reset. In src/cfg/cfg-traversal.h line 548 - 577 :

  void doWalkFunction(Function* func) {
    basicBlocks.clear();
    debugIds.clear();
+  exit = nullptr;                          // It is similar to how we handle basicBlocks, debugIds and entry.
+  hasSyntheticExit = false;       

    startBasicBlock();
    entry = currBasicBlock;
    ...
  }

[kripken]

I see, thanks @XMadrid . Yes, I guess that would be more consistent. Would you like to open a PR with it?

[XMadrid]

Sure. I will do this.