[PATCH] D131525: [analyzer] Fix false positive in use-after-move checker
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc74a204826da: [analyzer] Fix false positive in
use-after-move checker (authored by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131525/new/
https://reviews.llvm.org/D131525
Files:
clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
clang/test/Analysis/use-after-move.cpp
Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -900,6 +900,28 @@
}
}
+void checkExplicitDestructorCalls() {
+ // The below code segments invoke the destructor twice (explicit and
+ // implicit). While this is not a desired code behavior, it is
+ // not the use-after-move checker's responsibility to issue such a warning.
+ {
+ B* b = new B;
+ B a = std::move(*b);
+ b->~B(); // no-warning
+ delete b;
+ }
+ {
+B a, b;
+new (&a) B(reinterpret_cast(b));
+(&b)->~B(); // no-warning
+ }
+ {
+B b;
+B a = std::move(b);
+b.~B(); // no-warning
+ }
+}
+
struct MoveOnlyWithDestructor {
MoveOnlyWithDestructor();
~MoveOnlyWithDestructor();
Index: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -618,10 +618,6 @@
if (!IC)
return;
- // Calling a destructor on a moved object is fine.
- if (isa(IC))
-return;
-
const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
if (!ThisRegion)
return;
@@ -631,6 +627,10 @@
if (!MethodDecl)
return;
+ // Calling a destructor on a moved object is fine.
+ if (isa(MethodDecl))
+return;
+
// We want to investigate the whole object, not only sub-object of a parent
// class in which the encountered method defined.
ThisRegion = ThisRegion->getMostDerivedObjectRegion();
Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -900,6 +900,28 @@
}
}
+void checkExplicitDestructorCalls() {
+ // The below code segments invoke the destructor twice (explicit and
+ // implicit). While this is not a desired code behavior, it is
+ // not the use-after-move checker's responsibility to issue such a warning.
+ {
+ B* b = new B;
+ B a = std::move(*b);
+ b->~B(); // no-warning
+ delete b;
+ }
+ {
+B a, b;
+new (&a) B(reinterpret_cast(b));
+(&b)->~B(); // no-warning
+ }
+ {
+B b;
+B a = std::move(b);
+b.~B(); // no-warning
+ }
+}
+
struct MoveOnlyWithDestructor {
MoveOnlyWithDestructor();
~MoveOnlyWithDestructor();
Index: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
@@ -618,10 +618,6 @@
if (!IC)
return;
- // Calling a destructor on a moved object is fine.
- if (isa(IC))
-return;
-
const MemRegion *ThisRegion = IC->getCXXThisVal().getAsRegion();
if (!ThisRegion)
return;
@@ -631,6 +627,10 @@
if (!MethodDecl)
return;
+ // Calling a destructor on a moved object is fine.
+ if (isa(MethodDecl))
+return;
+
// We want to investigate the whole object, not only sub-object of a parent
// class in which the encountered method defined.
ThisRegion = ThisRegion->getMostDerivedObjectRegion();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140062: [-Wunsafe-buffer-usage] Safe-buffers re-architecture to introduce Fixable gadgets
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG50d4a1f70e11: [-Wunsafe-buffer-usage] Safe-buffers
re-architecture to introduce Fixable… (authored by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D140062?vs=486625&id=486952#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140062/new/
https://reviews.llvm.org/D140062
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -137,9 +137,9 @@
/// rigid AST structure that constitutes an operation on a pointer-type object.
/// Discovery of a gadget in the code corresponds to claiming that we understand
/// what this part of code is doing well enough to potentially improve it.
-/// Gadgets can be unsafe (immediately deserving a warning) or safe (not
-/// deserving a warning per se, but affecting our decision-making process
-/// nonetheless).
+/// Gadgets can be warning (immediately deserving a warning) or fixable (not
+/// always deserving a warning per se, but requires our attention to identify
+/// it warrants a fixit).
class Gadget {
public:
enum class Kind {
@@ -156,7 +156,7 @@
Kind getKind() const { return K; }
- virtual bool isSafe() const = 0;
+ virtual bool isWarningGadget() const = 0;
virtual const Stmt *getBaseStmt() const = 0;
/// Returns the list of pointer-type variables on which this gadget performs
@@ -164,53 +164,55 @@
/// of all DeclRefExprs in the gadget's AST!
virtual DeclUseList getClaimedVarUseSites() const = 0;
- /// Returns a fixit that would fix the current gadget according to
- /// the current strategy. Returns None if the fix cannot be produced;
- /// returns an empty list if no fixes are necessary.
- virtual std::optional getFixits(const Strategy &) const {
-return std::nullopt;
- }
-
virtual ~Gadget() = default;
private:
Kind K;
};
-using GadgetList = std::vector>;
-/// Unsafe gadgets correspond to unsafe code patterns that warrants
+/// Warning gadgets correspond to unsafe code patterns that warrants
/// an immediate warning.
-class UnsafeGadget : public Gadget {
+class WarningGadget : public Gadget {
public:
- UnsafeGadget(Kind K) : Gadget(K) {}
+ WarningGadget(Kind K) : Gadget(K) {}
- static bool classof(const Gadget *G) { return G->isSafe(); }
- bool isSafe() const final { return false; }
+ static bool classof(const Gadget *G) { return G->isWarningGadget(); }
+ bool isWarningGadget() const final { return true; }
};
-/// Safe gadgets correspond to code patterns that aren't unsafe but need to be
-/// properly recognized in order to emit correct warnings and fixes over unsafe
-/// gadgets. For example, if a raw pointer-type variable is replaced by
-/// a safe C++ container, every use of such variable may need to be
+/// Fixable gadgets correspond to code patterns that aren't always unsafe but need to be
+/// properly recognized in order to emit fixes. For example, if a raw pointer-type
+/// variable is replaced by a safe C++ container, every use of such variable must be
/// carefully considered and possibly updated.
-class SafeGadget : public Gadget {
+class FixableGadget : public Gadget {
public:
- SafeGadget(Kind K) : Gadget(K) {}
+ FixableGadget(Kind K) : Gadget(K) {}
+
+ static bool classof(const Gadget *G) { return !G->isWarningGadget(); }
+ bool isWarningGadget() const final { return false; }
+
+ /// Returns a fixit that would fix the current gadget according to
+ /// the current strategy. Returns None if the fix cannot be produced;
+ /// returns an empty list if no fixes are necessary.
+ virtual Optional getFixits(const Strategy &) const {
+return None;
+ }
- static bool classof(const Gadget *G) { return !G->isSafe(); }
- bool isSafe() const final { return true; }
};
+using FixableGadgetList = std::vector>;
+using WarningGadgetList = std::vector>;
+
/// An increment of a pointer-type value is unsafe as it may run the pointer
/// out of bounds.
-class IncrementGadget : public UnsafeGadget {
+class IncrementGadget : public WarningGadget {
static constexpr const char *const OpTag = "op";
const UnaryOperator *Op;
public:
IncrementGadget(const MatchFinder::MatchResult &Result)
- : UnsafeGadget(Kind::Increment),
+ : WarningGadget(Kind::Increment),
Op(Result.Nodes.getNodeAs(OpTag)) {}
static bool classof(const Gadget *G) {
@@ -239,13 +241,13 @@
/// A decrement of a pointer-type value is unsafe as it may run the pointer
/// out of bounds.
-class DecrementGadget : public UnsafeGadget {
+class Decr
[PATCH] D143206: [-Wunsafe-buffer-usage] Add Fixable for simple pointer dereference
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe7596a99fca6: [-Wunsafe-buffer-usage] Add Fixable for simple
pointer dereference (authored by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D143206?vs=507463&id=507529#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143206/new/
https://reviews.llvm.org/D143206
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-deref.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void basic_dereference() {
+ int tmp;
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+ tmp = p[5];
+ int val = *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:14}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"[0]"
+}
+
+int return_method() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+ int tmp = p[5];
+ return *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:11}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"[0]"
+}
+
+void foo(int v) {
+}
+
+void method_invocation() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+
+ foo(*p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:8}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:9-[[@LINE-2]]:9}:"[0]"
+}
+
+void binary_operation() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+
+ int k = *p + 20;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:12}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:13-[[@LINE-2]]:13}:"[0]"
+
+}
+
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -463,6 +463,45 @@
return {};
}
};
+
+class PointerDereferenceGadget : public FixableGadget {
+ static constexpr const char *const BaseDeclRefExprTag = "BaseDRE";
+ static constexpr const char *const OperatorTag = "op";
+
+ const DeclRefExpr *BaseDeclRefExpr = nullptr;
+ const UnaryOperator *Op = nullptr;
+
+public:
+ PointerDereferenceGadget(const MatchFinder::MatchResult &Result)
+ : FixableGadget(Kind::PointerDereference),
+BaseDeclRefExpr(
+Result.Nodes.getNodeAs(BaseDeclRefExprTag)),
+Op(Result.Nodes.getNodeAs(OperatorTag)) {}
+
+ static bool classof(const Gadget *G) {
+return G->getKind() == Kind::PointerDereference;
+ }
+
+ static Matcher matcher() {
+auto Target =
+unaryOperator(
+hasOperatorName("*"),
+has(expr(ignoringParenImpCasts(
+declRefExpr(to(varDecl())).bind(BaseDeclRefExprTag)
+.bind(OperatorTag);
+
+return expr(isInUnspecifiedLvalueContext(Target));
+ }
+
+ DeclUseList getClaimedVarUseSites() const override {
+return {BaseDeclRefExpr};
+ }
+
+ virtual const Stmt *getBaseStmt() const final { return Op; }
+
+ virtual std::optional getFixits(const Strategy &S) const override;
+};
+
} // namespace
namespace {
@@ -914,6 +953,37 @@
return std::nullopt; // something wrong or unsupported, give up
}
+std::optional
+PointerDereferenceGadget::getFixits(const Strategy &S) const {
+ const VarDecl *VD = cast(BaseDeclRefExpr->getDecl());
+ switch (S.lookup(VD)) {
+ case Strategy::Kind::Span: {
+ASTContext &Ctx = VD->getASTContext();
+SourceManager &SM = Ctx.getSourceManager();
+// Required changes: *(ptr); => (ptr[0]); and *ptr; => ptr[0]
+// Deletes the *operand
+CharSourceRange derefRange = clang::CharSourceRange::getCharRange(
+Op->getB
[PATCH] D143128: [-Wunsafe-buffer-usage] Fix-Its transforming `&DRE[any]` to `(DRE.data() + any)`
malavikasamak added inline comments.
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1096
+ } else {
+SS << "&" << getExprText(DRE, SM, LangOpts).str() << ".data()"
+ << "[" << getExprText(Idx, SM, LangOpts).str() << "]";
The title and summary of this patch indicates this gadget emits fixit of the
form &DRE.data() + any when it encounters &DRE[any], but the code seems to
emits &DRE.data()[any] instead? If yes, can you please update the summary?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143128/new/
https://reviews.llvm.org/D143128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144064: [-Wunsafe-buffer-usage] Match unsafe pointers being casted to bool or participating in pointer subtractions
malavikasamak added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:156 + // 2. the operand of a cast-to-(Integer or Boolean) operation; or + // 3. the operand of a pointer subtraction operation; or + // 4. the operand of a pointer comparison operation; or I don't understand why we are special handling only pointer subtraction here. Won't pointer addition be also considered UPC? If so, can we just add it to this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144064/new/ https://reviews.llvm.org/D144064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D144064: [-Wunsafe-buffer-usage] Match unsafe pointers being casted to bool or participating in pointer subtractions
malavikasamak added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:156 + // 2. the operand of a cast-to-(Integer or Boolean) operation; or + // 3. the operand of a pointer subtraction operation; or + // 4. the operand of a pointer comparison operation; or malavikasamak wrote: > I don't understand why we are special handling only pointer subtraction here. > Won't pointer addition be also considered UPC? If so, can we just add it to > this patch. Never mind. It looks like pointer addition is not even legal and subtraction is a special case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144064/new/ https://reviews.llvm.org/D144064 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143676: [-Wunsafe-buffer-usage] FixableGadget for handling stand alone pointers under UPC.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa046d1877201: [-Wunsafe-buffer-usage] FixableGadget for
handling stand alone pointers under… (authored by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D143676?vs=507918&id=511810#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143676/new/
https://reviews.llvm.org/D143676
Files:
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-pointer-access.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+void foo(int* v) {
+}
+
+void m1(int* v1, int* v2, int) {
+
+}
+
+void condition_check_nullptr() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+ if(p != nullptr) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+}
+
+void condition_check() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ auto q = new int[10];
+
+ int tmp = p[5];
+ if(p == q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if(q != p){}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:12}:".data()"
+
+ if(p < q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if(p <= q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if(p > q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if(p >= q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ if( p == q && p != nullptr) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:".data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:18-[[@LINE-2]]:18}:".data()"
+}
+
+void condition_check_two_usafe_buffers() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ auto q = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span q"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+ tmp = q[5];
+
+ if(p == q) {}
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:".data()"
+}
+
+void unsafe_method_invocation_single_param() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+ foo(p);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:8-[[@LINE-1]]:8}:".data()"
+
+}
+
+void safe_method_invocation_single_param() {
+ auto p = new int[10];
+ foo(p);
+}
+
+void unsafe_method_invocation_double_param() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+ m1(p, p, 10);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:".data()"
+
+ auto q = new int[10];
+
+ m1(p, q, 4);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:7-[[@LINE-1]]:7}:".data()"
+
+ m1(q, p, 9);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:10}:".data()"
+
+ m1(q, q, 8);
+}
+
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -165,17 +165,22 @@
// 1. an argument of a function call (except the callee has [[unsafe_...]]
// attribute), or
// 2. the operand of a cast operation; or
- // ...
+ // 3. the operand of
[PATCH] D144905: [-Wunsafe-buffer-usage] Handle unevaluated contexts that contain unsafe buffer usages
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG777eb4bcfc32: [-Wunsafe-buffer-usage] Handle unevaluated
contexts that contain unsafe buffer… (authored by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D144905?vs=515094&id=515114#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144905/new/
https://reviews.llvm.org/D144905
Files:
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp
@@ -102,12 +102,6 @@
foo(ap4[1]);// expected-note{{used in buffer access here}}
}
-//TODO: do not warn for unevaluated context
-void testUnevaluatedContext(int * p) {// expected-warning{{'p' is an unsafe pointer used for buffer access}}
- foo(sizeof(p[1]), // expected-note{{used in buffer access here}}
- sizeof(decltype(p[1]))); // expected-note{{used in buffer access here}}
-}
-
void testQualifiedParameters(const int * p, const int * const q, const int a[10], const int b[10][10]) {
// expected-warning@-1{{'p' is an unsafe pointer used for buffer access}}
// expected-warning@-2{{'q' is an unsafe pointer used for buffer access}}
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-warning-unevaluated-context.cpp
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -std=c++20 -Wno-all -Wunsafe-buffer-usage -fblocks -include %s -verify %s
+
+// RUN: %clang -x c++ -fsyntax-only -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s
+// RUN: %clang_cc1 -std=c++11 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s
+// RUN: %clang_cc1 -std=c++20 -fblocks -include %s %s 2>&1 | FileCheck --allow-empty %s
+// CHECK-NOT: [-Wunsafe-buffer-usage]
+
+#ifndef INCLUDED
+#define INCLUDED
+#pragma clang system_header
+
+// no spanification warnings for system headers
+void foo(...); // let arguments of `foo` to hold testing expressions
+#else
+
+namespace std {
+ class type_info;
+ class bad_cast;
+ class bad_typeid;
+}
+using size_t = __typeof(sizeof(int));
+void *malloc(size_t);
+
+void foo(int v) {
+}
+
+void foo(int *p){}
+
+void uneval_context_fix() {
+ auto p = new int[10]; // expected-warning{{'p' is an unsafe pointer used for buffer access}}
+
+ // Warn on the following DREs
+ _Generic(1, int: p[2], float: 3); // expected-note{{used in buffer access here}}
+
+ // Do not warn for following DREs
+ auto q = new int[10];
+ foo(sizeof(q[1]), // no-note
+ sizeof(decltype(q[1]))); // no-note
+ __typeof(q[5]) x; // no-note
+ int *r = (int *)malloc(sizeof(q[5])); // no-note
+ int y = sizeof(q[5]); // no-note
+ __is_pod(__typeof(q[5])); // no-note
+ __is_trivially_constructible(__typeof(q[5]), decltype(q[5])); // no-note
+ _Generic(q[1], int: 2, float: 3); // no-note
+ _Generic(1, int: 2, float: q[3]); // no-note
+ decltype(q[2]) var = y; // no-note
+ noexcept(q[2]); // no-note
+ typeid(q[3]); // no-note
+}
+#endif
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-unevaluated-context.cpp
@@ -0,0 +1,79 @@
+// RUN: %clang_cc1 -std=c++20 -Wunsafe-buffer-usage -fdiagnostics-parseable-fixits -fsyntax-only %s 2>&1 | FileCheck %s
+
+namespace std {
+ class type_info;
+ class bad_cast;
+ class bad_typeid;
+}
+using size_t = __typeof(sizeof(int));
+void *malloc(size_t);
+
+void foo(...);
+int bar(int *ptr);
+
+void uneval_context_fix_pointer_dereference() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+
+ int tmp = p[5];
+ typeid(foo(*p));
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:14-[[@LINE-1]]:15}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"[0]"
+ _Generic(*p, int: 2, float: 3);
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:12-[[@LINE-1]]:13}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:14-[[@LINE-2]]:14}:"[0]"
+}
+
+void uneval_context_fix_pointer_array_access() {
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-i
[PATCH] D146450: [-Wunsafe-buffer-usage] Bug fix: Handles the assertion violations for code within macros.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9bd0db80784e: [-Wunsafe-buffer-usage] Bug fix: Handles the
assertion violations for code… (authored by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146450/new/
https://reviews.llvm.org/D146450
Files:
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp
@@ -2,6 +2,8 @@
typedef int * Int_ptr_t;
typedef int Int_t;
+#define DEFINE_PTR(X) int* ptr = (X);
+
void local_array_subscript_simple() {
int tmp;
int *p = new int[10];
@@ -222,3 +224,28 @@
tmp = r[j] + r[k]; // both `j` and `k` are unsigned so they must be non-negative
tmp = r[(unsigned int)-1]; // a cast-to-unsigned-expression is also non-negative
}
+
+void all_vars_in_macro() {
+ int* local;
+ DEFINE_PTR(local)
+ ptr[1] = 0;
+}
+
+void few_vars_in_macro() {
+ int* local;
+ DEFINE_PTR(local)
+ ptr[1] = 0;
+ int tmp;
+ ptr[2] = 30;
+ auto p = new int[10];
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{"
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}"
+ tmp = p[5];
+ int val = *p;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:14}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"[0]"
+ val = *p + 30;
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:10}:""
+ // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:"[0]"
+}
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -1117,42 +1117,44 @@
// Return the source location of the last character of the AST `Node`.
template
-static SourceLocation getEndCharLoc(const NodeTy *Node, const SourceManager &SM,
-const LangOptions &LangOpts) {
+static std::optional
+getEndCharLoc(const NodeTy *Node, const SourceManager &SM,
+ const LangOptions &LangOpts) {
unsigned TkLen = Lexer::MeasureTokenLength(Node->getEndLoc(), SM, LangOpts);
SourceLocation Loc = Node->getEndLoc().getLocWithOffset(TkLen - 1);
- // We expect `Loc` to be valid. The location is obtained by moving forward
- // from the beginning of the token 'len(token)-1' characters. The file ID of
- // the locations within a token must be consistent.
- assert(Loc.isValid() && "Expected the source location of the last"
- "character of an AST Node to be always valid");
- return Loc;
+ if (Loc.isValid())
+return Loc;
+
+ return std::nullopt;
}
// Return the source location just past the last character of the AST `Node`.
template
-static SourceLocation getPastLoc(const NodeTy *Node, const SourceManager &SM,
- const LangOptions &LangOpts) {
+static std::optional getPastLoc(const NodeTy *Node,
+const SourceManager &SM,
+const LangOptions &LangOpts) {
SourceLocation Loc =
Lexer::getLocForEndOfToken(Node->getEndLoc(), 0, SM, LangOpts);
- // We expect `Loc` to be valid as it is either associated to a file ID or
- // it must be the end of a macro expansion. (see
- // `Lexer::getLocForEndOfToken`)
- assert(Loc.isValid() && "Expected the source location just past the last "
- "character of an AST Node to be always valid");
- return Loc;
+ if (Loc.isValid())
+return Loc;
+
+ return std::nullopt;
}
// Return text representation of an `Expr`.
-static StringRef getExprText(const Expr *E, const SourceManager &SM,
- const LangOptions &LangOpts) {
- SourceLocation LastCharLoc = getPastLoc(E, SM, LangOpts);
+static std::optional getExprText(const Expr *E,
+const SourceManager &SM,
+const LangOptions &LangOpts) {
+ std::optional LastCharLoc = getPastLoc(E, SM, LangOpts);
+
+ if (LastCharLoc)
+return Lexer::getSourceText(
+CharSourceRange::getCharRange(E->getBeginLoc(), *LastCharLoc), SM,
+LangOpts);
- return Lexer::getSourceText(
- CharSourceRange::getCharRange(E->getBeginLoc(), LastCharLoc), SM,
- LangOpts);
+ return std::nullopt;
}
std::optional
@@ -1191,12 +1
[PATCH] D146773: [-Wunsafe-buffer-usage] Make raw (ungrouped) warnings a bit more verbose.
malavikasamak added inline comments.
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11786
+ "%select{"
+"unsafe operation on raw pointer in expression|"
+"unsafe arithmetic on raw pointer|"
NoQ wrote:
> The first mode doesn't show up in any tests and it's probably dead code at
> this point.
What do you think of specifying the variable name in these warnings? It could
be helpful when there are more than one DREs in a statement.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146773/new/
https://reviews.llvm.org/D146773
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D135851: [clang] Support for read-only types
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG678ded017f21: [clang] Support for read-only types (authored
by malavikasamak).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D135851?vs=481148&id=483289#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135851/new/
https://reviews.llvm.org/D135851
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/Sema/attr-read-only-placement.cpp
Index: clang/test/Sema/attr-read-only-placement.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-read-only-placement.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_cc1 -Wread-only-types %s -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++2a -Wread-only-types %s -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++17 -Wread-only-types %s -verify -fsyntax-only
+
+struct __attribute__((enforce_read_only_placement)) A { // #A_DECL
+};
+
+A a1; // expected-warning {{object of type 'A' cannot be placed in read-only memory}}
+ // expected-note@#A_DECL {{type was declared read-only here}}
+const A a2[10]; // no-warning
+A a3[20]; // expected-warning {{object of type 'A' cannot be placed in read-only memory}}
+ // expected-note@#A_DECL {{type was declared read-only here}}
+
+
+
+struct B;
+struct __attribute__((enforce_read_only_placement)) B { //#B_DECL
+};
+
+B b1; // expected-warning {{object of type 'B' cannot be placed in read-only memory}}
+ // expected-note@#B_DECL {{type was declared read-only here}}
+const B b2; // no-warning
+const B b3[4]; // no-warning
+B b4[5]; // expected-warning {{object of type 'B' cannot be placed in read-only memory}}
+ // expected-note@#B_DECL {{type was declared read-only here}}
+B b5[5][5]; // expected-warning {{object of type 'B' cannot be placed in read-only memory}}
+// expected-note@#B_DECL {{type was declared read-only here}}
+B b10[5][5][5]; // expected-warning {{object of type 'B' cannot be placed in read-only memory}}
+// expected-note@#B_DECL {{type was declared read-only here}}
+
+void method1() {
+static const B b6;
+static B b7;// expected-warning {{object of type 'B' cannot be placed in read-only memory}}
+// expected-note@#B_DECL {{type was declared read-only here}}
+B b8; // no-warning
+const B b9; // no-warning
+}
+
+struct C;
+struct __attribute__((enforce_read_only_placement)) C; // expected-note {{type was declared read-only here}}
+struct C { // no-note. The note should be attached to the definition/declaration bearing the attribute
+};
+
+C c1; // expected-warning {{object of type 'C' cannot be placed in read-only memory}}
+
+// Cases to be handled by the follow-up patches.
+
+// Attaching and checking the attribute in reverse, where the attribute is attached after the
+// type definition
+struct D;
+struct D { //expected-note{{previous definition is here}}
+};
+struct __attribute__((enforce_read_only_placement)) D; // #3
+// expected-warning@#3{{attribute declaration must precede definition}}
+
+D d1; // We do not emit a warning here, as there is another warning for declaring
+ // a type after the definition
+
+
+// Cases where the attribute must be explicitly attached to another type
+// Case 1: Inheriting from a type that has the attribute
+struct E : C { // FIXME: warn the user declarations of type `E`, that extends `C`, won't be
+ // checked for read only placement because `E` is not marked as `C` is.
+};
+
+// Case 2: Declaring a field of the type that has the attribute
+struct F {
+C c1; // FIXME: warn the user type `F` that wraps type `C` won't be checked for
+ // read only placement
+};
+
+struct BaseWithoutAttribute {
+int a;
+};
+
+struct __attribute__((enforce_read_only_placement)) J : BaseWithoutAttribute { // no-warning
+};
+
+struct __attribute__((enforce_read_only_placement)) BaseWithAttribute {
+int i;
+};
+
+struct __attribute__((enforce_read_only_placement)) Derived : BaseWithAttribute { // no-warning
+int j;
+};
+
+struct __attribute__((enforce_read_only_placement)) WrapperToAttributeInstance { // no-warning
+BaseWithAttribute b;
+};
+
+struct __attribute__((enforce_read_only_placement)) WrapperToNoAttributeInstance { // no-warning
+BaseWithoutAttribute b;
+};
+
+// Cases where the const qualification doesn't ensure read-only memory placement
+// of an instance.
+
+// Case 1: The type defines/inherits mutable data members
+struct __att
