[clang] [experimental] Detect return-stack-addr using CFG (PR #124133)
Utkarsh Saxena via cfe-commits
cfe-commits at lists.llvm.org
Sun Feb 2 09:44:58 PST 2025
https://github.com/usx95 updated https://github.com/llvm/llvm-project/pull/124133
>From 22990789b61e9f9d22e88a6b008eb3166fd1cb56 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Thu, 23 Jan 2025 15:47:39 +0000
Subject: [PATCH 1/3] [experimental] Detect return-stack-addr using CFG
---
.../Analysis/Analyses/DanglingReference.h | 14 +
clang/include/clang/Basic/DiagnosticGroups.td | 3 +
.../clang/Basic/DiagnosticSemaKinds.td | 8 +
clang/lib/Analysis/CMakeLists.txt | 1 +
clang/lib/Analysis/DanglingReference.cpp | 351 ++++++++++++++++++
clang/lib/Sema/AnalysisBasedWarnings.cpp | 8 +
.../test/Sema/warn-lifetime-analysis-cfg.cpp | 136 +++++++
7 files changed, 521 insertions(+)
create mode 100644 clang/include/clang/Analysis/Analyses/DanglingReference.h
create mode 100644 clang/lib/Analysis/DanglingReference.cpp
create mode 100644 clang/test/Sema/warn-lifetime-analysis-cfg.cpp
diff --git a/clang/include/clang/Analysis/Analyses/DanglingReference.h b/clang/include/clang/Analysis/Analyses/DanglingReference.h
new file mode 100644
index 000000000000000..c9f5753eed070e6
--- /dev/null
+++ b/clang/include/clang/Analysis/Analyses/DanglingReference.h
@@ -0,0 +1,14 @@
+#ifndef LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H
+#define LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H
+#include "clang/AST/DeclBase.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Sema/Sema.h"
+
+namespace clang {
+void runDanglingReferenceAnalysis(const DeclContext &dc, const CFG &cfg,
+ AnalysisDeclContext &ac, Sema &S);
+
+} // namespace clang
+
+#endif // LLVM_CLANG_ANALYSIS_ANALYSES_DANGLING_REFERENCE_H
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 594e99a19b64d61..eeddd6eb82a3019 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -472,6 +472,9 @@ def Dangling : DiagGroup<"dangling", [DanglingAssignment,
DanglingInitializerList,
DanglingGsl,
ReturnStackAddress]>;
+def ReturnStackAddressCFG : DiagGroup<"return-stack-address-cfg">;
+def DanglingCFG : DiagGroup<"dangling-cfg", [ReturnStackAddressCFG]>;
+
def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">;
def ExcessInitializers : DiagGroup<"excess-initializers">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8be4f946dce1ccb..846cf6b3d45f8a8 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10169,6 +10169,14 @@ def err_lifetimebound_implicit_object_parameter_void_return_type : Error<
"parameter of a function that returns void; "
"did you mean 'lifetime_capture_by(X)'">;
+// CFG based lifetime analysis.
+def warn_ret_stack_variable_ref_cfg : Warning<
+ "returning reference to a stack variable">, InGroup<ReturnStackAddressCFG>, DefaultIgnore;
+def note_local_variable_declared_here : Note<"reference to this stack variable is returned">;
+
+def warn_ret_stack_temporary_ref_cfg : Warning<
+ "returning reference to a temporary object">, InGroup<ReturnStackAddressCFG>, DefaultIgnore;
+
// CHECK: returning address/reference of stack memory
def warn_ret_stack_addr_ref : Warning<
"%select{address of|reference to}0 stack memory associated with "
diff --git a/clang/lib/Analysis/CMakeLists.txt b/clang/lib/Analysis/CMakeLists.txt
index 7914c12d429ef95..d6ea1e907e7f09b 100644
--- a/clang/lib/Analysis/CMakeLists.txt
+++ b/clang/lib/Analysis/CMakeLists.txt
@@ -16,6 +16,7 @@ add_clang_library(clangAnalysis
ConstructionContext.cpp
Consumed.cpp
CodeInjector.cpp
+ DanglingReference.cpp
Dominators.cpp
ExprMutationAnalyzer.cpp
IntervalPartition.cpp
diff --git a/clang/lib/Analysis/DanglingReference.cpp b/clang/lib/Analysis/DanglingReference.cpp
new file mode 100644
index 000000000000000..2602cc597a36b86
--- /dev/null
+++ b/clang/lib/Analysis/DanglingReference.cpp
@@ -0,0 +1,351 @@
+#include "clang/Analysis/Analyses/DanglingReference.h"
+#include "clang/AST/Attrs.inc"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclVisitor.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/Type.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Basic/DiagnosticSema.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/Support/raw_ostream.h"
+#include <sstream>
+
+namespace clang {
+namespace {
+
+template <typename T> static bool isRecordWithAttr(QualType Type) {
+ auto *RD = Type->getAsCXXRecordDecl();
+ if (!RD)
+ return false;
+ bool Result = RD->hasAttr<T>();
+
+ if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
+ Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>();
+
+ return Result;
+}
+bool isOwner(const Expr *E) {
+ return isRecordWithAttr<OwnerAttr>(E->getType());
+}
+bool isOwner(const Decl *D) {
+ return isa<ValueDecl>(D) &&
+ isRecordWithAttr<OwnerAttr>(dyn_cast<ValueDecl>(D)->getType());
+}
+bool isPointer(const Expr *E) {
+ return isRecordWithAttr<PointerAttr>(E->getType());
+}
+bool isPointer(const Decl *D) {
+ return isa<ValueDecl>(D) &&
+ isRecordWithAttr<PointerAttr>(dyn_cast<ValueDecl>(D)->getType());
+}
+
+struct MemoryLoc {
+ enum MemoryType {
+ EMPTY, // Pointer is null.
+ STACK, // Pointer points to something on stack.
+ UNKNOWN, // Pointer points to an unknown entity.
+ } Loc;
+ // Details of stack location.
+ const Decl *D = nullptr;
+ const Expr *MTE = nullptr;
+
+ bool IsEmpty() { return Loc == EMPTY; }
+ bool IsOnStack() { return Loc == STACK; }
+ bool IsUnkown() { return Loc == UNKNOWN; }
+
+ const Decl *getDecl() { return D; }
+ const Expr *getExpr() { return MTE; }
+
+ static MemoryLoc Unknown() { return {UNKNOWN, nullptr, nullptr}; }
+ static MemoryLoc Empty() { return {EMPTY, nullptr, nullptr}; }
+ static MemoryLoc VarOnStack(const Decl *D) { return {STACK, D, nullptr}; }
+ static MemoryLoc Temporary(const Expr *MTE) { return {STACK, nullptr, MTE}; }
+
+ std::string str() {
+ std::ostringstream os;
+ switch (Loc) {
+ case EMPTY:
+ os << "Empty";
+ break;
+ case UNKNOWN:
+ os << "Unknown";
+ break;
+ case STACK:
+ os << "Stack";
+ if (auto *VD = dyn_cast_or_null<VarDecl>(D))
+ os << " \"" << VD->getName().str() << "\"";
+ if (MTE)
+ os << " (temporary)";
+ break;
+ }
+ return os.str();
+ }
+};
+
+class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
+public:
+ void Handle(const Stmt *S) {
+ if (auto *E = dyn_cast<Expr>(S);
+ E && ExprPointsTo.find(E) != ExprPointsTo.end())
+ return;
+ Visit(S);
+ }
+
+ void MaybeInitaliseDecl(const Decl *D) {
+ if (!D)
+ return;
+ auto *VD = dyn_cast<VarDecl>(D);
+ if (!VD)
+ return;
+ // Initialise the pointer if we are seeing it for the first time.
+ if (isPointer(VD)) {
+ if (DeclPointsTo.find(D) == DeclPointsTo.end())
+ UpdatePointer(VD, ResolveExpr(VD->getInit()));
+ }
+ if (isOwner(VD)) {
+ if (StackDecls.find(D) == StackDecls.end() && VD->hasLocalStorage())
+ AddToStack(VD);
+ }
+ }
+
+ // Merge above and below in VisitVarDecl !!
+ void VisitDeclStmt(const DeclStmt *DS) {
+ MaybeInitaliseDecl(DS->getSingleDecl());
+ }
+
+ void VisitDeclRefExpr(const DeclRefExpr *DRE) {
+ SetExprPointer(DRE, DRE->getDecl());
+ }
+
+ void VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *MTE) {
+ // Ignore MTE of pointer types.
+ if (isPointer(MTE)) {
+ Handle(MTE->getSubExpr());
+ SetExprPointer(MTE, MTE->getSubExpr());
+ }
+ if (isOwner(MTE)) {
+ // We have a temporary owner on stack.
+ AddToStack(MTE);
+ }
+ }
+
+ void VisitImplicitCastExpr(const ImplicitCastExpr *E) {
+ auto *SE = E->IgnoreImpCasts();
+ Handle(SE);
+ SetExprPointer(E, SE);
+ }
+
+ void VisitExprWithCleanups(const ExprWithCleanups *E) {
+ // Handle(E->getSubExpr());
+ SetExprPointer(E, E->getSubExpr());
+ }
+
+ void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
+ // Conversion from Owner to a Pointer.
+ const Expr *ConversionFrom = MCE->IgnoreConversionOperatorSingleStep();
+ if (ConversionFrom != MCE) {
+ if (isOwner(ConversionFrom) && isPointer(MCE)) {
+ SetExprPointer(MCE, ConversionFrom);
+ }
+ }
+ }
+
+ void VisitCXXConstructExpr(const CXXConstructExpr *CCE) {
+ if (!isPointer(CCE))
+ return;
+ if (CCE->getNumArgs() == 1)
+ SetExprPointer(CCE, CCE->getArg(0));
+ }
+
+ void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
+ if (OCE->isAssignmentOp()) {
+ assert(OCE->getNumArgs() == 2);
+ Handle(OCE->getArg(0));
+ Handle(OCE->getArg(1));
+ HandleAssignment(OCE->getArg(0), OCE->getArg(1));
+ }
+ }
+
+private:
+ // Returns the Decl that is aliased by this expression.
+ const Decl *DeclReferencedBy(const Expr *E) {
+ if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+ return DRE->getDecl();
+ }
+ return nullptr;
+ }
+
+ void HandleAssignment(const Expr *A, const Expr *B) {
+ if (!isPointer(A))
+ return;
+
+ if (const Decl *PointerDecl = DeclReferencedBy(A))
+ UpdatePointer(PointerDecl, B);
+ }
+
+ // Update the contents of a Pointer.
+ void UpdatePointer(const Decl *PointerD, MemoryLoc ML) {
+ assert(isPointer(PointerD));
+ DeclPointsTo[PointerD] = ML;
+ }
+ void UpdatePointer(const Decl *PointerD, const Expr *A) {
+ UpdatePointer(PointerD, ResolveExpr(A));
+ }
+
+ void SetExprPointer(const Expr *E, MemoryLoc ML) {
+ assert(ExprPointsTo.insert({E, ML}).second);
+ }
+ void SetExprPointer(const Expr *E, const Expr *PointeeE) {
+ SetExprPointer(E, ResolveExpr(PointeeE));
+ }
+ void SetExprPointer(const Expr *E, const Decl *D) {
+ SetExprPointer(E, ResolveDecl(D));
+ }
+
+public:
+ MemoryLoc ResolveExpr(const Expr *E) {
+ if (!E)
+ return MemoryLoc::Empty();
+ Handle(E);
+ auto Res = ExprPointsTo.find(E);
+ if (Res != ExprPointsTo.end()) {
+ return Res->getSecond();
+ }
+ return ExprPointsTo[E] = MemoryLoc::Unknown();
+ }
+
+ // Returns the memory location pointed to by D. If D is a pointer-type,
+ // returns the memory pointed to by the pointer.
+ MemoryLoc ResolveDecl(const Decl *D) {
+ MaybeInitaliseDecl(D);
+ if (isPointer(D))
+ return ResolvePointer(D);
+ if (isOwner(D))
+ return ResolveOwner(D);
+ return MemoryLoc::Unknown();
+ }
+
+ MemoryLoc ResolvePointer(const Decl *D) {
+ auto *VD = dyn_cast<VarDecl>(D);
+ assert(VD);
+ if (!VD->hasLocalStorage())
+ return MemoryLoc::Unknown();
+ assert(isPointer(D));
+ auto Res = DeclPointsTo.find(D);
+ assert(Res != DeclPointsTo.end());
+ return Res->getSecond();
+ }
+
+ MemoryLoc ResolveOwner(const Decl *D) {
+ assert(isOwner(D));
+ if (IsOnStack(D))
+ return MemoryLoc::VarOnStack(D);
+ return MemoryLoc::Unknown();
+ }
+
+ void AddToStack(const Decl *D) {
+ assert(isOwner(D));
+ StackDecls.insert(D);
+ }
+ void AddToStack(const Expr *E) {
+ assert(isa<MaterializeTemporaryExpr>(E));
+ assert(isOwner(E));
+ StackExprs.insert(E);
+ // Add a self edge.
+ assert(ExprPointsTo.insert({E, MemoryLoc::Temporary(E)}).second);
+ }
+ bool IsOnStack(const Decl *D) { return StackDecls.contains(D); }
+ bool IsOnStack(const Expr *E) { return StackExprs.contains(E); }
+
+ // ExpressionResolver &ExprResolver;
+ // Map from an expression of View type to its pointee and Owner type to the
+ // reference<TODO simplify>. This should follow single assignment because
+ // Expr* cannot be reassigned in the program.
+ llvm::DenseMap<const Expr *, MemoryLoc> ExprPointsTo;
+ // Map from a decl of View type to it pointee. This can be reassigned at
+ // various points in the program due to transfer functions.
+ llvm::DenseMap<const Decl *, MemoryLoc> DeclPointsTo;
+
+ // Collection of Expr* and Decl* stored on stack.
+ llvm::DenseSet<const Expr *> StackExprs;
+ llvm::DenseSet<const Decl *> StackDecls;
+};
+class DanglingReferenceAnalyzer {
+public:
+ DanglingReferenceAnalyzer(const DeclContext &DC, const CFG &cfg,
+ AnalysisDeclContext &AC, Sema &S)
+ : DC(DC), cfg(cfg), AC(AC), S(S) {}
+ void RunAnalysis() {
+
+ // For simplicity in protoytpe, avoid joins and stick to functions without
+ // branches.
+ if (!cfg.isLinear())
+ return;
+ cfg.dump(AC.getASTContext().getLangOpts(), true);
+ for (auto I = cfg.begin(), E = cfg.end(); I != E; ++I) {
+ for (CFGBlock::const_iterator BI = (*I)->begin(), BE = (*I)->end();
+ BI != BE; ++BI) {
+ if (auto cfgstmt = BI->getAs<CFGStmt>()) {
+ auto *stmt = cfgstmt->getStmt();
+ llvm::errs() << "================================================\n";
+ cfgstmt->dump();
+ if (auto *E = dyn_cast<Expr>(stmt))
+ E->dumpColor();
+ PointsTo.Handle(stmt);
+ if (auto *E = dyn_cast<Expr>(stmt)) {
+ llvm::errs() << "Points To : " << PointsTo.ResolveExpr(E).str()
+ << "\n";
+ }
+ if (auto *RS = dyn_cast<ReturnStmt>(stmt))
+ HandleReturnStmt(RS);
+ }
+ }
+ }
+ }
+
+private:
+ void HandleReturnStmt(const ReturnStmt *RS) {
+ // Diagnose possible return of stack variable.
+ if (!RS)
+ return;
+ const Expr *RetExpr = RS->getRetValue();
+ if (!RetExpr || !isPointer(RetExpr))
+ return;
+ PointsTo.Handle(RetExpr);
+ auto RetPointee = PointsTo.ResolveExpr(RetExpr);
+ // RetExpr->dumpColor();
+ // llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n";
+ if (RetPointee.IsOnStack()) {
+ // This points to something on stack!!
+ if (auto *D = RetPointee.getDecl()) {
+ S.Diag(RetExpr->getExprLoc(), diag::warn_ret_stack_variable_ref_cfg)
+ << RetExpr->getExprLoc();
+ S.Diag(D->getLocation(), diag::note_local_variable_declared_here)
+ << D->getLocation();
+ }
+ if (auto *E = RetPointee.getExpr())
+ S.Diag(E->getExprLoc(), diag::warn_ret_stack_temporary_ref_cfg)
+ << E->getExprLoc();
+ }
+ }
+
+ [[maybe_unused]] const DeclContext &DC;
+ const CFG &cfg;
+ [[maybe_unused]] AnalysisDeclContext &AC;
+ Sema &S;
+ // ExpressionResolver ExprResolver;
+ PointsToTracker PointsTo;
+};
+} // namespace
+
+void runDanglingReferenceAnalysis(const DeclContext &DC, const CFG &cfg,
+ AnalysisDeclContext &AC, Sema &S) {
+ DanglingReferenceAnalyzer DRA(DC, cfg, AC, S);
+ DRA.RunAnalysis();
+}
+
+} // namespace clang
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 589869d0186575b..721d4bdba3db545 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -29,6 +29,7 @@
#include "clang/Analysis/Analyses/CFGReachabilityAnalysis.h"
#include "clang/Analysis/Analyses/CalledOnceCheck.h"
#include "clang/Analysis/Analyses/Consumed.h"
+#include "clang/Analysis/Analyses/DanglingReference.h"
#include "clang/Analysis/Analyses/ReachableCode.h"
#include "clang/Analysis/Analyses/ThreadSafety.h"
#include "clang/Analysis/Analyses/UninitializedValues.h"
@@ -2826,6 +2827,13 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
}
}
+ if (S.getLangOpts().CPlusPlus &&
+ !Diags.isIgnored(diag::warn_ret_stack_variable_ref_cfg,
+ D->getBeginLoc())) {
+ if (CFG *cfg = AC.getCFG()) {
+ runDanglingReferenceAnalysis(*cast<DeclContext>(D), *cfg, AC, S);
+ }
+ }
// Check for violations of "called once" parameter properties.
if (S.getLangOpts().ObjC && !S.getLangOpts().CPlusPlus &&
shouldAnalyzeCalledOnceParameters(Diags, D->getBeginLoc())) {
diff --git a/clang/test/Sema/warn-lifetime-analysis-cfg.cpp b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp
new file mode 100644
index 000000000000000..e337c75f8344d2f
--- /dev/null
+++ b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp
@@ -0,0 +1,136 @@
+// RUN: %clang_cc1 -Wno-dangling -Wdangling-cfg -verify -std=c++11 %s
+#include "Inputs/lifetime-analysis.h"
+
+std::string_view unknown(std::string_view s);
+std::string_view unknown();
+std::string temporary();
+
+std::string_view simple_with_ctor() {
+ std::string s1; // expected-note {{reference to this stack variable is returned}}
+ std::string_view cs1 = s1;
+ return cs1; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view simple_with_assignment() {
+ std::string s1; // expected-note {{reference to this stack variable is returned}}
+ std::string_view cs1;
+ cs1 = s1;
+ return cs1; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view use_unknown() {
+ std::string s1;
+ std::string_view cs1 = s1;
+ cs1 = unknown();
+ return cs1; // ok.
+}
+
+std::string_view overwrite_unknown() {
+ std::string s1; // expected-note {{reference to this stack variable is returned}}
+ std::string_view cs1 = s1;
+ cs1 = unknown();
+ cs1 = s1;
+ return cs1; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view overwrite_with_unknown() {
+ std::string s1;
+ std::string_view cs1 = s1;
+ cs1 = s1;
+ cs1 = unknown();
+ return cs1; // Ok.
+}
+
+std::string_view return_unknown() {
+ std::string s1;
+ std::string_view cs1 = s1;
+ return unknown(cs1);
+}
+
+std::string_view multiple_assignments() {
+ std::string s1;
+ std::string s2 = s1; // expected-note {{reference to this stack variable is returned}}
+ std::string_view cs1 = s1;
+ std::string_view cs2 = s1;
+ s2 = s1; // Ignore owner transfers.
+ cs1 = s1;
+ cs2 = s2;
+ cs1 = cs2;
+ cs2 = s1;
+ return cs1; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view global_view;
+std::string_view ignore_global_view() {
+ std::string s;
+ global_view = s; // TODO: We can still track writes to static storage!
+ return global_view; // Ok.
+}
+std::string global_owner;
+std::string_view ignore_global_owner() {
+ std::string_view sv;
+ sv = global_owner;
+ return sv; // Ok.
+}
+
+std::string_view return_temporary() {
+ return
+ temporary(); // expected-warning {{returning reference to a temporary object}}
+}
+
+std::string_view store_temporary() {
+ std::string_view sv =
+ temporary(); // expected-warning {{returning reference to a temporary object}}
+ return sv;
+}
+
+std::string_view return_local() {
+ std::string local; // expected-note {{reference to this stack variable is returned}}
+ return // expected-warning {{returning reference to a stack variable}}
+ local;
+}
+
+// Parameters
+std::string_view params_owner_and_views(
+ std::string_view sv,
+ std::string s) { // expected-note {{reference to this stack variable is returned}}
+ sv = s;
+ return sv; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view param_owner(std::string s) { // expected-note {{reference to this stack variable is returned}}
+ return s; // expected-warning {{returning reference to a stack variable}}
+}
+std::string_view param_owner_ref(const std::string& s) {
+ return s;
+}
+
+std::string& get_str_ref();
+std::string_view ref_to_global_str() {
+ const std::string& s = get_str_ref();
+ std::string_view sv = s;
+ return sv;
+}
+std::string_view view_to_global_str() {
+ std::string_view s = get_str_ref();
+ std::string_view sv = s;
+ sv = s;
+ return sv;
+}
+std::string_view copy_of_global_str() {
+ std::string s = get_str_ref(); // expected-note {{reference to this stack variable is returned}}
+ std::string_view sv = s;
+ return sv; // expected-warning {{returning reference to a stack variable}}
+}
+
+// TODO: Use lifetimebound in function calls. Use Pointer to Owner of pointer
+// std::string_view containerOfString() {
+// std::vector<std::string> local;
+// return local.at(0);
+// }
+
+// std::string_view containerOfViewMultistmt() {
+// std::vector<std::string> local;
+// std::string_view sv = local.at(0);
+// return sv;
+// }
>From 46c76ead9a224d5f6e7bc04fafc194f352392151 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Thu, 23 Jan 2025 16:16:23 +0000
Subject: [PATCH 2/3] Move out Sema from Analysis
---
.../Analysis/Analyses/DanglingReference.h | 15 +++++--
.../clang/Basic/DiagnosticSemaKinds.td | 17 +++++---
clang/lib/Analysis/DanglingReference.cpp | 41 ++++++++-----------
clang/lib/Sema/AnalysisBasedWarnings.cpp | 25 ++++++++++-
4 files changed, 65 insertions(+), 33 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/DanglingReference.h b/clang/include/clang/Analysis/Analyses/DanglingReference.h
index c9f5753eed070e6..f49f967686600d2 100644
--- a/clang/include/clang/Analysis/Analyses/DanglingReference.h
+++ b/clang/include/clang/Analysis/Analyses/DanglingReference.h
@@ -3,11 +3,20 @@
#include "clang/AST/DeclBase.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
-#include "clang/Sema/Sema.h"
-
namespace clang {
+class DanglingReferenceReporter {
+public:
+ DanglingReferenceReporter() = default;
+ virtual ~DanglingReferenceReporter() = default;
+
+ virtual void ReportReturnLocalVar(const Expr *RetExpr,
+ const Decl *LocalDecl) {}
+ virtual void ReportReturnTemporaryExpr(const Expr *TemporaryExpr) {}
+};
+
void runDanglingReferenceAnalysis(const DeclContext &dc, const CFG &cfg,
- AnalysisDeclContext &ac, Sema &S);
+ AnalysisDeclContext &ac,
+ DanglingReferenceReporter *reporter);
} // namespace clang
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 846cf6b3d45f8a8..15f45971b4b2d47 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10170,12 +10170,17 @@ def err_lifetimebound_implicit_object_parameter_void_return_type : Error<
"did you mean 'lifetime_capture_by(X)'">;
// CFG based lifetime analysis.
-def warn_ret_stack_variable_ref_cfg : Warning<
- "returning reference to a stack variable">, InGroup<ReturnStackAddressCFG>, DefaultIgnore;
-def note_local_variable_declared_here : Note<"reference to this stack variable is returned">;
-
-def warn_ret_stack_temporary_ref_cfg : Warning<
- "returning reference to a temporary object">, InGroup<ReturnStackAddressCFG>, DefaultIgnore;
+def warn_ret_stack_variable_ref_cfg
+ : Warning<"returning reference to a stack variable">,
+ InGroup<ReturnStackAddressCFG>,
+ DefaultIgnore;
+def note_local_variable_declared_here
+ : Note<"reference to this stack variable is returned">;
+
+def warn_ret_stack_temporary_ref_cfg
+ : Warning<"returning reference to a temporary object">,
+ InGroup<ReturnStackAddressCFG>,
+ DefaultIgnore;
// CHECK: returning address/reference of stack memory
def warn_ret_stack_addr_ref : Warning<
diff --git a/clang/lib/Analysis/DanglingReference.cpp b/clang/lib/Analysis/DanglingReference.cpp
index 2602cc597a36b86..1ee79d26a843a8f 100644
--- a/clang/lib/Analysis/DanglingReference.cpp
+++ b/clang/lib/Analysis/DanglingReference.cpp
@@ -1,17 +1,14 @@
#include "clang/Analysis/Analyses/DanglingReference.h"
#include "clang/AST/Attrs.inc"
#include "clang/AST/Decl.h"
-#include "clang/AST/DeclVisitor.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/AST/Type.h"
#include "clang/Analysis/CFG.h"
-#include "clang/Basic/DiagnosticSema.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
-#include "llvm/Support/raw_ostream.h"
#include <sstream>
namespace clang {
@@ -277,28 +274,30 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
class DanglingReferenceAnalyzer {
public:
DanglingReferenceAnalyzer(const DeclContext &DC, const CFG &cfg,
- AnalysisDeclContext &AC, Sema &S)
- : DC(DC), cfg(cfg), AC(AC), S(S) {}
+ AnalysisDeclContext &AC,
+ DanglingReferenceReporter *Reporter)
+ : DC(DC), cfg(cfg), AC(AC), Reporter(Reporter) {}
void RunAnalysis() {
// For simplicity in protoytpe, avoid joins and stick to functions without
// branches.
if (!cfg.isLinear())
return;
- cfg.dump(AC.getASTContext().getLangOpts(), true);
+ // cfg.dump(AC.getASTContext().getLangOpts(), true);
for (auto I = cfg.begin(), E = cfg.end(); I != E; ++I) {
for (CFGBlock::const_iterator BI = (*I)->begin(), BE = (*I)->end();
BI != BE; ++BI) {
if (auto cfgstmt = BI->getAs<CFGStmt>()) {
auto *stmt = cfgstmt->getStmt();
- llvm::errs() << "================================================\n";
- cfgstmt->dump();
- if (auto *E = dyn_cast<Expr>(stmt))
- E->dumpColor();
+ // llvm::errs() <<
+ // "================================================\n";
+ // cfgstmt->dump();
+ // if (auto *E = dyn_cast<Expr>(stmt))
+ // E->dumpColor();
PointsTo.Handle(stmt);
if (auto *E = dyn_cast<Expr>(stmt)) {
- llvm::errs() << "Points To : " << PointsTo.ResolveExpr(E).str()
- << "\n";
+ // llvm::errs() << "Points To : " << PointsTo.ResolveExpr(E).str()
+ // << "\n";
}
if (auto *RS = dyn_cast<ReturnStmt>(stmt))
HandleReturnStmt(RS);
@@ -321,30 +320,26 @@ class DanglingReferenceAnalyzer {
// llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n";
if (RetPointee.IsOnStack()) {
// This points to something on stack!!
- if (auto *D = RetPointee.getDecl()) {
- S.Diag(RetExpr->getExprLoc(), diag::warn_ret_stack_variable_ref_cfg)
- << RetExpr->getExprLoc();
- S.Diag(D->getLocation(), diag::note_local_variable_declared_here)
- << D->getLocation();
- }
+ if (auto *D = RetPointee.getDecl())
+ Reporter->ReportReturnLocalVar(RetExpr, D);
if (auto *E = RetPointee.getExpr())
- S.Diag(E->getExprLoc(), diag::warn_ret_stack_temporary_ref_cfg)
- << E->getExprLoc();
+ Reporter->ReportReturnTemporaryExpr(E);
}
}
[[maybe_unused]] const DeclContext &DC;
const CFG &cfg;
[[maybe_unused]] AnalysisDeclContext &AC;
- Sema &S;
+ DanglingReferenceReporter *Reporter;
// ExpressionResolver ExprResolver;
PointsToTracker PointsTo;
};
} // namespace
void runDanglingReferenceAnalysis(const DeclContext &DC, const CFG &cfg,
- AnalysisDeclContext &AC, Sema &S) {
- DanglingReferenceAnalyzer DRA(DC, cfg, AC, S);
+ AnalysisDeclContext &AC,
+ DanglingReferenceReporter *Reporter) {
+ DanglingReferenceAnalyzer DRA(DC, cfg, AC, Reporter);
DRA.RunAnalysis();
}
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 721d4bdba3db545..618b00734ccb2ac 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2638,6 +2638,28 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
}
}
+namespace {
+
+class DanglingReferenceReporterImpl : public DanglingReferenceReporter {
+public:
+ DanglingReferenceReporterImpl(Sema &S) : S(S) {}
+
+ void ReportReturnLocalVar(const Expr *RetExpr,
+ const Decl *LocalDecl) override {
+ S.Diag(RetExpr->getExprLoc(), diag::warn_ret_stack_variable_ref_cfg)
+ << RetExpr->getExprLoc();
+ S.Diag(LocalDecl->getLocation(), diag::note_local_variable_declared_here)
+ << LocalDecl->getLocation();
+ }
+ void ReportReturnTemporaryExpr(const Expr *TemporaryExpr) override {
+ S.Diag(TemporaryExpr->getExprLoc(), diag::warn_ret_stack_temporary_ref_cfg)
+ << TemporaryExpr->getExprLoc();
+ }
+
+private:
+ Sema &S;
+};
+} // namespace
void clang::sema::AnalysisBasedWarnings::IssueWarnings(
sema::AnalysisBasedWarnings::Policy P, sema::FunctionScopeInfo *fscope,
const Decl *D, QualType BlockType) {
@@ -2831,7 +2853,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
!Diags.isIgnored(diag::warn_ret_stack_variable_ref_cfg,
D->getBeginLoc())) {
if (CFG *cfg = AC.getCFG()) {
- runDanglingReferenceAnalysis(*cast<DeclContext>(D), *cfg, AC, S);
+ DanglingReferenceReporterImpl Reporter(S);
+ runDanglingReferenceAnalysis(*cast<DeclContext>(D), *cfg, AC, &Reporter);
}
}
// Check for violations of "called once" parameter properties.
>From fcdadc7d0cfe456d9def8bea1f99cd2d46f42fa4 Mon Sep 17 00:00:00 2001
From: Utkarsh Saxena <usx at google.com>
Date: Fri, 24 Jan 2025 14:45:17 +0000
Subject: [PATCH 3/3] Add equivalence for no-cfg tests
---
clang/lib/Analysis/DanglingReference.cpp | 579 ++++++++++++++----
clang/lib/Sema/AnalysisBasedWarnings.cpp | 4 +
clang/test/Sema/Inputs/lifetime-analysis.h | 1 +
.../test/Sema/warn-lifetime-analysis-cfg.cpp | 296 ++++++++-
.../Sema/warn-lifetime-analysis-nocfg.cpp | 83 ++-
5 files changed, 809 insertions(+), 154 deletions(-)
diff --git a/clang/lib/Analysis/DanglingReference.cpp b/clang/lib/Analysis/DanglingReference.cpp
index 1ee79d26a843a8f..89b718052c8b91d 100644
--- a/clang/lib/Analysis/DanglingReference.cpp
+++ b/clang/lib/Analysis/DanglingReference.cpp
@@ -1,15 +1,28 @@
#include "clang/Analysis/Analyses/DanglingReference.h"
#include "clang/AST/Attrs.inc"
#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/DataflowWorklist.h"
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/ImmutableMap.h"
+#include "llvm/ADT/ImmutableSet.h"
+#include "llvm/Support/raw_ostream.h"
+#include <cassert>
#include <sstream>
+#include <stdbool.h>
+#include <string>
namespace clang {
namespace {
@@ -22,26 +35,98 @@ template <typename T> static bool isRecordWithAttr(QualType Type) {
if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD))
Result |= CTSD->getSpecializedTemplate()->getTemplatedDecl()->hasAttr<T>();
-
+ if (RD->hasDefinition())
+ for (auto B : RD->bases())
+ Result |= isRecordWithAttr<T>(B.getType());
return Result;
}
-bool isOwner(const Expr *E) {
- return isRecordWithAttr<OwnerAttr>(E->getType());
-}
+
+bool isOwner(QualType Q) { return isRecordWithAttr<OwnerAttr>(Q); }
+bool isOwner(const Expr *E) { return isOwner(E->getType()); }
bool isOwner(const Decl *D) {
return isa<ValueDecl>(D) &&
isRecordWithAttr<OwnerAttr>(dyn_cast<ValueDecl>(D)->getType());
}
-bool isPointer(const Expr *E) {
- return isRecordWithAttr<PointerAttr>(E->getType());
+bool isPointer(QualType Q) {
+ return Q->isNullPtrType() || Q->isPointerType() ||
+ isRecordWithAttr<PointerAttr>(Q);
}
+bool isPointer(const Expr *E) { return isPointer(E->getType()); }
bool isPointer(const Decl *D) {
- return isa<ValueDecl>(D) &&
- isRecordWithAttr<PointerAttr>(dyn_cast<ValueDecl>(D)->getType());
+ auto *VD = dyn_cast<ValueDecl>(D);
+ return VD && isPointer(VD->getType());
+}
+
+static bool isInStlNamespace(const Decl *D) {
+ const DeclContext *DC = D->getDeclContext();
+ if (!DC)
+ return false;
+ if (const auto *ND = dyn_cast<NamespaceDecl>(DC))
+ if (const IdentifierInfo *II = ND->getIdentifier()) {
+ StringRef Name = II->getName();
+ if (Name.size() >= 2 && Name.front() == '_' &&
+ (Name[1] == '_' || isUppercase(Name[1])))
+ return true;
+ }
+
+ return DC->isStdNamespace();
+}
+
+static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
+ if (!Callee)
+ return false;
+ if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
+ if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()) &&
+ Callee->getParent()->hasAttr<OwnerAttr>())
+ return true;
+ if (!isInStlNamespace(Callee->getParent()))
+ return false;
+ if (!isRecordWithAttr<PointerAttr>(
+ Callee->getFunctionObjectParameterType()) &&
+ !isRecordWithAttr<OwnerAttr>(Callee->getFunctionObjectParameterType()))
+ return false;
+ if (isPointer(Callee->getReturnType())) {
+ if (!Callee->getIdentifier())
+ return false;
+ return llvm::StringSwitch<bool>(Callee->getName())
+ .Cases("begin", "rbegin", "cbegin", "crbegin", true)
+ .Cases("end", "rend", "cend", "crend", true)
+ .Cases("c_str", "data", "get", true)
+ // Map and set types.
+ .Cases("find", "equal_range", "lower_bound", "upper_bound", true)
+ .Default(false);
+ }
+ if (Callee->getReturnType()->isReferenceType()) {
+ if (!Callee->getIdentifier()) {
+ auto OO = Callee->getOverloadedOperator();
+ if (!Callee->getParent()->hasAttr<OwnerAttr>())
+ return false;
+ return OO == OverloadedOperatorKind::OO_Subscript ||
+ OO == OverloadedOperatorKind::OO_Star;
+ }
+ return llvm::StringSwitch<bool>(Callee->getName())
+ .Cases("front", "back", "at", "top", "value", true)
+ .Default(false);
+ }
+ return false;
+}
+
+bool implicitObjectParamIsLifetimeBound(const FunctionDecl *FD) {
+ const TypeSourceInfo *TSI = FD->getTypeSourceInfo();
+ if (!TSI)
+ return false;
+ AttributedTypeLoc ATL;
+ for (TypeLoc TL = TSI->getTypeLoc();
+ (ATL = TL.getAsAdjusted<AttributedTypeLoc>());
+ TL = ATL.getModifiedLoc()) {
+ if (ATL.getAttrAs<LifetimeBoundAttr>())
+ return true;
+ }
+ return false;
}
struct MemoryLoc {
- enum MemoryType {
+ enum Storage {
EMPTY, // Pointer is null.
STACK, // Pointer points to something on stack.
UNKNOWN, // Pointer points to an unknown entity.
@@ -62,7 +147,7 @@ struct MemoryLoc {
static MemoryLoc VarOnStack(const Decl *D) { return {STACK, D, nullptr}; }
static MemoryLoc Temporary(const Expr *MTE) { return {STACK, nullptr, MTE}; }
- std::string str() {
+ std::string str() const {
std::ostringstream os;
switch (Loc) {
case EMPTY:
@@ -81,13 +166,229 @@ struct MemoryLoc {
}
return os.str();
}
+
+ static inline void Profile(llvm::FoldingSetNodeID &ID, const MemoryLoc &M) {
+ ID.AddInteger(M.Loc);
+ ID.AddPointer(M.D);
+ ID.AddPointer(M.MTE);
+ }
+
+ inline void Profile(llvm::FoldingSetNodeID &ID) const {
+ return Profile(ID, *this);
+ }
+};
+inline bool operator==(const MemoryLoc &LHS, const MemoryLoc &RHS) {
+ return LHS.Loc == RHS.Loc && LHS.D == RHS.D && LHS.MTE == RHS.MTE;
+}
+inline bool operator!=(const MemoryLoc &LHS, const MemoryLoc &RHS) {
+ return !(LHS == RHS);
+}
+
+class PointsToFactory {
+public:
+ PointsToFactory()
+ : ESetFact(false), DSetFact(false), EPointsToFact(false),
+ DPointsToFact(false) {}
+ llvm::ImmutableSet<const Expr *>::Factory ESetFact;
+ llvm::ImmutableSet<const Decl *>::Factory DSetFact;
+ llvm::ImmutableMap<const Expr *, MemoryLoc>::Factory EPointsToFact;
+ llvm::ImmutableMap<const Decl *, MemoryLoc>::Factory DPointsToFact;
+};
+
+class PointsToSet {
+
+public:
+ explicit PointsToSet(PointsToFactory &Factory)
+ : Factory(Factory), ExprPointsTo(Factory.EPointsToFact.getEmptyMap()),
+ DeclPointsTo(Factory.DPointsToFact.getEmptyMap()),
+ StackExprs(Factory.ESetFact.getEmptySet()),
+ StackDecls(Factory.DSetFact.getEmptySet()),
+ UnknownDecls(Factory.DSetFact.getEmptySet()) {}
+
+ PointsToSet(const PointsToSet &P)
+ : Factory(P.Factory), ExprPointsTo(P.ExprPointsTo),
+ DeclPointsTo(P.DeclPointsTo), StackExprs(P.StackExprs),
+ StackDecls(P.StackDecls), UnknownDecls(P.UnknownDecls) {}
+
+ PointsToSet &operator=(const PointsToSet &P) {
+ ExprPointsTo = P.ExprPointsTo;
+ DeclPointsTo = P.DeclPointsTo;
+ StackExprs = P.StackExprs;
+ StackDecls = P.StackDecls;
+ UnknownDecls = P.UnknownDecls;
+ return *this;
+ }
+
+ bool ContainsExprPointsTo(const Expr *E) const {
+ return E && ExprPointsTo.contains(E);
+ }
+ bool ContainsDeclPointsTo(const Decl *D) const {
+ return D && DeclPointsTo.contains(D);
+ }
+ MemoryLoc GetExprPointsTo(const Expr *E) const {
+ assert(ContainsExprPointsTo(E));
+ return *ExprPointsTo.lookup(E);
+ }
+ MemoryLoc GetDeclPointsTo(const Decl *D) const {
+ assert(ContainsDeclPointsTo(D));
+ return *DeclPointsTo.lookup(D);
+ }
+ MemoryLoc SetExprPointer(const Expr *E, MemoryLoc M) {
+ ExprPointsTo = Factory.EPointsToFact.add(ExprPointsTo, E, M);
+ return M;
+ }
+ MemoryLoc SetDeclPointer(const Decl *D, MemoryLoc M) {
+ if (DeclPointsTo.contains(D))
+ DeclPointsTo = Factory.DPointsToFact.remove(DeclPointsTo, D);
+ DeclPointsTo = Factory.DPointsToFact.add(DeclPointsTo, D, M);
+ return M;
+ }
+
+ void AddToStack(const Decl *D) {
+ StackDecls = Factory.DSetFact.add(StackDecls, D);
+ }
+ void MarkAsUnknown(const Decl *D) {
+ UnknownDecls = Factory.DSetFact.add(UnknownDecls, D);
+ }
+ void AddToStack(const Expr *E) {
+ StackExprs = Factory.ESetFact.add(StackExprs, E);
+ }
+
+ bool IsOnStack(const Decl *D) const {
+ return StackDecls.contains(D) && !UnknownDecls.contains(D);
+ }
+ bool IsOnStack(const Expr *E) const { return StackExprs.contains(E); }
+
+ int size() const {
+ return ExprPointsTo.getHeight() + DeclPointsTo.getHeight() +
+ StackExprs.getHeight() + StackDecls.getHeight() +
+ UnknownDecls.getHeight();
+ }
+ void dump() {
+ std::ostringstream os;
+ for (const auto &x : DeclPointsTo) {
+ auto *VD = dyn_cast<VarDecl>(x.first);
+ if (VD) {
+ llvm::errs() << VD->getNameAsString() << " ----> Points to ---> "
+ << x.second.str() << "\n";
+ }
+ }
+ for (const auto &x : UnknownDecls) {
+ auto *VD = dyn_cast<VarDecl>(x);
+ if (VD) {
+ llvm::errs() << "\tunknown: " << VD->getNameAsString() << "\n";
+ }
+ }
+ }
+
+private:
+ PointsToFactory &Factory;
+
+ // TODO: Make private.
+public:
+ // TODO: We only need decl information. not expr!
+
+ // Map from an expression of View type to its pointee and Owner type to the
+ // reference<TODO simplify>. This should follow single assignment because
+ // Expr* cannot be reassigned in the program.
+ llvm::ImmutableMap<const Expr *, MemoryLoc> ExprPointsTo;
+ // Map from a decl of View type to it pointee. This can be reassigned at
+ // various points in the program due to transfer functions.
+ llvm::ImmutableMap<const Decl *, MemoryLoc> DeclPointsTo;
+
+ // Collection of Expr* and Decl* stored on stack.
+ llvm::ImmutableSet<const Expr *> StackExprs;
+ llvm::ImmutableSet<const Decl *> StackDecls;
+ // Decl was previously on stack but was then moved to some unknown storage.
+ // For example: std::unique_ptr<> P can be moved to unknown storage using
+ // std::move(P) or P.release();
+ llvm::ImmutableSet<const Decl *> UnknownDecls;
};
-class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
+PointsToSet Merge(const PointsToSet a, const PointsToSet b) {
+ if (a.size() < b.size())
+ return Merge(b, a);
+ PointsToSet res = a;
+ for (const auto &v : b.ExprPointsTo) {
+ if (!res.ContainsExprPointsTo(v.first))
+ res.SetExprPointer(v.first, v.second);
+ else if (res.GetExprPointsTo(v.first) != v.second)
+ res.SetExprPointer(v.first, MemoryLoc::Unknown());
+ }
+ for (const auto &v : b.DeclPointsTo) {
+ if (!res.ContainsDeclPointsTo(v.first))
+ res.SetDeclPointer(v.first, v.second);
+ else if (res.GetDeclPointsTo(v.first) != v.second)
+ res.SetDeclPointer(v.first, MemoryLoc::Unknown());
+ }
+ for (const auto *D : b.StackDecls) {
+ res.AddToStack(D);
+ }
+ for (const auto *E : b.StackExprs) {
+ res.AddToStack(E);
+ }
+ for (const auto *D : b.UnknownDecls) {
+ res.MarkAsUnknown(D);
+ }
+ return res;
+}
+
+class BlockVisitor : public ConstStmtVisitor<BlockVisitor> {
public:
+ BlockVisitor(PointsToSet Incoming, const DeclContext &DC,
+ DanglingReferenceReporter *Reporter)
+ : PointsTo(Incoming), DC(DC), Reporter(Reporter) {}
+ void Handle(const clang::CFGBlock *B) {
+ for (CFGBlock::const_iterator BI = B->begin(), BE = B->end(); BI != BE;
+ ++BI) {
+ if (auto cfgstmt = BI->getAs<CFGStmt>()) {
+ // llvm::errs() << "================================================\n";
+ // cfgstmt->dump();
+ auto *stmt = cfgstmt->getStmt();
+ // if (auto *E = dyn_cast<Expr>(stmt))
+ // E->dumpColor();
+
+ Handle(stmt);
+
+ // if (auto *E = dyn_cast<Expr>(stmt)) {
+ // E->dumpColor();
+ // auto Loc = PointsTo.ResolveExpr(E);
+ // llvm::errs() << "Points To : " << Loc.str() << "\n";
+ // }
+
+ if (auto *RS = dyn_cast<ReturnStmt>(stmt))
+ DiagnoseReturnStmt(RS);
+ }
+ }
+ }
+ PointsToSet getOutgoing() { return PointsTo; }
+
+private:
+ // Diagnose possible return of stack variable.
+ void DiagnoseReturnStmt(const ReturnStmt *RS) {
+ if (!RS)
+ return;
+ const Expr *RetExpr = RS->getRetValue();
+ if (!RetExpr || !isPointer(RetExpr))
+ return;
+ Handle(RetExpr);
+ auto RetPointee = ResolveExpr(RetExpr);
+ // RetExpr->dumpColor();
+ // llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n";
+ if (RetPointee.IsOnStack()) {
+ llvm::errs() << "=================================================\n";
+ llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n\n";
+ RetExpr->dumpColor();
+ // This points to something on stack!!
+ if (auto *D = RetPointee.getDecl(); D && PointsTo.IsOnStack(D))
+ Reporter->ReportReturnLocalVar(RetExpr, D);
+ if (auto *E = RetPointee.getExpr())
+ Reporter->ReportReturnTemporaryExpr(E);
+ }
+ }
+
void Handle(const Stmt *S) {
- if (auto *E = dyn_cast<Expr>(S);
- E && ExprPointsTo.find(E) != ExprPointsTo.end())
+ if (auto *E = dyn_cast<Expr>(S); PointsTo.ContainsExprPointsTo(E))
return;
Visit(S);
}
@@ -100,16 +401,27 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
return;
// Initialise the pointer if we are seeing it for the first time.
if (isPointer(VD)) {
- if (DeclPointsTo.find(D) == DeclPointsTo.end())
- UpdatePointer(VD, ResolveExpr(VD->getInit()));
- }
- if (isOwner(VD)) {
- if (StackDecls.find(D) == StackDecls.end() && VD->hasLocalStorage())
- AddToStack(VD);
+ if (PointsTo.ContainsDeclPointsTo(D))
+ return;
+ // Pointer params are always unknown.
+ if (isa<ParmVarDecl>(VD)) {
+ UpdatePointer(VD, MemoryLoc::Unknown());
+ return;
+ }
+ UpdatePointer(VD, ResolveExpr(VD->getInit()));
+ return;
}
+ // Ignore non-stack variable.
+ // TODO: Track reference variables.
+ if (!DC.containsDecl(const_cast<Decl *>(D)) || !VD->hasLocalStorage() ||
+ VD->getType()->isReferenceType())
+ return;
+
+ if (!PointsTo.IsOnStack(D))
+ AddToStack(VD);
}
- // Merge above and below in VisitVarDecl !!
+public:
void VisitDeclStmt(const DeclStmt *DS) {
MaybeInitaliseDecl(DS->getSingleDecl());
}
@@ -123,11 +435,10 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
if (isPointer(MTE)) {
Handle(MTE->getSubExpr());
SetExprPointer(MTE, MTE->getSubExpr());
+ return;
}
- if (isOwner(MTE)) {
- // We have a temporary owner on stack.
- AddToStack(MTE);
- }
+ // We have a temporary on stack.
+ AddToStack(MTE);
}
void VisitImplicitCastExpr(const ImplicitCastExpr *E) {
@@ -137,16 +448,51 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
}
void VisitExprWithCleanups(const ExprWithCleanups *E) {
- // Handle(E->getSubExpr());
SetExprPointer(E, E->getSubExpr());
}
-
- void VisitCXXMemberCallExpr(const CXXMemberCallExpr *MCE) {
- // Conversion from Owner to a Pointer.
- const Expr *ConversionFrom = MCE->IgnoreConversionOperatorSingleStep();
- if (ConversionFrom != MCE) {
- if (isOwner(ConversionFrom) && isPointer(MCE)) {
- SetExprPointer(MCE, ConversionFrom);
+ bool IsUniquePtrRelease(const CXXMemberCallExpr *MCE) {
+ if (!MCE || !MCE->getCalleeDecl())
+ return false;
+ auto *FD = MCE->getCalleeDecl()->getAsFunction();
+ if (!FD)
+ return false;
+ return FD->getIdentifier() && FD->getName() == "release";
+ }
+ void VisitCallExpr(const CallExpr *CE) {
+ if (CE->isCallToStdMove()) {
+ assert(CE->getNumArgs() == 1);
+ MarkDeclAsUnknown(CE->getArg(0));
+ return;
+ }
+ // TODO: Can be merged with above std::move.
+ if (auto *MCE = dyn_cast<CXXMemberCallExpr>(CE)) {
+ if (IsUniquePtrRelease(MCE))
+ MarkDeclAsUnknown(MCE->getImplicitObjectArgument());
+ }
+ // Lifetimebound function calls.
+ auto *FD = dyn_cast_or_null<FunctionDecl>(CE->getCalleeDecl());
+ if (!FD)
+ return;
+ // FIXME: This should only be done for GSL pointer args and not all args!
+ QualType RetType = FD->getReturnType();
+ if (RetType->isReferenceType() && !isOwner(RetType->getPointeeType()))
+ return;
+ Expr *ObjectArg = nullptr;
+ if (auto *MCE = dyn_cast<CXXMemberCallExpr>(CE)) {
+ ObjectArg = MCE->getImplicitObjectArgument();
+ if (ObjectArg && (implicitObjectParamIsLifetimeBound(FD) ||
+ shouldTrackImplicitObjectArg(MCE->getMethodDecl()))) {
+ // TODO: Track more args. Not just the first one!
+ SetExprPointer(CE, ObjectArg);
+ return;
+ }
+ }
+ for (unsigned I = 0; I < FD->getNumParams(); ++I) {
+ const ParmVarDecl *PVD = FD->getParamDecl(I);
+ if (CE->getArg(I) && PVD->hasAttr<LifetimeBoundAttr>()) {
+ // TODO: Track more args. Not just the first one!
+ SetExprPointer(CE, CE->getArg(I));
+ return;
}
}
}
@@ -155,7 +501,8 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
if (!isPointer(CCE))
return;
if (CCE->getNumArgs() == 1)
- SetExprPointer(CCE, CCE->getArg(0));
+ if (isOwner(CCE->getArg(0)) || isPointer(CCE->getArg(0)))
+ SetExprPointer(CCE, CCE->getArg(0));
}
void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *OCE) {
@@ -170,31 +517,41 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
private:
// Returns the Decl that is aliased by this expression.
const Decl *DeclReferencedBy(const Expr *E) {
- if (auto *DRE = dyn_cast<DeclRefExpr>(E)) {
+ if (auto *DRE = dyn_cast_or_null<DeclRefExpr>(E)) {
return DRE->getDecl();
}
return nullptr;
}
+ void MarkDeclAsUnknown(const Expr *E) {
+ if (!E)
+ return;
+ if (const Decl *D = DeclReferencedBy(E)) {
+ PointsTo.MarkAsUnknown(D);
+ }
+ }
+
void HandleAssignment(const Expr *A, const Expr *B) {
if (!isPointer(A))
return;
- if (const Decl *PointerDecl = DeclReferencedBy(A))
+ if (const Decl *PointerDecl = DeclReferencedBy(A);
+ PointerDecl && isPointer(PointerDecl))
UpdatePointer(PointerDecl, B);
}
// Update the contents of a Pointer.
void UpdatePointer(const Decl *PointerD, MemoryLoc ML) {
assert(isPointer(PointerD));
- DeclPointsTo[PointerD] = ML;
+ PointsTo.SetDeclPointer(PointerD, ML);
}
void UpdatePointer(const Decl *PointerD, const Expr *A) {
UpdatePointer(PointerD, ResolveExpr(A));
}
void SetExprPointer(const Expr *E, MemoryLoc ML) {
- assert(ExprPointsTo.insert({E, ML}).second);
+ assert(!PointsTo.ContainsExprPointsTo(E));
+ PointsTo.SetExprPointer(E, ML);
}
void SetExprPointer(const Expr *E, const Expr *PointeeE) {
SetExprPointer(E, ResolveExpr(PointeeE));
@@ -203,16 +560,16 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
SetExprPointer(E, ResolveDecl(D));
}
-public:
MemoryLoc ResolveExpr(const Expr *E) {
if (!E)
return MemoryLoc::Empty();
- Handle(E);
- auto Res = ExprPointsTo.find(E);
- if (Res != ExprPointsTo.end()) {
- return Res->getSecond();
+ if (PointsTo.ContainsExprPointsTo(E)) {
+ return PointsTo.GetExprPointsTo(E);
}
- return ExprPointsTo[E] = MemoryLoc::Unknown();
+ Handle(E);
+ if (PointsTo.ContainsExprPointsTo(E))
+ return PointsTo.GetExprPointsTo(E);
+ return PointsTo.SetExprPointer(E, MemoryLoc::Unknown());
}
// Returns the memory location pointed to by D. If D is a pointer-type,
@@ -221,56 +578,43 @@ class PointsToTracker : public ConstStmtVisitor<PointsToTracker> {
MaybeInitaliseDecl(D);
if (isPointer(D))
return ResolvePointer(D);
- if (isOwner(D))
- return ResolveOwner(D);
- return MemoryLoc::Unknown();
+ return ResolveNonPointer(D);
}
MemoryLoc ResolvePointer(const Decl *D) {
auto *VD = dyn_cast<VarDecl>(D);
- assert(VD);
- if (!VD->hasLocalStorage())
+ // TODO: Handle other decls like field.
+ if (!VD || !VD->hasLocalStorage())
return MemoryLoc::Unknown();
assert(isPointer(D));
- auto Res = DeclPointsTo.find(D);
- assert(Res != DeclPointsTo.end());
- return Res->getSecond();
+ return PointsTo.GetDeclPointsTo(D);
}
- MemoryLoc ResolveOwner(const Decl *D) {
- assert(isOwner(D));
- if (IsOnStack(D))
+ MemoryLoc ResolveNonPointer(const Decl *D) {
+ assert(!isPointer(D));
+ if (IsOnStack(D)) {
return MemoryLoc::VarOnStack(D);
+ }
return MemoryLoc::Unknown();
}
- void AddToStack(const Decl *D) {
- assert(isOwner(D));
- StackDecls.insert(D);
- }
+ void AddToStack(const Decl *D) { PointsTo.AddToStack(D); }
void AddToStack(const Expr *E) {
assert(isa<MaterializeTemporaryExpr>(E));
- assert(isOwner(E));
- StackExprs.insert(E);
+ assert(!isPointer(E));
+ PointsTo.AddToStack(E);
// Add a self edge.
- assert(ExprPointsTo.insert({E, MemoryLoc::Temporary(E)}).second);
+ assert(!PointsTo.ContainsExprPointsTo(E));
+ PointsTo.SetExprPointer(E, MemoryLoc::Temporary(E));
}
- bool IsOnStack(const Decl *D) { return StackDecls.contains(D); }
- bool IsOnStack(const Expr *E) { return StackExprs.contains(E); }
+ bool IsOnStack(const Decl *D) { return PointsTo.IsOnStack(D); }
+ bool IsOnStack(const Expr *E) { return PointsTo.IsOnStack(E); }
- // ExpressionResolver &ExprResolver;
- // Map from an expression of View type to its pointee and Owner type to the
- // reference<TODO simplify>. This should follow single assignment because
- // Expr* cannot be reassigned in the program.
- llvm::DenseMap<const Expr *, MemoryLoc> ExprPointsTo;
- // Map from a decl of View type to it pointee. This can be reassigned at
- // various points in the program due to transfer functions.
- llvm::DenseMap<const Decl *, MemoryLoc> DeclPointsTo;
-
- // Collection of Expr* and Decl* stored on stack.
- llvm::DenseSet<const Expr *> StackExprs;
- llvm::DenseSet<const Decl *> StackDecls;
+ PointsToSet PointsTo;
+ const DeclContext &DC;
+ DanglingReferenceReporter *Reporter;
};
+
class DanglingReferenceAnalyzer {
public:
DanglingReferenceAnalyzer(const DeclContext &DC, const CFG &cfg,
@@ -278,61 +622,58 @@ class DanglingReferenceAnalyzer {
DanglingReferenceReporter *Reporter)
: DC(DC), cfg(cfg), AC(AC), Reporter(Reporter) {}
void RunAnalysis() {
-
// For simplicity in protoytpe, avoid joins and stick to functions without
// branches.
- if (!cfg.isLinear())
- return;
+ // if (!cfg.isLinear())
+ // return;
// cfg.dump(AC.getASTContext().getLangOpts(), true);
- for (auto I = cfg.begin(), E = cfg.end(); I != E; ++I) {
- for (CFGBlock::const_iterator BI = (*I)->begin(), BE = (*I)->end();
- BI != BE; ++BI) {
- if (auto cfgstmt = BI->getAs<CFGStmt>()) {
- auto *stmt = cfgstmt->getStmt();
- // llvm::errs() <<
- // "================================================\n";
- // cfgstmt->dump();
- // if (auto *E = dyn_cast<Expr>(stmt))
- // E->dumpColor();
- PointsTo.Handle(stmt);
- if (auto *E = dyn_cast<Expr>(stmt)) {
- // llvm::errs() << "Points To : " << PointsTo.ResolveExpr(E).str()
- // << "\n";
- }
- if (auto *RS = dyn_cast<ReturnStmt>(stmt))
- HandleReturnStmt(RS);
- }
- }
+ ForwardDataflowWorklist worklist(cfg, AC);
+ worklist.enqueueSuccessors(&cfg.getEntry());
+ PointsToSets.insert({&cfg.getEntry(), PointsToSet(Factory)});
+
+ llvm::BitVector Visited(cfg.getNumBlockIDs());
+
+ while (const CFGBlock *Block = worklist.dequeue()) {
+ unsigned BlockID = Block->getBlockID();
+ if (Visited[BlockID])
+ continue;
+ // llvm::errs() << "====================================\n";
+ // Block->dump();
+ PointsToSet IncomingPointsTo = MergePredecessors(Block);
+ BlockVisitor BV(IncomingPointsTo, DC, Reporter);
+ BV.Handle(Block);
+ PointsToSets.insert({Block, BV.getOutgoing()});
+
+ // llvm::errs() << "Incoming points to:\n";
+ // IncomingPointsTo.dump();
+ // llvm::errs() << "Outgoing points to:\n";
+ // BV.getOutgoing().dump();
+
+ worklist.enqueueSuccessors(Block);
+ Visited[BlockID] = true;
}
}
-private:
- void HandleReturnStmt(const ReturnStmt *RS) {
- // Diagnose possible return of stack variable.
- if (!RS)
- return;
- const Expr *RetExpr = RS->getRetValue();
- if (!RetExpr || !isPointer(RetExpr))
- return;
- PointsTo.Handle(RetExpr);
- auto RetPointee = PointsTo.ResolveExpr(RetExpr);
- // RetExpr->dumpColor();
- // llvm::errs() << "Returning pointer to " << RetPointee.str() << "\n";
- if (RetPointee.IsOnStack()) {
- // This points to something on stack!!
- if (auto *D = RetPointee.getDecl())
- Reporter->ReportReturnLocalVar(RetExpr, D);
- if (auto *E = RetPointee.getExpr())
- Reporter->ReportReturnTemporaryExpr(E);
+ PointsToSet MergePredecessors(const CFGBlock *Block) {
+ PointsToSet Result(Factory);
+ for (const auto &Pred : Block->preds()) {
+ if (PointsToSets.contains(Pred)) {
+ Result = Merge(Result, PointsToSets.find(Pred)->getSecond());
+ // llvm::errs() << "Merging:\n";
+ // PointsToSets.find(Pred)->getSecond().dump();
+ }
}
+ // llvm::errs() << "Merged result:\n";
+ // Result.dump();
+ return Result;
}
+ PointsToFactory Factory;
[[maybe_unused]] const DeclContext &DC;
const CFG &cfg;
- [[maybe_unused]] AnalysisDeclContext &AC;
+ AnalysisDeclContext &AC;
DanglingReferenceReporter *Reporter;
- // ExpressionResolver ExprResolver;
- PointsToTracker PointsTo;
+ llvm::DenseMap<const CFGBlock *, PointsToSet> PointsToSets;
};
} // namespace
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 618b00734ccb2ac..3fc98b4388ee941 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2650,10 +2650,14 @@ class DanglingReferenceReporterImpl : public DanglingReferenceReporter {
<< RetExpr->getExprLoc();
S.Diag(LocalDecl->getLocation(), diag::note_local_variable_declared_here)
<< LocalDecl->getLocation();
+ llvm::errs() << "Debug: Return local var:\n";
+ RetExpr->getExprLoc().dump(S.getSourceManager());
}
void ReportReturnTemporaryExpr(const Expr *TemporaryExpr) override {
S.Diag(TemporaryExpr->getExprLoc(), diag::warn_ret_stack_temporary_ref_cfg)
<< TemporaryExpr->getExprLoc();
+ llvm::errs() << "Debug: Return temporary expr:\n";
+ TemporaryExpr->getExprLoc().dump(S.getSourceManager());
}
private:
diff --git a/clang/test/Sema/Inputs/lifetime-analysis.h b/clang/test/Sema/Inputs/lifetime-analysis.h
index f888e6ab94bb6aa..f176a4636f94144 100644
--- a/clang/test/Sema/Inputs/lifetime-analysis.h
+++ b/clang/test/Sema/Inputs/lifetime-analysis.h
@@ -87,6 +87,7 @@ template<typename T>
struct unique_ptr {
T &operator*();
T *get() const;
+ T *release();
};
template<typename T>
diff --git a/clang/test/Sema/warn-lifetime-analysis-cfg.cpp b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp
index e337c75f8344d2f..824cdf4510f1123 100644
--- a/clang/test/Sema/warn-lifetime-analysis-cfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-cfg.cpp
@@ -90,6 +90,11 @@ std::string_view return_local() {
local;
}
+int* return_null_ptr() {
+ auto T = nullptr;
+ return T;
+}
+
// Parameters
std::string_view params_owner_and_views(
std::string_view sv,
@@ -123,14 +128,285 @@ std::string_view copy_of_global_str() {
return sv; // expected-warning {{returning reference to a stack variable}}
}
-// TODO: Use lifetimebound in function calls. Use Pointer to Owner of pointer
-// std::string_view containerOfString() {
-// std::vector<std::string> local;
-// return local.at(0);
-// }
+struct Struct { std::string s; };
+std::string_view field() {
+ Struct s;
+ std::string_view sv;
+ sv = s.s;
+ return sv; // FIXME.
+}
+
+std::string_view loopReturnFor(int n) {
+ for (int i = 0; i < n; ++i) {
+ std::string local = "inside-loop"; // expected-note {{reference to this stack variable is returned}}
+ return local; // expected-warning {{returning reference to a stack variable}}
+ }
+ return "safe"; // Safe return.
+}
+
+std::string_view loopReturnWhile(bool condition) {
+ while (condition) {
+ std::string local = "inside-loop"; // expected-note {{reference to this stack variable is returned}}
+ return local; // expected-warning {{returning reference to a stack variable}}
+ }
+ return "safe"; // Safe return.
+}
+
+std::string_view loopReturnDoWhile() {
+ do {
+ std::string local = "inside-loop"; // expected-note {{reference to this stack variable is returned}}
+ return local; // expected-warning {{returning reference to a stack variable}}
+ } while (false);
+}
+
+std::string_view nestedConditionals(bool a, bool b) {
+ std::string local = "nested"; // expected-note {{reference to this stack variable is returned}}
+ if (a) {
+ if (b) {
+ return local; // expected-warning {{returning reference to a stack variable}}
+ }
+ }
+ return "safe";
+}
+
+std::string_view nestedLoops(int n) {
+ std::string local = "loop"; // expected-note {{reference to this stack variable is returned}}
+ for (int i = 0; i < n; ++i) {
+ for (int j = 0; j < n; ++j) {
+ return local; // expected-warning {{returning reference to a stack variable}}
+ }
+ }
+ return "safe";
+}
+
+namespace lifetimebound {
+
+std::string_view func_lb_sv(std::string_view sv [[clang::lifetimebound]]);
+std::string_view use_func_lb_sv() {
+ std::string s; // expected-note {{reference to this stack variable is returned}}
+ std::string_view sv = func_lb_sv(s);
+ sv = func_lb_sv(s);
+ std::string_view sv2;
+ sv2 = sv;
+ return sv2; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view use_string_func_lb_sv() {
+ std::string s; // expected-note {{reference to this stack variable is returned}}
+ std::string_view sv = s;
+ std::string_view sv_lb = func_lb_sv(sv);
+ return sv_lb; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view direct_return() {
+ std::string s; // expected-note {{reference to this stack variable is returned}}
+ std::string_view sv = s;
+ return func_lb_sv(sv); // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view if_branches(std::string param, bool cond) { // expected-note 4 {{reference to this stack variable is returned}}
+ std::string_view view;
+ view = func_lb_sv(param);
+ view = func_lb_sv(view);
+ std::string_view stack = view;
+ if (cond) view = func_lb_sv(view);
+ if (cond) view = func_lb_sv(view);
+ if (cond)
+ return view; // expected-warning {{returning reference to a stack variable}}
+ if (cond)
+ view = unknown();
+ // Now it is potentially safe!
+ if (cond)
+ return view; // Ok. We can't know for sure.
+ if (cond) {
+ view = stack;
+ return view; // expected-warning {{returning reference to a stack variable}}
+ }
+ // Unconditionally restore to stack.
+ view = stack;
+ if (cond)
+ return view; // expected-warning {{returning reference to a stack variable}}
+ return view; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view nested_early_return(std::string param, bool a, bool b) { // expected-note 2 {{reference to this stack variable is returned}}
+ std::string_view view;
+ view = func_lb_sv(param);
+ view = func_lb_sv(view);
+ std::string_view stack = view;
+
+ if (a) {
+ if (b) {
+ return view; // expected-warning {{returning reference to a stack variable}}
+ }
+ view = unknown();
+ } else {
+ if (b) {
+ view = stack;
+ return view; // expected-warning {{returning reference to a stack variable}}
+ }
+ }
+ return view; // Ok, because one branch set it to unknown.
+}
+
+std::string_view func_with_branches(std::string param, bool cond1, bool cond2) { // expected-note 2 {{reference to this stack variable is returned}}
+ std::string_view view;
+ view = func_lb_sv(param);
+ view = func_lb_sv(view);
+ std::string_view stack = view;
+
+ if (cond1)
+ view = func_lb_sv(view);
+
+ if (cond2)
+ view = unknown();
+ else if (cond1)
+ return view; // expected-warning {{returning reference to a stack variable}}
+
+ if (cond1 && cond2)
+ return view; // Ok. We can't be sure.
+
+ view = stack;
+ return view; // expected-warning {{returning reference to a stack variable}}
+}
+
+
+namespace MemberFunctions {
+struct S {
+ std::string return_string() const;
+ const std::string& return_string_ref() const [[clang::lifetimebound]];
+};
+
+std::string_view use_return_string() {
+ S s;
+ return s.return_string(); // expected-warning {{returning reference to a temporary object}}
+}
+std::string_view use_return_string_ref() {
+ S s; // expected-note {{reference to this stack variable is returned}}
+ return s.return_string_ref(); // expected-warning {{returning reference to a stack variable}}
+}
+std::string_view use_return_string_ref(const S* s) {
+ return s->return_string_ref();
+}
+std::string_view use_return_string_ref_temporary() {
+ return S{}.return_string_ref(); // expected-warning {{returning reference to a temporary object}}
+}
+} // namespace MemberFunctions
+
+namespace Optional {
+std::optional<std::string_view> getOptional(std::string_view);
+std::string_view usOptional(std::string_view s) {
+ return getOptional(s).value();
+}
+} // namespace Optional
+struct ThisIsLB {
+std::string_view get() [[clang::lifetimebound]];
+};
+
+std::string_view use_lifetimebound_member_fn() {
+ ThisIsLB obj; // expected-note {{reference to this stack variable is returned}}
+ return obj.get(); // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view return_temporary_get() {
+ return ThisIsLB{}.get(); // expected-warning {{returning reference to a temporary object}}
+}
+
+std::string_view store_temporary_get() {
+ // FIXME: Move this diagnostic to the return loc!!
+ std::string_view sv1 = ThisIsLB{}.get(); // expected-warning {{returning reference to a temporary object}}
+ std::string_view sv2 = sv1;
+ std::string_view sv3 = func_lb_sv(sv2);
+ return sv3;
+}
+
+std::string_view multiple_lifetimebound_calls() {
+ std::string s; // expected-note {{reference to this stack variable is returned}}
+ std::string_view sv = func_lb_sv(func_lb_sv(func_lb_sv(s)));
+ sv = func_lb_sv(func_lb_sv(func_lb_sv(sv)));
+ return sv; // expected-warning {{returning reference to a stack variable}}
+}
+
+} // namespace lifetimebound
+
+std::string_view return_char_star() {
+ const char* key;
+ key = "foo";
+ return key;
+}
+
+void lambda() {
+ std::string s;
+ auto l1 = [&s]() {
+ std::string_view sv = s;
+ return sv;
+ };
+ auto l2 = []() {
+ std::string s; // expected-note {{reference to this stack variable is returned}}
+ std::string_view sv = s;
+ return sv; // expected-warning {{returning reference to a stack variable}}
+ };
+}
+
+std::string_view default_arg(std::string_view sv = std::string()) {
+ return sv; // Ok!
+}
+std::string_view default_arg_overwritten(std::string_view sv = std::string()) {
+ std::string s; // expected-note {{reference to this stack variable is returned}}
+ sv = s;
+ return sv; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view containerOfString(bool cond) {
+ if (cond) {
+ return std::vector<std::string>{"one"}.at(0); // expected-warning {{returning reference to a temporary object}}
+ }
+ std::vector<std::string> local; // expected-note 2 {{reference to this stack variable is returned}}
+ if (cond) {
+ return local.at(0); // expected-warning {{returning reference to a stack variable}}
+ }
+ std::string_view sv = local.at(0);
+ if (cond) {
+ return sv; // expected-warning {{returning reference to a stack variable}}
+ }
+ return unknown();
+}
+
+std::string_view conditionalReturn(bool flag) {
+ std::string local1 = "if-branch"; // expected-note {{reference to this stack variable is returned}}
+ std::string local2 = "else-branch"; // expected-note {{reference to this stack variable is returned}}
+ if (flag)
+ return local1; // expected-warning {{returning reference to a stack variable}}
+ else
+ return local2; // expected-warning {{returning reference to a stack variable}}
+}
+
+std::string_view conditionalReturnMixed(bool flag, const std::string& external) {
+ std::string local = "stack-var"; // expected-note {{reference to this stack variable is returned}}
+ if (flag)
+ return local; // expected-warning {{returning reference to a stack variable}}
+ else
+ return external; // OK, returning reference to external.
+}
-// std::string_view containerOfViewMultistmt() {
-// std::vector<std::string> local;
-// std::string_view sv = local.at(0);
-// return sv;
-// }
+void move_to_arena(std::unique_ptr<int>);
+void move_to_arena(int*);
+int* move_unique_ptr(bool cond) {
+ std::unique_ptr<int> local; // expected-note 3 {{reference to this stack variable is returned}}
+ int* pointer = local.get();
+ if (cond)
+ return local.get(); // expected-warning {{returning reference to a stack variable}}
+ if (cond)
+ return pointer; // expected-warning {{returning reference to a stack variable}}
+ if (cond) {
+ // 'local' definitely moved to unknown.
+ move_to_arena(std::move(local));
+ return pointer;
+ }
+ if(cond)
+ return pointer; // expected-warning {{returning reference to a stack variable}}
+ // Maybe 'local' is moved to unknown.
+ if (cond)
+ move_to_arena(local.release());
+ return pointer; // Ok.
+}
\ No newline at end of file
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 4c19367bb7f3ddc..38ff380edf99021 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -Wdangling -Wdangling-field -Wreturn-stack-address -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-dangling -Wdangling-cfg -verify=cfg %s
#include "Inputs/lifetime-analysis.h"
struct [[gsl::Owner(int)]] MyIntOwner {
MyIntOwner();
@@ -88,27 +89,32 @@ MyIntPointer returningLocalPointer() {
}
MyIntPointer daglingGslPtrFromLocalOwner() {
- MyIntOwner localOwner;
+ MyIntOwner localOwner; // cfg-note {{reference to this stack variable is returned}}
return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}}
+ // cfg-warning at -1 {{returning reference to a stack variable}}
}
MyLongPointerFromConversion daglingGslPtrFromLocalOwnerConv() {
- MyLongOwnerWithConversion localOwner;
+ MyLongOwnerWithConversion localOwner; // cfg-note {{reference to this stack variable is returned}}
return localOwner; // expected-warning {{address of stack memory associated with local variable 'localOwner' returned}}
+ // cfg-warning at -1 {{returning reference to a stack variable}}
}
MyIntPointer danglingGslPtrFromTemporary() {
return MyIntOwner{}; // expected-warning {{returning address of local temporary object}}
+ // cfg-warning at -1 {{returning reference to a temporary object}}
}
MyIntOwner makeTempOwner();
MyIntPointer danglingGslPtrFromTemporary2() {
return makeTempOwner(); // expected-warning {{returning address of local temporary object}}
+ // cfg-warning at -1 {{returning reference to a temporary object}}
}
MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() {
return MyLongOwnerWithConversion{}; // expected-warning {{returning address of local temporary object}}
+ // cfg-warning at -1 {{returning reference to a temporary object}}
}
int *noFalsePositive(MyIntOwner &o) {
@@ -144,6 +150,7 @@ void modelIterators() {
std::vector<int>::iterator modelIteratorReturn() {
return std::vector<int>().begin(); // expected-warning {{returning address of local temporary object}}
+ // cfg-warning at -1{{returning reference to a temporary object}}
}
const int *modelFreeFunctions() {
@@ -163,8 +170,9 @@ int modelAnyCast3() {
}
const char *danglingRawPtrFromLocal() {
- std::basic_string<char> s;
+ std::basic_string<char> s; // cfg-note {{reference to this stack variable is returned}}
return s.c_str(); // expected-warning {{address of stack memory associated with local variable 's' returned}}
+ // cfg-warning at -1 {{returning reference to a stack variable}}
}
int &danglingRawPtrFromLocal2() {
@@ -185,8 +193,9 @@ std::string_view containerWithAnnotatedElements() {
// no warning on constructing from gsl-pointer
std::string_view c2 = std::vector<std::string_view>().at(0);
- std::vector<std::string> local;
+ std::vector<std::string> local; // cfg-note {{reference to this stack variable is returned}}
return local.at(0); // expected-warning {{address of stack memory associated with local variable}}
+ // cfg-warning at -1 {{returning reference to a stack variable}}
}
std::string_view localUniquePtr(int i) {
@@ -198,25 +207,29 @@ std::string_view localUniquePtr(int i) {
}
std::string_view localOptional(int i) {
- std::optional<std::string> o;
+ std::optional<std::string> o; // cfg-note {{reference to this stack variable is returned}}
if (i)
return o.value(); // expected-warning {{address of stack memory associated with local variable}}
+ // cfg-warning at -1{{returning reference to a stack variable}}
std::optional<std::string_view> abc;
return abc.value(); // expect no warning
}
const char *danglingRawPtrFromTemp() {
return std::basic_string<char>().c_str(); // expected-warning {{returning address of local temporary object}}
+ // cfg-warning at -1{{returning reference to a temporary object}}
}
std::unique_ptr<int> getUniquePtr();
int *danglingUniquePtrFromTemp() {
return getUniquePtr().get(); // expected-warning {{returning address of local temporary object}}
+ // cfg-warning at -1{{returning reference to a temporary object}}
}
int *danglingUniquePtrFromTemp2() {
return std::unique_ptr<int>().get(); // expected-warning {{returning address of local temporary object}}
+ // cfg-warning at -1{{returning reference to a temporary object}}
}
void danglingReferenceFromTempOwner() {
@@ -300,7 +313,7 @@ void danglingStringviewAssignment(std::string_view a1, std::string_view a2) {
std::reference_wrapper<int> danglingPtrFromNonOwnerLocal() {
int i = 5;
- return i; // TODO
+ return i;
}
std::reference_wrapper<int> danglingPtrFromNonOwnerLocal2() {
@@ -411,6 +424,7 @@ MyIntPointer handleDerivedToBaseCast1(MySpecialIntPointer ptr) {
MyIntPointer handleDerivedToBaseCast2(MyOwnerIntPointer ptr) {
return ptr; // expected-warning {{address of stack memory associated with parameter 'ptr' returned}}
+ // FIXME(cfg): Detect this! we are traversing the type hierarchy for the attr.
}
std::vector<int>::iterator noFalsePositiveWithVectorOfPointers() {
@@ -615,10 +629,14 @@ std::string_view test5() {
// Pointer<Pointer> from Owner<Pointer>
// Prevent regression GH108463
-Span<int*> test6(std::vector<int*> v) {
+Span<int*> test6(std::vector<int*> v, bool cond) { // cfg-note {{reference to this stack variable is returned}}
Span<int *> dangling = std::vector<int*>(); // expected-warning {{object backing the pointer}}
+ // cfg-warning at -1 {{returning reference to a temporary object}}
+ if(cond) return dangling;
+
dangling = std::vector<int*>(); // expected-warning {{object backing the pointer}}
return v; // expected-warning {{address of stack memory}}
+ // cfg-warning at -1 {{returning reference to a stack variable}}
}
/////// From Owner<Owner<Pointer>> ///////
@@ -636,16 +654,18 @@ std::vector<int*> test8(StatusOr<std::vector<int*>> aa) {
}
// Pointer<Pointer> from Owner<Owner<Pointer>>
-Span<int*> test9(StatusOr<std::vector<int*>> aa) {
+Span<int*> test9(StatusOr<std::vector<int*>> aa) { // cfg-note {{reference to this stack variable is returned}}
return aa.valueLB(); // expected-warning {{address of stack memory associated}}
+ // cfg-warning at -1 {{returning reference to a stack variable}}
return aa.valueNoLB(); // OK.
}
/////// From Owner<Owner> ///////
// Pointer<Owner>> from Owner<Owner>
-Span<std::string> test10(StatusOr<std::vector<std::string>> aa) {
+Span<std::string> test10(StatusOr<std::vector<std::string>> aa) { // cfg-note {{reference to this stack variable is returned}}
return aa.valueLB(); // expected-warning {{address of stack memory}}
+ // cfg-warning at -1 {{returning reference to a stack variable}}
return aa.valueNoLB(); // OK.
}
@@ -663,11 +683,12 @@ const int& test12(Span<int> a) {
return a.getFieldNoLB(); // OK.
}
-void test13() {
+auto test13() {
// FIXME: RHS is Owner<Pointer>, we skip this case to avoid false positives.
std::optional<Span<int*>> abc = std::vector<int*>{};
std::optional<Span<int>> t = std::vector<int> {}; // expected-warning {{object backing the pointer will be destroyed}}
+ return t;
}
} // namespace GH100526
@@ -690,8 +711,10 @@ class set {
} // namespace std
namespace GH118064{
-void test() {
+auto test() {
auto y = std::set<int>{}.begin(); // expected-warning {{object backing the pointer}}
+ // cfg-warning at -1{{returning reference to a temporary object}}
+ return y;
}
} // namespace GH118064
@@ -703,20 +726,26 @@ std::string_view TakeSv(std::string_view abc [[clang::lifetimebound]]);
std::string_view TakeStrRef(const std::string& abc [[clang::lifetimebound]]);
std::string_view TakeStr(std::string abc [[clang::lifetimebound]]);
+bool foo();
std::string_view test1() {
- std::string_view t1 = Ref(std::string()); // expected-warning {{object backing}}
- t1 = Ref(std::string()); // expected-warning {{object backing}}
- return Ref(std::string()); // expected-warning {{returning address}}
-
- std::string_view t2 = TakeSv(std::string()); // expected-warning {{object backing}}
- t2 = TakeSv(std::string()); // expected-warning {{object backing}}
- return TakeSv(std::string()); // expected-warning {{returning address}}
-
- std::string_view t3 = TakeStrRef(std::string()); // expected-warning {{temporary}}
- t3 = TakeStrRef(std::string()); // expected-warning {{object backing}}
- return TakeStrRef(std::string()); // expected-warning {{returning address}}
-
-
+ if(foo()) {
+ std::string_view t1 = Ref(std::string()); // expected-warning {{object backing}}
+ t1 = Ref(std::string()); // expected-warning {{object backing}}
+ return Ref(std::string()); // expected-warning {{returning address}}
+ // cfg-warning at -1 {{returning reference to a temporary object}}
+ }
+ if(foo()) {
+ std::string_view t2 = TakeSv(std::string()); // expected-warning {{object backing}}
+ t2 = TakeSv(std::string()); // expected-warning {{object backing}}
+ return TakeSv(std::string()); // expected-warning {{returning address}}
+ // cfg-warning at -1 {{returning reference to a temporary object}}
+ }
+ if(foo()) {
+ std::string_view t3 = TakeStrRef(std::string()); // expected-warning {{temporary}}
+ t3 = TakeStrRef(std::string()); // expected-warning {{object backing}}
+ return TakeStrRef(std::string()); // expected-warning {{returning address}}
+ // cfg-warning at -1 {{returning reference to a temporary object}}
+ }
std::string_view t4 = TakeStr(std::string());
t4 = TakeStr(std::string());
return TakeStr(std::string());
@@ -727,10 +756,13 @@ struct Foo {
const T& get() const [[clang::lifetimebound]];
const T& getNoLB() const;
};
-std::string_view test2(Foo<std::string> r1, Foo<std::string_view> r2) {
+std::string_view test2(
+ Foo<std::string> r1, // cfg-note {{reference to this stack variable is returned}}
+ Foo<std::string_view> r2) {
std::string_view t1 = Foo<std::string>().get(); // expected-warning {{object backing}}
t1 = Foo<std::string>().get(); // expected-warning {{object backing}}
return r1.get(); // expected-warning {{address of stack}}
+ // cfg-warning at -1 {{returning reference to a stack variable}}
std::string_view t2 = Foo<std::string_view>().get();
t2 = Foo<std::string_view>().get();
@@ -750,6 +782,7 @@ Pointer test3(Bar bar) {
Pointer p = Pointer(Bar()); // expected-warning {{temporary}}
p = Pointer(Bar()); // expected-warning {{object backing}}
return bar; // expected-warning {{address of stack}}
+ // FIXME(cfg): Handle Lifetimebound constructors.
}
template<typename T>
More information about the cfe-commits
mailing list