-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Diagnose Infinite Recursion #11869
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
Diagnose Infinite Recursion #11869
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,163 @@ | ||
//==-- DiagnoseInfiniteRecursion.cpp - Find infinitely-recursive applies --==// | ||
// | ||
// This source file is part of the Swift.org open source project | ||
// | ||
// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors | ||
// Licensed under Apache License v2.0 with Runtime Library Exception | ||
// | ||
// See https://swift.org/LICENSE.txt for license information | ||
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
// | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This file implements a diagnostic pass that detects deleterious forms of | ||
// recursive functions. | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#define DEBUG_TYPE "infinite-recursion" | ||
#include "swift/AST/DiagnosticsSIL.h" | ||
#include "swift/AST/Expr.h" | ||
#include "swift/Parse/Lexer.h" | ||
#include "swift/SIL/CFG.h" | ||
#include "swift/SIL/SILArgument.h" | ||
#include "swift/SIL/SILInstruction.h" | ||
#include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h" | ||
#include "swift/SILOptimizer/PassManager/Passes.h" | ||
#include "swift/SILOptimizer/PassManager/Transforms.h" | ||
#include "swift/SILOptimizer/Utils/Devirtualize.h" | ||
#include "llvm/ADT/PostOrderIterator.h" | ||
#include "llvm/ADT/iterator_range.h" | ||
#include "llvm/Support/Debug.h" | ||
|
||
using namespace swift; | ||
|
||
template<typename...T, typename...U> | ||
static void diagnose(ASTContext &Context, SourceLoc loc, Diag<T...> diag, | ||
U &&...args) { | ||
Context.Diags.diagnose(loc, | ||
diag, std::forward<U>(args)...); | ||
} | ||
|
||
static bool hasRecursiveCallInPath(SILBasicBlock &Block, | ||
SILFunction *Target, | ||
ModuleDecl *TargetModule) { | ||
// Process all instructions in the block to find applies that reference | ||
// the parent function. Also looks through vtables for statically | ||
// dispatched (witness) methods. | ||
for (auto &I : Block) { | ||
auto *AI = dyn_cast<ApplyInst>(&I); | ||
if (AI && AI->getCalleeFunction() && AI->getCalleeFunction() == Target) | ||
return true; | ||
|
||
if (FullApplySite FAI = FullApplySite::isa(&I)) { | ||
// Don't touch dynamic dispatch. | ||
if (isa<ObjCMethodInst>(FAI.getCallee())) | ||
continue; | ||
|
||
auto &M = FAI.getModule(); | ||
if (auto *CMI = dyn_cast<ClassMethodInst>(FAI.getCallee())) { | ||
auto ClassType = CMI->getOperand()->getType(); | ||
|
||
// FIXME: If we're not inside the module context of the method, | ||
// we may have to deserialize vtables. If the serialized tables | ||
// are damaged, the pass will crash. | ||
// | ||
// Though, this has the added bonus of not looking into vtables | ||
// outside the current module. Because we're not doing IPA, let | ||
// alone cross-module IPA, this is all well and good. | ||
auto *BGC = ClassType.getNominalOrBoundGenericNominal(); | ||
if (BGC && BGC->getModuleContext() != TargetModule) { | ||
continue; | ||
} | ||
|
||
auto *F = getTargetClassMethod(M, ClassType, CMI); | ||
if (F == Target) | ||
return true; | ||
|
||
continue; | ||
} | ||
|
||
if (auto *WMI = dyn_cast<WitnessMethodInst>(FAI.getCallee())) { | ||
SILFunction *F; | ||
SILWitnessTable *WT; | ||
|
||
std::tie(F, WT) = M.lookUpFunctionInWitnessTable( | ||
WMI->getConformance(), WMI->getMember()); | ||
if (F == Target) | ||
return true; | ||
|
||
continue; | ||
} | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
static bool hasInfinitelyRecursiveApply(SILFunction &Fn, | ||
SILFunction *TargetFn) { | ||
SmallPtrSet<SILBasicBlock *, 16> Visited; | ||
SmallVector<SILBasicBlock *, 16> WorkList; | ||
// Keep track of whether we found at least one recursive path. | ||
bool foundRecursion = false; | ||
|
||
auto *TargetModule = TargetFn->getModule().getSwiftModule(); | ||
auto analyzeSuccessor = [&](SILBasicBlock *Succ) -> bool { | ||
if (!Visited.insert(Succ).second) | ||
return false; | ||
|
||
// If the successor block contains a recursive call, end analysis there. | ||
if (!hasRecursiveCallInPath(*Succ, TargetFn, TargetModule)) { | ||
WorkList.push_back(Succ); | ||
return false; | ||
} | ||
return true; | ||
}; | ||
|
||
// Seed the work list with the entry block. | ||
foundRecursion |= analyzeSuccessor(Fn.getEntryBlock()); | ||
|
||
while (!WorkList.empty()) { | ||
SILBasicBlock *CurBlock = WorkList.pop_back_val(); | ||
// Found a path to the exit node without a recursive call. | ||
if (CurBlock->getTerminator()->isFunctionExiting()) | ||
return false; | ||
|
||
for (SILBasicBlock *Succ : CurBlock->getSuccessorBlocks()) | ||
foundRecursion |= analyzeSuccessor(Succ); | ||
} | ||
return foundRecursion; | ||
} | ||
|
||
namespace { | ||
class DiagnoseInfiniteRecursion : public SILFunctionTransform { | ||
public: | ||
DiagnoseInfiniteRecursion() {} | ||
|
||
private: | ||
void run() override { | ||
SILFunction *Fn = getFunction(); | ||
// Don't rerun diagnostics on deserialized functions. | ||
if (Fn->wasDeserializedCanonical()) | ||
return; | ||
|
||
// Ignore empty functions and straight-line thunks. | ||
if (Fn->empty() || Fn->isThunk() != IsNotThunk) | ||
return; | ||
|
||
// If we can't diagnose it, there's no sense analyzing it. | ||
if (!Fn->hasLocation() || Fn->getLocation().getSourceLoc().isInvalid()) | ||
return; | ||
|
||
if (hasInfinitelyRecursiveApply(*Fn, Fn)) { | ||
diagnose(Fn->getModule().getASTContext(), | ||
Fn->getLocation().getSourceLoc(), | ||
diag::warn_infinite_recursive_function); | ||
} | ||
} | ||
}; | ||
} // end anonymous namespace | ||
|
||
SILTransform *swift::createDiagnoseInfiniteRecursion() { | ||
return new DiagnoseInfiniteRecursion(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
// RUN: %target-swift-frontend -emit-sil -primary-file %s -o /dev/null -verify | ||
|
||
func a() { // expected-warning {{all paths through this function will call itself}} | ||
a() | ||
} | ||
|
||
func b(_ x : Int) { // expected-warning {{all paths through this function will call itself}} | ||
if x != 0 { | ||
b(x) | ||
} else { | ||
b(x+1) | ||
} | ||
} | ||
|
||
func c(_ x : Int) { | ||
if x != 0 { | ||
c(5) | ||
} | ||
} | ||
|
||
func d(_ x : Int) { // expected-warning {{all paths through this function will call itself}} | ||
var x = x | ||
if x != 0 { | ||
x += 1 | ||
} | ||
return d(x) | ||
} | ||
|
||
// Doesn't warn on mutually recursive functions | ||
|
||
func e() { f() } | ||
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() // expected-warning {{will never be executed}} | ||
} | ||
|
||
func h(_ x : Int) { | ||
while (x < 5) { | ||
h(x+1) | ||
} | ||
} | ||
|
||
func i(_ x : Int) { // expected-warning {{all paths through this function will call itself}} | ||
var x = x | ||
while (x < 5) { | ||
x -= 1 | ||
} | ||
i(0) | ||
} | ||
|
||
func j() -> Int { // expected-warning {{all paths through this function will call itself}} | ||
return 5 + j() | ||
} | ||
|
||
func k() -> Any { // expected-warning {{all paths through this function will call itself}} | ||
return type(of: k()) | ||
} | ||
|
||
class S { | ||
convenience init(a: Int) { // expected-warning {{all paths through this function will call itself}} | ||
self.init(a: a) | ||
} | ||
init(a: Int?) {} | ||
|
||
static func a() { // expected-warning {{all paths through this function will call itself}} | ||
return a() | ||
} | ||
|
||
func b() { // expected-warning {{all paths through this function will call itself}} | ||
var i = 0 | ||
repeat { | ||
i += 1 | ||
b() | ||
} while (i > 5) | ||
} | ||
} | ||
|
||
class T: S { | ||
// No warning, calls super | ||
override func b() { | ||
var i = 0 | ||
repeat { | ||
i += 1 | ||
super.b() | ||
} while (i > 5) | ||
} | ||
} | ||
|
||
func == (l: S?, r: S?) -> Bool { // expected-warning {{all paths through this function will call itself}} | ||
if l == nil && r == nil { return true } | ||
guard let l = l, let r = r else { return false } | ||
return l === r | ||
} | ||
|
||
public func == <Element>(lhs: Array<Element>, rhs: Array<Element>) -> Bool { // expected-warning {{all paths through this function will call itself}} | ||
return lhs == rhs | ||
} | ||
|
||
func factorial(_ n : UInt) -> UInt { // expected-warning {{all paths through this function will call itself}} | ||
return (n != 0) ? factorial(n - 1) * n : factorial(1) | ||
} | ||
|
||
func tr(_ key: String) -> String { // expected-warning {{all paths through this function will call itself}} | ||
return tr(key) ?? key // expected-warning {{left side of nil coalescing operator '??' has non-optional type}} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add some negative test-cases as well? I.e. those ones which cannot be detected with a simple analysis yet, but could be eventually detected in the future? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There’s already a negative test for mutual recursion. Can you think of any others? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May be something where class_method or witness_method cannot be devirtualized, i.e. you cannot determine the target? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a new regression test for |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// RUN: %target-swift-frontend -emit-sil -primary-file %s -o /dev/null -verify | ||
|
||
// REQUIRES: objc_interop | ||
|
||
// A negative test that the infinite recursion pass doesn't diagnose dynamic | ||
// dispatch. | ||
|
||
import Foundation | ||
|
||
class MyRecursiveClass { | ||
required init() {} | ||
@objc dynamic func foo() { | ||
return type(of: self).foo(self)() | ||
} | ||
|
||
@objc dynamic func foo2() { | ||
return self.foo() | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you comment here on why you want this to run on IsReabstractionThunk but not other thunks? Is this to save work? Would we misdiagnose something? Do reabstraction thunks have a different shape?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be run on thunks. If the thunk
IsRebastractionThunk
it will fail this condition.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that condition makes sense. I guess the answer here is that it saves some work to ignore these.