Adding use-after-return in Lifetime Analysis #165370
Open
+329
−206
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang Author: Kashika Akhouri (kashika0112) ChangesAdding "use-after-return" in Lifetime Analysis. Patch is 32.46 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/165370.diff 9 Files Affected:
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
index 6a90aeb01e638..712e1d42e2966 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h
@@ -44,6 +44,8 @@ class Fact {
Use,
/// A marker for a specific point in the code, for testing.
TestPoint,
+ OriginEscapes,
+ // An origin that stores a loan escapes the function.
};
private:
@@ -142,6 +144,23 @@ class ReturnOfOriginFact : public Fact {
const OriginManager &OM) const override;
};
+class OriginEscapesFact : public Fact {
+ OriginID OID;
+ const Stmt *EscapeSource;
+
+public:
+ static bool classof(const Fact *F) {
+ return F->getKind() == Kind::OriginEscapes;
+ }
+
+ OriginEscapesFact(OriginID OID, const Stmt *Source)
+ : Fact(Kind::OriginEscapes), OID(OID), EscapeSource(Source) {}
+ OriginID getEscapedOriginID() const { return OID; }
+ const Stmt *getEscapeSource() const { return EscapeSource; }
+ void dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &OM) const override;
+};
+
class UseFact : public Fact {
const Expr *UseExpr;
// True if this use is a write operation (e.g., left-hand side of assignment).
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
index 5e58abee2bbb3..ec3c130bb6c66 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h
@@ -93,6 +93,7 @@ class FactsGenerator : public ConstStmtVisitor<FactsGenerator> {
FactManager &FactMgr;
AnalysisDeclContext &AC;
llvm::SmallVector<Fact *> CurrentBlockFacts;
+ llvm::SmallVector<Fact *> EscapesInCurrentBlock;
// To distinguish between reads and writes for use-after-free checks, this map
// stores the `UseFact` for each `DeclRefExpr`. We initially identify all
// `DeclRefExpr`s as "read" uses. When an assignment is processed, the use
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
index 91ffbb169f947..f5b1b3d9b6564 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
@@ -42,6 +42,11 @@ class LifetimeSafetyReporter {
virtual void reportUseAfterFree(const Expr *IssueExpr, const Expr *UseExpr,
SourceLocation FreeLoc,
Confidence Confidence) {}
+
+ virtual void reportUseAfterReturn(const Expr *IssueExpr,
+ const Stmt *EscapeStmt,
+ SourceLocation ExpiryLoc,
+ Confidence Confidence) {}
};
/// The main entry point for the analysis.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5ff4cc4b833d9..6ea5e34cab291 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10730,6 +10730,7 @@ def warn_lifetime_safety_loan_expires_strict : Warning<
InGroup<LifetimeSafetyStrict>, DefaultIgnore;
def note_lifetime_safety_used_here : Note<"later used here">;
def note_lifetime_safety_destroyed_here : Note<"destroyed here">;
+def note_lifetime_safety_returned_here : Note<"returned here">;
// For non-floating point, expressions of the form x == x or x != x
// should result in a warning, since these always evaluate to a constant.
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index c443c3a5d4f9b..ae5c62ef04d72 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -21,6 +21,7 @@
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallSet.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TimeProfiler.h"
@@ -61,10 +62,23 @@ class LifetimeChecker {
AnalysisDeclContext &ADC, LifetimeSafetyReporter *Reporter)
: LoanPropagation(LoanPropagation), LiveOrigins(LiveOrigins), FactMgr(FM),
Reporter(Reporter) {
- for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>())
- for (const Fact *F : FactMgr.getFacts(B))
- if (const auto *EF = F->getAs<ExpireFact>())
+ for (const CFGBlock *B : *ADC.getAnalysis<PostOrderCFGView>()) {
+ llvm::SmallVector<const ExpireFact *> BlockExpires;
+ llvm::SmallVector<const OriginEscapesFact *> BlockEscapes;
+ for (const Fact *F : FactMgr.getFacts(B)) {
+ if (const auto *EF = F->getAs<ExpireFact>()) {
checkExpiry(EF);
+ BlockExpires.push_back(EF);
+ } else if (const auto *OEF = F->getAs<OriginEscapesFact>()) {
+ BlockEscapes.push_back(OEF);
+ }
+ }
+ if (Reporter) {
+ for (const OriginEscapesFact *OEF : BlockEscapes) {
+ checkEscape(OEF, BlockExpires);
+ }
+ }
+ }
issuePendingWarnings();
}
@@ -106,6 +120,63 @@ class LifetimeChecker {
/*ConfidenceLevel=*/CurConfidence};
}
+ void checkEscape(const OriginEscapesFact *OEF,
+ llvm::ArrayRef<const ExpireFact *> BlockExpires) {
+
+ if (!Reporter) {
+ return;
+ }
+
+ OriginID returnedOID = OEF->getEscapedOriginID();
+ ProgramPoint PP = OEF;
+
+ LoanSet HeldLoans = LoanPropagation.getLoans(returnedOID, PP);
+ if (HeldLoans.isEmpty()) {
+ return;
+ }
+
+ llvm::SmallSet<LoanID, 4> ExpiredLoansInBlock;
+ llvm::DenseMap<LoanID, SourceLocation> ExpiryLocs;
+
+ for (const ExpireFact *EF : BlockExpires) {
+ ExpiredLoansInBlock.insert(EF->getLoanID());
+ ExpiryLocs[EF->getLoanID()] = EF->getExpiryLoc();
+ }
+
+ bool hasExpiredDependency = false;
+ bool allHeldLoansExpired = true;
+ LoanID exampleExpiredLoan = LoanID();
+
+ for (LoanID heldLoan : HeldLoans) {
+ if (ExpiredLoansInBlock.count(heldLoan)) {
+ hasExpiredDependency = true;
+ if (exampleExpiredLoan.Value == LoanID().Value) {
+ exampleExpiredLoan = heldLoan;
+ }
+ } else {
+ allHeldLoansExpired = false;
+ }
+ }
+
+ if (!hasExpiredDependency) {
+ return;
+ }
+
+ Confidence FinalConfidence;
+ if (allHeldLoansExpired) {
+ FinalConfidence = Confidence::Definite;
+ } else {
+ FinalConfidence = Confidence::Maybe;
+ }
+
+ const Loan &L = FactMgr.getLoanMgr().getLoan(exampleExpiredLoan);
+ SourceLocation ExpiryLoc = ExpiryLocs[exampleExpiredLoan];
+ const Stmt *EscapeSource = OEF->getEscapeSource();
+
+ Reporter->reportUseAfterReturn(L.IssueExpr, EscapeSource, ExpiryLoc,
+ FinalConfidence);
+ }
+
void issuePendingWarnings() {
if (!Reporter)
return;
diff --git a/clang/lib/Analysis/LifetimeSafety/Dataflow.h b/clang/lib/Analysis/LifetimeSafety/Dataflow.h
index 2f7bcb6e5dc81..37720ffa03618 100644
--- a/clang/lib/Analysis/LifetimeSafety/Dataflow.h
+++ b/clang/lib/Analysis/LifetimeSafety/Dataflow.h
@@ -168,6 +168,8 @@ class DataflowAnalysis {
return D->transfer(In, *F->getAs<OriginFlowFact>());
case Fact::Kind::ReturnOfOrigin:
return D->transfer(In, *F->getAs<ReturnOfOriginFact>());
+ case Fact::Kind::OriginEscapes:
+ return D->transfer(In, *F->getAs<OriginEscapesFact>());
case Fact::Kind::Use:
return D->transfer(In, *F->getAs<UseFact>());
case Fact::Kind::TestPoint:
@@ -181,6 +183,7 @@ class DataflowAnalysis {
Lattice transfer(Lattice In, const ExpireFact &) { return In; }
Lattice transfer(Lattice In, const OriginFlowFact &) { return In; }
Lattice transfer(Lattice In, const ReturnOfOriginFact &) { return In; }
+ Lattice transfer(Lattice In, const OriginEscapesFact &) { return In; }
Lattice transfer(Lattice In, const UseFact &) { return In; }
Lattice transfer(Lattice In, const TestPointFact &) { return In; }
};
diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
index 1aea64f864367..29c75959ba0fe 100644
--- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
@@ -50,6 +50,14 @@ void ReturnOfOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &,
OS << ")\n";
}
+void OriginEscapesFact::dump(llvm::raw_ostream &OS, const LoanManager &,
+ const OriginManager &OM) const {
+ OS << "OriginEscapes (";
+ OM.dump(getEscapedOriginID(), OS);
+ OS << ")\n";
+}
+
+
void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &,
const OriginManager &OM) const {
OS << "Use (";
diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 9b68de107e314..762ed07eb5bb8 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -58,6 +58,7 @@ void FactsGenerator::run() {
// initializations and destructions are processed in the correct sequence.
for (const CFGBlock *Block : *AC.getAnalysis<PostOrderCFGView>()) {
CurrentBlockFacts.clear();
+ EscapesInCurrentBlock.clear();
for (unsigned I = 0; I < Block->size(); ++I) {
const CFGElement &Element = Block->Elements[I];
if (std::optional<CFGStmt> CS = Element.getAs<CFGStmt>())
@@ -66,6 +67,8 @@ void FactsGenerator::run() {
Element.getAs<CFGAutomaticObjDtor>())
handleDestructor(*DtorOpt);
}
+ CurrentBlockFacts.append(EscapesInCurrentBlock.begin(),
+ EscapesInCurrentBlock.end());
FactMgr.addBlockFacts(Block, CurrentBlockFacts);
}
}
@@ -166,7 +169,8 @@ void FactsGenerator::VisitReturnStmt(const ReturnStmt *RS) {
if (const Expr *RetExpr = RS->getRetValue()) {
if (hasOrigin(RetExpr)) {
OriginID OID = FactMgr.getOriginMgr().getOrCreate(*RetExpr);
- CurrentBlockFacts.push_back(FactMgr.createFact<ReturnOfOriginFact>(OID));
+ EscapesInCurrentBlock.push_back(
+ FactMgr.createFact<OriginEscapesFact>(OID, RS));
}
}
}
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 140b709dbb651..879793575e0b4 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -65,61 +65,61 @@ using namespace clang;
//===----------------------------------------------------------------------===//
namespace {
- class UnreachableCodeHandler : public reachable_code::Callback {
- Sema &S;
- SourceRange PreviousSilenceableCondVal;
-
- public:
- UnreachableCodeHandler(Sema &s) : S(s) {}
-
- void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L,
- SourceRange SilenceableCondVal, SourceRange R1,
- SourceRange R2, bool HasFallThroughAttr) override {
- // If the diagnosed code is `[[fallthrough]];` and
- // `-Wunreachable-code-fallthrough` is enabled, suppress `code will never
- // be executed` warning to avoid generating diagnostic twice
- if (HasFallThroughAttr &&
- !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr,
- SourceLocation()))
- return;
+class UnreachableCodeHandler : public reachable_code::Callback {
+ Sema &S;
+ SourceRange PreviousSilenceableCondVal;
- // Avoid reporting multiple unreachable code diagnostics that are
- // triggered by the same conditional value.
+public:
+ UnreachableCodeHandler(Sema &s) : S(s) {}
+
+ void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L,
+ SourceRange SilenceableCondVal, SourceRange R1,
+ SourceRange R2, bool HasFallThroughAttr) override {
+ // If the diagnosed code is `[[fallthrough]];` and
+ // `-Wunreachable-code-fallthrough` is enabled, suppress `code will never
+ // be executed` warning to avoid generating diagnostic twice
+ if (HasFallThroughAttr &&
+ !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr,
+ SourceLocation()))
+ return;
+
+ // Avoid reporting multiple unreachable code diagnostics that are
+ // triggered by the same conditional value.
if (PreviousSilenceableCondVal.isValid() &&
SilenceableCondVal.isValid() &&
- PreviousSilenceableCondVal == SilenceableCondVal)
- return;
- PreviousSilenceableCondVal = SilenceableCondVal;
+ PreviousSilenceableCondVal == SilenceableCondVal)
+ return;
+ PreviousSilenceableCondVal = SilenceableCondVal;
- unsigned diag = diag::warn_unreachable;
- switch (UK) {
- case reachable_code::UK_Break:
- diag = diag::warn_unreachable_break;
- break;
- case reachable_code::UK_Return:
- diag = diag::warn_unreachable_return;
- break;
- case reachable_code::UK_Loop_Increment:
- diag = diag::warn_unreachable_loop_increment;
- break;
- case reachable_code::UK_Other:
- break;
- }
+ unsigned diag = diag::warn_unreachable;
+ switch (UK) {
+ case reachable_code::UK_Break:
+ diag = diag::warn_unreachable_break;
+ break;
+ case reachable_code::UK_Return:
+ diag = diag::warn_unreachable_return;
+ break;
+ case reachable_code::UK_Loop_Increment:
+ diag = diag::warn_unreachable_loop_increment;
+ break;
+ case reachable_code::UK_Other:
+ break;
+ }
- S.Diag(L, diag) << R1 << R2;
+ S.Diag(L, diag) << R1 << R2;
- SourceLocation Open = SilenceableCondVal.getBegin();
- if (Open.isValid()) {
- SourceLocation Close = SilenceableCondVal.getEnd();
- Close = S.getLocForEndOfToken(Close);
- if (Close.isValid()) {
- S.Diag(Open, diag::note_unreachable_silence)
+ SourceLocation Open = SilenceableCondVal.getBegin();
+ if (Open.isValid()) {
+ SourceLocation Close = SilenceableCondVal.getEnd();
+ Close = S.getLocForEndOfToken(Close);
+ if (Close.isValid()) {
+ S.Diag(Open, diag::note_unreachable_silence)
<< FixItHint::CreateInsertion(Open, "/* DISABLES CODE */ (")
<< FixItHint::CreateInsertion(Close, ")");
- }
}
}
- };
+ }
+};
} // anonymous namespace
/// CheckUnreachable - Check for unreachable code.
@@ -388,9 +388,9 @@ static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD,
if (BodyCFG->getExit().pred_empty())
return;
visitReachableThrows(BodyCFG, [&](const CXXThrowExpr *Throw, CFGBlock &Block) {
- if (throwEscapes(S, Throw, Block, BodyCFG))
- EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD);
- });
+ if (throwEscapes(S, Throw, Block, BodyCFG))
+ EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD);
+ });
}
static bool isNoexcept(const FunctionDecl *FD) {
@@ -803,7 +803,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
}
else if (isa<BlockDecl>(D)) {
if (const FunctionType *FT =
- BlockType->getPointeeType()->getAs<FunctionType>()) {
+ BlockType->getPointeeType()->getAs<FunctionType>()) {
if (FT->getReturnType()->isVoidType())
ReturnsVoid = true;
if (FT->getNoReturnAttr())
@@ -815,7 +815,7 @@ static void CheckFallThroughForBody(Sema &S, const Decl *D, const Stmt *Body,
// Short circuit for compilation speed.
if (CD.checkDiagnostics(Diags, ReturnsVoid, HasNoReturn))
- return;
+ return;
SourceLocation LBrace = Body->getBeginLoc(), RBrace = Body->getEndLoc();
// cpu_dispatch functions permit empty function bodies for ICC compatibility.
@@ -892,7 +892,7 @@ class ContainsReference : public ConstEvaluatedExprVisitor<ContainsReference> {
typedef ConstEvaluatedExprVisitor<ContainsReference> Inherited;
ContainsReference(ASTContext &Context, const DeclRefExpr *Needle)
- : Inherited(Context), FoundReference(false), Needle(Needle) {}
+ : Inherited(Context), FoundReference(false), Needle(Needle) {}
void VisitExpr(const Expr *E) {
// Stop evaluating if we already have a reference.
@@ -1016,7 +1016,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
int RemoveDiagKind = -1;
const char *FixitStr =
S.getLangOpts().CPlusPlus ? (I->Output ? "true" : "false")
- : (I->Output ? "1" : "0");
+ : (I->Output ? "1" : "0");
FixItHint Fixit1, Fixit2;
switch (Term ? Term->getStmtClass() : Stmt::DeclStmtClass) {
@@ -1124,7 +1124,7 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
<< IsCapturedByBlock << User->getSourceRange();
if (RemoveDiagKind != -1)
S.Diag(Fixit1.RemoveRange.getBegin(), diag::note_uninit_fixit_remove_cond)
- << RemoveDiagKind << Str << I->Output << Fixit1 << Fixit2;
+ << RemoveDiagKind << Str << I->Output << Fixit1 << Fixit2;
Diagnosed = true;
}
@@ -1294,32 +1294,32 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor {
// Don't care about other unreachable statements.
}
}
- // If there are no unreachable statements, this may be a special
- // case in CFG:
- // case X: {
- // A a; // A has a destructor.
- // break;
- // }
- // // <<<< This place is represented by a 'hanging' CFG block.
- // case Y:
- continue;
+ // If there are no unreachable statements, this may be a special
+ // case in CFG:
+ // case X: {
+ // A a; // A has a destructor.
+ // break;
+ // }
+ // // <<<< This place is represented by a 'hanging' CFG block.
+ // case Y:
+ continue;
}
- const Stmt *LastStmt = getLastStmt(*P);
- if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) {
- markFallthroughVisited(AS);
- ++AnnotatedCnt;
- continue; // Fallthrough annotation, good.
- }
+ const Stmt *LastStmt = getLastStmt(*P);
+ if (const AttributedStmt *AS = asFallThroughAttr(LastStmt)) {
+ markFallthroughVisited(AS);
+ ++AnnotatedCnt;
+ continue; // Fallthrough annotation, good.
+ }
- if (!LastStmt) { // This block contains no executable statements.
- // Traverse its predecessors.
- std::copy(P->pred_begin(), P->pred_end(),
- std::back_inserter(BlockQueue));
- continue;
- }
+ if (!LastStmt) { // This block contains no executable statements.
+ // Traverse its predecessors.
+ std::copy(P->pred_begin(), P->pred_end(),
+ std::back_inserter(BlockQueue));
+ continue;
+ }
- ++UnannotatedCnt;
+ ++UnannotatedCnt;
}
return !!UnannotatedCnt;
}
@@ -1335,48 +1335,48 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor {
return true;
}
- // We don't want to traverse local type declarations. We analyze their
- // methods separately.
- bool TraverseDecl(Decl *D) override { return true; }
+ // We don't want to traverse local type declarations. We analyze their
+ // methods separately.
+ bool TraverseDecl(Decl *D) override { return true; }
- // We analyze lambda bodies separately. Skip them here.
- bool TraverseLambdaExpr(LambdaExpr *LE) override {
- // Traverse the captures, but not the body.
- for (const auto C : zip(LE->captures(), LE->capture_inits()))
- TraverseLambdaCapture(LE, &std::get<0>(C), std::get<1>(C));
- return true;
- }
+ // We analyze lambda bodies separately. Skip them here.
+ bool TraverseLambdaExpr(LambdaExpr *LE) override {
+ // Trave...
[truncated]
|
Xazax-hun
reviewed
Oct 28, 2025
| /// A marker for a specific point in the code, for testing. | ||
| TestPoint, | ||
| OriginEscapes, | ||
| // An origin that stores a loan escapes the function. |
There was a problem hiding this comment.
Nit: the comment should probably go before the enum constant and might need some word smithing.
| .setAlwaysAdd(Stmt::DeclRefExprClass) | ||
| .setAlwaysAdd(Stmt::ImplicitCastExprClass) | ||
| .setAlwaysAdd(Stmt::UnaryOperatorClass); | ||
| .setAlwaysAdd(Stmt::BinaryOperatorClass) |
There was a problem hiding this comment.
Could you get rid of the formatting only changes? Would make the review easier and the patch smaller.
| } | ||
|
|
||
| void checkEscape(const OriginEscapesFact *OEF, | ||
| llvm::ArrayRef<const ExpireFact *> BlockExpires) { |
There was a problem hiding this comment.
Do we catch the error if the returned loan expired in a different block?
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp,h -- clang/include/clang/Analysis/Analyses/LifetimeSafety/Facts.h clang/include/clang/Analysis/Analyses/LifetimeSafety/FactsGenerator.h clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h clang/lib/Analysis/LifetimeSafety/Checker.cpp clang/lib/Analysis/LifetimeSafety/Dataflow.h clang/lib/Analysis/LifetimeSafety/Facts.cpp clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp clang/lib/Sema/AnalysisBasedWarnings.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/clang/lib/Analysis/LifetimeSafety/Facts.cpp b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
index 29c75959b..e22e281a6 100644
--- a/clang/lib/Analysis/LifetimeSafety/Facts.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Facts.cpp
@@ -51,13 +51,12 @@ void ReturnOfOriginFact::dump(llvm::raw_ostream &OS, const LoanManager &,
}
void OriginEscapesFact::dump(llvm::raw_ostream &OS, const LoanManager &,
- const OriginManager &OM) const {
+ const OriginManager &OM) const {
OS << "OriginEscapes (";
OM.dump(getEscapedOriginID(), OS);
OS << ")\n";
}
-
void UseFact::dump(llvm::raw_ostream &OS, const LoanManager &,
const OriginManager &OM) const {
OS << "Use (";
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 879793575..8be79e58a 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -85,8 +85,7 @@ public:
// Avoid reporting multiple unreachable code diagnostics that are
// triggered by the same conditional value.
- if (PreviousSilenceableCondVal.isValid() &&
- SilenceableCondVal.isValid() &&
+ if (PreviousSilenceableCondVal.isValid() && SilenceableCondVal.isValid() &&
PreviousSilenceableCondVal == SilenceableCondVal)
return;
PreviousSilenceableCondVal = SilenceableCondVal;
@@ -387,7 +386,8 @@ static void checkThrowInNonThrowingFunc(Sema &S, const FunctionDecl *FD,
return;
if (BodyCFG->getExit().pred_empty())
return;
- visitReachableThrows(BodyCFG, [&](const CXXThrowExpr *Throw, CFGBlock &Block) {
+ visitReachableThrows(
+ BodyCFG, [&](const CXXThrowExpr *Throw, CFGBlock &Block) {
if (throwEscapes(S, Throw, Block, BodyCFG))
EmitDiagForCXXThrowInNonThrowingFunc(S, Throw->getThrowLoc(), FD);
});
@@ -1014,8 +1014,8 @@ static void DiagUninitUse(Sema &S, const VarDecl *VD, const UninitUse &Use,
// For all binary terminators, branch 0 is taken if the condition is true,
// and branch 1 is taken if the condition is false.
int RemoveDiagKind = -1;
- const char *FixitStr =
- S.getLangOpts().CPlusPlus ? (I->Output ? "true" : "false")
+ const char *FixitStr = S.getLangOpts().CPlusPlus
+ ? (I->Output ? "true" : "false")
: (I->Output ? "1" : "0");
FixItHint Fixit1, Fixit2;
@@ -1348,7 +1348,6 @@ public:
}
private:
-
static const AttributedStmt *asFallThroughAttr(const Stmt *S) {
if (const AttributedStmt *AS = dyn_cast_or_null<AttributedStmt>(S)) {
if (hasSpecificAttr<FallThroughAttr>(AS->getAttrs()))
@@ -1382,11 +1381,9 @@ private:
static StringRef getFallthroughAttrSpelling(Preprocessor &PP,
SourceLocation Loc) {
- TokenValue FallthroughTokens[] = {
- tok::l_square, tok::l_square,
+ TokenValue FallthroughTokens[] = {tok::l_square, tok::l_square,
PP.getIdentifierInfo("fallthrough"),
- tok::r_square, tok::r_square
- };
+ tok::r_square, tok::r_square};
TokenValue ClangFallthroughTokens[] = {
tok::l_square, tok::l_square, PP.getIdentifierInfo("clang"),
@@ -2239,8 +2236,8 @@ public:
void handleNegativeNotHeld(StringRef Kind, Name LockName, Name Neg,
SourceLocation Loc) override {
- PartialDiagnosticAt Warning(Loc,
- S.PDiag(diag::warn_acquire_requires_negative_cap)
+ PartialDiagnosticAt Warning(
+ Loc, S.PDiag(diag::warn_acquire_requires_negative_cap)
<< Kind << LockName << Neg);
Warnings.emplace_back(std::move(Warning), getNotes());
}
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding "use-after-return" in Lifetime Analysis.
Detecting when a function returns a reference to its own stack memory.
Consider the following example:
The code adds a new Fact "OriginEscape" in the end of the CFG to determine any loan that is escaping the function as shown below:
The confidence of the report is determined by checking if at least one of the loans returned is not expired (strict). If all loans are expired it is considered permissive.