Drop unreachable blocks after inlining

Summary:
The HIR inliner could crash when inlining a callee that has no reachable
`return`, for example a `while True:` loop whose only exit is an exception.

`HIRBuilder::inlineHIR` builds the callee CFG but, unlike the normal `buildHIR`
path, does not run `removeUnreachableBlocks`.  When the callee has no reachable
return, the merged exit block (and the surrounding return-value
`Assign`/`Branch` blocks produced by `inlineFunctionCall`) end up with no
predecessors and are left in the caller's CFG.  `InlineFunctionCalls::Run` then
ran `CopyPropagation` and `CleanCFG` over those stale blocks; `CleanCFG` ->
`PhiElimination` -> `chaseAssignOperand` dereferenced instructions that had
already been freed during inlining, segfaulting inside
`InlineFunctionCalls::Run`.

Fixes #120.

Reviewed By: mpage

Differential Revision: D110056992

fbshipit-source-id: a0c450ad7c414d092f167bec2e1de4ea039519f4

Author: alexmalyshev

cinderx/Jit/hir/inliner.cpp

@@ -608,6 +608,7 @@ void InlineFunctionCalls::Run(Function& irfunc) {
   // unreachable and therefore make less work (less to inline), we cannot remove
   // unreachable blocks in the above loop.  It might delete instructions pointed
   // to by `calls`.
+  removeUnreachableBlocks(irfunc);
   CopyPropagation{}.Run(irfunc);
   CleanCFG{}.Run(irfunc);
 }

cinderx/PythonLib/test_cinderx/test_jit_inliner.py

@@ -190,6 +190,74 @@ def caller_of_warm_branch(x: int, take_cold: bool) -> int:
     return warm_branch(x, take_cold) + 1
 
 
+# Regression model for a JIT inliner crash.  `flag_set_loop` is a `while True:` loop
+# whose only exit is the KeyError raised by the lookup.  There is no reachable `return`
+# inside the loop so when it is inlined its merged exit block has no predecessors and is
+# unreachable.  The inliner used to run CopyPropagation and CleanCFG over that stale
+# block (left behind because it can't be removed during the inlining loop), which
+# dereferenced freed instructions and crashed.
+_FLAG_VALUES = {"a": 1, "b": 2, "i": 4, "L": 8}
+_LOCALE = 8
+_GLOBAL = 16
+
+
+class FlagSource:
+    def __init__(self, chars: str) -> None:
+        self._chars = chars
+        self.pos = 0
+
+    def get(self) -> str:
+        ch = self._chars[self.pos]
+        self.pos += 1
+        return ch
+
+    def match(self, ch: str) -> bool:
+        if self.pos < len(self._chars) and self._chars[self.pos] == ch:
+            self.pos += 1
+            return True
+        return False
+
+
+class FlagInfo:
+    inline_locale: bool = False
+
+
+def flag_set_loop(source: FlagSource) -> int:
+    flags = 0
+    saved_pos = 0
+    try:
+        while True:
+            saved_pos = source.pos
+            ch = source.get()
+            flags |= _FLAG_VALUES[ch]
+    except KeyError:
+        source.pos = saved_pos
+    return flags
+
+
+def parse_flags(source: FlagSource, info: FlagInfo) -> tuple[int, int]:
+    flags_on = flag_set_loop(source)
+    if source.match("-"):
+        flags_off = flag_set_loop(source)
+        if not flags_off:
+            raise ValueError("no flags after '-'")
+    else:
+        flags_off = 0
+    if flags_on & _LOCALE:
+        info.inline_locale = True
+    return flags_on, flags_off
+
+
+def parse_flags_subpattern(source: FlagSource, info: FlagInfo) -> tuple[int, int]:
+    flags_on, flags_off = parse_flags(source, info)
+    if flags_off & _GLOBAL:
+        raise ValueError("cannot turn off global flag")
+    if flags_on & flags_off:
+        raise ValueError("flag turned on and off")
+    flags_on &= ~_GLOBAL
+    return flags_on, flags_off
+
+
 @passUnless(INLINER, "Testing the inliner")
 class InlinedFunctionTests(unittest.TestCase):
     @jit_suppress
@@ -365,6 +433,27 @@ def test_inline_function_stats(self) -> None:
             next(iter(has_varargs)),
         )
 
+    @jit_suppress
+    def test_inlining_callee_without_reachable_return(self) -> None:
+        """Inlining a function whose only loop exit is an exception (so it has
+        no reachable return) leaves an unreachable exit block in the caller.
+        The inliner must drop that block before running CopyPropagation/CleanCFG;
+        otherwise those passes walk freed instructions and crash."""
+        cinderx.jit.force_compile(parse_flags_subpattern)
+        self.assertTrue(cinderx.jit.is_jit_compiled(parse_flags_subpattern))
+
+        # parse_flags is inlined, and flag_set_loop is inlined transitively
+        # through it (once for flags_on, and the source has no "-" so flags_off
+        # stays a constant), exercising the unreachable-exit-block path.
+        self.assertGreater(
+            cinderx.jit.get_num_inlined_functions(parse_flags_subpattern), 1
+        )
+
+        # "abiX" turns on flags a|b|i = 7; the trailing "X" is not a flag, so
+        # the lookup raises KeyError and ends the loop.  There is no "-", so
+        # flags_off is 0.
+        self.assertEqual(parse_flags_subpattern(FlagSource("abiX"), FlagInfo()), (7, 0))
+
     @jit_suppress
     def test_line_numbers_with_multiple_inlined_calls(self) -> None:
         """Verify that line numbers are correct for inlined calls that appear