llvm-project
llvm-project copied to clipboard
llvm
[LifetimeSafety] Cross-TU Vs Intra-TU Annotation Suggestions
Differentiate between cross-TU and intra-TU annotation suggestions.
This PR refines the lifetime annotation suggestion feature by introducing a distinction between cross-TU and intra-TU annotation suggestions. The primary purpose of this change is to differentiate between suggestions which can be replaced by inference and other cross-tu suggestions where inference is not sufficient to replace the need of explicit annotations.
We have introduced two new warning groups under Lifetime Safety Suggestions:
-
-Wexperimental-lifetime-safety-cross-tu-suggestions: These are for functions whose declarations are in header, definition in source file. These are higher priority as it affects callers in other translation units. Inference cannot replace these annotations across TU boundaries (even in its most powerful form). Annotation suggestion is on the header declaration. -
-Wexperimental-lifetime-safety-intra-tu-suggestions: For suggestions on functions which are in the same TU. These can be considered lower-priority suggestions as inference can potentially handle these within the same TU.
Example:
// Header file r.h
#include <iostream>
#include <string>
std::string_view public_func(std::string_view a);
inline std::string_view inline_header(std::string_view a) {
return a;
}
// Source (cpp file)
#include <iostream>
#include <string>
#include "r.h"
std::string_view public_func(std::string_view a) {
return a;
}
namespace {
std::string_view private_func(std::string_view a) {
return a;
}
}
The warnings generated are:
./r.h:6:39: warning: parameter in intra-TU function should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-intra-tu-suggestions]
6 | inline std::string_view inline_header(std::string_view a) {
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
./r.h:7:12: note: param returned here
7 | return a;
./r.h:4:30: warning: parameter in cross-TU function should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-cross-tu-suggestions]
4 | std::string_view public_func(std::string_view a);
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
r.cpp:6:10: note: param returned here
6 | return a;
| ^
r.cpp:10:33: warning: parameter in intra-TU function should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-intra-tu-suggestions]
10 | std::string_view private_func(std::string_view a) {
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
r.cpp:11:10: note: param returned here
11 | return a;
| ^
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-temporal-safety
Author: Kashika Akhouri (kashika0112)
Changes
Introduce between public and private annotation suggestions.
This PR refines the lifetime annotation suggestion feature by introducing a distinction between public and private annotation suggestions.
We have introduced two new warning groups under Lifetime Safety Suggestions:
-Wexperimental-lifetime-safety-public-suggestions: For suggestions on public functions (those with external linkage). These are treated as high-priority suggestions as they affect the public API of a module. For public functions that are declared in a header file and defined in a source file, the[[clang::lifetimebound]]annotation is now suggested on the declaration in the header file.-Wexperimental-lifetime-safety-private-suggestions: For suggestions on private functions (those with internal linkage, e.g., static functions or functions in an anonymous namespace). These can be considered lower-priority suggestions. For private functions, suggestion is placed on the definition.
Example:
// Header file r.h
#include <iostream>
#include <string>
std::string_view public_func(std::string_view a);
inline std::string_view inline_header(std::string_view a) {
return a;
}
// Source (cpp file)
#include <iostream>
#include <string>
#include "r.h"
std::string_view public_func(std::string_view a) {
return a;
}
namespace {
std::string_view private_func(std::string_view a) {
return a;
}
}
The warnings generated are:
./r.h:6:39: warning: externally visible param should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-public-suggestions]
6 | inline std::string_view inline_header(std::string_view a) {
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
./r.h:7:12: note: param returned here
7 | return a;
| ^
./r.h:4:30: warning: externally visible param should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-public-suggestions]
4 | std::string_view public_func(std::string_view a);
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
r.cpp:6:10: note: param returned here
6 | return a;
| ^
r.cpp:10:33: warning: param with internal linkage should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-private-suggestions]
10 | std::string_view private_func(std::string_view a) {
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
r.cpp:11:10: note: param returned here
11 | return a;
| ^
Full diff: https://github.com/llvm/llvm-project/pull/171972.diff
6 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+9-3)
- (modified) clang/include/clang/Basic/DiagnosticGroups.td (+11-1)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+10-3)
- (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+20-2)
- (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+24-5)
- (modified) clang/test/Sema/warn-lifetime-safety-suggestions.cpp (+90-32)
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
index 31fae55f60486..5a840f17203b5 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
@@ -48,9 +48,15 @@ class LifetimeSafetyReporter {
SourceLocation ExpiryLoc,
Confidence Confidence) {}
- // Suggests lifetime bound annotations for function paramters
- virtual void suggestAnnotation(const ParmVarDecl *PVD,
- const Expr *EscapeExpr) {}
+ // Suggest private lifetime bound annotations for function parameters internal
+ // to the existing file.
+ virtual void suggestAnnotationsPrivate(const ParmVarDecl *ParmToAnnotate,
+ const Expr *EscapeExpr) {}
+
+ // Suggest public lifetime bound annotations for function parameters external
+ // to other files.
+ virtual void suggestAnnotationsPublic(const ParmVarDecl *ParmToAnnotate,
+ const Expr *EscapeExpr) {}
};
/// The main entry point for the analysis.
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index e1dba0195f470..ac05f11ddcf11 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -541,8 +541,18 @@ def LifetimeSafety : DiagGroup<"experimental-lifetime-safety",
Experimental warnings to detect use-after-free and related temporal safety bugs based on lifetime safety analysis.
}];
}
+def LifetimeSafetyPublicSuggestions
+ : DiagGroup<"experimental-lifetime-safety-public-suggestions">;
+def LifetimeSafetyPrivateSuggestions
+ : DiagGroup<"experimental-lifetime-safety-private-suggestions">;
def LifetimeSafetySuggestions
- : DiagGroup<"experimental-lifetime-safety-suggestions">;
+ : DiagGroup<"experimental-lifetime-safety-suggestions",
+ [LifetimeSafetyPublicSuggestions,
+ LifetimeSafetyPrivateSuggestions]> {
+ code Documentation = [{
+ Lifetime annotation suggestions for function parameters that should be marked [[clang::lifetimebound]] based on lifetime analysis.
+ }];
+}
def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 381d1fb063eba..69e46aeecf6a9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10818,9 +10818,16 @@ 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">;
-def warn_lifetime_safety_suggest_lifetimebound
- : Warning<"param should be marked [[clang::lifetimebound]]">,
- InGroup<LifetimeSafetySuggestions>,
+def warn_lifetime_safety_private_suggestion
+ : Warning<"param with internal linkage should be marked "
+ "[[clang::lifetimebound]]">,
+ InGroup<LifetimeSafetyPrivateSuggestions>,
+ DefaultIgnore;
+
+def warn_lifetime_safety_public_suggestion
+ : Warning<
+ "externally visible param should be marked [[clang::lifetimebound]]">,
+ InGroup<LifetimeSafetyPublicSuggestions>,
DefaultIgnore;
def note_lifetime_safety_suggestion_returned_here : Note<"param returned here">;
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index 99071d6b46c1e..cd0454bae2d62 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -20,6 +20,7 @@
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TimeProfiler.h"
@@ -163,8 +164,25 @@ class LifetimeChecker {
void suggestAnnotations() {
if (!Reporter)
return;
- for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap)
- Reporter->suggestAnnotation(PVD, EscapeExpr);
+ SourceManager &SM = AST.getSourceManager();
+ for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) {
+ const auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext());
+ if (!FD)
+ continue;
+ // For public functions (external linkage), find the header declaration
+ // to annotate; otherwise, treat as private and annotate the definition.
+ if (FD->isExternallyVisible()) {
+ const FunctionDecl *CanonicalFD = FD->getCanonicalDecl();
+ const ParmVarDecl *ParmToAnnotate = PVD;
+ if (CanonicalFD != FD && SM.getFileID(FD->getLocation()) !=
+ SM.getFileID(CanonicalFD->getLocation()))
+ if (unsigned Index = PVD->getFunctionScopeIndex();
+ Index < CanonicalFD->getNumParams())
+ ParmToAnnotate = CanonicalFD->getParamDecl(Index);
+ Reporter->suggestAnnotationsPublic(ParmToAnnotate, EscapeExpr);
+ } else
+ Reporter->suggestAnnotationsPrivate(PVD, EscapeExpr);
+ }
}
void inferAnnotations() {
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 321445dd4e1ff..f2081d2e2bebe 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2883,14 +2883,33 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter {
<< EscapeExpr->getEndLoc();
}
- void suggestAnnotation(const ParmVarDecl *PVD,
- const Expr *EscapeExpr) override {
+ void suggestAnnotationsPrivate(const ParmVarDecl *ParmToAnnotate,
+ const Expr *EscapeExpr) override {
SourceLocation InsertionPoint = Lexer::getLocForEndOfToken(
- PVD->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts());
- S.Diag(PVD->getBeginLoc(), diag::warn_lifetime_safety_suggest_lifetimebound)
- << PVD->getSourceRange()
+ ParmToAnnotate->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts());
+
+ S.Diag(ParmToAnnotate->getBeginLoc(),
+ diag::warn_lifetime_safety_private_suggestion)
+ << ParmToAnnotate->getSourceRange()
+ << FixItHint::CreateInsertion(InsertionPoint,
+ " [[clang::lifetimebound]]");
+
+ S.Diag(EscapeExpr->getBeginLoc(),
+ diag::note_lifetime_safety_suggestion_returned_here)
+ << EscapeExpr->getSourceRange();
+ }
+
+ void suggestAnnotationsPublic(const ParmVarDecl *ParmToAnnotate,
+ const Expr *EscapeExpr) override {
+ SourceLocation InsertionPoint = Lexer::getLocForEndOfToken(
+ ParmToAnnotate->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts());
+
+ S.Diag(ParmToAnnotate->getBeginLoc(),
+ diag::warn_lifetime_safety_public_suggestion)
+ << ParmToAnnotate->getSourceRange()
<< FixItHint::CreateInsertion(InsertionPoint,
" [[clang::lifetimebound]]");
+
S.Diag(EscapeExpr->getBeginLoc(),
diag::note_lifetime_safety_suggestion_returned_here)
<< EscapeExpr->getSourceRange();
diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp
index 9f3ccb7fca770..e3096540d269b 100644
--- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -fexperimental-lifetime-safety-inference -Wexperimental-lifetime-safety-suggestions -Wexperimental-lifetime-safety -verify %s
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -fexperimental-lifetime-safety-inference -Wexperimental-lifetime-safety-suggestions -Wexperimental-lifetime-safety -I%t -verify %t/test_source.cpp
struct MyObj {
int id;
@@ -12,14 +14,52 @@ struct [[gsl::Pointer()]] View {
void use() const;
};
-View return_view_directly (View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+//===----------------------------------------------------------------------===//
+// Public Annotation Test Cases
+//===----------------------------------------------------------------------===//
+
+//--- test_header.h
+#ifndef TEST_HEADER_H
+#define TEST_HEADER_H
+
+struct MyObj {
+ int id;
+ ~MyObj() {} // Non-trivial destructor
+ MyObj operator+(MyObj);
+};
+
+struct [[gsl::Pointer()]] View {
+ View(const MyObj&); // Borrows from MyObj
+ View();
+ void use() const;
+};
+
+View return_view_directly(View a); // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+
+View conditional_return_view(
+ View a, // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+ View b, // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+ bool c);
+
+int* return_pointer_directly(int* a); // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+
+MyObj* return_pointer_object(MyObj* a); // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+
+inline View inline_header_return_view(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+ return a; // expected-note {{param returned here}}
+}
+
+#endif // TEST_HEADER_H
+
+//--- test_source.cpp
+
+#include "test_header.h"
+
+View return_view_directly(View a) {
return a; // expected-note {{param returned here}}
}
-View conditional_return_view (
- View a, // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
- View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
- bool c) {
+View conditional_return_view(View a, View b, bool c) {
View res;
if (c)
res = a;
@@ -28,29 +68,20 @@ View conditional_return_view (
return res; // expected-note 2 {{param returned here}}
}
-// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently.
-MyObj& return_reference (MyObj& a, MyObj& b, bool c) {
- if(c) {
- return a;
- }
- return b;
-}
-
-// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently.
-View return_view_from_reference (MyObj& p) {
- return p;
-}
-
-int* return_pointer_directly (int* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+int* return_pointer_directly(int* a) {
return a; // expected-note {{param returned here}}
}
-MyObj* return_pointer_object (MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+MyObj* return_pointer_object(MyObj* a) {
return a; // expected-note {{param returned here}}
}
+//===----------------------------------------------------------------------===//
+// Private Annotation Test Cases
+//===----------------------------------------------------------------------===//
+namespace {
View only_one_paramter_annotated (View a [[clang::lifetimebound]],
- View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+ View b, // expected-warning {{param with internal linkage should be marked [[clang::lifetimebound]]}}.
bool c) {
if(c)
return a;
@@ -59,11 +90,25 @@ View only_one_paramter_annotated (View a [[clang::lifetimebound]],
View reassigned_to_another_parameter (
View a,
- View b) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+ View b) { // expected-warning {{param with internal linkage should be marked [[clang::lifetimebound]]}}.
a = b;
return a; // expected-note {{param returned here}}
}
+View private_func_redecl (View a);
+View private_func_redecl (View a) { // expected-warning {{param with internal linkage should be marked [[clang::lifetimebound]]}}.
+ return a; // expected-note {{param returned here}}
+}
+}
+
+static View return_view_static (View a) { // expected-warning {{param with internal linkage should be marked [[clang::lifetimebound]]}}.
+ return a; // expected-note {{param returned here}}
+}
+
+//===----------------------------------------------------------------------===//
+// FIXME Test Cases
+//===----------------------------------------------------------------------===//
+
struct ReturnsSelf {
const ReturnsSelf& get() const {
return *this;
@@ -89,16 +134,29 @@ void test_getView_on_temporary() {
(void)sv;
}
+// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently.
+MyObj& return_reference (MyObj& a, MyObj& b, bool c) {
+ if(c) {
+ return a;
+ }
+ return b;
+}
+
+// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently.
+View return_view_from_reference (MyObj& p) {
+ return p;
+}
+
//===----------------------------------------------------------------------===//
// Annotation Inference Test Cases
//===----------------------------------------------------------------------===//
namespace correct_order_inference {
-View return_view_by_func (View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View return_view_by_func (View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return return_view_directly(a); // expected-note {{param returned here}}
}
-MyObj* return_pointer_by_func (MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+MyObj* return_pointer_by_func (MyObj* a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return return_pointer_object(a); // expected-note {{param returned here}}
}
} // namespace correct_order_inference
@@ -111,7 +169,7 @@ View return_view_caller(View a) {
return return_view_callee(a);
}
-View return_view_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View return_view_callee(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
} // namespace incorrect_order_inference_view
@@ -124,17 +182,17 @@ MyObj* return_object_caller(MyObj* a) {
return return_object_callee(a);
}
-MyObj* return_object_callee(MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+MyObj* return_object_callee(MyObj* a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
} // namespace incorrect_order_inference_object
namespace simple_annotation_inference {
-View inference_callee_return_identity(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View inference_callee_return_identity(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
-View inference_caller_forwards_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View inference_caller_forwards_callee(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return inference_callee_return_identity(a); // expected-note {{param returned here}}
}
@@ -147,12 +205,12 @@ View inference_top_level_return_stack_view() {
namespace inference_in_order_with_redecls {
View inference_callee_return_identity(View a);
-View inference_callee_return_identity(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View inference_callee_return_identity(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
View inference_caller_forwards_callee(View a);
-View inference_caller_forwards_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View inference_caller_forwards_callee(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return inference_callee_return_identity(a); // expected-note {{param returned here}}
}
@@ -165,7 +223,7 @@ View inference_top_level_return_stack_view() {
namespace inference_with_templates {
template<typename T>
-T* template_identity(T* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+T* template_identity(T* a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
@llvm/pr-subscribers-clang-analysis
Author: Kashika Akhouri (kashika0112)
Changes
Introduce between public and private annotation suggestions.
This PR refines the lifetime annotation suggestion feature by introducing a distinction between public and private annotation suggestions.
We have introduced two new warning groups under Lifetime Safety Suggestions:
-Wexperimental-lifetime-safety-public-suggestions: For suggestions on public functions (those with external linkage). These are treated as high-priority suggestions as they affect the public API of a module. For public functions that are declared in a header file and defined in a source file, the[[clang::lifetimebound]]annotation is now suggested on the declaration in the header file.-Wexperimental-lifetime-safety-private-suggestions: For suggestions on private functions (those with internal linkage, e.g., static functions or functions in an anonymous namespace). These can be considered lower-priority suggestions. For private functions, suggestion is placed on the definition.
Example:
// Header file r.h
#include <iostream>
#include <string>
std::string_view public_func(std::string_view a);
inline std::string_view inline_header(std::string_view a) {
return a;
}
// Source (cpp file)
#include <iostream>
#include <string>
#include "r.h"
std::string_view public_func(std::string_view a) {
return a;
}
namespace {
std::string_view private_func(std::string_view a) {
return a;
}
}
The warnings generated are:
./r.h:6:39: warning: externally visible param should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-public-suggestions]
6 | inline std::string_view inline_header(std::string_view a) {
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
./r.h:7:12: note: param returned here
7 | return a;
| ^
./r.h:4:30: warning: externally visible param should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-public-suggestions]
4 | std::string_view public_func(std::string_view a);
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
r.cpp:6:10: note: param returned here
6 | return a;
| ^
r.cpp:10:33: warning: param with internal linkage should be marked [[clang::lifetimebound]] [-Wexperimental-lifetime-safety-private-suggestions]
10 | std::string_view private_func(std::string_view a) {
| ^~~~~~~~~~~~~~~~~~
| [[clang::lifetimebound]]
r.cpp:11:10: note: param returned here
11 | return a;
| ^
Full diff: https://github.com/llvm/llvm-project/pull/171972.diff
6 Files Affected:
- (modified) clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h (+9-3)
- (modified) clang/include/clang/Basic/DiagnosticGroups.td (+11-1)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+10-3)
- (modified) clang/lib/Analysis/LifetimeSafety/Checker.cpp (+20-2)
- (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+24-5)
- (modified) clang/test/Sema/warn-lifetime-safety-suggestions.cpp (+90-32)
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
index 31fae55f60486..5a840f17203b5 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
@@ -48,9 +48,15 @@ class LifetimeSafetyReporter {
SourceLocation ExpiryLoc,
Confidence Confidence) {}
- // Suggests lifetime bound annotations for function paramters
- virtual void suggestAnnotation(const ParmVarDecl *PVD,
- const Expr *EscapeExpr) {}
+ // Suggest private lifetime bound annotations for function parameters internal
+ // to the existing file.
+ virtual void suggestAnnotationsPrivate(const ParmVarDecl *ParmToAnnotate,
+ const Expr *EscapeExpr) {}
+
+ // Suggest public lifetime bound annotations for function parameters external
+ // to other files.
+ virtual void suggestAnnotationsPublic(const ParmVarDecl *ParmToAnnotate,
+ const Expr *EscapeExpr) {}
};
/// The main entry point for the analysis.
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index e1dba0195f470..ac05f11ddcf11 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -541,8 +541,18 @@ def LifetimeSafety : DiagGroup<"experimental-lifetime-safety",
Experimental warnings to detect use-after-free and related temporal safety bugs based on lifetime safety analysis.
}];
}
+def LifetimeSafetyPublicSuggestions
+ : DiagGroup<"experimental-lifetime-safety-public-suggestions">;
+def LifetimeSafetyPrivateSuggestions
+ : DiagGroup<"experimental-lifetime-safety-private-suggestions">;
def LifetimeSafetySuggestions
- : DiagGroup<"experimental-lifetime-safety-suggestions">;
+ : DiagGroup<"experimental-lifetime-safety-suggestions",
+ [LifetimeSafetyPublicSuggestions,
+ LifetimeSafetyPrivateSuggestions]> {
+ code Documentation = [{
+ Lifetime annotation suggestions for function parameters that should be marked [[clang::lifetimebound]] based on lifetime analysis.
+ }];
+}
def DistributedObjectModifiers : DiagGroup<"distributed-object-modifiers">;
def DllexportExplicitInstantiationDecl : DiagGroup<"dllexport-explicit-instantiation-decl">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 381d1fb063eba..69e46aeecf6a9 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10818,9 +10818,16 @@ 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">;
-def warn_lifetime_safety_suggest_lifetimebound
- : Warning<"param should be marked [[clang::lifetimebound]]">,
- InGroup<LifetimeSafetySuggestions>,
+def warn_lifetime_safety_private_suggestion
+ : Warning<"param with internal linkage should be marked "
+ "[[clang::lifetimebound]]">,
+ InGroup<LifetimeSafetyPrivateSuggestions>,
+ DefaultIgnore;
+
+def warn_lifetime_safety_public_suggestion
+ : Warning<
+ "externally visible param should be marked [[clang::lifetimebound]]">,
+ InGroup<LifetimeSafetyPublicSuggestions>,
DefaultIgnore;
def note_lifetime_safety_suggestion_returned_here : Note<"param returned here">;
diff --git a/clang/lib/Analysis/LifetimeSafety/Checker.cpp b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
index 99071d6b46c1e..cd0454bae2d62 100644
--- a/clang/lib/Analysis/LifetimeSafety/Checker.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Checker.cpp
@@ -20,6 +20,7 @@
#include "clang/Analysis/Analyses/PostOrderCFGView.h"
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TimeProfiler.h"
@@ -163,8 +164,25 @@ class LifetimeChecker {
void suggestAnnotations() {
if (!Reporter)
return;
- for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap)
- Reporter->suggestAnnotation(PVD, EscapeExpr);
+ SourceManager &SM = AST.getSourceManager();
+ for (const auto &[PVD, EscapeExpr] : AnnotationWarningsMap) {
+ const auto *FD = dyn_cast<FunctionDecl>(PVD->getDeclContext());
+ if (!FD)
+ continue;
+ // For public functions (external linkage), find the header declaration
+ // to annotate; otherwise, treat as private and annotate the definition.
+ if (FD->isExternallyVisible()) {
+ const FunctionDecl *CanonicalFD = FD->getCanonicalDecl();
+ const ParmVarDecl *ParmToAnnotate = PVD;
+ if (CanonicalFD != FD && SM.getFileID(FD->getLocation()) !=
+ SM.getFileID(CanonicalFD->getLocation()))
+ if (unsigned Index = PVD->getFunctionScopeIndex();
+ Index < CanonicalFD->getNumParams())
+ ParmToAnnotate = CanonicalFD->getParamDecl(Index);
+ Reporter->suggestAnnotationsPublic(ParmToAnnotate, EscapeExpr);
+ } else
+ Reporter->suggestAnnotationsPrivate(PVD, EscapeExpr);
+ }
}
void inferAnnotations() {
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 321445dd4e1ff..f2081d2e2bebe 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2883,14 +2883,33 @@ class LifetimeSafetyReporterImpl : public LifetimeSafetyReporter {
<< EscapeExpr->getEndLoc();
}
- void suggestAnnotation(const ParmVarDecl *PVD,
- const Expr *EscapeExpr) override {
+ void suggestAnnotationsPrivate(const ParmVarDecl *ParmToAnnotate,
+ const Expr *EscapeExpr) override {
SourceLocation InsertionPoint = Lexer::getLocForEndOfToken(
- PVD->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts());
- S.Diag(PVD->getBeginLoc(), diag::warn_lifetime_safety_suggest_lifetimebound)
- << PVD->getSourceRange()
+ ParmToAnnotate->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts());
+
+ S.Diag(ParmToAnnotate->getBeginLoc(),
+ diag::warn_lifetime_safety_private_suggestion)
+ << ParmToAnnotate->getSourceRange()
+ << FixItHint::CreateInsertion(InsertionPoint,
+ " [[clang::lifetimebound]]");
+
+ S.Diag(EscapeExpr->getBeginLoc(),
+ diag::note_lifetime_safety_suggestion_returned_here)
+ << EscapeExpr->getSourceRange();
+ }
+
+ void suggestAnnotationsPublic(const ParmVarDecl *ParmToAnnotate,
+ const Expr *EscapeExpr) override {
+ SourceLocation InsertionPoint = Lexer::getLocForEndOfToken(
+ ParmToAnnotate->getEndLoc(), 0, S.getSourceManager(), S.getLangOpts());
+
+ S.Diag(ParmToAnnotate->getBeginLoc(),
+ diag::warn_lifetime_safety_public_suggestion)
+ << ParmToAnnotate->getSourceRange()
<< FixItHint::CreateInsertion(InsertionPoint,
" [[clang::lifetimebound]]");
+
S.Diag(EscapeExpr->getBeginLoc(),
diag::note_lifetime_safety_suggestion_returned_here)
<< EscapeExpr->getSourceRange();
diff --git a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp
index 9f3ccb7fca770..e3096540d269b 100644
--- a/clang/test/Sema/warn-lifetime-safety-suggestions.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-suggestions.cpp
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -fexperimental-lifetime-safety-inference -Wexperimental-lifetime-safety-suggestions -Wexperimental-lifetime-safety -verify %s
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fsyntax-only -fexperimental-lifetime-safety -fexperimental-lifetime-safety-inference -Wexperimental-lifetime-safety-suggestions -Wexperimental-lifetime-safety -I%t -verify %t/test_source.cpp
struct MyObj {
int id;
@@ -12,14 +14,52 @@ struct [[gsl::Pointer()]] View {
void use() const;
};
-View return_view_directly (View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+//===----------------------------------------------------------------------===//
+// Public Annotation Test Cases
+//===----------------------------------------------------------------------===//
+
+//--- test_header.h
+#ifndef TEST_HEADER_H
+#define TEST_HEADER_H
+
+struct MyObj {
+ int id;
+ ~MyObj() {} // Non-trivial destructor
+ MyObj operator+(MyObj);
+};
+
+struct [[gsl::Pointer()]] View {
+ View(const MyObj&); // Borrows from MyObj
+ View();
+ void use() const;
+};
+
+View return_view_directly(View a); // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+
+View conditional_return_view(
+ View a, // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+ View b, // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+ bool c);
+
+int* return_pointer_directly(int* a); // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+
+MyObj* return_pointer_object(MyObj* a); // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+
+inline View inline_header_return_view(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}
+ return a; // expected-note {{param returned here}}
+}
+
+#endif // TEST_HEADER_H
+
+//--- test_source.cpp
+
+#include "test_header.h"
+
+View return_view_directly(View a) {
return a; // expected-note {{param returned here}}
}
-View conditional_return_view (
- View a, // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
- View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
- bool c) {
+View conditional_return_view(View a, View b, bool c) {
View res;
if (c)
res = a;
@@ -28,29 +68,20 @@ View conditional_return_view (
return res; // expected-note 2 {{param returned here}}
}
-// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently.
-MyObj& return_reference (MyObj& a, MyObj& b, bool c) {
- if(c) {
- return a;
- }
- return b;
-}
-
-// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently.
-View return_view_from_reference (MyObj& p) {
- return p;
-}
-
-int* return_pointer_directly (int* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+int* return_pointer_directly(int* a) {
return a; // expected-note {{param returned here}}
}
-MyObj* return_pointer_object (MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+MyObj* return_pointer_object(MyObj* a) {
return a; // expected-note {{param returned here}}
}
+//===----------------------------------------------------------------------===//
+// Private Annotation Test Cases
+//===----------------------------------------------------------------------===//
+namespace {
View only_one_paramter_annotated (View a [[clang::lifetimebound]],
- View b, // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+ View b, // expected-warning {{param with internal linkage should be marked [[clang::lifetimebound]]}}.
bool c) {
if(c)
return a;
@@ -59,11 +90,25 @@ View only_one_paramter_annotated (View a [[clang::lifetimebound]],
View reassigned_to_another_parameter (
View a,
- View b) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+ View b) { // expected-warning {{param with internal linkage should be marked [[clang::lifetimebound]]}}.
a = b;
return a; // expected-note {{param returned here}}
}
+View private_func_redecl (View a);
+View private_func_redecl (View a) { // expected-warning {{param with internal linkage should be marked [[clang::lifetimebound]]}}.
+ return a; // expected-note {{param returned here}}
+}
+}
+
+static View return_view_static (View a) { // expected-warning {{param with internal linkage should be marked [[clang::lifetimebound]]}}.
+ return a; // expected-note {{param returned here}}
+}
+
+//===----------------------------------------------------------------------===//
+// FIXME Test Cases
+//===----------------------------------------------------------------------===//
+
struct ReturnsSelf {
const ReturnsSelf& get() const {
return *this;
@@ -89,16 +134,29 @@ void test_getView_on_temporary() {
(void)sv;
}
+// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently.
+MyObj& return_reference (MyObj& a, MyObj& b, bool c) {
+ if(c) {
+ return a;
+ }
+ return b;
+}
+
+// FIXME: Fails to generate lifetime suggestion for reference types as these are not handled currently.
+View return_view_from_reference (MyObj& p) {
+ return p;
+}
+
//===----------------------------------------------------------------------===//
// Annotation Inference Test Cases
//===----------------------------------------------------------------------===//
namespace correct_order_inference {
-View return_view_by_func (View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View return_view_by_func (View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return return_view_directly(a); // expected-note {{param returned here}}
}
-MyObj* return_pointer_by_func (MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+MyObj* return_pointer_by_func (MyObj* a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return return_pointer_object(a); // expected-note {{param returned here}}
}
} // namespace correct_order_inference
@@ -111,7 +169,7 @@ View return_view_caller(View a) {
return return_view_callee(a);
}
-View return_view_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View return_view_callee(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
} // namespace incorrect_order_inference_view
@@ -124,17 +182,17 @@ MyObj* return_object_caller(MyObj* a) {
return return_object_callee(a);
}
-MyObj* return_object_callee(MyObj* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+MyObj* return_object_callee(MyObj* a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
} // namespace incorrect_order_inference_object
namespace simple_annotation_inference {
-View inference_callee_return_identity(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View inference_callee_return_identity(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
-View inference_caller_forwards_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View inference_caller_forwards_callee(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return inference_callee_return_identity(a); // expected-note {{param returned here}}
}
@@ -147,12 +205,12 @@ View inference_top_level_return_stack_view() {
namespace inference_in_order_with_redecls {
View inference_callee_return_identity(View a);
-View inference_callee_return_identity(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View inference_callee_return_identity(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
View inference_caller_forwards_callee(View a);
-View inference_caller_forwards_callee(View a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+View inference_caller_forwards_callee(View a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return inference_callee_return_identity(a); // expected-note {{param returned here}}
}
@@ -165,7 +223,7 @@ View inference_top_level_return_stack_view() {
namespace inference_with_templates {
template<typename T>
-T* template_identity(T* a) { // expected-warning {{param should be marked [[clang::lifetimebound]]}}.
+T* template_identity(T* a) { // expected-warning {{externally visible param should be marked [[clang::lifetimebound]]}}.
return a; // expected-note {{param returned here}}
}
LGTM. Please also update the PR title and description.
Also mention that the primary purpose of this change is to differentiate between suggestions which can be replaced by inference and other cross-tu suggestions where inference is not sufficient to replace the need of explicit annotations.
Thank you, have updated the PR title and description to reflect the same.
:window: Windows x64 Test Results
- 53080 tests passed
- 2082 tests skipped
:white_check_mark: The build succeeded and all tests passed.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
:penguin: Linux x64 Test Results
- 112121 tests passed
- 4518 tests skipped
:white_check_mark: The build succeeded and all tests passed.
I will land this for now. Precommit CI passes and PR has approvals from all reviewers.
