llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-analysis
Author: Kashika Akhouri (kashika0112)
<details>
<summary>Changes</summary>
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
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}}
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/171972
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
