https://github.com/danakj updated
https://github.com/llvm/llvm-project/pull/91777
>From 8b318dadac6d0ec53b5d26461edfe19a391845ec Mon Sep 17 00:00:00 2001
From: danakj <dan...@chromium.org>
Date: Fri, 10 May 2024 13:31:17 -0400
Subject: [PATCH 1/4] Respect the [[clang::unsafe_buffer_usage]] attribute for
constructors
The -Wunsafe-buffer-usage warning should fire on any call to a function
annotated with [[clang::unsafe_buffer_usage]], however it omitted calls
to constructors, since the expression is a CXXConstructExpr which does
not subclass CallExpr. Thus the matcher on callExpr() does not find
these expressions.
Add a new WarningGadget that matches cxxConstructExpr that are calling
a CXXConstructDecl annotated by [[clang::unsafe_buffer_usage]] and fires
the warning. The new UnsafeBufferUsageCtorAttrGadget gadget explicitly
avoids matching against the std::span(ptr, size) constructor because
that is handled by SpanTwoParamConstructorGadget and we never want two
gadgets to match the same thing (and this is guarded by asserts).
The gadgets themselves do not report the warnings, instead each gadget's
Stmt is passed to the UnsafeBufferUsageHandler (implemented by
UnsafeBufferUsageReporter). The Reporter is previously hardcoded that a
CXXConstructExpr statement must be a match for std::span(ptr, size),
but that is no longer the case. We want the Reporter to generate
different warnings (in the -Wunsafe-buffer-usage-in-container subgroup)
for the span contructor. And we will want it to report more warnings for
other std-container-specific gadgets in the future. To handle this we
allow the gadget to control if the warning is general
(it calls handleUnsafeBufferUsage()) or is a std-container-specific
warning (it calls handleUnsafeOperationInContainer()).
Then the WarningGadget grows a virtual method to dispatch to the
appropriate path in the UnsafeBufferUsageHandler. By doing so, we no
longer need getBaseStmt in the Gadget interface. The only use of it for
FixableGadgets was to get the SourceLocation, so we make an explicit
virtual method for that on Gadget. Then the handleUnsafeOperation()
dispatcher can be a virtual method that is only in WarningGadget.
The SpanTwoParamConstructorGadget gadget dispatches to
handleUnsafeOperationInContainer() while the other WarningGadgets all
dispatch to the original handleUnsafeBufferUsage().
Tests are added for annotated constructors, conversion operattors, call
operators, fold expressions, and regular methods.
---
.../Analysis/Analyses/UnsafeBufferUsage.h | 5 +
.../Analyses/UnsafeBufferUsageGadgets.def | 1 +
clang/lib/Analysis/UnsafeBufferUsage.cpp | 167 ++++++++++++------
clang/lib/Sema/AnalysisBasedWarnings.cpp | 22 ++-
...warn-unsafe-buffer-usage-function-attr.cpp | 38 ++++
5 files changed, 176 insertions(+), 57 deletions(-)
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 5d16dcc824c50..228b4ae1e3e11 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -106,6 +106,11 @@ class UnsafeBufferUsageHandler {
virtual void handleUnsafeOperation(const Stmt *Operation,
bool IsRelatedToDecl, ASTContext &Ctx) =
0;
+ /// Invoked when an unsafe operation with a std container is found.
+ virtual void handleUnsafeOperationInContainer(const Stmt *Operation,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) = 0;
+
/// Invoked when a fix is suggested against a variable. This function groups
/// all variables that must be fixed together (i.e their types must be
changed
/// to the same target type to prevent type mismatches) into a single fixit.
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 3273c642eed51..242ad763ba62b 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -36,6 +36,7 @@ WARNING_GADGET(Decrement)
WARNING_GADGET(ArraySubscript)
WARNING_GADGET(PointerArithmetic)
WARNING_GADGET(UnsafeBufferUsageAttr)
+WARNING_GADGET(UnsafeBufferUsageCtorAttr)
WARNING_GADGET(DataInvocation)
WARNING_CONTAINER_GADGET(SpanTwoParamConstructor) // Uses of `std::span(arg0,
arg1)`
FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified
Lvalue Context
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index c42e70d5b95ac..b24588e722aaa 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -492,7 +492,7 @@ class Gadget {
#endif
virtual bool isWarningGadget() const = 0;
- virtual const Stmt *getBaseStmt() const = 0;
+ virtual SourceLocation getSourceLoc() const = 0;
/// Returns the list of pointer-type variables on which this gadget performs
/// its operation. Typically, there's only one variable. This isn't a list
@@ -513,6 +513,10 @@ class WarningGadget : public Gadget {
static bool classof(const Gadget *G) { return G->isWarningGadget(); }
bool isWarningGadget() const final { return true; }
+
+ virtual void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const = 0;
};
/// Fixable gadgets correspond to code patterns that aren't always unsafe but
@@ -572,7 +576,12 @@ class IncrementGadget : public WarningGadget {
.bind(OpTag));
}
- const UnaryOperator *getBaseStmt() const override { return Op; }
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ Handler.handleUnsafeOperation(Op, IsRelatedToDecl, Ctx);
+ }
+ SourceLocation getSourceLoc() const override { return Op->getBeginLoc(); }
DeclUseList getClaimedVarUseSites() const override {
SmallVector<const DeclRefExpr *, 2> Uses;
@@ -607,7 +616,12 @@ class DecrementGadget : public WarningGadget {
.bind(OpTag));
}
- const UnaryOperator *getBaseStmt() const override { return Op; }
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ Handler.handleUnsafeOperation(Op, IsRelatedToDecl, Ctx);
+ }
+ SourceLocation getSourceLoc() const override { return Op->getBeginLoc(); }
DeclUseList getClaimedVarUseSites() const override {
if (const auto *DRE =
@@ -648,7 +662,12 @@ class ArraySubscriptGadget : public WarningGadget {
// clang-format on
}
- const ArraySubscriptExpr *getBaseStmt() const override { return ASE; }
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ Handler.handleUnsafeOperation(ASE, IsRelatedToDecl, Ctx);
+ }
+ SourceLocation getSourceLoc() const override { return ASE->getBeginLoc(); }
DeclUseList getClaimedVarUseSites() const override {
if (const auto *DRE =
@@ -696,7 +715,12 @@ class PointerArithmeticGadget : public WarningGadget {
.bind(PointerArithmeticTag));
}
- const Stmt *getBaseStmt() const override { return PA; }
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ Handler.handleUnsafeOperation(PA, IsRelatedToDecl, Ctx);
+ }
+ SourceLocation getSourceLoc() const override { return PA->getBeginLoc(); }
DeclUseList getClaimedVarUseSites() const override {
if (const auto *DRE = dyn_cast<DeclRefExpr>(Ptr->IgnoreParenImpCasts())) {
@@ -734,7 +758,12 @@ class SpanTwoParamConstructorGadget : public WarningGadget
{
.bind(SpanTwoParamConstructorTag));
}
- const Stmt *getBaseStmt() const override { return Ctor; }
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ Handler.handleUnsafeOperationInContainer(Ctor, IsRelatedToDecl, Ctx);
+ }
+ SourceLocation getSourceLoc() const override { return Ctor->getBeginLoc(); }
DeclUseList getClaimedVarUseSites() const override {
// If the constructor call is of the form `std::span{var, n}`, `var` is
@@ -780,11 +809,8 @@ class PointerInitGadget : public FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &S) const override;
-
- virtual const Stmt *getBaseStmt() const override {
- // FIXME: This needs to be the entire DeclStmt, assuming that this method
- // makes sense at all on a FixableGadget.
- return PtrInitRHS;
+ SourceLocation getSourceLoc() const override {
+ return PtrInitRHS->getBeginLoc();
}
virtual DeclUseList getClaimedVarUseSites() const override {
@@ -833,12 +859,7 @@ class PtrToPtrAssignmentGadget : public FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &S) const override;
-
- virtual const Stmt *getBaseStmt() const override {
- // FIXME: This should be the binary operator, assuming that this method
- // makes sense at all on a FixableGadget.
- return PtrLHS;
- }
+ SourceLocation getSourceLoc() const override { return PtrLHS->getBeginLoc();
}
virtual DeclUseList getClaimedVarUseSites() const override {
return DeclUseList{PtrLHS, PtrRHS};
@@ -888,12 +909,7 @@ class CArrayToPtrAssignmentGadget : public FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &S) const override;
-
- virtual const Stmt *getBaseStmt() const override {
- // FIXME: This should be the binary operator, assuming that this method
- // makes sense at all on a FixableGadget.
- return PtrLHS;
- }
+ SourceLocation getSourceLoc() const override { return PtrLHS->getBeginLoc();
}
virtual DeclUseList getClaimedVarUseSites() const override {
return DeclUseList{PtrLHS, PtrRHS};
@@ -921,10 +937,55 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget {
}
static Matcher matcher() {
- return
stmt(callExpr(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage))))
- .bind(OpTag));
+ auto HasUnsafeFnDecl =
+ callee(functionDecl(hasAttr(attr::UnsafeBufferUsage)));
+ return stmt(callExpr(HasUnsafeFnDecl).bind(OpTag));
+ }
+
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ Handler.handleUnsafeOperation(Op, IsRelatedToDecl, Ctx);
+ }
+ SourceLocation getSourceLoc() const override { return Op->getBeginLoc(); }
+
+ DeclUseList getClaimedVarUseSites() const override { return {}; }
+};
+
+/// A call of a constructor that performs unchecked buffer operations
+/// over one of its pointer parameters, or constructs a class object that will
+/// perform buffer operations that depend on the correctness of the parameters.
+class UnsafeBufferUsageCtorAttrGadget : public WarningGadget {
+ constexpr static const char *const OpTag = "cxx_construct_expr";
+ const CXXConstructExpr *Op;
+
+public:
+ UnsafeBufferUsageCtorAttrGadget(const MatchFinder::MatchResult &Result)
+ : WarningGadget(Kind::UnsafeBufferUsageCtorAttr),
+ Op(Result.Nodes.getNodeAs<CXXConstructExpr>(OpTag)) {}
+
+ static bool classof(const Gadget *G) {
+ return G->getKind() == Kind::UnsafeBufferUsageCtorAttr;
+ }
+
+ static Matcher matcher() {
+ auto HasUnsafeCtorDecl =
+ hasDeclaration(cxxConstructorDecl(hasAttr(attr::UnsafeBufferUsage)));
+ // std::span(ptr, size) ctor is handled by SpanTwoParamConstructorGadget.
+ auto HasTwoParamSpanCtorDecl = hasDeclaration(
+ cxxConstructorDecl(hasDeclContext(isInStdNamespace()), hasName("span"),
+ parameterCountIs(2)));
+ return stmt(
+ cxxConstructExpr(HasUnsafeCtorDecl, unless(HasTwoParamSpanCtorDecl))
+ .bind(OpTag));
+ }
+
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ Handler.handleUnsafeOperation(Op, IsRelatedToDecl, Ctx);
}
- const Stmt *getBaseStmt() const override { return Op; }
+ SourceLocation getSourceLoc() const override { return Op->getBeginLoc(); }
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
@@ -953,7 +1014,13 @@ class DataInvocationGadget : public WarningGadget {
explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)))))
.bind(OpTag));
}
- const Stmt *getBaseStmt() const override { return Op; }
+
+ void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) const override {
+ Handler.handleUnsafeOperation(Op, IsRelatedToDecl, Ctx);
+ }
+ SourceLocation getSourceLoc() const override { return Op->getBeginLoc(); }
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
@@ -990,8 +1057,7 @@ class ULCArraySubscriptGadget : public FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &S) const override;
-
- virtual const Stmt *getBaseStmt() const override { return Node; }
+ SourceLocation getSourceLoc() const override { return Node->getBeginLoc(); }
virtual DeclUseList getClaimedVarUseSites() const override {
if (const auto *DRE =
@@ -1031,8 +1097,7 @@ class UPCStandalonePointerGadget : public FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &S) const override;
-
- virtual const Stmt *getBaseStmt() const override { return Node; }
+ SourceLocation getSourceLoc() const override { return Node->getBeginLoc(); }
virtual DeclUseList getClaimedVarUseSites() const override { return {Node}; }
};
@@ -1070,10 +1135,9 @@ class PointerDereferenceGadget : public FixableGadget {
return {BaseDeclRefExpr};
}
- virtual const Stmt *getBaseStmt() const final { return Op; }
-
virtual std::optional<FixItList>
getFixits(const FixitStrategy &S) const override;
+ SourceLocation getSourceLoc() const override { return Op->getBeginLoc(); }
};
// Represents expressions of the form `&DRE[any]` in the Unspecified Pointer
@@ -1108,8 +1172,7 @@ class UPCAddressofArraySubscriptGadget : public
FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &) const override;
-
- virtual const Stmt *getBaseStmt() const override { return Node; }
+ SourceLocation getSourceLoc() const override { return Node->getBeginLoc(); }
virtual DeclUseList getClaimedVarUseSites() const override {
const auto *ArraySubst = cast<ArraySubscriptExpr>(Node->getSubExpr());
@@ -1218,8 +1281,7 @@ class UPCPreIncrementGadget : public FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &S) const override;
-
- virtual const Stmt *getBaseStmt() const override { return Node; }
+ SourceLocation getSourceLoc() const override { return Node->getBeginLoc(); }
virtual DeclUseList getClaimedVarUseSites() const override {
return {dyn_cast<DeclRefExpr>(Node->getSubExpr())};
@@ -1264,8 +1326,7 @@ class UUCAddAssignGadget : public FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &S) const override;
-
- virtual const Stmt *getBaseStmt() const override { return Node; }
+ SourceLocation getSourceLoc() const override { return Node->getBeginLoc(); }
virtual DeclUseList getClaimedVarUseSites() const override {
return {dyn_cast<DeclRefExpr>(Node->getLHS())};
@@ -1315,9 +1376,9 @@ class DerefSimplePtrArithFixableGadget : public
FixableGadget {
virtual std::optional<FixItList>
getFixits(const FixitStrategy &s) const final;
-
- // TODO remove this method from FixableGadget interface
- virtual const Stmt *getBaseStmt() const final { return nullptr; }
+ SourceLocation getSourceLoc() const override {
+ return DerefOp->getBeginLoc();
+ }
virtual DeclUseList getClaimedVarUseSites() const final {
return {BaseDeclRefExpr};
@@ -2070,7 +2131,7 @@ UUCAddAssignGadget::getFixits(const FixitStrategy &S)
const {
if (S.lookup(VD) == FixitStrategy::Kind::Span) {
FixItList Fixes;
- const Stmt *AddAssignNode = getBaseStmt();
+ const Stmt *AddAssignNode = Node;
StringRef varName = VD->getName();
const ASTContext &Ctx = VD->getASTContext();
@@ -2112,7 +2173,6 @@ UPCPreIncrementGadget::getFixits(const FixitStrategy &S)
const {
if (S.lookup(VD) == FixitStrategy::Kind::Span) {
FixItList Fixes;
std::stringstream SS;
- const Stmt *PreIncNode = getBaseStmt();
StringRef varName = VD->getName();
const ASTContext &Ctx = VD->getASTContext();
@@ -2120,12 +2180,12 @@ UPCPreIncrementGadget::getFixits(const FixitStrategy
&S) const {
SS << "(" << varName.data() << " = " << varName.data()
<< ".subspan(1)).data()";
std::optional<SourceLocation> PreIncLocation =
- getEndCharLoc(PreIncNode, Ctx.getSourceManager(), Ctx.getLangOpts());
+ getEndCharLoc(Node, Ctx.getSourceManager(), Ctx.getLangOpts());
if (!PreIncLocation)
return std::nullopt;
Fixes.push_back(FixItHint::CreateReplacement(
- SourceRange(PreIncNode->getBeginLoc(), *PreIncLocation), SS.str()));
+ SourceRange(Node->getBeginLoc(), *PreIncLocation), SS.str()));
return Fixes;
}
}
@@ -2856,7 +2916,7 @@ getFixIts(FixableGadgetSets &FixablesForAllVars, const
FixitStrategy &S,
}
#ifndef NDEBUG
Handler.addDebugNoteForVar(
- VD, F->getBaseStmt()->getBeginLoc(),
+ VD, F->getSourceLoc(),
("gadget '" + F->getDebugName() + "' refused to produce a fix")
.str());
#endif
@@ -3008,8 +3068,9 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
// every problematic operation and consider it done. No need to deal
// with fixable gadgets, no need to group operations by variable.
for (const auto &G : WarningGadgets) {
- Handler.handleUnsafeOperation(G->getBaseStmt(),
/*IsRelatedToDecl=*/false,
- D->getASTContext());
+ llvm::errs() << "Warnings not EmitSuggestions\n";
+ G->handleUnsafeOperation(Handler, /*IsRelatedToDecl=*/false,
+ D->getASTContext());
}
// This return guarantees that most of the machine doesn't run when
@@ -3251,8 +3312,8 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
Tracker, Handler, VarGrpMgr);
for (const auto &G : UnsafeOps.noVar) {
- Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/false,
- D->getASTContext());
+ G->handleUnsafeOperation(Handler, /*IsRelatedToDecl=*/false,
+ D->getASTContext());
}
for (const auto &[VD, WarningGadgets] : UnsafeOps.byVar) {
@@ -3263,8 +3324,8 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
: FixItList{},
D, NaiveStrategy);
for (const auto &G : WarningGadgets) {
- Handler.handleUnsafeOperation(G->getBaseStmt(), /*IsRelatedToDecl=*/true,
- D->getASTContext());
+ G->handleUnsafeOperation(Handler, /*IsRelatedToDecl=*/true,
+ D->getASTContext());
}
}
}
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 6992ba9ad9a75..b3dd4bc6aa8f5 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2256,11 +2256,8 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
Range = UO->getSubExpr()->getSourceRange();
MsgParam = 1;
}
- } else if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(Operation)) {
- S.Diag(CtorExpr->getLocation(),
- diag::warn_unsafe_buffer_usage_in_container);
} else {
- if (isa<CallExpr>(Operation)) {
+ if (isa<CallExpr>(Operation) || isa<CXXConstructExpr>(Operation)) {
// note_unsafe_buffer_operation doesn't have this mode yet.
assert(!IsRelatedToDecl && "Not implemented yet!");
MsgParam = 3;
@@ -2295,6 +2292,23 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
}
}
+ void handleUnsafeOperationInContainer(const Stmt *Operation,
+ bool IsRelatedToDecl,
+ ASTContext &Ctx) override {
+ SourceLocation Loc;
+ SourceRange Range;
+ unsigned MsgParam = 0;
+ if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(Operation)) {
+ Loc = CtorExpr->getLocation();
+ }
+ S.Diag(Loc, diag::warn_unsafe_buffer_usage_in_container);
+ if (IsRelatedToDecl) {
+ assert(!SuggestSuggestions &&
+ "Variables blamed for unsafe buffer usage without suggestions!");
+ S.Diag(Loc, diag::note_unsafe_buffer_operation) << MsgParam << Range;
+ }
+ }
+
void handleUnsafeVariableGroup(const VarDecl *Variable,
const VariableGroupsManager &VarGrpMgr,
FixItList &&Fixes, const Decl *D,
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp
index 7df01c46438c7..bfc34b55c1f66 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp
@@ -85,3 +85,41 @@ void testInheritance() {
BC->func(); // expected-warning{{function introduces unsafe buffer
manipulation}}
BC->func1();
}
+
+class UnsafeMembers {
+public:
+ UnsafeMembers() {}
+
+ [[clang::unsafe_buffer_usage]]
+ UnsafeMembers(int) {}
+
+ [[clang::unsafe_buffer_usage]]
+ explicit operator int() { return 0; }
+
+ [[clang::unsafe_buffer_usage]]
+ void Method() {}
+
+ [[clang::unsafe_buffer_usage]]
+ void operator()() {}
+
+ [[clang::unsafe_buffer_usage]]
+ int operator+(UnsafeMembers) { return 0; }
+};
+
+template <class... Vs>
+int testFoldExpression(Vs&&... v) {
+ return (... + v); // expected-warning{{function introduces unsafe buffer
manipulation}}
+}
+
+// https://github.com/llvm/llvm-project/issues/80482
+void testClassMembers() {
+ UnsafeMembers(3); // expected-warning{{function introduces unsafe buffer
manipulation}}
+
+ (void)static_cast<int>(UnsafeMembers()); // expected-warning{{function
introduces unsafe buffer manipulation}}
+
+ UnsafeMembers().Method(); // expected-warning{{function introduces unsafe
buffer manipulation}}
+
+ UnsafeMembers()(); // expected-warning{{function introduces unsafe buffer
manipulation}}
+
+ testFoldExpression(UnsafeMembers(), UnsafeMembers());
+}
>From 84da0fe9aa5d80c41ef78e79f6d47730f5be7880 Mon Sep 17 00:00:00 2001
From: danakj <dan...@chromium.org>
Date: Fri, 10 May 2024 13:53:23 -0400
Subject: [PATCH 2/4] Remove debug logging
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index b24588e722aaa..9e07b9cbb6aea 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -3068,7 +3068,6 @@ void clang::checkUnsafeBufferUsage(const Decl *D,
// every problematic operation and consider it done. No need to deal
// with fixable gadgets, no need to group operations by variable.
for (const auto &G : WarningGadgets) {
- llvm::errs() << "Warnings not EmitSuggestions\n";
G->handleUnsafeOperation(Handler, /*IsRelatedToDecl=*/false,
D->getASTContext());
}
>From d9e7b26a515c3a397b23e4e07dff0a209dcd8094 Mon Sep 17 00:00:00 2001
From: danakj <dan...@chromium.org>
Date: Tue, 14 May 2024 16:51:05 -0400
Subject: [PATCH 3/4] Apply review suggestions
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 4 +---
clang/lib/Sema/AnalysisBasedWarnings.cpp | 9 ++++++---
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 9e07b9cbb6aea..65747ad9ad6d3 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -972,9 +972,7 @@ class UnsafeBufferUsageCtorAttrGadget : public
WarningGadget {
auto HasUnsafeCtorDecl =
hasDeclaration(cxxConstructorDecl(hasAttr(attr::UnsafeBufferUsage)));
// std::span(ptr, size) ctor is handled by SpanTwoParamConstructorGadget.
- auto HasTwoParamSpanCtorDecl = hasDeclaration(
- cxxConstructorDecl(hasDeclContext(isInStdNamespace()), hasName("span"),
- parameterCountIs(2)));
+ auto HasTwoParamSpanCtorDecl = SpanTwoParamConstructorGadget::matcher();
return stmt(
cxxConstructExpr(HasUnsafeCtorDecl, unless(HasTwoParamSpanCtorDecl))
.bind(OpTag));
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index b3dd4bc6aa8f5..b9d0b59ef1db7 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2298,9 +2298,12 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
SourceLocation Loc;
SourceRange Range;
unsigned MsgParam = 0;
- if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(Operation)) {
- Loc = CtorExpr->getLocation();
- }
+
+ // This function only handles SpanTwoParamConstructorGadget so far, which
+ // always gives a CXXConstructExpr.
+ const auto *CtorExpr = cast<CXXConstructExpr>(Operation);
+ Loc = CtorExpr->getLocation();
+
S.Diag(Loc, diag::warn_unsafe_buffer_usage_in_container);
if (IsRelatedToDecl) {
assert(!SuggestSuggestions &&
>From c156c31b1e95f77e36db4420d26becd14eef8bbe Mon Sep 17 00:00:00 2001
From: danakj <dan...@chromium.org>
Date: Wed, 15 May 2024 16:19:54 -0400
Subject: [PATCH 4/4] Add TODO comment back, but in the Gadget class
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 65747ad9ad6d3..38b5d561e6ef0 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -492,6 +492,7 @@ class Gadget {
#endif
virtual bool isWarningGadget() const = 0;
+ // TODO remove this method from FixableGadget interface
virtual SourceLocation getSourceLoc() const = 0;
/// Returns the list of pointer-type variables on which this gadget performs
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
