[clang] While refactoring projects to eradicate unsafe buffer accesses using … (PR #75650)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/75650
…-Wunsafe-buffer-usage,
there maybe accidental re-introduction of new OutOfBound accesses into the code
bases. One such case is invoking span::data() method on a span variable to
retrieve a pointer, which is then cast to a larger type and dereferenced. Such
dereferences can introduce OutOfBound accesses.
To address this, a new WarningGadget is being introduced to warn against such
invocations.
>From 809bf6f4237f634feaeb7e5b0b88be3a2e4de455 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 15 Dec 2023 11:40:55 -0800
Subject: [PATCH] While refactoring projects to eradicate unsafe buffer
accesses using -Wunsafe-buffer-usage, there maybe accidental re-introduction
of new OutOfBound accesses into the code bases. One such case is invoking
span::data() method on a span variable to retrieve a pointer, which is then
cast to a larger type and dereferenced. Such dereferences can introduce
OutOfBound accesses.
To address this, a new WarningGadget is being introduced to warn against such
invocations.
---
.../Analysis/Analyses/UnsafeBufferUsage.h | 2 +-
.../Analyses/UnsafeBufferUsageGadgets.def | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 37 -
clang/lib/Sema/AnalysisBasedWarnings.cpp | 19 ++-
...e-buffer-usage-warning-data-invocation.cpp | 133 ++
6 files changed, 186 insertions(+), 8 deletions(-)
create mode 100644
clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 8a2d56668e32f9..b28f2c6b99c50e 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -66,7 +66,7 @@ class UnsafeBufferUsageHandler {
/// Invoked when an unsafe operation over raw pointers is found.
virtual void handleUnsafeOperation(const Stmt *Operation,
- bool IsRelatedToDecl) = 0;
+ 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
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 757ee452ced748..c9766168836510 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -30,6 +30,7 @@ WARNING_GADGET(Decrement)
WARNING_GADGET(ArraySubscript)
WARNING_GADGET(PointerArithmetic)
WARNING_GADGET(UnsafeBufferUsageAttr)
+WARNING_GADGET(DataInvocation)
FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified
Lvalue Context
FIXABLE_GADGET(DerefSimplePtrArithFixable)
FIXABLE_GADGET(PointerDereference)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 94e97a891baedc..9038dd879f54ae 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12058,7 +12058,7 @@ def warn_unsafe_buffer_variable : Warning<
InGroup, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsafe pointer arithmetic|"
- "unsafe buffer access|function introduces unsafe buffer manipulation}0">,
+ "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe
invocation of span::data}0">,
InGroup, DefaultIgnore;
def note_unsafe_buffer_operation : Note<
"used%select{| in pointer arithmetic| in buffer access}0 here">;
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 70eec1cee57f8e..e4eca9939b10d5 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -721,6 +721,33 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget {
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
+// Warning gadget for unsafe invocation of span::data method.
+// Triggers when the pointer returned by the invocation is immediately
+// cast to a larger type.
+
+class DataInvocationGadget : public WarningGadget {
+ constexpr static const char *const OpTag = "data_invocation_expr";
+ const ExplicitCastExpr *Op;
+
+ public:
+ DataInvocationGadget(const MatchFinder::MatchResult &Result)
+ : WarningGadget(Kind::DataInvocation),
+Op(Result.Nodes.getNodeAs(OpTag)) {}
+
+ static bool classof(const Gadget *G) {
+return G->getKind() == Kind::DataInvocation;
+ }
+
+ static Matcher matcher() {
+return stmt(
+explicitCastExpr(has(cxxMemberCallExpr(callee(
+
[clang] Warning for unsafe invocation of span::data (PR #75650)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/75650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Warning for unsafe invocation of span::data (PR #75650)
@@ -721,6 +721,33 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget {
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
+// Warning gadget for unsafe invocation of span::data method.
+// Triggers when the pointer returned by the invocation is immediately
+// cast to a larger type.
+
+class DataInvocationGadget : public WarningGadget {
+ constexpr static const char *const OpTag = "data_invocation_expr";
+ const ExplicitCastExpr *Op;
+
+ public:
+ DataInvocationGadget(const MatchFinder::MatchResult &Result)
+ : WarningGadget(Kind::DataInvocation),
+Op(Result.Nodes.getNodeAs(OpTag)) {}
+
+ static bool classof(const Gadget *G) {
+return G->getKind() == Kind::DataInvocation;
+ }
+
+ static Matcher matcher() {
+return stmt(
+explicitCastExpr(has(cxxMemberCallExpr(callee(
+ cxxMethodDecl(hasName("data")).bind(OpTag));
malavikasamak wrote:
Yes. It matches all method invocations with name "data". However, we wouldn't
be warning on all of them as the "handleUnsafeOperation" checks if the method
belongs to std::span class. It maybe better to move it to the matcher as you
point out, so there is less overlap between gadgets.
https://github.com/llvm/llvm-project/pull/75650
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Warning for unsafe invocation of span::data (PR #75650)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/75650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Warning for unsafe invocation of span::data (PR #75650)
@@ -2261,6 +2261,21 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
// note_unsafe_buffer_operation doesn't have this mode yet.
assert(!IsRelatedToDecl && "Not implemented yet!");
MsgParam = 3;
+ } else if (const auto *ECE = dyn_cast(Operation)) {
+QualType destType = ECE->getType();
+const uint64_t dSize =
Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
+if(const auto *CE =dyn_cast(ECE->getSubExpr())) {
+
+
if(CE->getRecordDecl()->getQualifiedNameAsString().compare("std::span"))
malavikasamak wrote:
Hmmm. I think it shouldn't be an issue as we are getting the qualified name.
However, I will add a test case to check this.
https://github.com/llvm/llvm-project/pull/75650
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Warning for unsafe invocation of span::data (PR #75650)
@@ -721,6 +721,33 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget {
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
+// Warning gadget for unsafe invocation of span::data method.
+// Triggers when the pointer returned by the invocation is immediately
+// cast to a larger type.
+
+class DataInvocationGadget : public WarningGadget {
+ constexpr static const char *const OpTag = "data_invocation_expr";
malavikasamak wrote:
Thank you. Will fix this.
https://github.com/llvm/llvm-project/pull/75650
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Warning for unsafe invocation of span::data (PR #75650)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/75650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Warning for unsafe invocation of span::data (PR #75650)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/75650
>From 809bf6f4237f634feaeb7e5b0b88be3a2e4de455 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 15 Dec 2023 11:40:55 -0800
Subject: [PATCH 1/2] While refactoring projects to eradicate unsafe buffer
accesses using -Wunsafe-buffer-usage, there maybe accidental re-introduction
of new OutOfBound accesses into the code bases. One such case is invoking
span::data() method on a span variable to retrieve a pointer, which is then
cast to a larger type and dereferenced. Such dereferences can introduce
OutOfBound accesses.
To address this, a new WarningGadget is being introduced to warn against such
invocations.
---
.../Analysis/Analyses/UnsafeBufferUsage.h | 2 +-
.../Analyses/UnsafeBufferUsageGadgets.def | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 37 -
clang/lib/Sema/AnalysisBasedWarnings.cpp | 19 ++-
...e-buffer-usage-warning-data-invocation.cpp | 133 ++
6 files changed, 186 insertions(+), 8 deletions(-)
create mode 100644
clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 8a2d56668e32f9..b28f2c6b99c50e 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -66,7 +66,7 @@ class UnsafeBufferUsageHandler {
/// Invoked when an unsafe operation over raw pointers is found.
virtual void handleUnsafeOperation(const Stmt *Operation,
- bool IsRelatedToDecl) = 0;
+ 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
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 757ee452ced748..c9766168836510 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -30,6 +30,7 @@ WARNING_GADGET(Decrement)
WARNING_GADGET(ArraySubscript)
WARNING_GADGET(PointerArithmetic)
WARNING_GADGET(UnsafeBufferUsageAttr)
+WARNING_GADGET(DataInvocation)
FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified
Lvalue Context
FIXABLE_GADGET(DerefSimplePtrArithFixable)
FIXABLE_GADGET(PointerDereference)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 94e97a891baedc..9038dd879f54ae 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12058,7 +12058,7 @@ def warn_unsafe_buffer_variable : Warning<
InGroup, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsafe pointer arithmetic|"
- "unsafe buffer access|function introduces unsafe buffer manipulation}0">,
+ "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe
invocation of span::data}0">,
InGroup, DefaultIgnore;
def note_unsafe_buffer_operation : Note<
"used%select{| in pointer arithmetic| in buffer access}0 here">;
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 70eec1cee57f8e..e4eca9939b10d5 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -721,6 +721,33 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget {
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
+// Warning gadget for unsafe invocation of span::data method.
+// Triggers when the pointer returned by the invocation is immediately
+// cast to a larger type.
+
+class DataInvocationGadget : public WarningGadget {
+ constexpr static const char *const OpTag = "data_invocation_expr";
+ const ExplicitCastExpr *Op;
+
+ public:
+ DataInvocationGadget(const MatchFinder::MatchResult &Result)
+ : WarningGadget(Kind::DataInvocation),
+Op(Result.Nodes.getNodeAs(OpTag)) {}
+
+ static bool classof(const Gadget *G) {
+return G->getKind() == Kind::DataInvocation;
+ }
+
+ static Matcher matcher() {
+return stmt(
+explicitCastExpr(has(cxxMemberCallExpr(callee(
+ cxxMethodDecl(hasName("data")).bind(OpTag));
+ }
+ const Stmt *getBaseStmt() const override { return Op; }
+
+ DeclUseList getClaimedVarUseSites() const override { return {}; }
+};
+
// Represents expressions of the form `DRE[*]` in the Unspecified Lvalue
// Context (see `isInUnspecifiedLvalueContext`).
// Note here `[]` is the built-in subscript operator.
@@ -2657,8 +2684,
[clang] Warning for unsafe invocation of span::data (PR #75650)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/75650
>From 809bf6f4237f634feaeb7e5b0b88be3a2e4de455 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 15 Dec 2023 11:40:55 -0800
Subject: [PATCH 1/3] While refactoring projects to eradicate unsafe buffer
accesses using -Wunsafe-buffer-usage, there maybe accidental re-introduction
of new OutOfBound accesses into the code bases. One such case is invoking
span::data() method on a span variable to retrieve a pointer, which is then
cast to a larger type and dereferenced. Such dereferences can introduce
OutOfBound accesses.
To address this, a new WarningGadget is being introduced to warn against such
invocations.
---
.../Analysis/Analyses/UnsafeBufferUsage.h | 2 +-
.../Analyses/UnsafeBufferUsageGadgets.def | 1 +
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 37 -
clang/lib/Sema/AnalysisBasedWarnings.cpp | 19 ++-
...e-buffer-usage-warning-data-invocation.cpp | 133 ++
6 files changed, 186 insertions(+), 8 deletions(-)
create mode 100644
clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
index 8a2d56668e32f9..b28f2c6b99c50e 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h
@@ -66,7 +66,7 @@ class UnsafeBufferUsageHandler {
/// Invoked when an unsafe operation over raw pointers is found.
virtual void handleUnsafeOperation(const Stmt *Operation,
- bool IsRelatedToDecl) = 0;
+ 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
diff --git a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 757ee452ced748..c9766168836510 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -30,6 +30,7 @@ WARNING_GADGET(Decrement)
WARNING_GADGET(ArraySubscript)
WARNING_GADGET(PointerArithmetic)
WARNING_GADGET(UnsafeBufferUsageAttr)
+WARNING_GADGET(DataInvocation)
FIXABLE_GADGET(ULCArraySubscript) // `DRE[any]` in an Unspecified
Lvalue Context
FIXABLE_GADGET(DerefSimplePtrArithFixable)
FIXABLE_GADGET(PointerDereference)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 94e97a891baedc..9038dd879f54ae 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12058,7 +12058,7 @@ def warn_unsafe_buffer_variable : Warning<
InGroup, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsafe pointer arithmetic|"
- "unsafe buffer access|function introduces unsafe buffer manipulation}0">,
+ "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe
invocation of span::data}0">,
InGroup, DefaultIgnore;
def note_unsafe_buffer_operation : Note<
"used%select{| in pointer arithmetic| in buffer access}0 here">;
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 70eec1cee57f8e..e4eca9939b10d5 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -721,6 +721,33 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget {
DeclUseList getClaimedVarUseSites() const override { return {}; }
};
+// Warning gadget for unsafe invocation of span::data method.
+// Triggers when the pointer returned by the invocation is immediately
+// cast to a larger type.
+
+class DataInvocationGadget : public WarningGadget {
+ constexpr static const char *const OpTag = "data_invocation_expr";
+ const ExplicitCastExpr *Op;
+
+ public:
+ DataInvocationGadget(const MatchFinder::MatchResult &Result)
+ : WarningGadget(Kind::DataInvocation),
+Op(Result.Nodes.getNodeAs(OpTag)) {}
+
+ static bool classof(const Gadget *G) {
+return G->getKind() == Kind::DataInvocation;
+ }
+
+ static Matcher matcher() {
+return stmt(
+explicitCastExpr(has(cxxMemberCallExpr(callee(
+ cxxMethodDecl(hasName("data")).bind(OpTag));
+ }
+ const Stmt *getBaseStmt() const override { return Op; }
+
+ DeclUseList getClaimedVarUseSites() const override { return {}; }
+};
+
// Represents expressions of the form `DRE[*]` in the Unspecified Lvalue
// Context (see `isInUnspecifiedLvalueContext`).
// Note here `[]` is the built-in subscript operator.
@@ -2657,8 +2684,
[clang] [-Wunsafe-buffer-usage] Warning for unsafe invocation of span::data (PR #75650)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/75650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Warning for unsafe invocation of span::data (PR #75650)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/75650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (PR #78815)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/78815
The patch fixes the crash introduced by the DataInvocation warning gadget
designed to warn against unsafe invocations of span::data method.
Radar: 121223051
>From 6334cd361f79fc79f32b8ca95c6f31a083704332 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 19 Jan 2024 15:16:12 -0800
Subject: [PATCH] [-Wunsafe-buffer-usage] Fix the crash introduced by the
unsafe invocation of span::data warning
Radar: 121223051
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 5 ++--
clang/lib/Sema/AnalysisBasedWarnings.cpp | 26 ++-
...e-buffer-usage-warning-data-invocation.cpp | 24 +++--
3 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 724c4304a07242..7df706beb22662 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -739,9 +739,10 @@ class DataInvocationGadget : public WarningGadget {
}
static Matcher matcher() {
+Matcher callExpr = cxxMemberCallExpr(
+callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span");
return stmt(
-explicitCastExpr(has(cxxMemberCallExpr(callee(cxxMethodDecl(
- hasName("data"),
ofClass(hasName("std::span")))
+explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)
.bind(OpTag));
}
const Stmt *getBaseStmt() const override { return Op; }
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 9eb1df5f024059..749655d03342cc 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2263,15 +2263,27 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
MsgParam = 3;
} else if (const auto *ECE = dyn_cast(Operation)) {
QualType destType = ECE->getType();
-const uint64_t dSize =
-Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
-if (const auto *CE = dyn_cast(ECE->getSubExpr())) {
- QualType srcType = CE->getType();
- const uint64_t sSize =
- Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
- if (sSize >= dSize)
+if (!isa(destType))
+ return;
+
+const Expr *subExpr = ECE->getSubExpr();
+// Check if related to DataInvocation warning gadget.
+if (!isa(subExpr)) {
+ if (const auto *SE = dyn_cast(subExpr)) {
+if (!isa(SE->getSubExpr()))
+ return;
+ } else
return;
}
+const uint64_t dSize =
+Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
+
+QualType srcType = ECE->getSubExpr()->getType();
+const uint64_t sSize =
+Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
+if (sSize >= dSize)
+ return;
+
MsgParam = 4;
}
Loc = Operation->getBeginLoc();
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
index 79eb3bb4bacc6e..7b39bef0411423 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
@@ -22,6 +22,8 @@ namespace std {
using size_t = __typeof(sizeof(int));
void *malloc(size_t);
+typedef long int intptr_t;
+
void foo(int v) {
}
@@ -90,15 +92,18 @@ void cast_without_data(int *ptr) {
void warned_patterns(std::span span_ptr, std::span base_span,
span span_without_qual) {
A *a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of
span::data}}
a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of
span::data}}
-
-A *a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe
invocation of span::data}}
-
-// TODO:: Should we warn when we cast from base to derived type?
-Derived *b = dynamic_cast (base_span.data());//
expected-warning{{unsafe invocation of span::data}}
- // TODO:: This pattern is safe. We can add special handling for it, if we
decide this
- // is the recommended fixit for the unsafe invocations.
- A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); //
expected-warning{{unsafe invocation of span::data}}
+a1 = (A*)(span_ptr.data()); // expected-warning{{unsafe invocation of
span::data}}
+A *a2 = (A*) (span_without_qual.data()); // expected-warning{{unsafe
invocation of span::data}}
+
+a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation
of span::data}}
+
+ // TODO:: Should we warn when we cast from base to derived type?
+ Derived *b = dynamic_cast (base_span.data());//
expected-warning{{unsafe invocation of span::data}}
+
+// TOD
[clang] [-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (PR #78815)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/78815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (PR #78815)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/78815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (PR #78815)
@@ -2263,15 +2263,27 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
MsgParam = 3;
} else if (const auto *ECE = dyn_cast(Operation)) {
QualType destType = ECE->getType();
-const uint64_t dSize =
-Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
-if (const auto *CE = dyn_cast(ECE->getSubExpr())) {
- QualType srcType = CE->getType();
- const uint64_t sSize =
- Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
- if (sSize >= dSize)
+if (!isa(destType))
+ return;
+
+const Expr *subExpr = ECE->getSubExpr();
+// Check if related to DataInvocation warning gadget.
malavikasamak wrote:
You are right that we currently don't have any other warning gadget that this
could correspond to. I was being a little cautious that a future gadget may
accidentally match here. Let's get rid of it for now.
https://github.com/llvm/llvm-project/pull/78815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (PR #78815)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/78815
>From 6334cd361f79fc79f32b8ca95c6f31a083704332 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 19 Jan 2024 15:16:12 -0800
Subject: [PATCH 1/2] [-Wunsafe-buffer-usage] Fix the crash introduced by the
unsafe invocation of span::data warning
Radar: 121223051
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 5 ++--
clang/lib/Sema/AnalysisBasedWarnings.cpp | 26 ++-
...e-buffer-usage-warning-data-invocation.cpp | 24 +++--
3 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 724c4304a072420..7df706beb22662c 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -739,9 +739,10 @@ class DataInvocationGadget : public WarningGadget {
}
static Matcher matcher() {
+Matcher callExpr = cxxMemberCallExpr(
+callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span");
return stmt(
-explicitCastExpr(has(cxxMemberCallExpr(callee(cxxMethodDecl(
- hasName("data"),
ofClass(hasName("std::span")))
+explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)
.bind(OpTag));
}
const Stmt *getBaseStmt() const override { return Op; }
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 9eb1df5f0240596..749655d03342cca 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2263,15 +2263,27 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
MsgParam = 3;
} else if (const auto *ECE = dyn_cast(Operation)) {
QualType destType = ECE->getType();
-const uint64_t dSize =
-Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
-if (const auto *CE = dyn_cast(ECE->getSubExpr())) {
- QualType srcType = CE->getType();
- const uint64_t sSize =
- Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
- if (sSize >= dSize)
+if (!isa(destType))
+ return;
+
+const Expr *subExpr = ECE->getSubExpr();
+// Check if related to DataInvocation warning gadget.
+if (!isa(subExpr)) {
+ if (const auto *SE = dyn_cast(subExpr)) {
+if (!isa(SE->getSubExpr()))
+ return;
+ } else
return;
}
+const uint64_t dSize =
+Ctx.getTypeSize(destType.getTypePtr()->getPointeeType());
+
+QualType srcType = ECE->getSubExpr()->getType();
+const uint64_t sSize =
+Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
+if (sSize >= dSize)
+ return;
+
MsgParam = 4;
}
Loc = Operation->getBeginLoc();
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
index 79eb3bb4bacc6e7..7b39bef04114236 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
@@ -22,6 +22,8 @@ namespace std {
using size_t = __typeof(sizeof(int));
void *malloc(size_t);
+typedef long int intptr_t;
+
void foo(int v) {
}
@@ -90,15 +92,18 @@ void cast_without_data(int *ptr) {
void warned_patterns(std::span span_ptr, std::span base_span,
span span_without_qual) {
A *a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of
span::data}}
a1 = (A*)span_ptr.data(); // expected-warning{{unsafe invocation of
span::data}}
-
-A *a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe
invocation of span::data}}
-
-// TODO:: Should we warn when we cast from base to derived type?
-Derived *b = dynamic_cast (base_span.data());//
expected-warning{{unsafe invocation of span::data}}
- // TODO:: This pattern is safe. We can add special handling for it, if we
decide this
- // is the recommended fixit for the unsafe invocations.
- A *a3 = (A*)span_ptr.subspan(0, sizeof(A)).data(); //
expected-warning{{unsafe invocation of span::data}}
+a1 = (A*)(span_ptr.data()); // expected-warning{{unsafe invocation of
span::data}}
+A *a2 = (A*) (span_without_qual.data()); // expected-warning{{unsafe
invocation of span::data}}
+
+a2 = (A*) span_without_qual.data(); // expected-warning{{unsafe invocation
of span::data}}
+
+ // TODO:: Should we warn when we cast from base to derived type?
+ Derived *b = dynamic_cast (base_span.data());//
expected-warning{{unsafe invocation of span::data}}
+
+// TODO:: This pattern is safe. We can add special handling for it, if we
decide this
+// is the recommended fixit for the unsafe invocations.
+A *
[clang] [-Wunsafe-buffer-usage] Fix the crash introduced by the unsafe invocation of span::data warning (PR #78815)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/78815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions -verify %s
+
+using size_t = __typeof(sizeof(int));
+
+namespace std {
+ class type_info;
+ class bad_cast;
+ class bad_typeid;
+
+ template class span {
+
+ private:
+T *elements;
+size_t size_;
+
+ public:
+span(T *, size_t){}
+
+constexpr T* data() const noexcept {
+ return elements;
+}
+
+constexpr size_t size() const noexcept {
+ return size_;
+}
+
+ };
+}
+
+struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr;
+
+size_t sz;
+};
+
+struct B {
+ A a;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[];
+};
+
+union Union {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr1;
+
+ int ptr2;
+};
+
+struct C {
+ Union ptr;
+};
+
+struct D {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr, *ptr2;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[10];
+
+ size_t sz;
+
+};
+
+void foo(int *ptr);
+
+void foo_safe(std::span sp);
+
+void test_attribute_union(C c) {
+ int *p = c.ptr.ptr1; //expected-warning{{field ptr1 prone to unsafe buffer
manipulation}}
+
+ // TODO: Warn here about the field
+ int address = c.ptr.ptr2;
+}
+
+int* test_atribute_struct(A a) {
+ int b = *(a.ptr); //expected-warning{{field ptr prone to unsafe buffer
manipulation}}
+ a.sz++;
+ // expected-warning@+1{{unsafe pointer arithmetic}}
+ return a.ptr++; //expected-warning{{field ptr prone to unsafe buffer
manipulation}}
+}
+
+void test_attribute_field_deref_chain(B b) {
+ int *ptr = b.a.ptr;//expected-warning{{field ptr prone to unsafe buffer
manipulation}}
+ foo(b.buf); //expected-warning{{field buf prone to unsafe buffer
manipulation}}
+}
+
+void test_safe_writes(std::span sp) {
+ A a;
+ // TODO: We should not warn for safe assignments from hardened types
malavikasamak wrote:
Now that we have allowed the attribute to be added to all record fields
irrespective of their type, such a special handling for pointers seems strange.
Will remove the comment in the next version. We can file a GitHub issue if we
decide otherwise in the future.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -926,22 +926,27 @@ class CArrayToPtrAssignmentGadget : public FixableGadget {
/// A call of a function or method that performs unchecked buffer operations
/// over one of its pointer parameters.
class UnsafeBufferUsageAttrGadget : public WarningGadget {
- constexpr static const char *const OpTag = "call_expr";
- const CallExpr *Op;
+ constexpr static const char *const OpTag = "attr_expr";
+ const Expr *Op;
public:
UnsafeBufferUsageAttrGadget(const MatchFinder::MatchResult &Result)
: WarningGadget(Kind::UnsafeBufferUsageAttr),
-Op(Result.Nodes.getNodeAs(OpTag)) {}
+Op(Result.Nodes.getNodeAs(OpTag)) {}
static bool classof(const Gadget *G) {
return G->getKind() == Kind::UnsafeBufferUsageAttr;
}
static Matcher matcher() {
+auto HasUnsafeFielDecl =
+member(fieldDecl(hasAttr(attr::UnsafeBufferUsage)));
+
auto HasUnsafeFnDecl =
callee(functionDecl(hasAttr(attr::UnsafeBufferUsage)));
-return stmt(callExpr(HasUnsafeFnDecl).bind(OpTag));
+
+return stmt(expr(anyOf(callExpr(HasUnsafeFnDecl).bind(OpTag),
malavikasamak wrote:
Fixing in the next version.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/101585
>From 1ecd91c03f6de92153809402b10f99e5f649787f Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Thu, 1 Aug 2024 11:01:36 -0700
Subject: [PATCH 1/3] [-Wunsafe-buffer-usage] Support adding
unsafe_buffer_usage aatribute to struct fields.
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/include/clang/Basic/AttrDocs.td | 45 +--
.../clang/Basic/DiagnosticSemaKinds.td| 3 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 17 ++-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 13 +-
.../warn-unsafe-buffer-usage-field-attr.cpp | 113 ++
6 files changed, 176 insertions(+), 17 deletions(-)
create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 4825979a974d22..2cc21d67ddffb2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4447,7 +4447,7 @@ def ReleaseHandle : InheritableParamAttr {
def UnsafeBufferUsage : InheritableAttr {
let Spellings = [Clang<"unsafe_buffer_usage">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, Field]>;
let Documentation = [UnsafeBufferUsageDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 99738812c81579..a52e3dd68a0ce2 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6763,15 +6763,18 @@ attribute requires a string literal argument to
identify the handle being releas
def UnsafeBufferUsageDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
-The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
-that need to be avoided as they are prone to buffer overflows. It is designed
to
-work together with the off-by-default compiler warning
``-Wunsafe-buffer-usage``
-to help codebases transition away from raw pointer based buffer management,
-in favor of safer abstractions such as C++20 ``std::span``. The attribute
causes
-``-Wunsafe-buffer-usage`` to warn on every use of the function, and it may
-enable ``-Wunsafe-buffer-usage`` to emit automatic fix-it hints
-which would help the user replace such unsafe functions with safe
-alternatives, though the attribute can be used even when the fix can't be
automated.
+The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
or
+struct fields that are buffers, that must to be avoided as they are prone to
+buffer overflows. It is designed to work together with the off-by-default
compiler
+warning ``-Wunsafe-buffer-usage``to help codebases transition away from raw
pointer
+based buffer management, in favor of safer abstractions such as C++20
``std::span``.
+The attribute causes ``-Wunsafe-buffer-usage`` to warn on every use of the
function or
+the field it is attached to, and it may enable ``-Wunsafe-buffer-usage`` to
emit
+automatic fix-it hints which would help the user replace the use of unsafe
+functions(/fields) with safe alternatives, though the attribute can be used
even when
+the fix can't be automated.
+
+Attribute attached to functions:
The attribute does not suppress ``-Wunsafe-buffer-usage`` inside the function
to which it is attached. These warnings still need to be addressed.
@@ -6835,6 +6838,30 @@ the proper solution would be to create a different
function (possibly
an overload of ``baz()``) that accepts a safe container like ``bar()``,
and then use the attribute on the original ``baz()`` to help the users
update their code to use the new function.
+
+Attribute attached to fields:
+
+The attribute should only be attached to struct fields, if the fields can not
be
+updated to a safe type with bounds check, such as std::span. In other words,
the
+buffers prone to unsafe accesses should always be updated to use safe
containers/views
+and attaching the attribute must be last resort when such an update is
infeasible.
+
+The attribute can be placed on individual fields or a set of them as shown
below.
+.. code-block:: c++
+
+ struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr1;
+
+[[clang::unsafe_buffer_usage]]
+int *ptr2, buf[10];
+
+size_t sz;
+ };
+
+Here, every read/write to the fields ptr1, ptr2 and buf will trigger a warning
that the
+field is marked unsafe due to unsafe-buffer operations on it.
+
}];
}
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 810abe4f23e31e..b0428d28667a51 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12371,7 +12371,8 @@ def warn_unsafe_buffer_variable : Warning<
InGroup, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsaf
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/101585
>From 1ecd91c03f6de92153809402b10f99e5f649787f Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Thu, 1 Aug 2024 11:01:36 -0700
Subject: [PATCH 1/4] [-Wunsafe-buffer-usage] Support adding
unsafe_buffer_usage aatribute to struct fields.
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/include/clang/Basic/AttrDocs.td | 45 +--
.../clang/Basic/DiagnosticSemaKinds.td| 3 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 17 ++-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 13 +-
.../warn-unsafe-buffer-usage-field-attr.cpp | 113 ++
6 files changed, 176 insertions(+), 17 deletions(-)
create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 4825979a974d22..2cc21d67ddffb2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4447,7 +4447,7 @@ def ReleaseHandle : InheritableParamAttr {
def UnsafeBufferUsage : InheritableAttr {
let Spellings = [Clang<"unsafe_buffer_usage">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, Field]>;
let Documentation = [UnsafeBufferUsageDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 99738812c81579..a52e3dd68a0ce2 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6763,15 +6763,18 @@ attribute requires a string literal argument to
identify the handle being releas
def UnsafeBufferUsageDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
-The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
-that need to be avoided as they are prone to buffer overflows. It is designed
to
-work together with the off-by-default compiler warning
``-Wunsafe-buffer-usage``
-to help codebases transition away from raw pointer based buffer management,
-in favor of safer abstractions such as C++20 ``std::span``. The attribute
causes
-``-Wunsafe-buffer-usage`` to warn on every use of the function, and it may
-enable ``-Wunsafe-buffer-usage`` to emit automatic fix-it hints
-which would help the user replace such unsafe functions with safe
-alternatives, though the attribute can be used even when the fix can't be
automated.
+The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
or
+struct fields that are buffers, that must to be avoided as they are prone to
+buffer overflows. It is designed to work together with the off-by-default
compiler
+warning ``-Wunsafe-buffer-usage``to help codebases transition away from raw
pointer
+based buffer management, in favor of safer abstractions such as C++20
``std::span``.
+The attribute causes ``-Wunsafe-buffer-usage`` to warn on every use of the
function or
+the field it is attached to, and it may enable ``-Wunsafe-buffer-usage`` to
emit
+automatic fix-it hints which would help the user replace the use of unsafe
+functions(/fields) with safe alternatives, though the attribute can be used
even when
+the fix can't be automated.
+
+Attribute attached to functions:
The attribute does not suppress ``-Wunsafe-buffer-usage`` inside the function
to which it is attached. These warnings still need to be addressed.
@@ -6835,6 +6838,30 @@ the proper solution would be to create a different
function (possibly
an overload of ``baz()``) that accepts a safe container like ``bar()``,
and then use the attribute on the original ``baz()`` to help the users
update their code to use the new function.
+
+Attribute attached to fields:
+
+The attribute should only be attached to struct fields, if the fields can not
be
+updated to a safe type with bounds check, such as std::span. In other words,
the
+buffers prone to unsafe accesses should always be updated to use safe
containers/views
+and attaching the attribute must be last resort when such an update is
infeasible.
+
+The attribute can be placed on individual fields or a set of them as shown
below.
+.. code-block:: c++
+
+ struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr1;
+
+[[clang::unsafe_buffer_usage]]
+int *ptr2, buf[10];
+
+size_t sz;
+ };
+
+Here, every read/write to the fields ptr1, ptr2 and buf will trigger a warning
that the
+field is marked unsafe due to unsafe-buffer operations on it.
+
}];
}
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 810abe4f23e31e..b0428d28667a51 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12371,7 +12371,8 @@ def warn_unsafe_buffer_variable : Warning<
InGroup, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsaf
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/101585
>From 1ecd91c03f6de92153809402b10f99e5f649787f Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Thu, 1 Aug 2024 11:01:36 -0700
Subject: [PATCH 1/5] [-Wunsafe-buffer-usage] Support adding
unsafe_buffer_usage aatribute to struct fields.
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/include/clang/Basic/AttrDocs.td | 45 +--
.../clang/Basic/DiagnosticSemaKinds.td| 3 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 17 ++-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 13 +-
.../warn-unsafe-buffer-usage-field-attr.cpp | 113 ++
6 files changed, 176 insertions(+), 17 deletions(-)
create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 4825979a974d22..2cc21d67ddffb2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4447,7 +4447,7 @@ def ReleaseHandle : InheritableParamAttr {
def UnsafeBufferUsage : InheritableAttr {
let Spellings = [Clang<"unsafe_buffer_usage">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, Field]>;
let Documentation = [UnsafeBufferUsageDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 99738812c81579..a52e3dd68a0ce2 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6763,15 +6763,18 @@ attribute requires a string literal argument to
identify the handle being releas
def UnsafeBufferUsageDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
-The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
-that need to be avoided as they are prone to buffer overflows. It is designed
to
-work together with the off-by-default compiler warning
``-Wunsafe-buffer-usage``
-to help codebases transition away from raw pointer based buffer management,
-in favor of safer abstractions such as C++20 ``std::span``. The attribute
causes
-``-Wunsafe-buffer-usage`` to warn on every use of the function, and it may
-enable ``-Wunsafe-buffer-usage`` to emit automatic fix-it hints
-which would help the user replace such unsafe functions with safe
-alternatives, though the attribute can be used even when the fix can't be
automated.
+The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
or
+struct fields that are buffers, that must to be avoided as they are prone to
+buffer overflows. It is designed to work together with the off-by-default
compiler
+warning ``-Wunsafe-buffer-usage``to help codebases transition away from raw
pointer
+based buffer management, in favor of safer abstractions such as C++20
``std::span``.
+The attribute causes ``-Wunsafe-buffer-usage`` to warn on every use of the
function or
+the field it is attached to, and it may enable ``-Wunsafe-buffer-usage`` to
emit
+automatic fix-it hints which would help the user replace the use of unsafe
+functions(/fields) with safe alternatives, though the attribute can be used
even when
+the fix can't be automated.
+
+Attribute attached to functions:
The attribute does not suppress ``-Wunsafe-buffer-usage`` inside the function
to which it is attached. These warnings still need to be addressed.
@@ -6835,6 +6838,30 @@ the proper solution would be to create a different
function (possibly
an overload of ``baz()``) that accepts a safe container like ``bar()``,
and then use the attribute on the original ``baz()`` to help the users
update their code to use the new function.
+
+Attribute attached to fields:
+
+The attribute should only be attached to struct fields, if the fields can not
be
+updated to a safe type with bounds check, such as std::span. In other words,
the
+buffers prone to unsafe accesses should always be updated to use safe
containers/views
+and attaching the attribute must be last resort when such an update is
infeasible.
+
+The attribute can be placed on individual fields or a set of them as shown
below.
+.. code-block:: c++
+
+ struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr1;
+
+[[clang::unsafe_buffer_usage]]
+int *ptr2, buf[10];
+
+size_t sz;
+ };
+
+Here, every read/write to the fields ptr1, ptr2 and buf will trigger a warning
that the
+field is marked unsafe due to unsafe-buffer operations on it.
+
}];
}
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 810abe4f23e31e..b0428d28667a51 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12371,7 +12371,8 @@ def warn_unsafe_buffer_variable : Warning<
InGroup, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsaf
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/101585
>From 3228500bd42c032baa2cf12f3120a82906604fd5 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Thu, 1 Aug 2024 11:01:36 -0700
Subject: [PATCH] [attribute][-Wunsafe-buffer-usage] Support adding
unsafe_buffer_usage attribute to struct fields.
Extend unsafe_buffer_usage attribute to support struct fields.
Adding this attribute to the fields causes the compiler to issue a warning at
every access site
of the field. The attribute should be attached to struct fields to inform the
clients of the struct
that the warned fields are prone OOB accesses.
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/include/clang/Basic/AttrDocs.td | 136 +++--
.../clang/Basic/DiagnosticSemaKinds.td| 3 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 13 +-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 14 +-
...a-attribute-supported-attributes-list.test | 2 +-
.../warn-unsafe-buffer-usage-field-attr.cpp | 178 ++
7 files changed, 285 insertions(+), 63 deletions(-)
create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 4825979a974d22..2cc21d67ddffb2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4447,7 +4447,7 @@ def ReleaseHandle : InheritableParamAttr {
def UnsafeBufferUsage : InheritableAttr {
let Spellings = [Clang<"unsafe_buffer_usage">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, Field]>;
let Documentation = [UnsafeBufferUsageDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 99738812c81579..3cc6edad98599d 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6764,77 +6764,103 @@ def UnsafeBufferUsageDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
-that need to be avoided as they are prone to buffer overflows. It is designed
to
-work together with the off-by-default compiler warning
``-Wunsafe-buffer-usage``
-to help codebases transition away from raw pointer based buffer management,
-in favor of safer abstractions such as C++20 ``std::span``. The attribute
causes
-``-Wunsafe-buffer-usage`` to warn on every use of the function, and it may
-enable ``-Wunsafe-buffer-usage`` to emit automatic fix-it hints
-which would help the user replace such unsafe functions with safe
+that need to be avoided as they are prone to buffer overflows or unsafe buffer
+struct fields. It is designed to work together with the off-by-default compiler
+warning ``-Wunsafe-buffer-usage``to help codebases transition away from raw
pointer
+based buffer management, in favor of safer abstractions such as C++20
``std::span``.
+The attribute causes ``-Wunsafe-buffer-usage`` to warn on every use of the
function or
+the field it is attached to, and it may also lead to emission of automatic
fix-it
+hints which would help the user replace the use of unsafe functions(/fields)
with safe
alternatives, though the attribute can be used even when the fix can't be
automated.
-The attribute does not suppress ``-Wunsafe-buffer-usage`` inside the function
-to which it is attached. These warnings still need to be addressed.
+* Attribute attached to functions: The attribute does not suppress
+ ``-Wunsafe-buffer-usage`` inside the function to which it is attached.
+ These warnings still need to be addressed.
-The attribute is warranted even if the only way a function can overflow
-the buffer is by violating the function's preconditions. For example, it
-would make sense to put the attribute on function ``foo()`` below because
-passing an incorrect size parameter would cause a buffer overflow:
+ The attribute is warranted even if the only way a function can overflow
+ the buffer is by violating the function's preconditions. For example, it
+ would make sense to put the attribute on function ``foo()`` below because
+ passing an incorrect size parameter would cause a buffer overflow:
-.. code-block:: c++
+ .. code-block:: c++
- [[clang::unsafe_buffer_usage]]
- void foo(int *buf, size_t size) {
-for (size_t i = 0; i < size; ++i) {
- buf[i] = i;
+[[clang::unsafe_buffer_usage]]
+void foo(int *buf, size_t size) {
+ for (size_t i = 0; i < size; ++i) {
+buf[i] = i;
+ }
}
- }
-The attribute is NOT warranted when the function uses safe abstractions,
-assuming that these abstractions weren't misused outside the function.
-For example, function ``bar()`` below doesn't need the attribute,
-because assuming that the container ``buf`` is well-formed (has size that
-fits the original buffer it refers to), overflow cannot occur:
+ The attri
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -0,0 +1,180 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions -verify %s
+
+using size_t = __typeof(sizeof(int));
+
+namespace std {
+ class type_info;
+ class bad_cast;
+ class bad_typeid;
+
+ template class span {
+
+ private:
+T *elements;
+size_t size_;
+
+ public:
+span(T *, size_t){}
+
+constexpr T* data() const noexcept {
+ return elements;
+}
+
+constexpr size_t size() const noexcept {
+ return size_;
+}
+
+ };
+}
+
+struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr;
+
+size_t sz;
+};
+
+struct B {
+ A a;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[];
+};
+
+struct D {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr, *ptr2;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[10];
+
+ size_t sz;
+
+};
+
+void foo(int *ptr);
+
+void foo_safe(std::span sp);
+
+int* test_atribute_struct(A a) {
+ int b = *(a.ptr); //expected-warning{{field 'ptr' prone to unsafe buffer
manipulation}}
+ a.sz++;
+ // expected-warning@+1{{unsafe pointer arithmetic}}
+ return a.ptr++; //expected-warning{{field 'ptr' prone to unsafe buffer
manipulation}}
+}
+
+void test_attribute_field_deref_chain(B b) {
+ int *ptr = b.a.ptr;//expected-warning{{field 'ptr' prone to unsafe buffer
manipulation}}
+ foo(b.buf); //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
+}
+
+void test_safe_writes(std::span sp) {
+ A a;
+ // TODO: We should not warn for safe assignments from hardened types
+ a.ptr = sp.data(); //expected-warning{{field 'ptr' prone to unsafe buffer
manipulation}}
+ a.sz = sp.size();
+
+ a.ptr = nullptr; // expected-warning{{field 'ptr' prone to unsafe buffer
manipulation}}
+}
+
+void test_safe_reads(A a, A b) {
+ //expected-warning@+1{{the two-parameter std::span construction is unsafe as
it can introduce mismatch between buffer size and the bound information}}
+ std::span sp {a.ptr, a.sz}; //expected-warning{{field 'ptr' prone to
unsafe buffer manipulation}}
+
+ // expected-warning@+1 3{{field 'ptr' prone to unsafe buffer manipulation}}
+ if(a.ptr != nullptr && a.ptr != b.ptr) {
+foo_safe(sp);
+ }
+
+}
+
+void test_attribute_multiple_fields (D d) {
+ int *p =d.ptr; //expected-warning{{field 'ptr' prone to unsafe buffer
manipulation}}
+ p = d.ptr2; //expected-warning{{field 'ptr2' prone to unsafe buffer
manipulation}}
+
+ p = d.buf; //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
+
+ int v = d.buf[0]; //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
+
+ //expected-warning@+1{{unsafe buffer access}}
+ v = d.buf[5]; //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
+}
+
+template
+struct TemplateArray {
+ [[clang::unsafe_buffer_usage]]
+ T *buf;
+
+ [[clang::unsafe_buffer_usage]]
+ size_t sz;
+};
+
+
+void test_struct_template (TemplateArray t) {
+ int *p = t.buf; //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
+ size_t s = t.sz; //expected-warning{{field 'sz' prone to unsafe buffer
manipulation}}
+}
+
+class R {
+ [[clang::unsafe_buffer_usage]]
+ int *array;
+
+ public:
+ int* getArray() {
+ return array; //expected-warning{{field 'array' prone to unsafe buffer
manipulation}}
+ }
+
+ void setArray(int *arr) {
+ array = arr; //expected-warning{{field 'array' prone to unsafe buffer
manipulation}}
+ }
+};
+
+template
+class Q {
+ [[clang::unsafe_buffer_usage]]
+ P *array;
+
+ public:
+ P* getArray() {
+ return array; //expected-warning{{field 'array' prone to unsafe buffer
manipulation}}
+ }
+
+ void setArray(P *arr) {
+ array = arr; //expected-warning{{field 'array' prone to unsafe buffer
manipulation}}
+ }
+};
+
+void test_class_template(Q q) {
+ q.getArray();
+ q.setArray(nullptr);
+}
+
+struct AnonFields {
+ struct {
+ [[clang::unsafe_buffer_usage]]
+ int a;
+ };
+};
+
+void test_anon_fields(AnonFields anon) {
+ int val = anon.a; //expected-warning{{field 'a' prone to unsafe buffer
manipulation}}
malavikasamak wrote:
I will add this to the tests. Clang allows adding the attribute for the
anonymous struct in your example. Looks like it treats inner structs as a
member as well. However, because the attribute is not explicitly attached to
the field 'a', it doesn't issue any warning.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/101585
>From c9f2b131aea2c0d9d1405cb00c54dde859750d0c Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Thu, 1 Aug 2024 11:01:36 -0700
Subject: [PATCH] [attributes][-Wunsafe-buffer-usage] Support adding
unsafe_buffer_usage attribute to struct fields.
Extend unsafe_buffer_usage attribute to support struct fields.
Adding this attribute to the fields causes the compiler to issue a warning at
every access site
of the field. The attribute should be attached to struct fields to inform the
clients of the struct
that the warned fields are prone OOB accesses.
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/include/clang/Basic/AttrDocs.td | 136 -
.../clang/Basic/DiagnosticSemaKinds.td| 3 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 13 +-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 14 +-
...a-attribute-supported-attributes-list.test | 2 +-
.../warn-unsafe-buffer-usage-field-attr.cpp | 190 ++
7 files changed, 297 insertions(+), 63 deletions(-)
create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 4825979a974d22..2cc21d67ddffb2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4447,7 +4447,7 @@ def ReleaseHandle : InheritableParamAttr {
def UnsafeBufferUsage : InheritableAttr {
let Spellings = [Clang<"unsafe_buffer_usage">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, Field]>;
let Documentation = [UnsafeBufferUsageDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 99738812c81579..3cc6edad98599d 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6764,77 +6764,103 @@ def UnsafeBufferUsageDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
-that need to be avoided as they are prone to buffer overflows. It is designed
to
-work together with the off-by-default compiler warning
``-Wunsafe-buffer-usage``
-to help codebases transition away from raw pointer based buffer management,
-in favor of safer abstractions such as C++20 ``std::span``. The attribute
causes
-``-Wunsafe-buffer-usage`` to warn on every use of the function, and it may
-enable ``-Wunsafe-buffer-usage`` to emit automatic fix-it hints
-which would help the user replace such unsafe functions with safe
+that need to be avoided as they are prone to buffer overflows or unsafe buffer
+struct fields. It is designed to work together with the off-by-default compiler
+warning ``-Wunsafe-buffer-usage``to help codebases transition away from raw
pointer
+based buffer management, in favor of safer abstractions such as C++20
``std::span``.
+The attribute causes ``-Wunsafe-buffer-usage`` to warn on every use of the
function or
+the field it is attached to, and it may also lead to emission of automatic
fix-it
+hints which would help the user replace the use of unsafe functions(/fields)
with safe
alternatives, though the attribute can be used even when the fix can't be
automated.
-The attribute does not suppress ``-Wunsafe-buffer-usage`` inside the function
-to which it is attached. These warnings still need to be addressed.
+* Attribute attached to functions: The attribute does not suppress
+ ``-Wunsafe-buffer-usage`` inside the function to which it is attached.
+ These warnings still need to be addressed.
-The attribute is warranted even if the only way a function can overflow
-the buffer is by violating the function's preconditions. For example, it
-would make sense to put the attribute on function ``foo()`` below because
-passing an incorrect size parameter would cause a buffer overflow:
+ The attribute is warranted even if the only way a function can overflow
+ the buffer is by violating the function's preconditions. For example, it
+ would make sense to put the attribute on function ``foo()`` below because
+ passing an incorrect size parameter would cause a buffer overflow:
-.. code-block:: c++
+ .. code-block:: c++
- [[clang::unsafe_buffer_usage]]
- void foo(int *buf, size_t size) {
-for (size_t i = 0; i < size; ++i) {
- buf[i] = i;
+[[clang::unsafe_buffer_usage]]
+void foo(int *buf, size_t size) {
+ for (size_t i = 0; i < size; ++i) {
+buf[i] = i;
+ }
}
- }
-The attribute is NOT warranted when the function uses safe abstractions,
-assuming that these abstractions weren't misused outside the function.
-For example, function ``bar()`` below doesn't need the attribute,
-because assuming that the container ``buf`` is well-formed (has size that
-fits the original buffer it refers to), overflow cannot occur:
+ The attr
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/101585 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/101585
Extend the unsafe_buffer_usage attribute, so they can also be added to struct
fields. This will cause the compiler to warn about the unsafe field at their
access sites.
>From 1ecd91c03f6de92153809402b10f99e5f649787f Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Thu, 1 Aug 2024 11:01:36 -0700
Subject: [PATCH] [-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage
aatribute to struct fields.
---
clang/include/clang/Basic/Attr.td | 2 +-
clang/include/clang/Basic/AttrDocs.td | 45 +--
.../clang/Basic/DiagnosticSemaKinds.td| 3 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 17 ++-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 13 +-
.../warn-unsafe-buffer-usage-field-attr.cpp | 113 ++
6 files changed, 176 insertions(+), 17 deletions(-)
create mode 100644 clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index 4825979a974d2..2cc21d67ddffb 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4447,7 +4447,7 @@ def ReleaseHandle : InheritableParamAttr {
def UnsafeBufferUsage : InheritableAttr {
let Spellings = [Clang<"unsafe_buffer_usage">];
- let Subjects = SubjectList<[Function]>;
+ let Subjects = SubjectList<[Function, Field]>;
let Documentation = [UnsafeBufferUsageDocs];
}
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 99738812c8157..a52e3dd68a0ce 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -6763,15 +6763,18 @@ attribute requires a string literal argument to
identify the handle being releas
def UnsafeBufferUsageDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
-The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
-that need to be avoided as they are prone to buffer overflows. It is designed
to
-work together with the off-by-default compiler warning
``-Wunsafe-buffer-usage``
-to help codebases transition away from raw pointer based buffer management,
-in favor of safer abstractions such as C++20 ``std::span``. The attribute
causes
-``-Wunsafe-buffer-usage`` to warn on every use of the function, and it may
-enable ``-Wunsafe-buffer-usage`` to emit automatic fix-it hints
-which would help the user replace such unsafe functions with safe
-alternatives, though the attribute can be used even when the fix can't be
automated.
+The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
or
+struct fields that are buffers, that must to be avoided as they are prone to
+buffer overflows. It is designed to work together with the off-by-default
compiler
+warning ``-Wunsafe-buffer-usage``to help codebases transition away from raw
pointer
+based buffer management, in favor of safer abstractions such as C++20
``std::span``.
+The attribute causes ``-Wunsafe-buffer-usage`` to warn on every use of the
function or
+the field it is attached to, and it may enable ``-Wunsafe-buffer-usage`` to
emit
+automatic fix-it hints which would help the user replace the use of unsafe
+functions(/fields) with safe alternatives, though the attribute can be used
even when
+the fix can't be automated.
+
+Attribute attached to functions:
The attribute does not suppress ``-Wunsafe-buffer-usage`` inside the function
to which it is attached. These warnings still need to be addressed.
@@ -6835,6 +6838,30 @@ the proper solution would be to create a different
function (possibly
an overload of ``baz()``) that accepts a safe container like ``bar()``,
and then use the attribute on the original ``baz()`` to help the users
update their code to use the new function.
+
+Attribute attached to fields:
+
+The attribute should only be attached to struct fields, if the fields can not
be
+updated to a safe type with bounds check, such as std::span. In other words,
the
+buffers prone to unsafe accesses should always be updated to use safe
containers/views
+and attaching the attribute must be last resort when such an update is
infeasible.
+
+The attribute can be placed on individual fields or a set of them as shown
below.
+.. code-block:: c++
+
+ struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr1;
+
+[[clang::unsafe_buffer_usage]]
+int *ptr2, buf[10];
+
+size_t sz;
+ };
+
+Here, every read/write to the fields ptr1, ptr2 and buf will trigger a warning
that the
+field is marked unsafe due to unsafe-buffer operations on it.
+
}];
}
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 810abe4f23e31..b0428d28667a5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12371,7 +12371
[clang] Warning Libc functions (PR #101583)
malavikasamak wrote: Consider re-wording to: "string_view construction with raw pointers does not guarantee null-termination, construct with std::string instead" https://github.com/llvm/llvm-project/pull/101583 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -959,12 +966,12 @@ class UnsafeBufferUsageAttrGadget : public WarningGadget {
/// 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;
+ const Expr *Op;
malavikasamak wrote:
Yes. That was accidentally edited. Let me revert this.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions -verify %s
+
+using size_t = __typeof(sizeof(int));
+
+namespace std {
+ class type_info;
+ class bad_cast;
+ class bad_typeid;
+
+ template class span {
+
+ private:
+T *elements;
+size_t size_;
+
+ public:
+span(T *, size_t){}
+
+constexpr T* data() const noexcept {
+ return elements;
+}
+
+constexpr size_t size() const noexcept {
+ return size_;
+}
+
+ };
+}
+
+struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr;
+
+size_t sz;
+};
+
+struct B {
+ A a;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[];
+};
+
+union Union {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr1;
+
+ int ptr2;
+};
+
+struct C {
+ Union ptr;
+};
+
+struct D {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr, *ptr2;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[10];
+
+ size_t sz;
+
+};
+
+void foo(int *ptr);
+
+void foo_safe(std::span sp);
+
+void test_attribute_union(C c) {
+ int *p = c.ptr.ptr1; //expected-warning{{field ptr1 prone to unsafe buffer
manipulation}}
+
+ // TODO: Warn here about the field
+ int address = c.ptr.ptr2;
+}
+
+int* test_atribute_struct(A a) {
+ int b = *(a.ptr); //expected-warning{{field ptr prone to unsafe buffer
manipulation}}
+ a.sz++;
+ // expected-warning@+1{{unsafe pointer arithmetic}}
+ return a.ptr++; //expected-warning{{field ptr prone to unsafe buffer
manipulation}}
+}
+
+void test_attribute_field_deref_chain(B b) {
+ int *ptr = b.a.ptr;//expected-warning{{field ptr prone to unsafe buffer
manipulation}}
+ foo(b.buf); //expected-warning{{field buf prone to unsafe buffer
manipulation}}
+}
+
+void test_safe_writes(std::span sp) {
+ A a;
+ // TODO: We should not warn for safe assignments from hardened types
malavikasamak wrote:
I think we should consider excluding a few safe patterns. Leaving a TODO for
the future work.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions -verify %s
+
+using size_t = __typeof(sizeof(int));
+
+namespace std {
+ class type_info;
+ class bad_cast;
+ class bad_typeid;
+
+ template class span {
+
+ private:
+T *elements;
+size_t size_;
+
+ public:
+span(T *, size_t){}
+
+constexpr T* data() const noexcept {
+ return elements;
+}
+
+constexpr size_t size() const noexcept {
+ return size_;
+}
+
+ };
+}
+
+struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr;
+
+size_t sz;
+};
+
+struct B {
+ A a;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[];
+};
+
+union Union {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr1;
+
+ int ptr2;
+};
+
+struct C {
+ Union ptr;
+};
+
+struct D {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr, *ptr2;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[10];
+
+ size_t sz;
+
+};
+
+void foo(int *ptr);
+
+void foo_safe(std::span sp);
+
+void test_attribute_union(C c) {
+ int *p = c.ptr.ptr1; //expected-warning{{field ptr1 prone to unsafe buffer
manipulation}}
+
+ // TODO: Warn here about the field
malavikasamak wrote:
This could be a potentially be added to this patch. The question is if the
attribute is added to a member of a Union field, should we warn when other
fields of the said union are accessed. I think we probably should, as it is
referring to the same memory. Wanted to know if it's something we are
interested to do as part of this patch or a future patch.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -2261,6 +2262,12 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
// note_unsafe_buffer_operation doesn't have this mode yet.
assert(!IsRelatedToDecl && "Not implemented yet!");
MsgParam = 3;
+ } else if (isa(Operation)) {
+// note_unsafe_buffer_operation doesn't have this mode yet.
+assert(!IsRelatedToDecl && "Not implemented yet!");
+auto ME = dyn_cast(Operation);
+name = ME->getMemberDecl()->getName();
malavikasamak wrote:
Sounds good, I will use the Decl and also add a test. I wasn't aware that it is
unusual to put a field name in there.
Side note: we should probably also provide the Decl, when the attribute is
added to method overloads. Perhaps that needs to be a separate PR?
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -927,21 +927,28 @@ class CArrayToPtrAssignmentGadget : public FixableGadget {
/// over one of its pointer parameters.
class UnsafeBufferUsageAttrGadget : public WarningGadget {
constexpr static const char *const OpTag = "call_expr";
- const CallExpr *Op;
+ const Expr *Op;
public:
UnsafeBufferUsageAttrGadget(const MatchFinder::MatchResult &Result)
: WarningGadget(Kind::UnsafeBufferUsageAttr),
-Op(Result.Nodes.getNodeAs(OpTag)) {}
+Op(Result.Nodes.getNodeAs(OpTag)) {}
static bool classof(const Gadget *G) {
return G->getKind() == Kind::UnsafeBufferUsageAttr;
}
static Matcher matcher() {
+auto HasUnsafeFielDecl =
+member(fieldDecl(allOf(
+ anyOf(hasPointerType(), hasArrayType()),
malavikasamak wrote:
I was discussing offline about this with Devin and he said it is a good idea to
not restrict what types of fields we allow the attribute to be attached to.
This could be useful if someone accidentally only updates a field that stores
the bounds information.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -927,21 +927,28 @@ class CArrayToPtrAssignmentGadget : public FixableGadget {
/// over one of its pointer parameters.
class UnsafeBufferUsageAttrGadget : public WarningGadget {
constexpr static const char *const OpTag = "call_expr";
- const CallExpr *Op;
+ const Expr *Op;
public:
UnsafeBufferUsageAttrGadget(const MatchFinder::MatchResult &Result)
: WarningGadget(Kind::UnsafeBufferUsageAttr),
-Op(Result.Nodes.getNodeAs(OpTag)) {}
+Op(Result.Nodes.getNodeAs(OpTag)) {}
static bool classof(const Gadget *G) {
return G->getKind() == Kind::UnsafeBufferUsageAttr;
}
static Matcher matcher() {
+auto HasUnsafeFielDecl =
+member(fieldDecl(allOf(
+ anyOf(hasPointerType(), hasArrayType()),
malavikasamak wrote:
Also, we do not emit any warning when it is placed on other types, which could
be confusing. I will handle this in the next version and allow it to be added
to all types.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -927,21 +927,28 @@ class CArrayToPtrAssignmentGadget : public FixableGadget {
/// over one of its pointer parameters.
class UnsafeBufferUsageAttrGadget : public WarningGadget {
constexpr static const char *const OpTag = "call_expr";
- const CallExpr *Op;
+ const Expr *Op;
public:
UnsafeBufferUsageAttrGadget(const MatchFinder::MatchResult &Result)
: WarningGadget(Kind::UnsafeBufferUsageAttr),
-Op(Result.Nodes.getNodeAs(OpTag)) {}
+Op(Result.Nodes.getNodeAs(OpTag)) {}
static bool classof(const Gadget *G) {
return G->getKind() == Kind::UnsafeBufferUsageAttr;
}
static Matcher matcher() {
+auto HasUnsafeFielDecl =
malavikasamak wrote:
Sounds good. I didn't want to duplicate a bunch of code here, as the only
change was to the matcher. Perhaps when we extend the attribute to be added to
other program locations we can separate them if necessary?
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -6763,15 +6763,18 @@ attribute requires a string literal argument to
identify the handle being releas
def UnsafeBufferUsageDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
-The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
-that need to be avoided as they are prone to buffer overflows. It is designed
to
-work together with the off-by-default compiler warning
``-Wunsafe-buffer-usage``
-to help codebases transition away from raw pointer based buffer management,
-in favor of safer abstractions such as C++20 ``std::span``. The attribute
causes
-``-Wunsafe-buffer-usage`` to warn on every use of the function, and it may
-enable ``-Wunsafe-buffer-usage`` to emit automatic fix-it hints
-which would help the user replace such unsafe functions with safe
-alternatives, though the attribute can be used even when the fix can't be
automated.
+The attribute ``[[clang::unsafe_buffer_usage]]`` should be placed on functions
or
malavikasamak wrote:
I will update it in the next version.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions -verify %s
+
+using size_t = __typeof(sizeof(int));
+
+namespace std {
+ class type_info;
+ class bad_cast;
+ class bad_typeid;
+
+ template class span {
+
+ private:
+T *elements;
+size_t size_;
+
+ public:
+span(T *, size_t){}
+
+constexpr T* data() const noexcept {
+ return elements;
+}
+
+constexpr size_t size() const noexcept {
+ return size_;
+}
+
+ };
+}
+
+struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr;
+
+size_t sz;
+};
+
+struct B {
+ A a;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[];
+};
+
+union Union {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr1;
+
+ int ptr2;
+};
+
+struct C {
+ Union ptr;
+};
+
+struct D {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr, *ptr2;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[10];
+
+ size_t sz;
+
+};
+
+void foo(int *ptr);
+
+void foo_safe(std::span sp);
+
+void test_attribute_union(C c) {
+ int *p = c.ptr.ptr1; //expected-warning{{field ptr1 prone to unsafe buffer
manipulation}}
+
+ // TODO: Warn here about the field
malavikasamak wrote:
So after some off-line discussion, we have converged to not provide this
special handling for Unions. That means, just like struct fields, the warning
will only be issued if the specific union field bears explicitly bears
attribute.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][-Wunsafe-buffer-usage] Support adding unsafe_buffer_usage attribute to struct fields (PR #101585)
@@ -0,0 +1,113 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage \
+// RUN:-fsafe-buffer-usage-suggestions -verify %s
+
+using size_t = __typeof(sizeof(int));
+
+namespace std {
+ class type_info;
+ class bad_cast;
+ class bad_typeid;
+
+ template class span {
+
+ private:
+T *elements;
+size_t size_;
+
+ public:
+span(T *, size_t){}
+
+constexpr T* data() const noexcept {
+ return elements;
+}
+
+constexpr size_t size() const noexcept {
+ return size_;
+}
+
+ };
+}
+
+struct A {
+[[clang::unsafe_buffer_usage]]
+int *ptr;
+
+size_t sz;
+};
+
+struct B {
+ A a;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[];
+};
+
+union Union {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr1;
+
+ int ptr2;
+};
+
+struct C {
+ Union ptr;
+};
+
+struct D {
+ [[clang::unsafe_buffer_usage]]
+ int *ptr, *ptr2;
+
+ [[clang::unsafe_buffer_usage]]
+ int buf[10];
+
+ size_t sz;
+
+};
+
+void foo(int *ptr);
+
+void foo_safe(std::span sp);
+
+void test_attribute_union(C c) {
+ int *p = c.ptr.ptr1; //expected-warning{{field ptr1 prone to unsafe buffer
manipulation}}
+
+ // TODO: Warn here about the field
malavikasamak wrote:
Updating the test in the next version.
https://github.com/llvm/llvm-project/pull/101585
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/112284
Do not warn when a constant sized array is indexed with an expression that
contains bitwise and operation
involving constants and it always results in a bound safe access.
(rdar://136684050)
>From b557d1d8c323116a4ffcf5c9cec06bb4fb133f92 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH] [Wunsafe-buffer-usage] False positives for & expression
indexing constant size array (arr[anything & 0])
(rdar://136684050)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 45 +++
.../warn-unsafe-buffer-usage-array.cpp| 14 ++
2 files changed, 59 insertions(+)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..cdfdcc17536391 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -427,6 +427,45 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//- e. g. "Try harder to find a NamedDecl to point at in the note."
//already duplicated
// - call both from Sema and from here
+ std::function
+ SafeMaskedAccess;
+ unsigned int RecLimit = 5;
+ llvm::APInt Max;
+ bool Initialized = false;
+
+ SafeMaskedAccess = [&](const Expr *exp, const ConstantArrayType *CATy,
+ unsigned int RecLimit) -> bool {
+if (RecLimit == 0)
+ return false;
+
+RecLimit--;
+
+if (const auto *IntLit = dyn_cast(exp)) {
+ const APInt ArrIdx = IntLit->getValue();
+ if (ArrIdx.isNonNegative() &&
+ ArrIdx.getLimitedValue() < CATy->getLimitedSize())
+return true;
+ if (!Initialized) {
+Max = ArrIdx;
+Initialized = true;
+ } else {
+Max = Max & ArrIdx.getLimitedValue();
+ }
+ if (Max.getLimitedValue() < CATy->getLimitedSize())
+return true;
+}
+
+if (const auto *BinEx = dyn_cast(exp)) {
+ if (SafeMaskedAccess(BinEx->getLHS()->IgnoreParenCasts(), CATy,
RecLimit))
+return true;
+ else if (SafeMaskedAccess(BinEx->getRHS()->IgnoreParenCasts(), CATy,
+RecLimit))
+return true;
+}
+
+return false;
+ };
const auto *BaseDRE =
dyn_cast(Node.getBase()->IgnoreParenImpCasts());
@@ -446,6 +485,12 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
if (ArrIdx.isNonNegative() &&
ArrIdx.getLimitedValue() < CATy->getLimitedSize())
return true;
+ } else if (const auto *BinEx = dyn_cast(Node.getIdx())) {
+if (BinEx->getOpcode() != BO_And)
+ return false;
+
+Max.setAllBits();
+return SafeMaskedAccess(Node.getIdx(), CATy, RecLimit);
}
return false;
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..e5a5ca57f66883 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -33,6 +33,20 @@ void constant_idx_safe0(unsigned idx) {
buffer[0] = 0;
}
+int array[10]; // expected-warning{{'array' is an unsafe buffer that does not
perform bounds checks}}
+
+void masked_idx(unsigned long long idx) {
+ array[idx & 5] = 10; // no warning
+ array[idx & 11 & 5] = 3; // no warning
+ array[idx & 11] = 20; // expected-note{{used in buffer access here}}
+}
+
+int array2[5];
+
+void mased_idx_false() {
+ array2[6 & 5];
+}
+
void constant_idx_unsafe(unsigned idx) {
int buffer[10]; // expected-warning{{'buffer' is an unsafe buffer that
does not perform bounds checks}}
// expected-note@-1{{change type of 'buffer' to
'std::array' to label it for hardening}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Emit a warning if pointer returned by vector::data and array::data is cast to larger type (PR #111910)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/111910
>From 6bc56755624efd0c533ac4952b1e4b37a3c0a4a9 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Thu, 10 Oct 2024 13:43:39 -0700
Subject: [PATCH] [-Wunsafe-buffer-usage] Emit a warning if pointer returned by
vector::data and array::data is cast to larger type
Emit a warning when the raw pointer retrieved from std::vector and std::array
instances are
cast to a larger type. Such a cast followed by a field dereference to the
resulting pointer
could cause an OOB access. This is similar to the existing span::data warning.
(rdar://136704278)
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 7 +-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 9 ++
...e-buffer-usage-warning-data-invocation.cpp | 97 +--
4 files changed, 82 insertions(+), 33 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f4a2d4a3f0656a..301a0d46d88390 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12497,7 +12497,7 @@ def warn_unsafe_buffer_variable : Warning<
InGroup, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsafe pointer arithmetic|"
- "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe
invocation of span::data|"
+ "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe
invocation of %1|"
"field %1 prone to unsafe buffer manipulation}0">,
InGroup, DefaultIgnore;
def warn_unsafe_buffer_libc_call : Warning<
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..5e0ec9ecc92ea4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1499,8 +1499,11 @@ class DataInvocationGadget : public WarningGadget {
}
static Matcher matcher() {
-Matcher callExpr = cxxMemberCallExpr(
-callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span");
+
+Matcher callExpr = cxxMemberCallExpr(callee(
+cxxMethodDecl(hasName("data"),
+ ofClass(anyOf(hasName("std::span"),
hasName("std::array"),
+hasName("std::vector"));
return stmt(
explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)
.bind(OpTag));
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 6496a33b8f5a50..c76733e9a774b6 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2279,9 +2279,18 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
QualType srcType = ECE->getSubExpr()->getType();
const uint64_t sSize =
Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
+
if (sSize >= dSize)
return;
+if (const auto *CE = dyn_cast(
+ECE->getSubExpr()->IgnoreParens())) {
+ D = CE->getMethodDecl();
+}
+
+if (!D)
+ return;
+
MsgParam = 4;
}
Loc = Operation->getBeginLoc();
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
index 08707d7ff545d8..0228e42652bd95 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
@@ -32,38 +32,68 @@ void foo(int *p){}
namespace std{
template class span {
- T *elements;
+ T *elements;
- span(T *, unsigned){}
+ span(T *, unsigned){}
- public:
+ public:
- constexpr span subspan(size_t offset, size_t count) const {
-return span (elements+offset, count); // expected-warning{{unsafe
pointer arithmetic}}
- }
+ constexpr span subspan(size_t offset, size_t count) const {
+ return span (elements+offset, count); // expected-warning{{unsafe
pointer arithmetic}}
+ }
- constexpr T* data() const noexcept {
-return elements;
- }
+ constexpr T* data() const noexcept {
+ return elements;
+ }
+
+ constexpr T* hello() const noexcept {
+ return elements;
+ }
+ };
+
+ template class vector {
+
+ T *elements;
+
+ public:
+
+ vector(size_t n) {
+ elements = new T[n];
+ }
+
+ constexpr T* data() const noexcept {
+ return elements;
+ }
+
+ ~vector() {
+ delete[] elements;
+ }
+ };
+
+ template
+ class array {
+ T elements[N];
+
+ public:
+
+ constexpr const T* data() const noexcept {
+ return elements;
+ }
+
+ };
-
- constexpr T* hello() const noexcept {
- return elements;
- }
-};
-
template class span_duplicate {
- span_duplicate(
[clang] [-Wunsafe-buffer-usage] Emit a warning if pointer returned by vector::data and array::data is cast to larger type (PR #111910)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/111910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Emit a warning if pointer returned by vector::data and array::data is cast to larger type (PR #111910)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/111910
Emit a warning when the raw pointer retrieved from std::vector and std::array
instances are cast to a larger type. Such a cast followed by a field
dereference to the resulting pointer could cause an OOB access. This is similar
to the existing span::data warning.
(rdar://136704278)
>From 6bc56755624efd0c533ac4952b1e4b37a3c0a4a9 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Thu, 10 Oct 2024 13:43:39 -0700
Subject: [PATCH] [-Wunsafe-buffer-usage] Emit a warning if pointer returned by
vector::data and array::data is cast to larger type
Emit a warning when the raw pointer retrieved from std::vector and std::array
instances are
cast to a larger type. Such a cast followed by a field dereference to the
resulting pointer
could cause an OOB access. This is similar to the existing span::data warning.
(rdar://136704278)
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +-
clang/lib/Analysis/UnsafeBufferUsage.cpp | 7 +-
clang/lib/Sema/AnalysisBasedWarnings.cpp | 9 ++
...e-buffer-usage-warning-data-invocation.cpp | 97 +--
4 files changed, 82 insertions(+), 33 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f4a2d4a3f0656a..301a0d46d88390 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12497,7 +12497,7 @@ def warn_unsafe_buffer_variable : Warning<
InGroup, DefaultIgnore;
def warn_unsafe_buffer_operation : Warning<
"%select{unsafe pointer operation|unsafe pointer arithmetic|"
- "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe
invocation of span::data|"
+ "unsafe buffer access|function introduces unsafe buffer manipulation|unsafe
invocation of %1|"
"field %1 prone to unsafe buffer manipulation}0">,
InGroup, DefaultIgnore;
def warn_unsafe_buffer_libc_call : Warning<
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..5e0ec9ecc92ea4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1499,8 +1499,11 @@ class DataInvocationGadget : public WarningGadget {
}
static Matcher matcher() {
-Matcher callExpr = cxxMemberCallExpr(
-callee(cxxMethodDecl(hasName("data"), ofClass(hasName("std::span");
+
+Matcher callExpr = cxxMemberCallExpr(callee(
+cxxMethodDecl(hasName("data"),
+ ofClass(anyOf(hasName("std::span"),
hasName("std::array"),
+hasName("std::vector"));
return stmt(
explicitCastExpr(anyOf(has(callExpr), has(parenExpr(has(callExpr)
.bind(OpTag));
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 6496a33b8f5a50..c76733e9a774b6 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2279,9 +2279,18 @@ class UnsafeBufferUsageReporter : public
UnsafeBufferUsageHandler {
QualType srcType = ECE->getSubExpr()->getType();
const uint64_t sSize =
Ctx.getTypeSize(srcType.getTypePtr()->getPointeeType());
+
if (sSize >= dSize)
return;
+if (const auto *CE = dyn_cast(
+ECE->getSubExpr()->IgnoreParens())) {
+ D = CE->getMethodDecl();
+}
+
+if (!D)
+ return;
+
MsgParam = 4;
}
Loc = Operation->getBeginLoc();
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
index 08707d7ff545d8..0228e42652bd95 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-data-invocation.cpp
@@ -32,38 +32,68 @@ void foo(int *p){}
namespace std{
template class span {
- T *elements;
+ T *elements;
- span(T *, unsigned){}
+ span(T *, unsigned){}
- public:
+ public:
- constexpr span subspan(size_t offset, size_t count) const {
-return span (elements+offset, count); // expected-warning{{unsafe
pointer arithmetic}}
- }
+ constexpr span subspan(size_t offset, size_t count) const {
+ return span (elements+offset, count); // expected-warning{{unsafe
pointer arithmetic}}
+ }
- constexpr T* data() const noexcept {
-return elements;
- }
+ constexpr T* data() const noexcept {
+ return elements;
+ }
+
+ constexpr T* hello() const noexcept {
+ return elements;
+ }
+ };
+
+ template class vector {
+
+ T *elements;
+
+ public:
+
+ vector(size_t n) {
+ elements = new T[n];
+ }
+
+ constexpr T* data() const noexcept {
+ return elements;
+ }
+
+ ~vector() {
+ delete[] elements;
+ }
[clang] [WIP][Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/112284
>From 7e00765481784324450982e4789bf61bf66dbfdf Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH 1/2] [Wunsafe-buffer-usage] False positives for & expression
indexing constant size array (arr[anything & 0])
Do not warn when a constant sized array is indexed with an expression that
contains bitwise and operation
involving constants and it always results in a bound safe access.
(rdar://136684050)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 127 --
.../warn-unsafe-buffer-usage-array.cpp| 43 ++
2 files changed, 162 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..1ee1657e90d7aa 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -420,6 +420,118 @@ AST_MATCHER(CXXConstructExpr,
isSafeSpanTwoParamConstruct) {
return false;
}
+class MaxValueEval : public RecursiveASTVisitor {
+
+ std::vector val;
+ ASTContext &Context;
+ llvm::APInt Max;
+ unsigned bit_width;
+
+public:
+ typedef RecursiveASTVisitor VisitorBase;
+
+ explicit MaxValueEval(ASTContext &Ctx, const Expr *exp) : Context(Ctx) {
+bit_width = Ctx.getIntWidth(exp->getType());
+Max = llvm::APInt::getSignedMaxValue(bit_width);
+val.clear();
+ }
+
+ bool findMatch(Expr *exp) {
+TraverseStmt(exp);
+return true;
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *dre) {
+val.push_back(Max);
+return false;
+ }
+
+ bool VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
+val.push_back(Max);
+return false;
+ }
+
+ bool EvaluateExpression(Expr *exp) {
+Expr::EvalResult EVResult;
+if (exp->EvaluateAsInt(EVResult, Context)) {
+ llvm::APSInt Result = EVResult.Val.getInt();
+ val.push_back(Result);
+ return true;
+}
+return false;
+ }
+
+ bool VisitBinaryOperator(BinaryOperator *E) {
+
+if (EvaluateExpression(E)) {
+ return false;
+} else {
+ TraverseStmt(E->getLHS());
+ llvm::APInt LHS = val.back();
+ val.pop_back();
+
+ TraverseStmt(E->getRHS());
+ llvm::APInt RHS = val.back();
+ val.pop_back();
+ llvm::APInt Result = Max;
+
+ switch (E->getOpcode()) {
+ case BO_And:
+ case BO_AndAssign:
+Result = LHS & RHS;
+break;
+
+ case BO_Or:
+ case BO_OrAssign:
+Result = LHS | RHS;
+break;
+
+ case BO_Shl:
+ case BO_ShlAssign:
+if (RHS != Max.getLimitedValue())
+ Result = LHS << RHS.getLimitedValue();
+break;
+
+ case BO_Shr:
+ case BO_ShrAssign:
+if (RHS == Max.getLimitedValue())
+ Result = LHS;
+else
+ Result = LHS.getLimitedValue() >> RHS.getLimitedValue();
+break;
+
+ case BO_Rem:
+ case BO_RemAssign:
+if (LHS.getLimitedValue() < RHS.getLimitedValue())
+ Result = LHS;
+else
+ Result = --RHS;
+break;
+
+ default:
+break;
+ }
+ val.push_back(Result);
+ return false;
+}
+return true;
+ }
+
+ bool VisitExpr(Expr *E) {
+if (EvaluateExpression(E)) {
+ return false;
+}
+return VisitorBase::VisitExpr(E);
+ }
+
+ APInt getValue() {
+if (val.size() == 1)
+ return val[0];
+else // A pattern we didn't consider was encountered
+ return Max;
+ }
+};
+
AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
// FIXME: Proper solution:
// - refactor Sema::CheckArrayAccess
@@ -439,14 +551,13 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
if (!CATy)
return false;
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() &&
-ArrIdx.getLimitedValue() < CATy->getLimitedSize())
- return true;
- }
+ MaxValueEval Vis(Finder->getASTContext(), Node.getIdx());
+ Vis.findMatch(const_cast(Node.getIdx()));
+ APInt result = Vis.getValue();
+
+ if (result.isNonNegative() &&
+ result.getLimitedValue() < CATy->getLimitedSize())
+return true;
return false;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..6f0a974e45d9c6 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -33,6 +33,49 @@ void constant_idx_safe0(unsigned idx) {
buffer[0] = 0;
}
+int array[10]; // expected-warning 3{{'array' is an unsafe buffer that does
not perform bounds checks}}
+
+void masked_idx1(unsigned long long idx) {
+ // Bitwise and operation
+ array[idx & 5] = 10; // no warning
+ array[idx & 11 &
[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/112284 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
@@ -427,6 +427,48 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//- e. g. "Try harder to find a NamedDecl to point at in the note."
//already duplicated
// - call both from Sema and from here
+ std::function
+ SafeMaskedAccess;
+ unsigned int RecLimit = 5;
+
+ SafeMaskedAccess = [&](const Expr *exp, unsigned int RecLimit) ->
llvm::APInt {
+llvm::APInt Max =
llvm::APInt::getMaxValue(Finder->getASTContext().getIntWidth(exp->getType()));
+Max.setAllBits();
+
+if (RecLimit == 0)
+ return Max;
+
+//RecLimit--;
+
+if (const auto *IntLit = dyn_cast(exp)) {
+ const APInt ArrIdx = IntLit->getValue();
+ return ArrIdx;
+}
+
+if (const auto *BinEx = dyn_cast(exp)) {
+ llvm::APInt LHS = SafeMaskedAccess(BinEx->getLHS()->IgnoreParenCasts(),
RecLimit);
+ llvm::APInt RHS = SafeMaskedAccess(BinEx->getRHS()->IgnoreParenCasts(),
RecLimit);
malavikasamak wrote:
The code has now re-organized with ASTVisitors. I don't think casts should pose
a major challenges anymore, but please let me know if you think otherwise. Also
have added more type cast tests to cover all bases.
https://github.com/llvm/llvm-project/pull/112284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
@@ -420,6 +420,118 @@ AST_MATCHER(CXXConstructExpr,
isSafeSpanTwoParamConstruct) {
return false;
}
+class MaxValueEval : public RecursiveASTVisitor {
+
+ std::vector val;
+ ASTContext &Context;
+ llvm::APInt Max;
+ unsigned bit_width;
+
+public:
+ typedef RecursiveASTVisitor VisitorBase;
+
+ explicit MaxValueEval(ASTContext &Ctx, const Expr *exp) : Context(Ctx) {
+bit_width = Ctx.getIntWidth(exp->getType());
+Max = llvm::APInt::getSignedMaxValue(bit_width);
+val.clear();
+ }
+
+ bool findMatch(Expr *exp) {
+TraverseStmt(exp);
+return true;
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *dre) {
+val.push_back(Max);
+return false;
+ }
+
+ bool VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
+val.push_back(Max);
+return false;
+ }
+
+ bool EvaluateExpression(Expr *exp) {
+Expr::EvalResult EVResult;
+if (exp->EvaluateAsInt(EVResult, Context)) {
+ llvm::APSInt Result = EVResult.Val.getInt();
+ val.push_back(Result);
+ return true;
+}
+return false;
+ }
+
+ bool VisitBinaryOperator(BinaryOperator *E) {
malavikasamak wrote:
Well.. This is a bit tricky and we actually don't always depend on the LHS and
RHS evaluation. Here, I am first checking if the expr evaluates to a constant
value. If yes, we store the result and don't traverse any further. If it does
not, only then we process the LHS and RHS to extract their limits.
https://github.com/llvm/llvm-project/pull/112284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
@@ -420,6 +420,118 @@ AST_MATCHER(CXXConstructExpr,
isSafeSpanTwoParamConstruct) {
return false;
}
+class MaxValueEval : public RecursiveASTVisitor {
+
+ std::vector val;
+ ASTContext &Context;
+ llvm::APInt Max;
+ unsigned bit_width;
+
+public:
+ typedef RecursiveASTVisitor VisitorBase;
+
+ explicit MaxValueEval(ASTContext &Ctx, const Expr *exp) : Context(Ctx) {
+bit_width = Ctx.getIntWidth(exp->getType());
+Max = llvm::APInt::getSignedMaxValue(bit_width);
malavikasamak wrote:
Thanks. Will handle this in the next version.
https://github.com/llvm/llvm-project/pull/112284
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positives in handling string literals. (PR #115552)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/115552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix false positives in warning againt 2-parameter std::span constructor (PR #115797)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/115797
Do not warn when two parameter constructor receives pointer address from a
std::addressof method and the span size is set to 1.
(rdar://139298119)
>From a60c18973c0ea5b59c7c5f38813083e862f70e6e Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Mon, 11 Nov 2024 17:18:40 -0800
Subject: [PATCH] [-Wunsafe-buffer-usage] Fix false positive in warnging againt
2-parameter std::span constructor
Do not warn when two parameter constructor receives pointer address from a
std::addressof method
and the span size is set to 1.
(rdar://139298119)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp| 8
...buffer-usage-in-container-span-construct.cpp | 17 +
2 files changed, 25 insertions(+)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 2c68409b846bc8..507e61564fc3f8 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -410,6 +410,14 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct)
{
// Check form 3:
return Arg1CV && Arg1CV->isOne();
break;
+ case Stmt::CallExprClass:
+if(const auto *CE = dyn_cast(Arg0)) {
+ const auto FnDecl = CE->getDirectCallee();
+ if(FnDecl && FnDecl->getNameAsString() == "addressof" &&
FnDecl->isInStdNamespace()) {
+return Arg1CV && Arg1CV->isOne();
+ }
+}
+break;
default:
break;
}
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
index c138fe088b3ba9..30b6d4ba9fb904 100644
---
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
+++
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
@@ -21,6 +21,12 @@ namespace std {
template< class T >
T&& move( T&& t ) noexcept;
+
+ template
+ _Tp* addressof(_Tp& __x) {
+return &__x;
+ }
+
}
namespace irrelevant_constructors {
@@ -74,15 +80,26 @@ namespace construct_wt_ptr_size {
return std::span{p, 10};// expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
}
+ // addressof method defined outside std namespace.
+ template
+ _Tp* addressof(_Tp& __x) {
+return &__x;
+ }
+
void notWarnSafeCases(unsigned n, int *p) {
int X;
unsigned Y = 10;
std::span S = std::span{&X, 1}; // no-warning
+S = std::span{std::addressof(X), 1}; // no-warning
int Arr[10];
typedef int TenInts_t[10];
TenInts_t Arr2;
S = std::span{&X, 2};// expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
+S = std::span{std::addressof(X), 2}; // expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
+// Warn when a non std method also named addressof
+S = std::span{addressof(X), 1}; // expected-warning{{the two-parameter
std::span construction is unsafe as it can introduce mismatch between buffer
size and the bound information}}
+
S = std::span{new int[10], 10}; // no-warning
S = std::span{new int[n], n};// no-warning
S = std::span{new int, 1}; // no-warning
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positives in handling string literals. (PR #115552)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/115552
Do not warn when a string literal is indexed and the idex value is within the
bounds of the length of the string.
(rdar://139106996)
>From 3be112ec1f0b2e6e2948db082a7141d91b873a17 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 8 Nov 2024 13:40:20 -0800
Subject: [PATCH] [Wunsafe-buffer-usage] Fix false positives in handling string
literals.
Do not warn when a string literal is indexed and the idex value is within
the bounds of the length of the string.
(rdar://139106996)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 28 +--
.../warn-unsafe-buffer-usage-array.cpp| 7 +
2 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 2c68409b846bc8..650d51bebd66f7 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -436,21 +436,31 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
const auto *BaseDRE =
dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- if (!BaseDRE)
-return false;
- if (!BaseDRE->getDecl())
-return false;
- const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
- BaseDRE->getDecl()->getType());
- if (!CATy)
+ const auto *SLiteral =
dyn_cast(Node.getBase()->IgnoreParenImpCasts());
+ uint64_t size;
+
+ if (!BaseDRE && !SLiteral)
return false;
+ if(BaseDRE) {
+if (!BaseDRE->getDecl())
+ return false;
+const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
+BaseDRE->getDecl()->getType());
+if (!CATy) {
+ return false;
+}
+size = CATy->getLimitedSize();
+ } else if(SLiteral) {
+size = SLiteral->getLength();
+ }
+
if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
const APInt ArrIdx = IdxLit->getValue();
// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
// bug
if (ArrIdx.isNonNegative() &&
-ArrIdx.getLimitedValue() < CATy->getLimitedSize())
+ArrIdx.getLimitedValue() < size)
return true;
}
@@ -1142,7 +1152,7 @@ class ArraySubscriptGadget : public WarningGadget {
// clang-format off
return stmt(arraySubscriptExpr(
hasBase(ignoringParenImpCasts(
- anyOf(hasPointerType(), hasArrayType(,
+ anyOf(hasPointerType(), hasArrayType(), stringLiteral(,
unless(anyOf(
isSafeArraySubscript(),
hasIndex(
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..0a443543d3f604 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -38,3 +38,10 @@ void constant_idx_unsafe(unsigned idx) {
// expected-note@-1{{change type of 'buffer' to
'std::array' to label it for hardening}}
buffer[10] = 0; // expected-note{{used in buffer access here}}
}
+
+void constant_id_string() {
+ char safe_char = "abc"[1]; // no-warning
+ char abcd[5] = "abc";
+ abcd[2]; // no-warning
+ safe_char = "abc"[3]; //expected-warning{{unsafe buffer access}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positives in handling string literals. (PR #115552)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/115552 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positives in handling string literals. (PR #115552)
@@ -38,3 +38,17 @@ void constant_idx_unsafe(unsigned idx) {
// expected-note@-1{{change type of 'buffer' to
'std::array' to label it for hardening}}
buffer[10] = 0; // expected-note{{used in buffer access here}}
}
+
+void constant_id_string(unsigned idx) {
+ char safe_char = "abc"[1]; // no-warning
+ safe_char = ""[0];
+ safe_char = "\0"[0];
+
+ char abcd[5] = "abc";
+ abcd[2]; // no-warning
+
+ char unsafe_char = "abc"[3]; //expected-warning{{unsafe buffer access}}
+ unsafe_char = "abc"[-1]; //expected-warning{{unsafe buffer access}}
+ unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
malavikasamak wrote:
This I think should warn, as the length here including the null terminator is 1.
https://github.com/llvm/llvm-project/pull/115552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positives in handling string literals. (PR #115552)
@@ -38,3 +38,17 @@ void constant_idx_unsafe(unsigned idx) {
// expected-note@-1{{change type of 'buffer' to
'std::array' to label it for hardening}}
buffer[10] = 0; // expected-note{{used in buffer access here}}
}
+
+void constant_id_string(unsigned idx) {
+ char safe_char = "abc"[1]; // no-warning
+ safe_char = ""[0];
+ safe_char = "\0"[0];
+
+ char abcd[5] = "abc";
+ abcd[2]; // no-warning
+
+ char unsafe_char = "abc"[3]; //expected-warning{{unsafe buffer access}}
malavikasamak wrote:
Okay. Makes sense.
https://github.com/llvm/llvm-project/pull/115552
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positives in handling string literals. (PR #115552)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/115552
>From 94ba7d8b845f18b089e03c0fa41d9d1d20e79f0b Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 8 Nov 2024 13:40:20 -0800
Subject: [PATCH] [Wunsafe-buffer-usage] Fix false positives in handling string
literals.
Do not warn when a string literal is indexed and the idex value is within
the bounds of the length of the string.
(rdar://139106996)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 28 +--
.../warn-unsafe-buffer-usage-array.cpp| 14 ++
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 2c68409b846bc8..b683826503c74c 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -436,21 +436,31 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
const auto *BaseDRE =
dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- if (!BaseDRE)
-return false;
- if (!BaseDRE->getDecl())
-return false;
- const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
- BaseDRE->getDecl()->getType());
- if (!CATy)
+ const auto *SLiteral =
+ dyn_cast(Node.getBase()->IgnoreParenImpCasts());
+ uint64_t size;
+
+ if (!BaseDRE && !SLiteral)
return false;
+ if (BaseDRE) {
+if (!BaseDRE->getDecl())
+ return false;
+const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
+BaseDRE->getDecl()->getType());
+if (!CATy) {
+ return false;
+}
+size = CATy->getLimitedSize();
+ } else if (SLiteral) {
+size = SLiteral->getLength() + 1;
+ }
+
if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
const APInt ArrIdx = IdxLit->getValue();
// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
// bug
-if (ArrIdx.isNonNegative() &&
-ArrIdx.getLimitedValue() < CATy->getLimitedSize())
+if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
return true;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..c6c93a27e4b969 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -38,3 +38,17 @@ void constant_idx_unsafe(unsigned idx) {
// expected-note@-1{{change type of 'buffer' to
'std::array' to label it for hardening}}
buffer[10] = 0; // expected-note{{used in buffer access here}}
}
+
+void constant_id_string(unsigned idx) {
+ char safe_char = "abc"[1]; // no-warning
+ safe_char = ""[0];
+ safe_char = "\0"[0];
+
+ char abcd[5] = "abc";
+ abcd[2]; // no-warning
+
+ char unsafe_char = "abc"[3];
+ unsafe_char = "abc"[-1]; //expected-warning{{unsafe buffer access}}
+ unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
+ unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix false positives in warning againt 2-parameter std::span constructor (PR #115797)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/115797
>From a60c18973c0ea5b59c7c5f38813083e862f70e6e Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Mon, 11 Nov 2024 17:18:40 -0800
Subject: [PATCH 1/2] [-Wunsafe-buffer-usage] Fix false positive in warnging
againt 2-parameter std::span constructor
Do not warn when two parameter constructor receives pointer address from a
std::addressof method
and the span size is set to 1.
(rdar://139298119)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp| 8
...buffer-usage-in-container-span-construct.cpp | 17 +
2 files changed, 25 insertions(+)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 2c68409b846bc8..507e61564fc3f8 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -410,6 +410,14 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct)
{
// Check form 3:
return Arg1CV && Arg1CV->isOne();
break;
+ case Stmt::CallExprClass:
+if(const auto *CE = dyn_cast(Arg0)) {
+ const auto FnDecl = CE->getDirectCallee();
+ if(FnDecl && FnDecl->getNameAsString() == "addressof" &&
FnDecl->isInStdNamespace()) {
+return Arg1CV && Arg1CV->isOne();
+ }
+}
+break;
default:
break;
}
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
index c138fe088b3ba9..30b6d4ba9fb904 100644
---
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
+++
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
@@ -21,6 +21,12 @@ namespace std {
template< class T >
T&& move( T&& t ) noexcept;
+
+ template
+ _Tp* addressof(_Tp& __x) {
+return &__x;
+ }
+
}
namespace irrelevant_constructors {
@@ -74,15 +80,26 @@ namespace construct_wt_ptr_size {
return std::span{p, 10};// expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
}
+ // addressof method defined outside std namespace.
+ template
+ _Tp* addressof(_Tp& __x) {
+return &__x;
+ }
+
void notWarnSafeCases(unsigned n, int *p) {
int X;
unsigned Y = 10;
std::span S = std::span{&X, 1}; // no-warning
+S = std::span{std::addressof(X), 1}; // no-warning
int Arr[10];
typedef int TenInts_t[10];
TenInts_t Arr2;
S = std::span{&X, 2};// expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
+S = std::span{std::addressof(X), 2}; // expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
+// Warn when a non std method also named addressof
+S = std::span{addressof(X), 1}; // expected-warning{{the two-parameter
std::span construction is unsafe as it can introduce mismatch between buffer
size and the bound information}}
+
S = std::span{new int[10], 10}; // no-warning
S = std::span{new int[n], n};// no-warning
S = std::span{new int, 1}; // no-warning
>From 9b215f104ffa97c1be39f7666fe83dfedeaf8603 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Mon, 11 Nov 2024 22:24:41 -0800
Subject: [PATCH 2/2] Fixing clang-format issues.
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 507e61564fc3f8..ae6863fb7cb28f 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -411,9 +411,10 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct)
{
return Arg1CV && Arg1CV->isOne();
break;
case Stmt::CallExprClass:
-if(const auto *CE = dyn_cast(Arg0)) {
+if (const auto *CE = dyn_cast(Arg0)) {
const auto FnDecl = CE->getDirectCallee();
- if(FnDecl && FnDecl->getNameAsString() == "addressof" &&
FnDecl->isInStdNamespace()) {
+ if (FnDecl && FnDecl->getNameAsString() == "addressof" &&
+ FnDecl->isInStdNamespace()) {
return Arg1CV && Arg1CV->isOne();
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix false positives in warning againt 2-parameter std::span constructor (PR #115797)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/115797
>From d0a95e158f6d39fdb284c067f945299e27029dbc Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Mon, 11 Nov 2024 17:18:40 -0800
Subject: [PATCH] [-Wunsafe-buffer-usage] Fix false positive in warnging againt
2-parameter std::span constructor
Do not warn when two parameter constructor receives pointer address from a
std::addressof method
and the span size is set to 1.
(rdar://139298119)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp| 10 ++
...buffer-usage-in-container-span-construct.cpp | 17 +
2 files changed, 27 insertions(+)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 2c68409b846bc8..b0bec3832f4d67 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -366,6 +366,7 @@ isInUnspecifiedUntypedContext(internal::Matcher
InnerMatcher) {
// 4. `std::span{a, n}`, where `a` is of an array-of-T with constant size
// `n`
// 5. `std::span{any, 0}`
+// 6. `std::span{std::addressof(...), 1}`
AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct) {
assert(Node.getNumArgs() == 2 &&
"expecting a two-parameter std::span constructor");
@@ -410,6 +411,15 @@ AST_MATCHER(CXXConstructExpr, isSafeSpanTwoParamConstruct)
{
// Check form 3:
return Arg1CV && Arg1CV->isOne();
break;
+ case Stmt::CallExprClass:
+if (const auto *CE = dyn_cast(Arg0)) {
+ const auto FnDecl = CE->getDirectCallee();
+ if (FnDecl && FnDecl->getNameAsString() == "addressof" &&
+ FnDecl->isInStdNamespace()) {
+return Arg1CV && Arg1CV->isOne();
+ }
+}
+break;
default:
break;
}
diff --git
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
index c138fe088b3ba9..30b6d4ba9fb904 100644
---
a/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
+++
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-in-container-span-construct.cpp
@@ -21,6 +21,12 @@ namespace std {
template< class T >
T&& move( T&& t ) noexcept;
+
+ template
+ _Tp* addressof(_Tp& __x) {
+return &__x;
+ }
+
}
namespace irrelevant_constructors {
@@ -74,15 +80,26 @@ namespace construct_wt_ptr_size {
return std::span{p, 10};// expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
}
+ // addressof method defined outside std namespace.
+ template
+ _Tp* addressof(_Tp& __x) {
+return &__x;
+ }
+
void notWarnSafeCases(unsigned n, int *p) {
int X;
unsigned Y = 10;
std::span S = std::span{&X, 1}; // no-warning
+S = std::span{std::addressof(X), 1}; // no-warning
int Arr[10];
typedef int TenInts_t[10];
TenInts_t Arr2;
S = std::span{&X, 2};// expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
+S = std::span{std::addressof(X), 2}; // expected-warning{{the
two-parameter std::span construction is unsafe as it can introduce mismatch
between buffer size and the bound information}}
+// Warn when a non std method also named addressof
+S = std::span{addressof(X), 1}; // expected-warning{{the two-parameter
std::span construction is unsafe as it can introduce mismatch between buffer
size and the bound information}}
+
S = std::span{new int[10], 10}; // no-warning
S = std::span{new int[n], n};// no-warning
S = std::span{new int, 1}; // no-warning
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Fix false positives in warning againt 2-parameter std::span constructor (PR #115797)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/115797 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [WIP][Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/112284
>From 7e00765481784324450982e4789bf61bf66dbfdf Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH] [Wunsafe-buffer-usage] False positives for & expression
indexing constant size array (arr[anything & 0])
Do not warn when a constant sized array is indexed with an expression that
contains bitwise and operation
involving constants and it always results in a bound safe access.
(rdar://136684050)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 127 --
.../warn-unsafe-buffer-usage-array.cpp| 43 ++
2 files changed, 162 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..1ee1657e90d7aa 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -420,6 +420,118 @@ AST_MATCHER(CXXConstructExpr,
isSafeSpanTwoParamConstruct) {
return false;
}
+class MaxValueEval : public RecursiveASTVisitor {
+
+ std::vector val;
+ ASTContext &Context;
+ llvm::APInt Max;
+ unsigned bit_width;
+
+public:
+ typedef RecursiveASTVisitor VisitorBase;
+
+ explicit MaxValueEval(ASTContext &Ctx, const Expr *exp) : Context(Ctx) {
+bit_width = Ctx.getIntWidth(exp->getType());
+Max = llvm::APInt::getSignedMaxValue(bit_width);
+val.clear();
+ }
+
+ bool findMatch(Expr *exp) {
+TraverseStmt(exp);
+return true;
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *dre) {
+val.push_back(Max);
+return false;
+ }
+
+ bool VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
+val.push_back(Max);
+return false;
+ }
+
+ bool EvaluateExpression(Expr *exp) {
+Expr::EvalResult EVResult;
+if (exp->EvaluateAsInt(EVResult, Context)) {
+ llvm::APSInt Result = EVResult.Val.getInt();
+ val.push_back(Result);
+ return true;
+}
+return false;
+ }
+
+ bool VisitBinaryOperator(BinaryOperator *E) {
+
+if (EvaluateExpression(E)) {
+ return false;
+} else {
+ TraverseStmt(E->getLHS());
+ llvm::APInt LHS = val.back();
+ val.pop_back();
+
+ TraverseStmt(E->getRHS());
+ llvm::APInt RHS = val.back();
+ val.pop_back();
+ llvm::APInt Result = Max;
+
+ switch (E->getOpcode()) {
+ case BO_And:
+ case BO_AndAssign:
+Result = LHS & RHS;
+break;
+
+ case BO_Or:
+ case BO_OrAssign:
+Result = LHS | RHS;
+break;
+
+ case BO_Shl:
+ case BO_ShlAssign:
+if (RHS != Max.getLimitedValue())
+ Result = LHS << RHS.getLimitedValue();
+break;
+
+ case BO_Shr:
+ case BO_ShrAssign:
+if (RHS == Max.getLimitedValue())
+ Result = LHS;
+else
+ Result = LHS.getLimitedValue() >> RHS.getLimitedValue();
+break;
+
+ case BO_Rem:
+ case BO_RemAssign:
+if (LHS.getLimitedValue() < RHS.getLimitedValue())
+ Result = LHS;
+else
+ Result = --RHS;
+break;
+
+ default:
+break;
+ }
+ val.push_back(Result);
+ return false;
+}
+return true;
+ }
+
+ bool VisitExpr(Expr *E) {
+if (EvaluateExpression(E)) {
+ return false;
+}
+return VisitorBase::VisitExpr(E);
+ }
+
+ APInt getValue() {
+if (val.size() == 1)
+ return val[0];
+else // A pattern we didn't consider was encountered
+ return Max;
+ }
+};
+
AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
// FIXME: Proper solution:
// - refactor Sema::CheckArrayAccess
@@ -439,14 +551,13 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
if (!CATy)
return false;
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() &&
-ArrIdx.getLimitedValue() < CATy->getLimitedSize())
- return true;
- }
+ MaxValueEval Vis(Finder->getASTContext(), Node.getIdx());
+ Vis.findMatch(const_cast(Node.getIdx()));
+ APInt result = Vis.getValue();
+
+ if (result.isNonNegative() &&
+ result.getLimitedValue() < CATy->getLimitedSize())
+return true;
return false;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..6f0a974e45d9c6 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -33,6 +33,49 @@ void constant_idx_safe0(unsigned idx) {
buffer[0] = 0;
}
+int array[10]; // expected-warning 3{{'array' is an unsafe buffer that does
not perform bounds checks}}
+
+void masked_idx1(unsigned long long idx) {
+ // Bitwise and operation
+ array[idx & 5] = 10; // no warning
+ array[idx & 11 & 5] =
[clang] [WIP][Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/112284
>From 80e593f62c9f00e6d639b870ec4912de2b971864 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH] [Wunsafe-buffer-usage] False positives for & expression
indexing constant size array (arr[anything & 0])
Do not warn when a constant sized array is indexed with an expression that
contains bitwise and operation
involving constants and it always results in a bound safe access.
(rdar://136684050)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 142 +-
.../warn-unsafe-buffer-usage-array.cpp| 43 ++
2 files changed, 177 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..2477ab01d9eaa8 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -420,6 +420,122 @@ AST_MATCHER(CXXConstructExpr,
isSafeSpanTwoParamConstruct) {
return false;
}
+class MaxValueEval : public RecursiveASTVisitor {
+
+ std::vector val;
+ ASTContext &Context;
+ llvm::APInt Max;
+ unsigned bit_width;
+
+ public:
+
+ typedef RecursiveASTVisitor VisitorBase;
+
+ explicit MaxValueEval(ASTContext &Ctx, const Expr *exp): Context(Ctx) {
+bit_width = Ctx.getIntWidth(exp->getType());
+Max = llvm::APInt::getSignedMaxValue(bit_width);
+//Max.setAllBits();
+val.clear();
+ }
+
+ bool findMatch(Expr *exp) {
+TraverseStmt(exp);
+return true;
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *dre) {
+val.push_back(Max);
+return false;
+ }
+
+ bool VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
+val.push_back(Max);
+return false;
+ }
+
+ bool EvaluateExpression(Expr *exp) {
+Expr::EvalResult EVResult;
+if (exp->EvaluateAsInt(EVResult, Context)) {
+ llvm::APSInt Result = EVResult.Val.getInt();
+ val.push_back(Result);
+ return true;
+}
+return false;
+ }
+
+ bool VisitBinaryOperator(BinaryOperator *E) {
+
+ if(EvaluateExpression(E)) {
+ return false;
+ } else {
+ TraverseStmt(E->getLHS());
+ llvm::APInt LHS = val.back();
+ val.pop_back();
+
+ TraverseStmt(E->getRHS());
+ llvm::APInt RHS = val.back();
+ val.pop_back();
+ llvm::APInt Result = Max;
+
+ switch (E->getOpcode()) {
+case BO_And:
+case BO_AndAssign:
+ Result = LHS & RHS;
+ break;
+
+case BO_Or:
+case BO_OrAssign:
+ Result = LHS | RHS;
+ break;
+
+case BO_Shl:
+case BO_ShlAssign:
+ if(RHS != Max.getLimitedValue())
+Result = LHS << RHS.getLimitedValue();
+ break;
+
+case BO_Shr:
+case BO_ShrAssign:
+ if(RHS == Max.getLimitedValue())
+Result = LHS;
+ //else if(RHS.getLimitedValue() >= bit_width)
+ // Result = llvm::APInt::getZero(bit_width);
+ else
+Result = LHS.getLimitedValue() >> RHS.getLimitedValue();
+ break;
+
+case BO_Rem:
+case BO_RemAssign:
+ if(LHS.getLimitedValue() < RHS.getLimitedValue())
+Result = LHS;
+ else
+Result = --RHS;
+ break;
+
+default:
+ break;
+ }
+ val.push_back(Result);
+ return false;
+}
+return true;
+ }
+
+ bool VisitExpr(Expr *E) {
+if(EvaluateExpression(E)) {
+ return false;
+}
+return VisitorBase::VisitExpr(E);
+ }
+
+ APInt getValue() {
+if(val.size() == 1)
+ return val[0];
+else // A pattern we didn't consider was encountered
+ return Max;
+ }
+};
+
AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
// FIXME: Proper solution:
// - refactor Sema::CheckArrayAccess
@@ -439,14 +555,13 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
if (!CATy)
return false;
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() &&
-ArrIdx.getLimitedValue() < CATy->getLimitedSize())
- return true;
- }
+ MaxValueEval Vis(Finder->getASTContext(), Node.getIdx());
+ Vis.findMatch(const_cast(Node.getIdx()));
+ APInt result = Vis.getValue();
+
+ if (result.isNonNegative() &&
+ result.getLimitedValue() < CATy->getLimitedSize())
+return true;
return false;
}
@@ -1146,6 +1261,10 @@ class ArraySubscriptGadget : public WarningGadget {
// clang-format on
}
+ const ArraySubscriptExpr* getASE() const{
+return ASE;
+ }
+
void handleUnsafeOperation(UnsafeBufferUsageHandler &Handler,
bool IsRelatedToDecl,
ASTContext &Ctx) const override {
@@ -3904,6 +4023,13 @@ void clang::checkUnsafeBufferU
[clang] [WIP][Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/112284
>From 81af812176768eb663a09b5ccabe3c729b76 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH] [Wunsafe-buffer-usage] False positives for & expression
indexing constant size array (arr[anything & 0])
Do not warn when a constant sized array is indexed with an expression that
contains bitwise and operation
involving constants and it always results in a bound safe access.
(rdar://136684050)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 48 ++-
.../warn-unsafe-buffer-usage-array.cpp| 16 +++
2 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 97f1c4f16b8f4c..f80076ee90978b 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -427,6 +427,48 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//- e. g. "Try harder to find a NamedDecl to point at in the note."
//already duplicated
// - call both from Sema and from here
+ std::function
+ SafeMaskedAccess;
+ unsigned int RecLimit = 5;
+
+ SafeMaskedAccess = [&](const Expr *exp, unsigned int RecLimit) ->
llvm::APInt {
+llvm::APInt Max =
llvm::APInt::getMaxValue(Finder->getASTContext().getIntWidth(exp->getType()));
+Max.setAllBits();
+
+if (RecLimit == 0)
+ return Max;
+
+//RecLimit--;
+
+if (const auto *IntLit = dyn_cast(exp)) {
+ const APInt ArrIdx = IntLit->getValue();
+ return ArrIdx;
+}
+
+if (const auto *BinEx = dyn_cast(exp)) {
+ llvm::APInt LHS = SafeMaskedAccess(BinEx->getLHS()->IgnoreParenCasts(),
RecLimit);
+ llvm::APInt RHS = SafeMaskedAccess(BinEx->getRHS()->IgnoreParenCasts(),
RecLimit);
+
+ switch(BinEx->getOpcode()) {
+case BO_And:
+ LHS = LHS & RHS.getLimitedValue();
+ return LHS;
+case BO_Or:
+ LHS = LHS | RHS.getLimitedValue();
+ return LHS;
+case BO_Shl:
+ LHS = LHS << RHS.getLimitedValue();
+ return LHS;
+case BO_Xor:
+ LHS = LHS ^ RHS.getLimitedValue();
+ return LHS;
+default:
+ return Max;
+ }
+}
+
+return Max;
+ };
const auto *BaseDRE =
dyn_cast(Node.getBase()->IgnoreParenImpCasts());
@@ -446,6 +488,10 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
if (ArrIdx.isNonNegative() &&
ArrIdx.getLimitedValue() < CATy->getLimitedSize())
return true;
+ } else if (const auto *BinEx = dyn_cast(Node.getIdx())) {
+APInt result = SafeMaskedAccess(Node.getIdx(), RecLimit);
+if (result.isNonNegative() && result.getLimitedValue() <
CATy->getLimitedSize())
+ return true;
}
return false;
@@ -1152,7 +1198,7 @@ class ArraySubscriptGadget : public WarningGadget {
Handler.handleUnsafeOperation(ASE, IsRelatedToDecl, Ctx);
}
SourceLocation getSourceLoc() const override { return ASE->getBeginLoc(); }
-
+
DeclUseList getClaimedVarUseSites() const override {
if (const auto *DRE =
dyn_cast(ASE->getBase()->IgnoreParenImpCasts())) {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..2b3f1388c3f28b 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -33,6 +33,22 @@ void constant_idx_safe0(unsigned idx) {
buffer[0] = 0;
}
+int array[10]; // expected-warning{{'array' is an unsafe buffer that does not
perform bounds checks}}
+
+void masked_idx(unsigned long long idx) {
+ array[idx & 5] = 10; // no warning
+ array[idx & 11 & 5] = 3; // no warning
+ array[idx & 11] = 20; // expected-note{{used in buffer access here}}
+}
+
+int array2[5];
+
+void mased_idx_false(unsigned long long idx) {
+ array2[6 & 5]; // no warning
+ array2[6 & idx & (idx + 1) & 5]; // no warning
+ array2[6 & idx & 5 & (idx + 1) | 4];
+}
+
void constant_idx_unsafe(unsigned idx) {
int buffer[10]; // expected-warning{{'buffer' is an unsafe buffer that
does not perform bounds checks}}
// expected-note@-1{{change type of 'buffer' to
'std::array' to label it for hardening}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positives in handling string literals. (PR #115552)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/115552
>From a3f41d4b947739f97adccbcb3dcef0a37f2a508a Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 8 Nov 2024 13:40:20 -0800
Subject: [PATCH] [Wunsafe-buffer-usage] Fix false positives in handling string
literals.
Do not warn when a string literal is indexed and the idex value is within
the bounds of the length of the string.
(rdar://139106996)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 28 +--
.../warn-unsafe-buffer-usage-array.cpp| 14 ++
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 2c68409b846bc8..2b6a67fcff4b39 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -436,21 +436,31 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
const auto *BaseDRE =
dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- if (!BaseDRE)
-return false;
- if (!BaseDRE->getDecl())
-return false;
- const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
- BaseDRE->getDecl()->getType());
- if (!CATy)
+ const auto *SLiteral =
+ dyn_cast(Node.getBase()->IgnoreParenImpCasts());
+ uint64_t size;
+
+ if (!BaseDRE && !SLiteral)
return false;
+ if (BaseDRE) {
+if (!BaseDRE->getDecl())
+ return false;
+const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
+BaseDRE->getDecl()->getType());
+if (!CATy) {
+ return false;
+}
+size = CATy->getLimitedSize();
+ } else if (SLiteral) {
+size = SLiteral->getLength();
+ }
+
if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
const APInt ArrIdx = IdxLit->getValue();
// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
// bug
-if (ArrIdx.isNonNegative() &&
-ArrIdx.getLimitedValue() < CATy->getLimitedSize())
+if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
return true;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 8b2f103ec66708..eddd3ccc9a1bf2 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -38,3 +38,17 @@ void constant_idx_unsafe(unsigned idx) {
// expected-note@-1{{change type of 'buffer' to
'std::array' to label it for hardening}}
buffer[10] = 0; // expected-note{{used in buffer access here}}
}
+
+void constant_id_string(unsigned idx) {
+ char safe_char = "abc"[1]; // no-warning
+ safe_char = ""[0];
+ safe_char = "\0"[0];
+
+ char abcd[5] = "abc";
+ abcd[2]; // no-warning
+
+ char unsafe_char = "abc"[3]; //expected-warning{{unsafe buffer access}}
+ unsafe_char = "abc"[-1]; //expected-warning{{unsafe buffer access}}
+ unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
+ unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/118249
>From a3bc09e9ad3e12a2041eb41671872781638b7aa9 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning for
multi-dimensional constant arrays
Do not warn about unsafe buffer access, when multi-dimensional constant arrays
are accessed and their indices are within the bounds of the buffer. Warning in
such cases would be a false positive. Such a suppression already exists for 1-d
arrays and it is now extended to multi-dimensional arrays.
(rdar://137926311)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 58 ++-
.../warn-unsafe-buffer-usage-array.cpp| 6 ++
.../warn-unsafe-buffer-usage-field-attr.cpp | 1 -
...n-unsafe-buffer-usage-fixits-parm-main.cpp | 3 +-
...e-buffer-usage-fixits-parm-unsupported.cpp | 2 +-
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 54 +++--
6 files changed, 60 insertions(+), 64 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..d3aab0ccd589d6 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,37 +433,35 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+ std::function CheckBounds =
+ [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(ASE->getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else
return false;
-}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
+ if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+return CheckBounds(BaseASE);
+ }
return true;
- }
+}
+return false;
+ };
- return false;
+ return CheckBounds(&Node);
}
AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
@@ -1172,6 +1170,12 @@ class ArraySubscriptGadget : public WarningGadget {
if (const auto *DRE =
dyn_cast(ASE->getBase()->IgnoreParenImpCasts())) {
return {DRE};
+} else if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ if (const auto *DRE = dyn_cast(
+ BaseASE->getBase()->IgnoreParenImpCasts())) {
+return {DRE};
+ }
}
return {};
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..41cdc122b7d2fa 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,9 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+float multi_dimension_array(Float4x4& matrix) {
+ return matrix[1][1];
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
index 0ba605475925b9..1636c948da075a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
@@ -96,7 +96,6 @@ void test_attribute_multiple_fields (D d) {
int v = d.buf[0]; //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
- //expected-warning@+1{{unsafe buffer access}}
v = d.buf[5]; //expected-warning{{field 'buf'
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/118249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning when 2-D constant arrays are indexed (PR #118249)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/118249
>From 8ca3b6312bc9ab53d7f89d02e0de180fadb20679 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning for
multi-dimensional constant arrays
Do not warn about unsafe buffer access, when multi-dimensional constant arrays
are accessed and their indices are within the bounds of the buffer. Warning in
such cases would be a false postive. Such a suppression aleady exists for 1-d
arrays and it is now extended to multo-dimensional arrays.
(rdar://137926311)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 58 ++-
.../warn-unsafe-buffer-usage-array.cpp| 6 ++
.../warn-unsafe-buffer-usage-field-attr.cpp | 1 -
...n-unsafe-buffer-usage-fixits-parm-main.cpp | 3 +-
...e-buffer-usage-fixits-parm-unsupported.cpp | 2 +-
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 54 +++--
6 files changed, 60 insertions(+), 64 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..d3aab0ccd589d6 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,37 +433,35 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+ std::function CheckBounds =
+ [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(ASE->getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else
return false;
-}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
+ if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+return CheckBounds(BaseASE);
+ }
return true;
- }
+}
+return false;
+ };
- return false;
+ return CheckBounds(&Node);
}
AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
@@ -1172,6 +1170,12 @@ class ArraySubscriptGadget : public WarningGadget {
if (const auto *DRE =
dyn_cast(ASE->getBase()->IgnoreParenImpCasts())) {
return {DRE};
+} else if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ if (const auto *DRE = dyn_cast(
+ BaseASE->getBase()->IgnoreParenImpCasts())) {
+return {DRE};
+ }
}
return {};
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..41cdc122b7d2fa 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,9 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+float multi_dimension_array(Float4x4& matrix) {
+ return matrix[1][1];
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
index 0ba605475925b9..1636c948da075a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
@@ -96,7 +96,6 @@ void test_attribute_multiple_fields (D d) {
int v = d.buf[0]; //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
- //expected-warning@+1{{unsafe buffer access}}
v = d.buf[5]; //expected-warning{{field 'buf' p
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/118249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning when 2-D constant arrays are indexed (PR #118249)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/118249
Do not warn about unsafe buffer access, when 2-D constant arrays are accessed
and the indices are within the bounds of the buffer. Warning in such cases is a
false postive. Such a suppression aleady exists for 1-d arrays and it is now
extended to 2-D arrays.
(rdar://137926311)
>From 1b21a2eab2fabbb9bdc2e540e48a38aab90fd028 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning when 2-D constant
arrays are indexed
Do not warn about unsafe buffer access, when 2-D constant arrays are accessed
and the indices are within the bounds of the buffer. Warning in such cases is
a false postive. Such a suppression aleady exists for 1-d arrays and it is now
extended to 2-D arrays.
(rdar://137926311)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 64 ++-
.../warn-unsafe-buffer-usage-array.cpp| 6 ++
...n-unsafe-buffer-usage-fixits-parm-main.cpp | 3 +-
...e-buffer-usage-fixits-parm-unsupported.cpp | 2 +-
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 33 --
5 files changed, 68 insertions(+), 40 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..f1e3b28fc03249 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,20 +433,56 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
+ auto CheckBounds = [](const ArraySubscriptExpr *ASE, uint64_t limit) -> bool
{
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
+return true;
+}
+return false;
+ };
+
+ const auto *BaseASE =
+ dyn_cast(Node.getBase()->IgnoreParenImpCasts());
+ uint64_t size = 0;
+
+ if (BaseASE) {
+const auto *DRE =
+dyn_cast(BaseASE->getBase()->IgnoreParenImpCasts());
+
+if (!DRE)
+ return false;
+
+const auto *CATy = dyn_cast(
+DRE->getType()->getUnqualifiedDesugaredType());
+if (!CATy) {
+ return false;
+}
+size = CATy->getLimitedSize();
+
+if (!CheckBounds(BaseASE, size))
+ return false;
+
+CATy = Finder->getASTContext().getAsConstantArrayType(BaseASE->getType());
+if (!CATy) {
+ return false;
+}
+
+size = CATy->getLimitedSize();
+return CheckBounds(&Node, size);
+ }
+
+ const DeclRefExpr *BaseDRE =
dyn_cast(Node.getBase()->IgnoreParenImpCasts());
const auto *SLiteral =
dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
if (!BaseDRE && !SLiteral)
return false;
if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
+const auto *CATy =
+Finder->getASTContext().getAsConstantArrayType(BaseDRE->getType());
if (!CATy) {
return false;
}
@@ -455,15 +491,7 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
size = SLiteral->getLength() + 1;
}
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
- return true;
- }
-
- return false;
+ return CheckBounds(&Node, size);
}
AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
@@ -1172,6 +1200,12 @@ class ArraySubscriptGadget : public WarningGadget {
if (const auto *DRE =
dyn_cast(ASE->getBase()->IgnoreParenImpCasts())) {
return {DRE};
+} else if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ if (const auto *DRE = dyn_cast(
+ BaseASE->getBase()->IgnoreParenImpCasts())) {
+return {DRE};
+ }
}
return {};
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..41cdc122b7d2fa 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,9 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+float multi_dimension_array(Float4x4& matrix) {
+ return matrix[1][1];
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-main.cpp
index c6b095031e0e32..d1fd1c00a9ea34 100644
--
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/118249
>From 2d923a6c6a21ab68a968d49becfe5d22fc20096f Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning for
multi-dimensional constant arrays
Do not warn about unsafe buffer access, when multi-dimensional constant arrays
are accessed and their indices are within the bounds of the buffer. Warning in
such cases would be a false positive. Such a suppression already exists for 1-d
arrays and it is now extended to multi-dimensional arrays.
(rdar://137926311)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 57 ++-
.../warn-unsafe-buffer-usage-array.cpp| 34 +++
.../warn-unsafe-buffer-usage-field-attr.cpp | 1 -
...n-unsafe-buffer-usage-fixits-parm-main.cpp | 3 +-
...e-buffer-usage-fixits-parm-unsupported.cpp | 2 +-
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 54 +++---
6 files changed, 88 insertions(+), 63 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..372f46736596e4 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,37 +433,36 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+ std::function CheckBounds =
+ [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(ASE->getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else {
return false;
}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
+ if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+return CheckBounds(BaseASE);
+ }
return true;
- }
+}
+return false;
+ };
- return false;
+ return CheckBounds(&Node);
}
AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
@@ -1172,6 +1171,12 @@ class ArraySubscriptGadget : public WarningGadget {
if (const auto *DRE =
dyn_cast(ASE->getBase()->IgnoreParenImpCasts())) {
return {DRE};
+} else if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ if (const auto *DRE = dyn_cast(
+ BaseASE->getBase()->IgnoreParenImpCasts())) {
+return {DRE};
+ }
}
return {};
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..3c4f66eccb279e 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,37 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+// expected-warning@+1 {{'matrix' is an unsafe buffer that does not perform
bounds checks}}
+float two_dimension_array(Float4x4& matrix) {
+ // expected-note@+1{{used in buffer access here}}
+ float a = matrix[0][4];
+
+ a = matrix[0][3];
+
+ // expected-note@+1{{used in buffer access here}}
+ a = matrix[4][0];
+
+ return matrix[1][1];
+}
+
+typedef float Float2x3x4[2][3][4];
+float multi_dimension_array(Float2x3x4& matrix) {
+ float *f = matrix[0][2];
+ return matrix[1][2][3];
+}
+
+char array_strings[][11] = {
+"Apple", "Banana", "Cherry", "Date", "Elderberry"
+};
+
+char array_string[] = "123456";
+
+char acc
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/118249
>From 6281b990096f058cfb6ed862631b8d6436db23f7 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning for
multi-dimensional constant arrays
Do not warn about unsafe buffer access, when multi-dimensional constant arrays
are accessed and their indices are within the bounds of the buffer. Warning in
such cases would be a false positive. Such a suppression already exists for 1-d
arrays and it is now extended to multi-dimensional arrays.
(rdar://137926311)
(rdar://140320139)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 51 +--
.../warn-unsafe-buffer-usage-array.cpp| 34 +
.../warn-unsafe-buffer-usage-field-attr.cpp | 1 -
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 26 +-
4 files changed, 71 insertions(+), 41 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..e7ba2c324247d7 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,37 +433,36 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+ std::function CheckBounds =
+ [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(ASE->getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else {
return false;
}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
+ if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+return CheckBounds(BaseASE);
+ }
return true;
- }
+}
+return false;
+ };
- return false;
+ return CheckBounds(&Node);
}
AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..2d143a94bf86f4 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,37 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+// expected-warning@+1 {{'matrix' is an unsafe buffer that does not perform
bounds checks}}
+float two_dimension_array(Float4x4& matrix) {
+ // expected-warning@+1{{unsafe buffer access}}
+ float a = matrix[0][4];
+
+ a = matrix[0][3];
+
+ // expected-note@+1{{used in buffer access here}}
+ a = matrix[4][0];
+
+ return matrix[1][1];
+}
+
+typedef float Float2x3x4[2][3][4];
+float multi_dimension_array(Float2x3x4& matrix) {
+ float *f = matrix[0][2];
+ return matrix[1][2][3];
+}
+
+char array_strings[][11] = {
+"Apple", "Banana", "Cherry", "Date", "Elderberry"
+};
+
+char array_string[] = "123456";
+
+char access_strings() {
+ char c = array_strings[0][4];
+ c = array_strings[3][10];
+ c = array_string[5];
+ return c;
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
index 0ba605475925b9..1636c948da075a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
@@ -96,7 +96,6 @@ void test_attribute_multiple_fields (D d) {
int v = d.buf[0]; //expected-warning{{field 'buf' prone to u
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/118249
>From 3a73ee44036b4162115061751241f105147f5947 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning for
multi-dimensional constant arrays
Do not warn about unsafe buffer access, when multi-dimensional constant arrays
are accessed and their indices are within the bounds of the buffer. Warning in
such cases would be a false positive. Such a suppression already exists for 1-d
arrays and it is now extended to multi-dimensional arrays.
(rdar://137926311)
(rdar://140320139)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 51 +--
.../warn-unsafe-buffer-usage-array.cpp| 40 +++
.../warn-unsafe-buffer-usage-field-attr.cpp | 1 -
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 38 +-
4 files changed, 89 insertions(+), 41 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..e7ba2c324247d7 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,37 +433,36 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+ std::function CheckBounds =
+ [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(ASE->getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else {
return false;
}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
+ if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+return CheckBounds(BaseASE);
+ }
return true;
- }
+}
+return false;
+ };
- return false;
+ return CheckBounds(&Node);
}
AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..7dd6c83dbba2a8 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,43 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+// expected-warning@+1 {{'matrix' is an unsafe buffer that does not perform
bounds checks}}
+float two_dimension_array(Float4x4& matrix, unsigned idx) {
+ // expected-warning@+1{{unsafe buffer access}}
+ float a = matrix[0][4];
+
+ a = matrix[0][3];
+
+ // expected-note@+1{{used in buffer access here}}
+ a = matrix[4][0];
+
+ a = matrix[idx][0]; // expected-note{{used in buffer access here}}
+
+ a = matrix[0][idx]; //expected-warning{{unsafe buffer access}}
+
+ a = matrix[idx][idx]; //expected-warning{{unsafe buffer access}} //
expected-note{{used in buffer access here}}
+
+ return matrix[1][1];
+}
+
+typedef float Float2x3x4[2][3][4];
+float multi_dimension_array(Float2x3x4& matrix) {
+ float *f = matrix[0][2];
+ return matrix[1][2][3];
+}
+
+char array_strings[][11] = {
+ "Apple", "Banana", "Cherry", "Date", "Elderberry"
+};
+
+char array_string[] = "123456";
+
+char access_strings() {
+ char c = array_strings[0][4];
+ c = array_strings[3][10];
+ c = array_string[5];
+ return c;
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
index 0ba605475925b9..
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/118249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
@@ -52,3 +52,37 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+// expected-warning@+1 {{'matrix' is an unsafe buffer that does not perform
bounds checks}}
+float two_dimension_array(Float4x4& matrix) {
+ // expected-note@+1{{used in buffer access here}}
+ float a = matrix[0][4];
malavikasamak wrote:
Done
https://github.com/llvm/llvm-project/pull/118249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
@@ -433,37 +433,36 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+ std::function CheckBounds =
+ [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(ASE->getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else {
return false;
}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
+ if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+return CheckBounds(BaseASE);
malavikasamak wrote:
That is how it is supposed to work. However, we will have cases where array
dimensions go upto 3 and higher than 3 dimensions should be quite rare indeed.
For each dimension we do the following: retrieve the size of the type and
compare it against the index used by the operation. This to me presents as a
natural fit for a recursive solution, both in terms of code reuse and
readability. That said, I am open to other solutions.
https://github.com/llvm/llvm-project/pull/118249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
@@ -8,6 +8,5 @@
// main function
int main(int argc, char *argv[]) { // expected-warning{{'argv' is an unsafe
pointer used for buffer access}}
char tmp;
- tmp = argv[5][5];// expected-note{{used in buffer access
here}} \
- expected-warning{{unsafe buffer access}}
malavikasamak wrote:
Yes. That is doable. I can just take out the part that associates the
operations to DREs for 2-D array accesses. Perhaps we should file a radar to
claim operations that are clearly associated with DREs?
https://github.com/llvm/llvm-project/pull/118249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/118249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
@@ -8,6 +8,5 @@
// main function
int main(int argc, char *argv[]) { // expected-warning{{'argv' is an unsafe
pointer used for buffer access}}
char tmp;
- tmp = argv[5][5];// expected-note{{used in buffer access
here}} \
- expected-warning{{unsafe buffer access}}
malavikasamak wrote:
Resolved in the latest diff.
https://github.com/llvm/llvm-project/pull/118249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
@@ -52,3 +52,37 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+// expected-warning@+1 {{'matrix' is an unsafe buffer that does not perform
bounds checks}}
+float two_dimension_array(Float4x4& matrix) {
+ // expected-note@+1{{used in buffer access here}}
+ float a = matrix[0][4];
malavikasamak wrote:
Sure. I will add a test case that uses a variable.
https://github.com/llvm/llvm-project/pull/118249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Add additional tests to esnure safe accesses to const sized array are not warned (PR #125483)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/125483
Add more tests, where the index of the const array access evaluates to a
constant and depends on a template argument.
rdar://143759014
>From 12dc35b91a805c75017d58d400082d2a8959ee31 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 31 Jan 2025 13:40:55 +0530
Subject: [PATCH] [Wunsafe-buffer-usage] Add additional tests to esnure safe
accesses to const sized array are not warned
Add more tests, where the index of the const array access evaluates to a
constant and depends on a template
argument.
rdar://143759014
---
.../warn-unsafe-buffer-usage-array.cpp| 33 +++
1 file changed, 33 insertions(+)
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index e80b54b7c696779..03a7ae5c7a8fb5b 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -124,3 +124,36 @@ void array_indexed_const_expr(unsigned idx) {
k = arr[get_const(5)]; // expected-note {{used in buffer access here}}
k = arr[get_const(4)];
}
+
+template
+consteval bool isNullTerminated(const char (&literal)[length])
+{
+ return literal[length - 1] == '\0';
+}
+
+template
+T access2DArray(const T (&arr)[M][N]) {
+ return arr[M-1][N-1];
+}
+
+template
+constexpr int access_elements() {
+ int arr[idx + 20];
+ return arr[idx + 1];
+}
+
+void test_template_methods()
+{
+ constexpr char arr[] = "Good Morning!"; // = {'a', 'b', 'c', 'd', 'e'};
+ isNullTerminated(arr);
+ isNullTerminated("");
+ auto _ = isNullTerminated("hello world\n");
+ access_elements<5>();
+
+ int arr1[3][4] = {
+{1, 2, 3, 4},
+{5, 6, 7, 8},
+{9, 10, 11, 12}
+};
+ access2DArray(arr1);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Turn off unsafe-buffer warning for methods annotated with clang::unsafe_buffer_usage attribute (PR #125671)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/125671
Unsafe operation in methods that are already annotated with
clang::unsafe_buffer_usage attribute, should not trigger a warning. This is
because, the developer has already identified the method as unsafe and warning
at every unsafe operation is redundant.
rdar://138644831
>From afab62e75c9130e667935d775bc256b2c3a9c62d Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 31 Jan 2025 13:19:46 +0530
Subject: [PATCH] [Wunsafe-buffer-usage] Turn off unsafe-buffer warning for
methods annotated with clang::unsafe_buffer_usage attribute
Unsafe operation in methods that are already annotated with
clang::unsafe_buffer_usage attribute,
should not trigger a warning. This is because, the developer has already
identified
the method as unsafe and warning at every unsafe operation is redundant.
rdar://138644831
---
clang/lib/Sema/AnalysisBasedWarnings.cpp | 3 ++
...warn-unsafe-buffer-usage-function-attr.cpp | 47 ---
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp
b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 589869d0186575b..7111dfd523a1014 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -2610,6 +2610,9 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
// The Callback function that performs analyses:
auto CallAnalyzers = [&](const Decl *Node) -> void {
+if (isa(Node) && Node->hasAttr())
+return;
+
// Perform unsafe buffer usage analysis:
if (!Diags.isIgnored(diag::warn_unsafe_buffer_operation,
Node->getBeginLoc()) ||
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 724d444638b57e7..ef724acc761dbd1 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-function-attr.cpp
@@ -119,16 +119,15 @@ struct HoldsUnsafeMembers {
[[clang::unsafe_buffer_usage]]
HoldsUnsafeMembers(int i)
-: FromCtor(i), // expected-warning{{function introduces unsafe buffer
manipulation}}
- FromCtor2{i} // expected-warning{{function introduces unsafe buffer
manipulation}}
-{}
+: FromCtor(i),
+ FromCtor2{i} {}
HoldsUnsafeMembers(float f)
: HoldsUnsafeMembers(0) {} // expected-warning{{function introduces
unsafe buffer manipulation}}
UnsafeMembers FromCtor;
UnsafeMembers FromCtor2;
-UnsafeMembers FromField{3}; // expected-warning 2{{function introduces
unsafe buffer manipulation}}
+UnsafeMembers FromField{3}; // expected-warning {{function introduces
unsafe buffer manipulation}}
};
struct SubclassUnsafeMembers : public UnsafeMembers {
@@ -138,8 +137,7 @@ struct SubclassUnsafeMembers : public UnsafeMembers {
[[clang::unsafe_buffer_usage]]
SubclassUnsafeMembers(int i)
-: UnsafeMembers(i) // expected-warning{{function introduces unsafe
buffer manipulation}}
-{}
+: UnsafeMembers(i){}
};
// https://github.com/llvm/llvm-project/issues/80482
@@ -245,3 +243,40 @@ struct AggregateViaDefaultInit {
void testAggregateViaDefaultInit() {
AggregateViaDefaultInit A;
};
+
+struct A {
+ int arr[2];
+
+ [[clang::unsafe_buffer_usage]]
+ int *ptr;
+};
+
+namespace std{
+ template class span {
+
+ T *elements;
+
+ public:
+
+ constexpr span(T *, unsigned){}
+
+ template
+ constexpr span(Begin first, End last){}
+
+ constexpr T* data() const noexcept {
+ return elements;
+ }
+ };
+}
+
+[[clang::unsafe_buffer_usage]]
+void check_no_warnings(unsigned idx) {
+ int *arr = new int[20];
+
+ int k = arr[idx]; // no-warning
+
+ std::span sp = {arr, 20}; // no-warning
+ A *ptr = reinterpret_cast (sp.data()); // no-warning
+ A a;
+ a.ptr = arr; // no-warning
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Add additional tests to esnure safe accesses to const sized array are not warned (PR #125483)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/125483 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Add additional tests to esnure safe accesses to const sized array are not warned (PR #125483)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/125483
>From efd4f3200c4fc2b132157c56e0eaf6c2db844878 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 31 Jan 2025 13:40:55 +0530
Subject: [PATCH] [Wunsafe-buffer-usage] Add additional tests to esnure safe
accesses to const sized array are not warned
Add more tests, where the index of the const array access evaluates to a
constant and depends on a template
argument.
rdar://143759014
---
.../warn-unsafe-buffer-usage-array.cpp| 35 +++
1 file changed, 35 insertions(+)
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index e80b54b7c696779..9bfc31bd07b0eee 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -124,3 +124,38 @@ void array_indexed_const_expr(unsigned idx) {
k = arr[get_const(5)]; // expected-note {{used in buffer access here}}
k = arr[get_const(4)];
}
+
+template
+consteval bool isNullTerminated(const char (&literal)[length])
+{
+ return literal[length - 1] == '\0';
+}
+
+template
+T access2DArray(const T (&arr)[M][N]) {
+ return arr[M-1][N-1];
+}
+
+template
+constexpr int access_elements() {
+ int arr[idx + 20];
+ return arr[idx + 1];
+}
+
+// Test array accesses where const sized arrays are accessed safely with
indices
+// that evaluate to a const values and depend on template arguments.
+void test_template_methods()
+{
+ constexpr char arr[] = "Good Morning!"; // = {'a', 'b', 'c', 'd', 'e'};
+ isNullTerminated(arr);
+ isNullTerminated("");
+ auto _ = isNullTerminated("hello world\n");
+ access_elements<5>();
+
+ int arr1[3][4] = {
+{1, 2, 3, 4},
+{5, 6, 7, 8},
+{9, 10, 11, 12}
+};
+ access2DArray(arr1);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/112284
>From f5b583585b96557377e6e2cb524c8e87fab81dd7 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH] [Wunsafe-buffer-usage] False positives for & expression
indexing constant size array (arr[anything & 0])
Do not warn when a constant sized array is indexed with an expression that
contains bitwise and operation
involving constants and it always results in a bound safe access.
rdar://136684050
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 123 ++
.../warn-unsafe-buffer-usage-array.cpp| 31 +
2 files changed, 154 insertions(+)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index c064aa30e8aedc..dd593945cbc286 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -431,6 +431,122 @@ AST_MATCHER(CXXConstructExpr,
isSafeSpanTwoParamConstruct) {
return false;
}
+class MaxValueEval : public ConstStmtVisitor {
+
+ ASTContext &Context;
+ llvm::APInt Max;
+ unsigned bit_width;
+
+public:
+ typedef ConstStmtVisitor VisitorBase;
+
+ explicit MaxValueEval(ASTContext &Ctx, const Expr *exp) : Context(Ctx) {
+bit_width = Ctx.getIntWidth(exp->getType());
+Max = llvm::APInt::getSignedMaxValue(bit_width);
+// val.clear();
+ }
+
+ llvm::APInt findMatch(Expr *exp) { return TraverseStmt(exp); }
+
+ llvm::APInt TraverseStmt(Stmt *S) {
+if (Expr *E = dyn_cast(S)) {
+ Expr::EvalResult EVResult;
+ if (ImplicitCastExpr *ICE = dyn_cast(E)) {
+return TraverseImplicitCastExpr(ICE);
+ } else if (dyn_cast(E)) {
+return Max;
+ } else if (dyn_cast(E)) {
+return Max;
+ } else if (BinaryOperator *BO = dyn_cast(E)) {
+return TraverseBinaryOperator(BO);
+ } else if (IntegerLiteral *IL = dyn_cast(E)) {
+return IL->getValue();
+ }
+}
+return Max;
+ }
+
+ llvm::APInt TraverseImplicitCastExpr(ImplicitCastExpr *E) {
+Expr::EvalResult EVResult;
+if (EvaluateExpression(E, EVResult)) {
+ return EVResult.Val.getInt();
+} else {
+ Expr *eExpr = E->getSubExpr();
+ llvm::APInt subEValue = TraverseStmt(eExpr);
+ switch (E->getCastKind()) {
+ case CK_LValueToRValue:
+return subEValue;
+ case CK_IntegralCast: {
+Expr *eExpr = E->getSubExpr();
+clang::IntegerLiteral *intLiteral = clang::IntegerLiteral::Create(
+Context, subEValue, eExpr->getType(), {});
+E->setSubExpr(intLiteral);
+bool evaluated = EvaluateExpression(E, EVResult);
+E->setSubExpr(eExpr);
+if (evaluated) {
+ return EVResult.Val.getInt();
+}
+break;
+ }
+ default:
+break;
+ }
+ return Max;
+}
+ }
+
+ bool EvaluateExpression(Expr *exp, Expr::EvalResult &EVResult) {
+if (exp->EvaluateAsInt(EVResult, Context)) {
+ return true;
+}
+return false;
+ }
+
+ llvm::APInt TraverseBinaryOperator(BinaryOperator *E) {
+unsigned bwidth = Context.getIntWidth(E->getType());
+
+auto evaluateSubExpr = [&, bwidth](Expr *E) -> llvm::APInt {
+ llvm::APInt Result = TraverseStmt(E);
+ unsigned width = Result.getBitWidth();
+
+ // Fix the bit length.
+ if (bwidth < width)
+Result = Result.trunc(bwidth);
+ else if (bwidth > width)
+Result =
+APInt(bwidth, Result.getLimitedValue(),
Result.isSignedIntN(width));
+ return Result;
+};
+
+Expr::EvalResult EVResult;
+if (EvaluateExpression(E, EVResult)) {
+ return EVResult.Val.getInt();
+} else {
+ Expr *LHSExpr = E->getLHS()->IgnoreParenCasts();
+ Expr *RHSExpr = E->getRHS()->IgnoreParenCasts();
+
+ unsigned bwidth = Context.getIntWidth(E->getType());
+
+ llvm::APInt LHS = evaluateSubExpr(LHSExpr);
+ llvm::APInt RHS = evaluateSubExpr(RHSExpr);
+
+ llvm::APInt Result = Max;
+
+ switch (E->getOpcode()) {
+ case BO_And:
+ case BO_AndAssign:
+Result = LHS & RHS;
+break;
+
+ default:
+break;
+ }
+ return Result;
+}
+return Max;
+ }
+};
+
AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
// FIXME: Proper solution:
// - refactor Sema::CheckArrayAccess
@@ -463,6 +579,13 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
return true;
}
+
+ MaxValueEval Vis(Finder->getASTContext(), Node.getIdx());
+ APInt result = Vis.findMatch(const_cast(Node.getIdx()));
+
+ if (result.isNonNegative() && result.getLimitedValue() < limit)
+return true;
+
return false;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index e80b54b7c69677..93a3be5d9d1f23 100644
--- a/
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
malavikasamak wrote: @jkorous-apple: If there are no objections, I will go ahead and merge this PR. https://github.com/llvm/llvm-project/pull/118249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
malavikasamak wrote: > LGTM! Thanks. Thanks @ziqingluo-90 https://github.com/llvm/llvm-project/pull/118249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/118249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] False positives for & expression indexing constant size array (arr[anything & 0]) (PR #112284)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/112284
>From ea92377ca9b449e62dcb9385184c9edda8e14fdb Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 11 Oct 2024 12:24:58 -0700
Subject: [PATCH] [Wunsafe-buffer-usage] False positives for & expression
indexing constant size array (arr[anything & 0])
Do not warn when a constant sized array is indexed with an expression that
contains bitwise and operation
involving constants and it always results in a bound safe access.
(rdar://136684050)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 156 +-
.../warn-unsafe-buffer-usage-array.cpp| 70
2 files changed, 221 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a9aff39df64746..06c7cfb5c442e1 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -431,6 +431,151 @@ AST_MATCHER(CXXConstructExpr,
isSafeSpanTwoParamConstruct) {
return false;
}
+class MaxValueEval : public ConstStmtVisitor {
+
+ ASTContext &Context;
+ llvm::APInt Max;
+ unsigned bit_width;
+
+public:
+ typedef ConstStmtVisitor VisitorBase;
+
+ explicit MaxValueEval(ASTContext &Ctx, const Expr *exp) : Context(Ctx) {
+bit_width = Ctx.getIntWidth(exp->getType());
+Max = llvm::APInt::getSignedMaxValue(bit_width);
+// val.clear();
+ }
+
+ llvm::APInt findMatch(Expr *exp) { return TraverseStmt(exp); }
+
+ llvm::APInt TraverseStmt(Stmt *S) {
+if (Expr *E = dyn_cast(S)) {
+ Expr::EvalResult EVResult;
+ if (EvaluateExpression(E, EVResult)) {
+return EVResult.Val.getInt();
+ } else if (ImplicitCastExpr *ICE = dyn_cast(E)) {
+return TraverseImplicitCastExpr(ICE);
+ } else if (DeclRefExpr *DRE = dyn_cast(E)) {
+return Max;
+ } else if (ArraySubscriptExpr *ASE = dyn_cast(E)) {
+return Max;
+ } else if (BinaryOperator *BO = dyn_cast(E)) {
+return TraverseBinaryOperator(BO);
+ } else if (IntegerLiteral *IL = dyn_cast(E)) {
+return IL->getValue();
+ }
+}
+return Max;
+ }
+
+ llvm::APInt TraverseImplicitCastExpr(ImplicitCastExpr *E) {
+Expr::EvalResult EVResult;
+if (EvaluateExpression(E, EVResult)) {
+ return EVResult.Val.getInt();
+} else {
+ Expr *eExpr = E->getSubExpr();
+ llvm::APInt subEValue = TraverseStmt(eExpr);
+ switch (E->getCastKind()) {
+ case CK_LValueToRValue:
+return subEValue;
+ case CK_IntegralCast: {
+Expr *eExpr = E->getSubExpr();
+clang::IntegerLiteral *intLiteral = clang::IntegerLiteral::Create(
+Context, subEValue, eExpr->getType(), {});
+E->setSubExpr(intLiteral);
+bool evaluated = EvaluateExpression(E, EVResult);
+E->setSubExpr(eExpr);
+if (evaluated) {
+ return EVResult.Val.getInt();
+}
+break;
+ }
+ default:
+break;
+ }
+ return Max;
+}
+ }
+
+ bool EvaluateExpression(Expr *exp, Expr::EvalResult &EVResult) {
+if (exp->EvaluateAsInt(EVResult, Context)) {
+ return true;
+}
+return false;
+ }
+
+ llvm::APInt TraverseBinaryOperator(BinaryOperator *E) {
+unsigned bwidth = Context.getIntWidth(E->getType());
+
+auto evaluateSubExpr = [&, bwidth](Expr *E) -> llvm::APInt {
+ llvm::APInt Result = TraverseStmt(E);
+ unsigned width = Result.getBitWidth();
+
+ // Fix the bit length.
+ if (bwidth < width)
+Result = Result.trunc(bwidth);
+ else if (bwidth > width)
+Result =
+APInt(bwidth, Result.getLimitedValue(),
Result.isSignedIntN(width));
+ return Result;
+};
+
+Expr::EvalResult EVResult;
+if (EvaluateExpression(E, EVResult)) {
+ return EVResult.Val.getInt();
+} else {
+ Expr *LHSExpr = E->getLHS()->IgnoreParenCasts();
+ Expr *RHSExpr = E->getRHS()->IgnoreParenCasts();
+
+ unsigned bwidth = Context.getIntWidth(E->getType());
+
+ llvm::APInt LHS = evaluateSubExpr(LHSExpr);
+ llvm::APInt RHS = evaluateSubExpr(RHSExpr);
+
+ llvm::APInt Result = Max;
+
+ switch (E->getOpcode()) {
+ case BO_And:
+ case BO_AndAssign:
+Result = LHS & RHS;
+break;
+
+ case BO_Or:
+ case BO_OrAssign:
+Result = LHS | RHS;
+break;
+
+ case BO_Shl:
+ case BO_ShlAssign:
+if (RHS != Max.getLimitedValue())
+ Result = LHS << RHS.getLimitedValue();
+break;
+
+ case BO_Shr:
+ case BO_ShrAssign:
+if (RHS == Max.getLimitedValue())
+ Result = LHS;
+else
+ Result = LHS.getLimitedValue() >> RHS.getLimitedValue();
+break;
+
+ case BO_Rem:
+ case BO_RemAssign:
+if (LHS.getLimitedValue() < RHS.getLimitedValue())
+ Result = LHS;
+else
+ Result = --RHS;
+brea
[clang] [-Wunsafe-buffer-usage] Add alloc_size knowledge to the 2-param span constructor warning (PR #114894)
malavikasamak wrote: Very interesting. Potentially an API that could be exposed to other analyses too! https://github.com/llvm/llvm-project/pull/114894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Add alloc_size knowledge to the 2-param span constructor warning (PR #114894)
malavikasamak wrote:
Can you please rebase this PR? The isSafeSpanTwoParamConstruct matcher has
changes on the main branch that are not present here. For instance, case 6 is
now used by :`std::span{std::addressof(...), 1}` on the main branch.
https://github.com/llvm/llvm-project/pull/114894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
@@ -8,6 +8,5 @@
// main function
int main(int argc, char *argv[]) { // expected-warning{{'argv' is an unsafe
pointer used for buffer access}}
char tmp;
- tmp = argv[5][5];// expected-note{{used in buffer access
here}} \
- expected-warning{{unsafe buffer access}}
malavikasamak wrote:
Ah! Yes you are right.
https://github.com/llvm/llvm-project/pull/118249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
@@ -433,37 +433,36 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+ std::function CheckBounds =
+ [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(ASE->getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else {
return false;
}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
+ if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+return CheckBounds(BaseASE);
malavikasamak wrote:
Ah! Now I think I understand your concern. You are saying that when we are
analyzing a[5][5] we are checking for the a[5], whereas a[5] should be checked
again separately. That's a valid point. Let me take another pass at it.
https://github.com/llvm/llvm-project/pull/118249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/118249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/118249
>From e1eadf464509ab74ff4f097f92e7140f43c61262 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning for
multi-dimensional constant arrays
Do not warn about unsafe buffer access, when multi-dimensional constant arrays
are accessed and their indices are within the bounds of the buffer. Warning in
such cases would be a false positive. Such a suppression already exists for 1-d
arrays and it is now extended to multi-dimensional arrays.
(rdar://137926311)
(rdar://140320139)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 44 +++
.../warn-unsafe-buffer-usage-array.cpp| 40 +
.../warn-unsafe-buffer-usage-field-attr.cpp | 1 -
...e-buffer-usage-fixits-parm-unsupported.cpp | 2 +-
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 39 +---
5 files changed, 82 insertions(+), 44 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..c3f580ec42eba3 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,37 +433,27 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(Node.getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ Node.getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else {
return false;
}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
return true;
- }
-
- return false;
+}
+return false;
}
AST_MATCHER_P(CallExpr, hasNumArgs, unsigned, Num) {
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..7dd6c83dbba2a8 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,43 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+// expected-warning@+1 {{'matrix' is an unsafe buffer that does not perform
bounds checks}}
+float two_dimension_array(Float4x4& matrix, unsigned idx) {
+ // expected-warning@+1{{unsafe buffer access}}
+ float a = matrix[0][4];
+
+ a = matrix[0][3];
+
+ // expected-note@+1{{used in buffer access here}}
+ a = matrix[4][0];
+
+ a = matrix[idx][0]; // expected-note{{used in buffer access here}}
+
+ a = matrix[0][idx]; //expected-warning{{unsafe buffer access}}
+
+ a = matrix[idx][idx]; //expected-warning{{unsafe buffer access}} //
expected-note{{used in buffer access here}}
+
+ return matrix[1][1];
+}
+
+typedef float Float2x3x4[2][3][4];
+float multi_dimension_array(Float2x3x4& matrix) {
+ float *f = matrix[0][2];
+ return matrix[1][2][3];
+}
+
+char array_strings[][11] = {
+ "Apple", "Banana", "Cherry", "Date", "Elderberry"
+};
+
+char array_string[] = "123456";
+
+char access_strings() {
+ char c = array_strings[0][4];
+ c = array_strings[3][10];
+ c = array_string[5];
+ return c;
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
index 0ba605475925b9..1636c948da075a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
@@ -96,7 +96,6 @@ void test_attribute_multiple_fields (D d) {
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
@@ -433,37 +433,36 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
-return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
+ std::function CheckBounds =
+ [&CheckBounds](const ArraySubscriptExpr *ASE) -> bool {
+uint64_t limit;
+if (const auto *CATy =
+dyn_cast(ASE->getBase()
+->IgnoreParenImpCasts()
+->getType()
+->getUnqualifiedDesugaredType())) {
+ limit = CATy->getLimitedSize();
+} else if (const auto *SLiteral = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+ limit = SLiteral->getLength() + 1;
+} else {
return false;
}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
- }
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (const auto *IdxLit = dyn_cast(ASE->getIdx())) {
+ const APInt ArrIdx = IdxLit->getValue();
+ if (!ArrIdx.isNonNegative() || ArrIdx.getLimitedValue() >= limit)
+return false;
+ if (const auto *BaseASE = dyn_cast(
+ ASE->getBase()->IgnoreParenImpCasts())) {
+return CheckBounds(BaseASE);
malavikasamak wrote:
Done.
https://github.com/llvm/llvm-project/pull/118249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [-Wunsafe-buffer-usage] Suppress warning for multi-dimensional constant arrays (PR #118249)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/118249
>From c1f7c2b37948271ccc8de7910d2bab91dfb2f87e Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Fri, 29 Nov 2024 14:53:37 +0530
Subject: [PATCH] [-Wunsafe-buffer-usage] Suppress warning for
multi-dimensional constant arrays
Do not warn about unsafe buffer access, when multi-dimensional constant arrays
are accessed and their indices are within the bounds of the buffer. Warning in
such cases would be a false positive. Such a suppression already exists for 1-d
arrays and it is now extended to multi-dimensional arrays.
(rdar://137926311)
(rdar://140320139)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 35 ++--
.../warn-unsafe-buffer-usage-array.cpp| 40 +++
.../warn-unsafe-buffer-usage-field-attr.cpp | 1 -
...e-buffer-usage-fixits-parm-unsupported.cpp | 2 +-
.../test/SemaCXX/warn-unsafe-buffer-usage.cpp | 39 +++---
5 files changed, 77 insertions(+), 40 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 5f36ffa926b269..261b600ccddc15 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -433,36 +433,25 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
//already duplicated
// - call both from Sema and from here
- const auto *BaseDRE =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- const auto *SLiteral =
- dyn_cast(Node.getBase()->IgnoreParenImpCasts());
- uint64_t size;
-
- if (!BaseDRE && !SLiteral)
+ uint64_t limit;
+ if (const auto *CATy =
+ dyn_cast(Node.getBase()
+ ->IgnoreParenImpCasts()
+ ->getType()
+ ->getUnqualifiedDesugaredType())) {
+limit = CATy->getLimitedSize();
+ } else if (const auto *SLiteral = dyn_cast(
+ Node.getBase()->IgnoreParenImpCasts())) {
+limit = SLiteral->getLength() + 1;
+ } else {
return false;
-
- if (BaseDRE) {
-if (!BaseDRE->getDecl())
- return false;
-const auto *CATy = Finder->getASTContext().getAsConstantArrayType(
-BaseDRE->getDecl()->getType());
-if (!CATy) {
- return false;
-}
-size = CATy->getLimitedSize();
- } else if (SLiteral) {
-size = SLiteral->getLength() + 1;
}
if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
const APInt ArrIdx = IdxLit->getValue();
-// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
-// bug
-if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < size)
+if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
return true;
}
-
return false;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index c6c93a27e4b969..7dd6c83dbba2a8 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -52,3 +52,43 @@ void constant_id_string(unsigned idx) {
unsafe_char = ""[1]; //expected-warning{{unsafe buffer access}}
unsafe_char = ""[idx]; //expected-warning{{unsafe buffer access}}
}
+
+typedef float Float4x4[4][4];
+
+// expected-warning@+1 {{'matrix' is an unsafe buffer that does not perform
bounds checks}}
+float two_dimension_array(Float4x4& matrix, unsigned idx) {
+ // expected-warning@+1{{unsafe buffer access}}
+ float a = matrix[0][4];
+
+ a = matrix[0][3];
+
+ // expected-note@+1{{used in buffer access here}}
+ a = matrix[4][0];
+
+ a = matrix[idx][0]; // expected-note{{used in buffer access here}}
+
+ a = matrix[0][idx]; //expected-warning{{unsafe buffer access}}
+
+ a = matrix[idx][idx]; //expected-warning{{unsafe buffer access}} //
expected-note{{used in buffer access here}}
+
+ return matrix[1][1];
+}
+
+typedef float Float2x3x4[2][3][4];
+float multi_dimension_array(Float2x3x4& matrix) {
+ float *f = matrix[0][2];
+ return matrix[1][2][3];
+}
+
+char array_strings[][11] = {
+ "Apple", "Banana", "Cherry", "Date", "Elderberry"
+};
+
+char array_string[] = "123456";
+
+char access_strings() {
+ char c = array_strings[0][4];
+ c = array_strings[3][10];
+ c = array_string[5];
+ return c;
+}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
index 0ba605475925b9..1636c948da075a 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-field-attr.cpp
@@ -96,7 +96,6 @@ void test_attribute_multiple_fields (D d) {
int v = d.buf[0]; //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
- //expected-warning@+1{{unsafe buffer access}}
v = d.buf[5]; //expected-warning{{field 'buf' prone to unsafe buffer
manipulation}}
[clang] "Reland "[Wunsafe-buffer-usage] Fix false positive when const sized array is indexed by const evaluatable expressions (#119340)"" (PR #123713)
https://github.com/malavikasamak created
https://github.com/llvm/llvm-project/pull/123713
This reverts commit 7dd34baf5505d689161c3a8678322a394d7a2929.
Fixed the assertion violation reported by
7dd34baf5505d689161c3a8678322a394d7a2929
>From b3b7a079e861432554eb3c85a2cc235980f53f65 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Tue, 21 Jan 2025 14:01:25 +0530
Subject: [PATCH] "Reland "[Wunsafe-buffer-usage] Fix false positive when const
sized array is indexed by const evaluatable expressions (#119340)""
This reverts commit 7dd34baf5505d689161c3a8678322a394d7a2929.
Fixed the assertion violation reported by
7dd34baf5505d689161c3a8678322a394d7a2929
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 9 --
.../warn-unsafe-buffer-usage-array.cpp| 32 +++
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a9aff39df64746..c064aa30e8aedc 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -453,8 +453,13 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
return false;
}
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
+ Expr::EvalResult EVResult;
+ const Expr *IndexExpr = Node.getIdx();
+ if (!IndexExpr->isValueDependent() &&
+ IndexExpr->EvaluateAsInt(EVResult, Finder->getASTContext())) {
+llvm::APSInt ArrIdx = EVResult.Val.getInt();
+// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
+// bug
if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
return true;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 7dd6c83dbba2a8..e80b54b7c69677 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -92,3 +92,35 @@ char access_strings() {
c = array_string[5];
return c;
}
+
+struct T {
+ int array[10];
+};
+
+const int index = 1;
+
+constexpr int get_const(int x) {
+ if(x < 3)
+return ++x;
+ else
+return x + 5;
+};
+
+void array_indexed_const_expr(unsigned idx) {
+ // expected-note@+2 {{change type of 'arr' to 'std::array' to label it for
hardening}}
+ // expected-warning@+1{{'arr' is an unsafe buffer that does not perform
bounds checks}}
+ int arr[10];
+ arr[sizeof(int)] = 5;
+
+ int array[sizeof(T)];
+ array[sizeof(int)] = 5;
+ array[sizeof(T) -1 ] = 3;
+
+ int k = arr[6 & 5];
+ k = arr[2 << index];
+ k = arr[8 << index]; // expected-note {{used in buffer access here}}
+ k = arr[16 >> 1];
+ k = arr[get_const(index)];
+ k = arr[get_const(5)]; // expected-note {{used in buffer access here}}
+ k = arr[get_const(4)];
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] "Reland "[Wunsafe-buffer-usage] Fix false positive when const sized array is indexed by const evaluatable expressions (#119340)"" (PR #123713)
https://github.com/malavikasamak closed https://github.com/llvm/llvm-project/pull/123713 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positive when const sized array is indexed by const evaluatable expressions (PR #119340)
malavikasamak wrote: @nico Thanks for the reproducer. I have relanded this change with the fix for the failing assert. https://github.com/llvm/llvm-project/commit/2a8c12b29f8dc777a62868512bed1a2dae1ef8b2 https://github.com/llvm/llvm-project/pull/119340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Fix false positive when const sized array is indexed by const evaluatable expressions (PR #119340)
https://github.com/malavikasamak updated
https://github.com/llvm/llvm-project/pull/119340
>From ca068b2383b784d783a33f3678f6bb90a6544861 Mon Sep 17 00:00:00 2001
From: MalavikaSamak
Date: Tue, 10 Dec 2024 11:05:21 +0530
Subject: [PATCH] [Wunsafe-buffer-usage] Fix false positive when const sized
array is indexed by const evaluated expressions
Do not warn when constant sized array is indexed by expressions that evaluate
to a const value. For instance,
sizeof(T) expression value can be evaluated at compile time and if an array is
indexed by such an expression,
it's bounds can be validated.
(rdar://140320289)
---
clang/lib/Analysis/UnsafeBufferUsage.cpp | 7 ++--
.../warn-unsafe-buffer-usage-array.cpp| 32 +++
2 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index a9aff39df64746..bef5fa8624ce48 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -453,8 +453,11 @@ AST_MATCHER(ArraySubscriptExpr, isSafeArraySubscript) {
return false;
}
- if (const auto *IdxLit = dyn_cast(Node.getIdx())) {
-const APInt ArrIdx = IdxLit->getValue();
+ Expr::EvalResult EVResult;
+ if (Node.getIdx()->EvaluateAsInt(EVResult, Finder->getASTContext())) {
+llvm::APSInt ArrIdx = EVResult.Val.getInt();
+// FIXME: ArrIdx.isNegative() we could immediately emit an error as that's
a
+// bug
if (ArrIdx.isNonNegative() && ArrIdx.getLimitedValue() < limit)
return true;
}
diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
index 7dd6c83dbba2a8..e80b54b7c69677 100644
--- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
+++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-array.cpp
@@ -92,3 +92,35 @@ char access_strings() {
c = array_string[5];
return c;
}
+
+struct T {
+ int array[10];
+};
+
+const int index = 1;
+
+constexpr int get_const(int x) {
+ if(x < 3)
+return ++x;
+ else
+return x + 5;
+};
+
+void array_indexed_const_expr(unsigned idx) {
+ // expected-note@+2 {{change type of 'arr' to 'std::array' to label it for
hardening}}
+ // expected-warning@+1{{'arr' is an unsafe buffer that does not perform
bounds checks}}
+ int arr[10];
+ arr[sizeof(int)] = 5;
+
+ int array[sizeof(T)];
+ array[sizeof(int)] = 5;
+ array[sizeof(T) -1 ] = 3;
+
+ int k = arr[6 & 5];
+ k = arr[2 << index];
+ k = arr[8 << index]; // expected-note {{used in buffer access here}}
+ k = arr[16 >> 1];
+ k = arr[get_const(index)];
+ k = arr[get_const(5)]; // expected-note {{used in buffer access here}}
+ k = arr[get_const(4)];
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Wunsafe-buffer-usage] Turn off unsafe-buffer warning for methods annotated with clang::unsafe_buffer_usage attribute (PR #125671)
malavikasamak wrote: @dtarditi FYI. https://github.com/llvm/llvm-project/pull/125671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CXX Safe Buffer] Update the safe buffer attribute documentation (PR #135087)
https://github.com/malavikasamak created https://github.com/llvm/llvm-project/pull/135087 Update the documentation for the unsafe_buffer_usage attribute to capture the new behavior introduced by https://github.com/llvm/llvm-project/pull/125671 >From c3fbfd791d0c2f6d9ed0825fd2a043d3319da058 Mon Sep 17 00:00:00 2001 From: MalavikaSamak Date: Wed, 9 Apr 2025 14:31:06 -0700 Subject: [PATCH] Update the documentation for the unsafe_buffer_usage attribute to capture the new beahvior introduced by https://github.com/llvm/llvm-project/pull/125671 --- clang/include/clang/Basic/AttrDocs.td | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index c8b371280e35d..1546d849f6856 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -7517,9 +7517,11 @@ the field it is attached to, and it may also lead to emission of automatic fix-i hints which would help the user replace the use of unsafe functions(/fields) with safe alternatives, though the attribute can be used even when the fix can't be automated. -* Attribute attached to functions: The attribute does not suppress - ``-Wunsafe-buffer-usage`` inside the function to which it is attached. - These warnings still need to be addressed. +* Attribute attached to functions: The attribute suppresses all the + ``-Wunsafe-buffer-usage`` warnings inside the function to which it is attached, since + it is now classified as an unsafe function. The attribute must be placed judiciously + as there won't be any warning for unsafe operations within the function and this can + also cause it to mask new usafe operations introduced in the future. The attribute is warranted even if the only way a function can overflow the buffer is by violating the function's preconditions. For example, it ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CXX Safe Buffer] Update the documentation for unsafe_buffer_usage attribute (PR #135087)
https://github.com/malavikasamak edited https://github.com/llvm/llvm-project/pull/135087 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
