[SILOptimizer] Don't diagnose infinite recursion if a branch terminates the program (#19781)

* [SILOptimizer] Don't diagnose infinite recursion if a branch terminates the program

This patch augments the infinite recursion checker to not warn if a
branch terminates, but still warns if a branch calls into something with
@_semantics("programtermination_point"). This way, calling fatalError
doesn't disqualify you for the diagnostic, but calling exit does.

This also removes the warning workaround in the standard library, and
annotates the internal _assertionFailure functions as
programtermination_points, so they get this treatment too.

* Fix formatting in SILInstructions.cpp

* Re-add missing test

Author: Harlan

docs/ARCOptimization.rst

@@ -373,7 +373,7 @@ Semantic Tags
 ARC takes advantage of certain semantic tags. This section documents these
 semantics and their meanings.
 
-arc.programtermination_point
+programtermination_point
 ----------------------------
 
 If this semantic tag is applied to a function, then we know that:
@@ -396,7 +396,7 @@ scope's lifetime may never end. This means that:
 
 1. While we can not ignore all such unreachable terminated blocks for ARC
 purposes for instance, if we sink a retain past a br into a non
-arc.programtermination_point block, we must sink the retain into the block.
+programtermination_point block, we must sink the retain into the block.
 
 2. If we are able to infer that an object's lifetime scope would never end due
 to the unreachable/no-return function, then we do not need to end the lifetime

include/swift/SIL/ApplySite.h

@@ -171,6 +171,12 @@ class ApplySite {
     return SILFunctionConventions(getSubstCalleeType(), getModule());
   }
 
+  /// Returns true if the callee function is annotated with
+  /// @_semantics("programtermination_point")
+  bool isCalleeKnownProgramTerminationPoint() const {
+    FOREACH_IMPL_RETURN(isCalleeKnownProgramTerminationPoint());
+  }
+
   /// Check if this is a call of a never-returning function.
   bool isCalleeNoReturn() const { FOREACH_IMPL_RETURN(isCalleeNoReturn()); }
 

include/swift/SIL/SILBasicBlock.h

@@ -370,7 +370,7 @@ public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock> {
   /// no-return apply or builtin.
   bool isNoReturn() const;
 
-  /// Returns true if this instruction only contains a branch instruction.
+  /// Returns true if this block only contains a branch instruction.
   bool isTrampoline() const;
 
   /// Returns true if it is legal to hoist instructions into this block.

include/swift/SIL/SILInstruction.h

@@ -35,6 +35,7 @@
 #include "swift/SIL/SILLocation.h"
 #include "swift/SIL/SILSuccessor.h"
 #include "swift/SIL/SILValue.h"
+#include "swift/Strings.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/ilist.h"
@@ -1818,6 +1819,14 @@ class ApplyInstBase<Impl, Base, false> : public Base {
     return Rep == FunctionType::Representation::Thin;
   }
 
+  /// Returns true if the callee function is annotated with
+  /// @_semantics("programtermination_point")
+  bool isCalleeKnownProgramTerminationPoint() const {
+    auto calleeFn = getCalleeFunction();
+    if (!calleeFn) return false;
+    return calleeFn->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT);
+  }
+
   /// True if this application has generic substitutions.
   bool hasSubstitutions() const {
     return Substitutions.hasAnySubstitutableParams();
@@ -6645,6 +6654,9 @@ class TermInst : public NonValueInstruction {
   /// Returns true if this terminator exits the function.
   bool isFunctionExiting() const;
 
+  /// Returns true if this terminator terminates the program.
+  bool isProgramTerminating() const;
+
   TermKind getTermKind() const { return TermKind(getKind()); }
 };
 

include/swift/Strings.h

@@ -77,8 +77,8 @@ constexpr static const char BUILTIN_TYPE_NAME_VEC[] = "Builtin.Vec";
 constexpr static const char BUILTIN_TYPE_NAME_SILTOKEN[] = "Builtin.SILToken";
 /// The name of the Builtin type for Word
 constexpr static const char BUILTIN_TYPE_NAME_WORD[] = "Builtin.Word";
-constexpr static StringLiteral SEMANTICS_ARC_PROGRAMTERMINATION_POINT =
-    "arc.programtermination_point";
+constexpr static StringLiteral SEMANTICS_PROGRAMTERMINATION_POINT =
+    "programtermination_point";
 } // end namespace swift
 
 #endif // SWIFT_STRINGS_H

lib/SIL/SILBasicBlock.cpp

@@ -24,6 +24,7 @@
 #include "swift/SIL/SILFunction.h"
 #include "swift/SIL/SILInstruction.h"
 #include "swift/SIL/SILModule.h"
+#include "swift/Strings.h"
 using namespace swift;
 
 //===----------------------------------------------------------------------===//

lib/SIL/SILInstructions.cpp

@@ -1061,6 +1061,30 @@ bool TermInst::isFunctionExiting() const {
   llvm_unreachable("Unhandled TermKind in switch.");
 }
 
+bool TermInst::isProgramTerminating() const {
+  switch (getTermKind()) {
+  case TermKind::BranchInst:
+  case TermKind::CondBranchInst:
+  case TermKind::SwitchValueInst:
+  case TermKind::SwitchEnumInst:
+  case TermKind::SwitchEnumAddrInst:
+  case TermKind::DynamicMethodBranchInst:
+  case TermKind::CheckedCastBranchInst:
+  case TermKind::CheckedCastValueBranchInst:
+  case TermKind::CheckedCastAddrBranchInst:
+  case TermKind::ReturnInst:
+  case TermKind::ThrowInst:
+  case TermKind::UnwindInst:
+  case TermKind::TryApplyInst:
+  case TermKind::YieldInst:
+    return false;
+  case TermKind::UnreachableInst:
+    return true;
+  }
+
+  llvm_unreachable("Unhandled TermKind in switch.");
+}
+
 TermInst::SuccessorBlockArgumentsListTy
 TermInst::getSuccessorBlockArguments() const {
   function_ref<PhiArgumentArrayRef(const SILSuccessor &)> op;

lib/SILOptimizer/Analysis/ARCAnalysis.cpp

@@ -1092,11 +1092,8 @@ bool swift::getFinalReleasesForValue(SILValue V, ReleaseTracker &Tracker) {
 //===----------------------------------------------------------------------===//
 
 static bool ignorableApplyInstInUnreachableBlock(const ApplyInst *AI) {
-  const auto *Fn = AI->getReferencedFunction();
-  if (!Fn)
-    return false;
-
-  return Fn->hasSemanticsAttr("arc.programtermination_point");
+  auto applySite = FullApplySite(const_cast<ApplyInst *>(AI));
+  return applySite.isCalleeKnownProgramTerminationPoint();
 }
 
 static bool ignorableBuiltinInstInUnreachableBlock(const BuiltinInst *BI) {

lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp

@@ -321,7 +321,7 @@ FunctionSideEffectFlags *FunctionSideEffects::getEffectsOn(SILValue Addr) {
 // Return true if the given function has defined effects that were successfully
 // recorded in this FunctionSideEffects object.
 bool FunctionSideEffects::setDefinedEffects(SILFunction *F) {
-  if (F->hasSemanticsAttr("arc.programtermination_point")) {
+  if (F->hasSemanticsAttr(SEMANTICS_PROGRAMTERMINATION_POINT)) {
     Traps = true;
     return true;
   }

lib/SILOptimizer/Mandatory/DiagnoseInfiniteRecursion.cpp

@@ -94,6 +94,24 @@ static bool hasRecursiveCallInPath(SILBasicBlock &Block,
   return false;
 }
 
+/// Returns true if the block has a call to a function marked with
+/// @_semantics("programtermination_point").
+static bool isKnownProgramTerminationPoint(const SILBasicBlock *bb) {
+  // Skip checking anything if this block doesn't end in a program terminator.
+  if (!bb->getTerminator()->isProgramTerminating())
+    return false;
+
+  // Check each instruction for a call to something that's a known
+  // programtermination_point
+  for (auto it = bb->rbegin(); it != bb->rend(); ++it) {
+    auto applySite = FullApplySite::isa(const_cast<SILInstruction *>(&*it));
+    if (!applySite) continue;
+    if (applySite.isCalleeKnownProgramTerminationPoint())
+      return true;
+  }
+  return false;
+}
+
 /// Perform a DFS through the target function to find any paths to an exit node
 /// that do not call into the target.
 static bool hasInfinitelyRecursiveApply(SILFunction *targetFn) {
@@ -109,6 +127,13 @@ static bool hasInfinitelyRecursiveApply(SILFunction *targetFn) {
   while (!workList.empty()) {
     SILBasicBlock *curBlock = workList.pop_back_val();
 
+    // Before checking for infinite recursion, see if we're calling something
+    // that's @_semantics("programtermination_point"). We explicitly don't
+    // want this call to disqualify the warning for infinite recursion,
+    // because they're reserved for exceptional circumstances.
+    if (isKnownProgramTerminationPoint(curBlock))
+      continue;
+
     // We're looking for functions that are recursive on _all_ paths. If this
     // block is recursive, mark that we found recursion and check the next
     // block in the work list.
@@ -119,7 +144,8 @@ static bool hasInfinitelyRecursiveApply(SILFunction *targetFn) {
 
     // If this block doesn't have a recursive call, and it exits the function,
     // then we know the function is not infinitely recursive.
-    if (curBlock->getTerminator()->isFunctionExiting())
+    auto term = curBlock->getTerminator();
+    if (term->isFunctionExiting() || term->isProgramTerminating())
       return false;
 
     // Otherwise, push the successors onto the stack if we haven't visited them.

lib/SILOptimizer/Transforms/ARCCodeMotion.cpp

@@ -1072,9 +1072,7 @@ static void eliminateRetainsPrecedingProgramTerminationPoints(SILFunction *f) {
     // such a case, we can ignore it. All other functions though imply we must
     // bail. If we don't have a function here, check for side
     if (auto apply = FullApplySite::isa(&*iter)) {
-      SILFunction *callee = apply.getCalleeFunction();
-      if (!callee ||
-          !callee->hasSemanticsAttr(SEMANTICS_ARC_PROGRAMTERMINATION_POINT)) {
+      if (!apply.isCalleeKnownProgramTerminationPoint()) {
         continue;
       }
     } else {

stdlib/public/core/AssertCommon.swift

@@ -76,6 +76,7 @@ internal func _fatalErrorFlags() -> UInt32 {
 /// bloats code.
 @usableFromInline
 @inline(never)
+@_semantics("programtermination_point")
 internal func _assertionFailure(
   _ prefix: StaticString, _ message: StaticString,
   file: StaticString, line: UInt,
@@ -106,6 +107,7 @@ internal func _assertionFailure(
 /// bloats code.
 @usableFromInline
 @inline(never)
+@_semantics("programtermination_point")
 internal func _assertionFailure(
   _ prefix: StaticString, _ message: String,
   file: StaticString, line: UInt,
@@ -136,6 +138,7 @@ internal func _assertionFailure(
 /// bloats code.
 @usableFromInline
 @inline(never)
+@_semantics("programtermination_point")
 internal func _assertionFailure(
   _ prefix: StaticString, _ message: String,
   flags: UInt32
@@ -161,27 +164,18 @@ internal func _assertionFailure(
 /// bloats code.
 @usableFromInline
 @inline(never)
-@_semantics("arc.programtermination_point")
+@_semantics("programtermination_point")
 internal func _fatalErrorMessage(
   _ prefix: StaticString, _ message: StaticString,
   file: StaticString, line: UInt,
   flags: UInt32
 ) -> Never {
-  // This function breaks the infinite recursion detection pass by introducing
-  // an edge the pass doesn't look through.
-  func _withUTF8Buffer<R>(
-    _ string: StaticString,
-    _ body: (UnsafeBufferPointer<UInt8>) -> R
-  ) -> R {
-    return string.withUTF8Buffer(body)
-  }
-
 #if INTERNAL_CHECKS_ENABLED
-  _withUTF8Buffer(prefix) {
+  prefix.withUTF8Buffer() {
     (prefix) in
-    _withUTF8Buffer(message) {
+    message.withUTF8Buffer() {
       (message) in
-      _withUTF8Buffer(file) {
+      file.withUTF8Buffer() {
         (file) in
         _swift_stdlib_reportFatalErrorInFile(
           prefix.baseAddress!, CInt(prefix.count),
@@ -192,9 +186,9 @@ internal func _fatalErrorMessage(
     }
   }
 #else
-  _withUTF8Buffer(prefix) {
+  prefix.withUTF8Buffer() {
     (prefix) in
-    _withUTF8Buffer(message) {
+    message.withUTF8Buffer() {
       (message) in
       _swift_stdlib_reportFatalError(
         prefix.baseAddress!, CInt(prefix.count),

test/SILOptimizer/arcsequenceopts.sil

@@ -1553,7 +1553,7 @@ bb0(%0 : $<τ_0_0> { var τ_0_0 } <Builtin.Int32>):
   return %2 : $()
 }
 
-sil [_semantics "arc.programtermination_point"] @fatalError : $@convention(thin) (StaticString, StaticString, StaticString) -> Never
+sil [_semantics "programtermination_point"] @fatalError : $@convention(thin) (StaticString, StaticString, StaticString) -> Never
 
 // CHECK-LABEL: sil @ignore_fatalErrorMsgBB : $@convention(thin) (Builtin.NativeObject) -> () {
 // CHECK-NOT: strong_retain

test/SILOptimizer/infinite_recursion.swift

@@ -33,7 +33,7 @@ func f() { e() }
 
 func g() { // expected-warning {{all paths through this function will call itself}}
   while true { // expected-note {{condition always evaluates to true}}
-    g() 
+    g()
   }
 
   g() // expected-warning {{will never be executed}}
@@ -61,6 +61,57 @@ func k() -> Any {  // expected-warning {{all paths through this function will ca
   return type(of: k())
 }
 
+@_silgen_name("exit") func exit(_: Int32) -> Never
+
+func l() {
+  guard Bool.random() else {
+    exit(0) // no warning; calling 'exit' terminates the program
+  }
+  l()
+}
+
+func m() { // expected-warning {{all paths through this function will call itself}}
+  guard Bool.random() else {
+    fatalError() // we _do_ warn here, because fatalError is a programtermination_point
+  }
+  m()
+}
+
+enum MyNever {}
+
+func blackHole() -> MyNever { // expected-warning {{all paths through this function will call itself}}
+  blackHole()
+}
+
+@_semantics("programtermination_point")
+func terminateMe() -> MyNever {
+  terminateMe() // no warning; terminateMe is a programtermination_point
+}
+
+func n() -> MyNever {
+  if Bool.random() {
+    blackHole() // no warning; blackHole() will terminate the program
+  }
+  n()
+}
+
+func o() -> MyNever {
+  if Bool.random() {
+    o()
+  }
+  blackHole() // no warning; blackHole() will terminate the program
+}
+
+func mayHaveSideEffects() {}
+
+func p() { // expected-warning {{all paths through this function will call itself}}
+  if Bool.random() {
+    mayHaveSideEffects() // presence of side-effects doesn't alter the check for the programtermination_point apply
+    fatalError()
+  }
+  p()
+}
+
 class S {
   convenience init(a: Int) { // expected-warning {{all paths through this function will call itself}}
     self.init(a: a)

test/SILOptimizer/retain_release_code_motion.sil

@@ -667,7 +667,7 @@ bb1:
   return %5 : $()
 }
 
-sil [_semantics "arc.programtermination_point"] @fatalError : $@convention(thin) () -> Never
+sil [_semantics "programtermination_point"] @fatalError : $@convention(thin) () -> Never
 
 // This should eliminate all retains except for the first one b/c of user.
 // CHECK-LABEL: sil @eliminate_retains_on_fatalError_path_single_bb : $@convention(thin) (@owned Builtin.NativeObject) -> () {

test/SILOptimizer/side-effect.sil

@@ -33,7 +33,7 @@ class X {
   init()
 }
 
-sil [_semantics "arc.programtermination_point"] @exitfunc : $@convention(thin) () -> Never
+sil [_semantics "programtermination_point"] @exitfunc : $@convention(thin) () -> Never
 sil [readnone] @pure_func : $@convention(thin) () -> ()
 sil [releasenone] @releasenone_func : $@convention(thin) () -> ()
 sil [readonly] @readonly_owned : $@convention(thin) (@owned X) -> ()