Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/ARCOptimization.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions include/swift/SIL/ApplySite.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()); }

Expand Down
2 changes: 1 addition & 1 deletion include/swift/SIL/SILBasicBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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()); }
};

Expand Down
4 changes: 2 additions & 2 deletions include/swift/Strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions lib/SIL/SILBasicBlock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

//===----------------------------------------------------------------------===//
Expand Down
24 changes: 24 additions & 0 deletions lib/SIL/SILInstructions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 2 additions & 5 deletions lib/SILOptimizer/Analysis/ARCAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/SILOptimizer/Analysis/SideEffectAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
28 changes: 27 additions & 1 deletion lib/SILOptimizer/Mandatory/DiagnoseInfiniteRecursion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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.
Expand All @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions lib/SILOptimizer/Transforms/ARCCodeMotion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
24 changes: 9 additions & 15 deletions stdlib/public/core/AssertCommon.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -136,6 +138,7 @@ internal func _assertionFailure(
/// bloats code.
@usableFromInline
@inline(never)
@_semantics("programtermination_point")
internal func _assertionFailure(
_ prefix: StaticString, _ message: String,
flags: UInt32
Expand All @@ -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),
Expand All @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/arcsequenceopts.sil
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 52 additions & 1 deletion test/SILOptimizer/infinite_recursion.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/retain_release_code_motion.sil
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> () {
Expand Down
2 changes: 1 addition & 1 deletion test/SILOptimizer/side-effect.sil
Original file line number Diff line number Diff line change
Expand Up @@ -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) -> ()
Expand Down