[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/balazske closed https://github.com/llvm/llvm-project/pull/95899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/95899
From 1eb6e7ebde0e97e1cd077dc27ffd3ebd6ed0e93d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Tue, 18 Jun 2024 10:09:24 +0200
Subject: [PATCH 1/4] [clang][analyzer] Add notes to PointerSubChecker
Notes are added to indicate the array declarations of the
arrays in a found invalid pointer subtraction.
---
.../Checkers/PointerSubChecker.cpp| 45 ---
clang/test/Analysis/pointer-sub-notes.c | 34 ++
2 files changed, 64 insertions(+), 15 deletions(-)
create mode 100644 clang/test/Analysis/pointer-sub-notes.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index b73534136fdf0..a983e66df0818 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -43,8 +43,6 @@ class PointerSubChecker
bool checkArrayBounds(CheckerContext , const Expr *E,
const ElementRegion *ElemReg,
const MemRegion *Reg) const;
- void reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const;
public:
void checkPreStmt(const BinaryOperator *B, CheckerContext ) const;
@@ -57,6 +55,14 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (!ElemReg)
return true;
+ auto ReportBug = [&](const llvm::StringLiteral ) {
+if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+ auto R = std::make_unique(BT, Msg, N);
+ R->addRange(E->getSourceRange());
+ C.emitReport(std::move(R));
+}
+ };
+
ProgramStateRef State = C.getState();
const MemRegion *SuperReg = ElemReg->getSuperRegion();
SValBuilder = C.getSValBuilder();
@@ -64,7 +70,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (SuperReg == Reg) {
if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
I && (!I->isOne() && !I->isZero()))
- reportBug(C, E, Msg_BadVarIndex);
+ ReportBug(Msg_BadVarIndex);
return false;
}
@@ -77,7 +83,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge);
if (S1 && !S2) {
- reportBug(C, E, Msg_LargeArrayIndex);
+ ReportBug(Msg_LargeArrayIndex);
return false;
}
}
@@ -89,22 +95,13 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = State->assume(*IndexTooSmall);
if (S1 && !S2) {
- reportBug(C, E, Msg_NegativeArrayIndex);
+ ReportBug(Msg_NegativeArrayIndex);
return false;
}
}
return true;
}
-void PointerSubChecker::reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const {
- if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
-auto R = std::make_unique(BT, Msg, N);
-R->addRange(E->getSourceRange());
-C.emitReport(std::move(R));
- }
-}
-
void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
CheckerContext ) const {
// When doing pointer subtraction, if the two pointers do not point to the
@@ -136,6 +133,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR))
return;
+ const ValueDecl *DiffDeclL = nullptr;
+ const ValueDecl *DiffDeclR = nullptr;
+
if (ElemLR && ElemRR) {
const MemRegion *SuperLR = ElemLR->getSuperRegion();
const MemRegion *SuperRR = ElemRR->getSuperRegion();
@@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
// Allow arithmetic on different symbolic regions.
if (isa(SuperLR) || isa(SuperRR))
return;
+if (const auto *SuperDLR = dyn_cast(SuperLR))
+ DiffDeclL = SuperDLR->getDecl();
+if (const auto *SuperDRR = dyn_cast(SuperRR))
+ DiffDeclR = SuperDRR->getDecl();
}
- reportBug(C, B, Msg_MemRegionDifferent);
+ if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+auto R =
+std::make_unique(BT, Msg_MemRegionDifferent,
N);
+R->addRange(B->getSourceRange());
+if (DiffDeclL)
+ R->addNote("Array at the left-hand side of subtraction",
+ {DiffDeclL, C.getSourceManager()});
+if (DiffDeclR)
+ R->addNote("Array at the right-hand side of subtraction",
+ {DiffDeclR, C.getSourceManager()});
+C.emitReport(std::move(R));
+ }
}
void ento::registerPointerSubChecker(CheckerManager ) {
diff --git a/clang/test/Analysis/pointer-sub-notes.c
b/clang/test/Analysis/pointer-sub-notes.c
new file mode 100644
index 0..dfdace3a58deb
--- /dev/null
+++ b/clang/test/Analysis/pointer-sub-notes.c
@@ -0,0 +1,34 @@
+//
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
@@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, // Allow arithmetic on different symbolic regions. if (isa(SuperLR) || isa(SuperRR)) return; +if (const auto *SuperDLR = dyn_cast(SuperLR)) + DiffDeclL = SuperDLR->getDecl(); +if (const auto *SuperDRR = dyn_cast(SuperRR)) + DiffDeclR = SuperDRR->getDecl(); balazske wrote: Warning is now omitted if both would be at the same declaration. https://github.com/llvm/llvm-project/pull/95899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/95899
From 1eb6e7ebde0e97e1cd077dc27ffd3ebd6ed0e93d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Tue, 18 Jun 2024 10:09:24 +0200
Subject: [PATCH 1/3] [clang][analyzer] Add notes to PointerSubChecker
Notes are added to indicate the array declarations of the
arrays in a found invalid pointer subtraction.
---
.../Checkers/PointerSubChecker.cpp| 45 ---
clang/test/Analysis/pointer-sub-notes.c | 34 ++
2 files changed, 64 insertions(+), 15 deletions(-)
create mode 100644 clang/test/Analysis/pointer-sub-notes.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index b73534136fdf0..a983e66df0818 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -43,8 +43,6 @@ class PointerSubChecker
bool checkArrayBounds(CheckerContext , const Expr *E,
const ElementRegion *ElemReg,
const MemRegion *Reg) const;
- void reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const;
public:
void checkPreStmt(const BinaryOperator *B, CheckerContext ) const;
@@ -57,6 +55,14 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (!ElemReg)
return true;
+ auto ReportBug = [&](const llvm::StringLiteral ) {
+if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+ auto R = std::make_unique(BT, Msg, N);
+ R->addRange(E->getSourceRange());
+ C.emitReport(std::move(R));
+}
+ };
+
ProgramStateRef State = C.getState();
const MemRegion *SuperReg = ElemReg->getSuperRegion();
SValBuilder = C.getSValBuilder();
@@ -64,7 +70,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (SuperReg == Reg) {
if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
I && (!I->isOne() && !I->isZero()))
- reportBug(C, E, Msg_BadVarIndex);
+ ReportBug(Msg_BadVarIndex);
return false;
}
@@ -77,7 +83,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge);
if (S1 && !S2) {
- reportBug(C, E, Msg_LargeArrayIndex);
+ ReportBug(Msg_LargeArrayIndex);
return false;
}
}
@@ -89,22 +95,13 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = State->assume(*IndexTooSmall);
if (S1 && !S2) {
- reportBug(C, E, Msg_NegativeArrayIndex);
+ ReportBug(Msg_NegativeArrayIndex);
return false;
}
}
return true;
}
-void PointerSubChecker::reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const {
- if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
-auto R = std::make_unique(BT, Msg, N);
-R->addRange(E->getSourceRange());
-C.emitReport(std::move(R));
- }
-}
-
void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
CheckerContext ) const {
// When doing pointer subtraction, if the two pointers do not point to the
@@ -136,6 +133,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR))
return;
+ const ValueDecl *DiffDeclL = nullptr;
+ const ValueDecl *DiffDeclR = nullptr;
+
if (ElemLR && ElemRR) {
const MemRegion *SuperLR = ElemLR->getSuperRegion();
const MemRegion *SuperRR = ElemRR->getSuperRegion();
@@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
// Allow arithmetic on different symbolic regions.
if (isa(SuperLR) || isa(SuperRR))
return;
+if (const auto *SuperDLR = dyn_cast(SuperLR))
+ DiffDeclL = SuperDLR->getDecl();
+if (const auto *SuperDRR = dyn_cast(SuperRR))
+ DiffDeclR = SuperDRR->getDecl();
}
- reportBug(C, B, Msg_MemRegionDifferent);
+ if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+auto R =
+std::make_unique(BT, Msg_MemRegionDifferent,
N);
+R->addRange(B->getSourceRange());
+if (DiffDeclL)
+ R->addNote("Array at the left-hand side of subtraction",
+ {DiffDeclL, C.getSourceManager()});
+if (DiffDeclR)
+ R->addNote("Array at the right-hand side of subtraction",
+ {DiffDeclR, C.getSourceManager()});
+C.emitReport(std::move(R));
+ }
}
void ento::registerPointerSubChecker(CheckerManager ) {
diff --git a/clang/test/Analysis/pointer-sub-notes.c
b/clang/test/Analysis/pointer-sub-notes.c
new file mode 100644
index 0..dfdace3a58deb
--- /dev/null
+++ b/clang/test/Analysis/pointer-sub-notes.c
@@ -0,0 +1,34 @@
+//
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/95899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
@@ -32,3 +32,20 @@ void different_2() {
int d = p2 - p1; // expected-warning{{Subtraction of two pointers that do
not point into the same array is undefined behavior}} \
// expected-note{{Subtraction of two pointers that do not
point into the same array is undefined behavior}}
}
+
+int different_3() {
+ struct {
+int array[5];
+ } a, b;
+ return [3] - [2]; // expected-warning{{Subtraction of two
pointers that do not point into the same array is undefined behavior}} \
+// expected-note{{Subtraction of two
pointers that do not point into the same array is undefined behavior}}
+}
+
+int different_5() {
NagyDonat wrote:
```suggestion
int different_4() {
```
Very minor nitpick: `different_3` was followed by `different_5`.
https://github.com/llvm/llvm-project/pull/95899
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
@@ -154,12 +154,14 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
auto R =
std::make_unique(BT, Msg_MemRegionDifferent,
N);
R->addRange(B->getSourceRange());
-if (DiffDeclL)
- R->addNote("Array at the left-hand side of subtraction",
- {DiffDeclL, C.getSourceManager()});
-if (DiffDeclR)
- R->addNote("Array at the right-hand side of subtraction",
- {DiffDeclR, C.getSourceManager()});
+if (DiffDeclL != DiffDeclR) {
NagyDonat wrote:
```suggestion
if (DiffDeclL != DiffDeclR) {
// The declarations may be identical even if the regions are different,
// if they are field regions within different objects:
// struct { int array[10]; } a, b;
// do_something_with(a.array[5] - b.array[5]);
// In this case the notes would be confusing, so don't emit them.
```
Add a comment that explains this `if (DiffDeclL != DiffDeclR)` check because
otherwise it would be very surprising.
https://github.com/llvm/llvm-project/pull/95899
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/NagyDonat approved this pull request. LGTM, thanks for the update. I added two minor edit suggestions, but after that the commit can be merged. https://github.com/llvm/llvm-project/pull/95899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/95899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/balazske updated
https://github.com/llvm/llvm-project/pull/95899
From 1eb6e7ebde0e97e1cd077dc27ffd3ebd6ed0e93d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Tue, 18 Jun 2024 10:09:24 +0200
Subject: [PATCH 1/2] [clang][analyzer] Add notes to PointerSubChecker
Notes are added to indicate the array declarations of the
arrays in a found invalid pointer subtraction.
---
.../Checkers/PointerSubChecker.cpp| 45 ---
clang/test/Analysis/pointer-sub-notes.c | 34 ++
2 files changed, 64 insertions(+), 15 deletions(-)
create mode 100644 clang/test/Analysis/pointer-sub-notes.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index b73534136fdf0..a983e66df0818 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -43,8 +43,6 @@ class PointerSubChecker
bool checkArrayBounds(CheckerContext , const Expr *E,
const ElementRegion *ElemReg,
const MemRegion *Reg) const;
- void reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const;
public:
void checkPreStmt(const BinaryOperator *B, CheckerContext ) const;
@@ -57,6 +55,14 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (!ElemReg)
return true;
+ auto ReportBug = [&](const llvm::StringLiteral ) {
+if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+ auto R = std::make_unique(BT, Msg, N);
+ R->addRange(E->getSourceRange());
+ C.emitReport(std::move(R));
+}
+ };
+
ProgramStateRef State = C.getState();
const MemRegion *SuperReg = ElemReg->getSuperRegion();
SValBuilder = C.getSValBuilder();
@@ -64,7 +70,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (SuperReg == Reg) {
if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
I && (!I->isOne() && !I->isZero()))
- reportBug(C, E, Msg_BadVarIndex);
+ ReportBug(Msg_BadVarIndex);
return false;
}
@@ -77,7 +83,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge);
if (S1 && !S2) {
- reportBug(C, E, Msg_LargeArrayIndex);
+ ReportBug(Msg_LargeArrayIndex);
return false;
}
}
@@ -89,22 +95,13 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = State->assume(*IndexTooSmall);
if (S1 && !S2) {
- reportBug(C, E, Msg_NegativeArrayIndex);
+ ReportBug(Msg_NegativeArrayIndex);
return false;
}
}
return true;
}
-void PointerSubChecker::reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const {
- if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
-auto R = std::make_unique(BT, Msg, N);
-R->addRange(E->getSourceRange());
-C.emitReport(std::move(R));
- }
-}
-
void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
CheckerContext ) const {
// When doing pointer subtraction, if the two pointers do not point to the
@@ -136,6 +133,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR))
return;
+ const ValueDecl *DiffDeclL = nullptr;
+ const ValueDecl *DiffDeclR = nullptr;
+
if (ElemLR && ElemRR) {
const MemRegion *SuperLR = ElemLR->getSuperRegion();
const MemRegion *SuperRR = ElemRR->getSuperRegion();
@@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
// Allow arithmetic on different symbolic regions.
if (isa(SuperLR) || isa(SuperRR))
return;
+if (const auto *SuperDLR = dyn_cast(SuperLR))
+ DiffDeclL = SuperDLR->getDecl();
+if (const auto *SuperDRR = dyn_cast(SuperRR))
+ DiffDeclR = SuperDRR->getDecl();
}
- reportBug(C, B, Msg_MemRegionDifferent);
+ if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+auto R =
+std::make_unique(BT, Msg_MemRegionDifferent,
N);
+R->addRange(B->getSourceRange());
+if (DiffDeclL)
+ R->addNote("Array at the left-hand side of subtraction",
+ {DiffDeclL, C.getSourceManager()});
+if (DiffDeclR)
+ R->addNote("Array at the right-hand side of subtraction",
+ {DiffDeclR, C.getSourceManager()});
+C.emitReport(std::move(R));
+ }
}
void ento::registerPointerSubChecker(CheckerManager ) {
diff --git a/clang/test/Analysis/pointer-sub-notes.c
b/clang/test/Analysis/pointer-sub-notes.c
new file mode 100644
index 0..dfdace3a58deb
--- /dev/null
+++ b/clang/test/Analysis/pointer-sub-notes.c
@@ -0,0 +1,34 @@
+//
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
balazske wrote: I found difficult results from the checker where it is not obvious what the problem is. One type is this case where a negative index is found (any of these results, or check the first one): https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=curl_curl-7_66_0_pointersub=off=New=alpha.core.PointerSub This type of problem is not fixed with the patch. Other problem is when different arrays are found: https://codechecker-demo.eastus.cloudapp.azure.com/Default/report-detail?run=vim_v8.2.1920_pointersub=off=New=alpha.core.PointerSub=5494864=9a4c8e84e0c5227d61f321ec217e88ec=%2Flocal%2Fpersistent_docker%2FCSA-measurements-driver-2913%2Fmeasurements_workspace%2Fvim%2Fsrc%2Fevalvars.c This case should improve with the patch (but not tested yet). https://github.com/llvm/llvm-project/pull/95899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
@@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
// Allow arithmetic on different symbolic regions.
if (isa(SuperLR) || isa(SuperRR))
return;
+if (const auto *SuperDLR = dyn_cast(SuperLR))
+ DiffDeclL = SuperDLR->getDecl();
+if (const auto *SuperDRR = dyn_cast(SuperRR))
+ DiffDeclR = SuperDRR->getDecl();
NagyDonat wrote:
Note that `FieldRegion`s are `DeclRegion`s, so these declarations may be data
member declarations within a `struct` or `class`. This is usually not a
problem, but there is a corner case where `SuperLR != SuperRR`, but the
corresponding declarations are identical:
```
struct {
int array[5];
} a, b;
int func(void) {
return [3] - [2];
}
```
In this case the current code would place both notes onto the declaration of
`field`, which would be confusing for the user.
Consider adding some code that handles this situation explicitly. (Either
simply skip note creation when `DiffDeclL == DiffDeclR`, or create a
specialized note for this case.)
https://github.com/llvm/llvm-project/pull/95899
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/NagyDonat commented: Looks good overall, I have one remark about a rare case that would require some specialized code. https://github.com/llvm/llvm-project/pull/95899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/NagyDonat edited https://github.com/llvm/llvm-project/pull/95899 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Balázs Kéri (balazske)
Changes
Notes are added to indicate the array declarations of the arrays in a found
invalid pointer subtraction.
---
Full diff: https://github.com/llvm/llvm-project/pull/95899.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp (+30-15)
- (added) clang/test/Analysis/pointer-sub-notes.c (+34)
``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index b73534136fdf0..a983e66df0818 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -43,8 +43,6 @@ class PointerSubChecker
bool checkArrayBounds(CheckerContext , const Expr *E,
const ElementRegion *ElemReg,
const MemRegion *Reg) const;
- void reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const;
public:
void checkPreStmt(const BinaryOperator *B, CheckerContext ) const;
@@ -57,6 +55,14 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (!ElemReg)
return true;
+ auto ReportBug = [&](const llvm::StringLiteral ) {
+if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+ auto R = std::make_unique(BT, Msg, N);
+ R->addRange(E->getSourceRange());
+ C.emitReport(std::move(R));
+}
+ };
+
ProgramStateRef State = C.getState();
const MemRegion *SuperReg = ElemReg->getSuperRegion();
SValBuilder = C.getSValBuilder();
@@ -64,7 +70,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (SuperReg == Reg) {
if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
I && (!I->isOne() && !I->isZero()))
- reportBug(C, E, Msg_BadVarIndex);
+ ReportBug(Msg_BadVarIndex);
return false;
}
@@ -77,7 +83,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge);
if (S1 && !S2) {
- reportBug(C, E, Msg_LargeArrayIndex);
+ ReportBug(Msg_LargeArrayIndex);
return false;
}
}
@@ -89,22 +95,13 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = State->assume(*IndexTooSmall);
if (S1 && !S2) {
- reportBug(C, E, Msg_NegativeArrayIndex);
+ ReportBug(Msg_NegativeArrayIndex);
return false;
}
}
return true;
}
-void PointerSubChecker::reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const {
- if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
-auto R = std::make_unique(BT, Msg, N);
-R->addRange(E->getSourceRange());
-C.emitReport(std::move(R));
- }
-}
-
void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
CheckerContext ) const {
// When doing pointer subtraction, if the two pointers do not point to the
@@ -136,6 +133,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR))
return;
+ const ValueDecl *DiffDeclL = nullptr;
+ const ValueDecl *DiffDeclR = nullptr;
+
if (ElemLR && ElemRR) {
const MemRegion *SuperLR = ElemLR->getSuperRegion();
const MemRegion *SuperRR = ElemRR->getSuperRegion();
@@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
// Allow arithmetic on different symbolic regions.
if (isa(SuperLR) || isa(SuperRR))
return;
+if (const auto *SuperDLR = dyn_cast(SuperLR))
+ DiffDeclL = SuperDLR->getDecl();
+if (const auto *SuperDRR = dyn_cast(SuperRR))
+ DiffDeclR = SuperDRR->getDecl();
}
- reportBug(C, B, Msg_MemRegionDifferent);
+ if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+auto R =
+std::make_unique(BT, Msg_MemRegionDifferent,
N);
+R->addRange(B->getSourceRange());
+if (DiffDeclL)
+ R->addNote("Array at the left-hand side of subtraction",
+ {DiffDeclL, C.getSourceManager()});
+if (DiffDeclR)
+ R->addNote("Array at the right-hand side of subtraction",
+ {DiffDeclR, C.getSourceManager()});
+C.emitReport(std::move(R));
+ }
}
void ento::registerPointerSubChecker(CheckerManager ) {
diff --git a/clang/test/Analysis/pointer-sub-notes.c
b/clang/test/Analysis/pointer-sub-notes.c
new file mode 100644
index 0..dfdace3a58deb
--- /dev/null
+++ b/clang/test/Analysis/pointer-sub-notes.c
@@ -0,0 +1,34 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub
-analyzer-output=text -verify %s
+
+void negative_1() {
+ int a[3];
+ int x = -1;
+ // FIXME: should indicate that 'x' is -1
+ int d = [x] - [0]; //
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
llvmbot wrote:
@llvm/pr-subscribers-clang-static-analyzer-1
Author: Balázs Kéri (balazske)
Changes
Notes are added to indicate the array declarations of the arrays in a found
invalid pointer subtraction.
---
Full diff: https://github.com/llvm/llvm-project/pull/95899.diff
2 Files Affected:
- (modified) clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp (+30-15)
- (added) clang/test/Analysis/pointer-sub-notes.c (+34)
``diff
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index b73534136fdf0..a983e66df0818 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -43,8 +43,6 @@ class PointerSubChecker
bool checkArrayBounds(CheckerContext , const Expr *E,
const ElementRegion *ElemReg,
const MemRegion *Reg) const;
- void reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const;
public:
void checkPreStmt(const BinaryOperator *B, CheckerContext ) const;
@@ -57,6 +55,14 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (!ElemReg)
return true;
+ auto ReportBug = [&](const llvm::StringLiteral ) {
+if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+ auto R = std::make_unique(BT, Msg, N);
+ R->addRange(E->getSourceRange());
+ C.emitReport(std::move(R));
+}
+ };
+
ProgramStateRef State = C.getState();
const MemRegion *SuperReg = ElemReg->getSuperRegion();
SValBuilder = C.getSValBuilder();
@@ -64,7 +70,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (SuperReg == Reg) {
if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
I && (!I->isOne() && !I->isZero()))
- reportBug(C, E, Msg_BadVarIndex);
+ ReportBug(Msg_BadVarIndex);
return false;
}
@@ -77,7 +83,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge);
if (S1 && !S2) {
- reportBug(C, E, Msg_LargeArrayIndex);
+ ReportBug(Msg_LargeArrayIndex);
return false;
}
}
@@ -89,22 +95,13 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = State->assume(*IndexTooSmall);
if (S1 && !S2) {
- reportBug(C, E, Msg_NegativeArrayIndex);
+ ReportBug(Msg_NegativeArrayIndex);
return false;
}
}
return true;
}
-void PointerSubChecker::reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const {
- if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
-auto R = std::make_unique(BT, Msg, N);
-R->addRange(E->getSourceRange());
-C.emitReport(std::move(R));
- }
-}
-
void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
CheckerContext ) const {
// When doing pointer subtraction, if the two pointers do not point to the
@@ -136,6 +133,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR))
return;
+ const ValueDecl *DiffDeclL = nullptr;
+ const ValueDecl *DiffDeclR = nullptr;
+
if (ElemLR && ElemRR) {
const MemRegion *SuperLR = ElemLR->getSuperRegion();
const MemRegion *SuperRR = ElemRR->getSuperRegion();
@@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
// Allow arithmetic on different symbolic regions.
if (isa(SuperLR) || isa(SuperRR))
return;
+if (const auto *SuperDLR = dyn_cast(SuperLR))
+ DiffDeclL = SuperDLR->getDecl();
+if (const auto *SuperDRR = dyn_cast(SuperRR))
+ DiffDeclR = SuperDRR->getDecl();
}
- reportBug(C, B, Msg_MemRegionDifferent);
+ if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+auto R =
+std::make_unique(BT, Msg_MemRegionDifferent,
N);
+R->addRange(B->getSourceRange());
+if (DiffDeclL)
+ R->addNote("Array at the left-hand side of subtraction",
+ {DiffDeclL, C.getSourceManager()});
+if (DiffDeclR)
+ R->addNote("Array at the right-hand side of subtraction",
+ {DiffDeclR, C.getSourceManager()});
+C.emitReport(std::move(R));
+ }
}
void ento::registerPointerSubChecker(CheckerManager ) {
diff --git a/clang/test/Analysis/pointer-sub-notes.c
b/clang/test/Analysis/pointer-sub-notes.c
new file mode 100644
index 0..dfdace3a58deb
--- /dev/null
+++ b/clang/test/Analysis/pointer-sub-notes.c
@@ -0,0 +1,34 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub
-analyzer-output=text -verify %s
+
+void negative_1() {
+ int a[3];
+ int x = -1;
+ // FIXME: should indicate that 'x' is -1
+ int d = [x] - [0];
[clang] [clang][analyzer] Add notes to PointerSubChecker (PR #95899)
https://github.com/balazske created
https://github.com/llvm/llvm-project/pull/95899
Notes are added to indicate the array declarations of the arrays in a found
invalid pointer subtraction.
From 1eb6e7ebde0e97e1cd077dc27ffd3ebd6ed0e93d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?=
Date: Tue, 18 Jun 2024 10:09:24 +0200
Subject: [PATCH] [clang][analyzer] Add notes to PointerSubChecker
Notes are added to indicate the array declarations of the
arrays in a found invalid pointer subtraction.
---
.../Checkers/PointerSubChecker.cpp| 45 ---
clang/test/Analysis/pointer-sub-notes.c | 34 ++
2 files changed, 64 insertions(+), 15 deletions(-)
create mode 100644 clang/test/Analysis/pointer-sub-notes.c
diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
index b73534136fdf0..a983e66df0818 100644
--- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp
@@ -43,8 +43,6 @@ class PointerSubChecker
bool checkArrayBounds(CheckerContext , const Expr *E,
const ElementRegion *ElemReg,
const MemRegion *Reg) const;
- void reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const;
public:
void checkPreStmt(const BinaryOperator *B, CheckerContext ) const;
@@ -57,6 +55,14 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (!ElemReg)
return true;
+ auto ReportBug = [&](const llvm::StringLiteral ) {
+if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+ auto R = std::make_unique(BT, Msg, N);
+ R->addRange(E->getSourceRange());
+ C.emitReport(std::move(R));
+}
+ };
+
ProgramStateRef State = C.getState();
const MemRegion *SuperReg = ElemReg->getSuperRegion();
SValBuilder = C.getSValBuilder();
@@ -64,7 +70,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
if (SuperReg == Reg) {
if (const llvm::APSInt *I = SVB.getKnownValue(State, ElemReg->getIndex());
I && (!I->isOne() && !I->isZero()))
- reportBug(C, E, Msg_BadVarIndex);
+ ReportBug(Msg_BadVarIndex);
return false;
}
@@ -77,7 +83,7 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = C.getState()->assume(*IndexTooLarge);
if (S1 && !S2) {
- reportBug(C, E, Msg_LargeArrayIndex);
+ ReportBug(Msg_LargeArrayIndex);
return false;
}
}
@@ -89,22 +95,13 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext ,
const Expr *E,
ProgramStateRef S1, S2;
std::tie(S1, S2) = State->assume(*IndexTooSmall);
if (S1 && !S2) {
- reportBug(C, E, Msg_NegativeArrayIndex);
+ ReportBug(Msg_NegativeArrayIndex);
return false;
}
}
return true;
}
-void PointerSubChecker::reportBug(CheckerContext , const Expr *E,
- const llvm::StringLiteral ) const {
- if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
-auto R = std::make_unique(BT, Msg, N);
-R->addRange(E->getSourceRange());
-C.emitReport(std::move(R));
- }
-}
-
void PointerSubChecker::checkPreStmt(const BinaryOperator *B,
CheckerContext ) const {
// When doing pointer subtraction, if the two pointers do not point to the
@@ -136,6 +133,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
if (!checkArrayBounds(C, B->getRHS(), ElemRR, LR))
return;
+ const ValueDecl *DiffDeclL = nullptr;
+ const ValueDecl *DiffDeclR = nullptr;
+
if (ElemLR && ElemRR) {
const MemRegion *SuperLR = ElemLR->getSuperRegion();
const MemRegion *SuperRR = ElemRR->getSuperRegion();
@@ -144,9 +144,24 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator
*B,
// Allow arithmetic on different symbolic regions.
if (isa(SuperLR) || isa(SuperRR))
return;
+if (const auto *SuperDLR = dyn_cast(SuperLR))
+ DiffDeclL = SuperDLR->getDecl();
+if (const auto *SuperDRR = dyn_cast(SuperRR))
+ DiffDeclR = SuperDRR->getDecl();
}
- reportBug(C, B, Msg_MemRegionDifferent);
+ if (ExplodedNode *N = C.generateNonFatalErrorNode()) {
+auto R =
+std::make_unique(BT, Msg_MemRegionDifferent,
N);
+R->addRange(B->getSourceRange());
+if (DiffDeclL)
+ R->addNote("Array at the left-hand side of subtraction",
+ {DiffDeclL, C.getSourceManager()});
+if (DiffDeclR)
+ R->addNote("Array at the right-hand side of subtraction",
+ {DiffDeclR, C.getSourceManager()});
+C.emitReport(std::move(R));
+ }
}
void ento::registerPointerSubChecker(CheckerManager ) {
diff --git a/clang/test/Analysis/pointer-sub-notes.c
b/clang/test/Analysis/pointer-sub-notes.c
new file mode 100644
index
