CFGFast: Do not judge a function by the block past a non-returning call

drop_bad_functions() deletes a short non-returning function when one of its
blocks has no successors and the bytes right after that block are classified as
data or as nodecode. The comment says the test is for a block that "might be
size-limited during CFGNode generation", but the code never checks that anything
cut the block short, and being followed by data is what the last block of every
real function looks like.

The block that trips it is nearly always the one CFGFast recovers past a call.
The callee's returning status is usually still unknown when the scan reaches the
call site, so CFGFast adds an Ijk_FakeRet edge over the call and recovers
whatever follows. When the callee never comes back, that is padding -- MSVC's
int3, the TOC restore GCC emits after a PowerPC64 bl -- and the alignment behind
it says nothing about the function.

Ignore such a block, and only when the call has a recovered callee with blocks
of its own: a stray int or syscall in decoded data gets a fall-through edge too,
and there the block really is the tail of something that was never code.

angr/analyses/cfg/cfg_fast.py

@@ -4697,6 +4697,34 @@ class CFGFast(ForwardAnalysis[CFGNode, CFGNode, CFGJob, int, object], CFGBase):
         # if node.addr in self.kb.functions.callgraph:
         #    self.kb.functions.callgraph.remove_node(node.addr)
 
+    def _reached_only_by_call_fallthrough(self, cfg_node: CFGNode) -> bool:
+        """
+        Is this block reachable only as the fall-through of a call to a function we recovered?
+
+        Whatever a compiler leaves after a call it treats as non-returning is not executed: MSVC pads with
+        ``int3``, GCC emits the TOC restore after every PowerPC64 ``bl``, and both are followed by the
+        alignment of the next function. CFGFast recovers that block anyway, because the callee's returning
+        status is usually still unknown when the scan reaches the call site. What is beyond such a block says
+        nothing about whether the function it hangs off was decoded out of data.
+
+        The callee has to be a function with blocks of its own. A stray ``int`` or ``syscall`` in decoded data
+        gets a fall-through edge too, and so does a call whose target is not in the image; a block behind one of
+        those is the tail of a decode rather than the padding after a call.
+        """
+        graph = self.model.graph
+        in_edges = list(graph.in_edges(cfg_node, data=True))
+        if not in_edges:
+            return False
+        for src, _, data in in_edges:
+            if data.get("jumpkind") != "Ijk_FakeRet":
+                return False
+            if not any(
+                edge_data.get("jumpkind") == "Ijk_Call" and self.kb.functions.get_func_block_count(dst.addr)
+                for _, dst, edge_data in graph.out_edges(src, data=True)
+            ):
+                return False
+        return True
+
     def drop_bad_functions(self):
         # remove all functions that are bad, i.e., likely the result of decoding data as code
         # - if a function jumps to data, then it's likely bad
@@ -4735,6 +4763,8 @@ class CFGFast(ForwardAnalysis[CFGNode, CFGNode, CFGJob, int, object], CFGBase):
                 cfg_node = self.model.get_any_node(block_addr)
                 if cfg_node is not None and cfg_node.size > 0:
                     out_degree = self.model.graph.out_degree[cfg_node]
+                    if out_degree < 2 and self._reached_only_by_call_fallthrough(cfg_node):
+                        continue
                     # is it jumping to data?
                     if out_degree == 0:
                         # might be size-limited during CFGNode generation; check the location after the end of the block

tests/analyses/cfg/test_cfgfast.py

@@ -1031,6 +1031,31 @@ class TestCfgfast(unittest.TestCase):
 
         assert len(cfg.kb.functions) < 150, f"32 KB of random data produced {len(cfg.kb.functions)} functions"
 
+    def test_msvc_function_ending_in_a_noreturning_call_is_kept(self):
+        # each of these five ends in a call MSVC treats as non-returning, and the block CFGFast recovers past
+        # that call is the single int3 MSVC leaves there. drop_bad_functions() used to read the run of int3
+        # padding that follows as the function running into data and delete the whole function; the image's own
+        # exception directory names all five.
+        proj = angr.Project(os.path.join(test_location, "x86_64", "windows", "ipnathlp.dll"), auto_load_libs=False)
+        cfg = proj.analyses.CFGFast(normalize=True)
+
+        for addr in (0x180004D20, 0x18001A3EC, 0x18001FDFC, 0x180024064, 0x180024080):
+            assert addr in cfg.kb.functions, f"{addr:#x} was dropped"
+            assert cfg.model.get_any_node(addr) is not None, f"no block covers {addr:#x}"
+        # the one-block int3 the linear scan picked up out of the padding is still not a function
+        assert 0x180004681 not in cfg.kb.functions
+
+    def test_ppc64_function_ending_in_a_noreturning_call_is_kept(self):
+        # rejected(): puts() and then exit(). Its .opd descriptor at 0x10010e20 puts it at 0x100007bc with a
+        # size of 60, so all three blocks below are inside it. GCC emits the TOC restore after the bl to exit()
+        # and pads the rest of the section with zeroes, so the block past that call runs into bytes that do not
+        # decode -- which said nothing about the function, and cost it all three blocks.
+        proj = angr.Project(os.path.join(test_location, "ppc64", "fauxware"), auto_load_libs=False)
+        cfg = proj.analyses.CFGFast(normalize=True)
+
+        assert 0x100007BC in cfg.kb.functions
+        assert {0x100007BC, 0x100007DC, 0x100007E8} <= cfg.kb.functions[0x100007BC].block_addrs_set
+
 
 if __name__ == "__main__":
     unittest.main()