@kbeyls FYI

@elhewaty, I believe you're working on a fix for this?

Yes, I am working on this, Sorry for my late reply. I was thinking of using CallGraph class and CallGraphWalker class to build a pass that mark all no-return functions and cash the results in some std::unordered_map.

Here are some thoughts:
1 - Do not build the graph if the no-return functions were not called [__cxa_throw@PLT _Unwind_Resume@PLT __cxa_rethrow@PLT exit@PLT abort@PLT] for ELF at least. I didn't find an something like getBinaryFunction(string &) ] in the Interface, so I think I should iterate over all functions, so this is not useful.

2 - using the call graph and the call graph walker we can start traversing the call graph in any order(I guess), although the the traversal in the walker is topological, but it's suffecient enough for this purpose as it has a worklist that keeps track of the caller function if the callee is later marked as a no-return.

3 - As mentioned, the walker visits every node to ensure some property using the callback function. we can make the callback function a DFS traversal and go from it to see if all it's successors end with a no-return call and mark it as a no-return [I think using this will make updating the worklist unnecessary, as we will visit the leaves of the graph at the first node and cash the results of each node, so when we traverse a visited node from non-visited one the result was cached from before so the traversing stops]. if we go with this we will not need the walker -I guess-

the code will be something like this:

unordered_map<Node, uint8_t> color; // 0 --> not visited, 1 ---> still there (cycle), 2 ---> traversed
unordered_map<Node, bool> memo;
bool dfs(Node cur) {
  if (isNoReturn(cur)) return true;
  if (memo.count(cur)) return memo[cur];
  if (isLeaf(cur)) return false;
  memo[cur] = 1; // assume it's no-return
  color[cur] = 1;
  for (every successor S of cur) {
    if (!color.count(S))
       memo[cur] &= dfs(S);
  }
  return memo[cur];
}

// pass
for every node N in the CG
  dfs(N)

So what do you think?

Some thoughts:

• fixing this is made of two different steps: working out which functions are noreturn, then modifying CFG to account for these calls.
• because of this, I think it would be nice to store isNoReturn on the BinaryFunctions themselves (instead of a cache), so the information can be used in any other way as well later on

I didn't find an something like getBinaryFunction(string &) ] in the Interface

I have also looked at this, and found we can get the BinaryFunction that is used in a call this way:

const MCSymbol* TargetSymbol = BC.MIB->getTargetSymbol(Inst); // Inst is a call
BinaryFunction* TargetFunction = BC.SymbolToFunctionMap[TargetSymbol];

we can make the callback function a DFS traversal and go from it to see if all it's successors end with a no-return call and mark it as a no-return

So are you saying, if a function only calls noreturn functions then itself is noreturn as well?

If so, I agree. But for leaf functions we need to check that they have no returns (obviously), and I think they should end in an instruction that is a terminator.