https://github.com/balazske updated https://github.com/llvm/llvm-project/pull/96501
From b431151f83fa2980e4a132191ccf5713ab69806b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Mon, 24 Jun 2024 16:48:54 +0200 Subject: [PATCH 1/2] [clang][analyzer] Improve PointerSubChecker The checker could report false positives if pointer arithmetic was done on pointers to non-array data before pointer subtraction. Another problem is fixed that could cause false positive if members of the same structure but in different memory objects are subtracted. --- .../Checkers/PointerSubChecker.cpp | 22 ++++++++++++-- clang/test/Analysis/pointer-sub.c | 29 +++++++++++++------ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index eea93a41f1384..63ed4df67d6d5 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -49,12 +49,28 @@ class PointerSubChecker }; } +static bool isArrayVar(const MemRegion *R) { + while (R) { + if (isa<VarRegion>(R)) + return true; + if (const auto *ER = dyn_cast<ElementRegion>(R)) + R = ER->getSuperRegion(); + else + return false; + } + return false; +} + bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E, const ElementRegion *ElemReg, const MemRegion *Reg) const { if (!ElemReg) return true; + const MemRegion *SuperReg = ElemReg->getSuperRegion(); + if (!isArrayVar(SuperReg)) + return true; + auto ReportBug = [&](const llvm::StringLiteral &Msg) { if (ExplodedNode *N = C.generateNonFatalErrorNode()) { auto R = std::make_unique<PathSensitiveBugReport>(BT, Msg, N); @@ -64,7 +80,6 @@ bool PointerSubChecker::checkArrayBounds(CheckerContext &C, const Expr *E, }; ProgramStateRef State = C.getState(); - const MemRegion *SuperReg = ElemReg->getSuperRegion(); SValBuilder &SVB = C.getSValBuilder(); if (SuperReg == Reg) { @@ -121,8 +136,9 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, if (LR == RR) return; - // No warning if one operand is unknown. - if (isa<SymbolicRegion>(LR) || isa<SymbolicRegion>(RR)) + // No warning if one operand is unknown or resides in a region that could be + // equal to the other. + if (LR->getSymbolicBase() || RR->getSymbolicBase()) return; const auto *ElemLR = dyn_cast<ElementRegion>(LR); diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c index 88e6dec2d172f..404b8530b89c0 100644 --- a/clang/test/Analysis/pointer-sub.c +++ b/clang/test/Analysis/pointer-sub.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.core.PointerSub -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core.PointerSub -analyzer-output=text-minimal -verify %s void f1(void) { int x, y, z[10]; @@ -73,15 +73,15 @@ void f4(void) { d = a[2] - a[1]; // expected-warning{{Subtraction of two pointers that}} } -typedef struct { +struct S { int a; int b; int c[10]; // expected-note2{{Array at the right-hand side of subtraction}} int d[10]; // expected-note2{{Array at the left-hand side of subtraction}} -} S; +}; void f5(void) { - S s; + struct S s; int y; int d; @@ -92,18 +92,18 @@ void f5(void) { d = &s.d[3] - &s.c[2]; // expected-warning{{Subtraction of two pointers that}} d = s.d - s.c; // expected-warning{{Subtraction of two pointers that}} - S sa[10]; + struct S sa[10]; d = &sa[2] - &sa[1]; d = &sa[2].a - &sa[1].b; // expected-warning{{Subtraction of two pointers that}} } void f6(void) { - long long l; + long long l = 2; char *a1 = (char *)&l; int d = a1[3] - l; - long long la1[3]; // expected-note{{Array at the right-hand side of subtraction}} - long long la2[3]; // expected-note{{Array at the left-hand side of subtraction}} + long long la1[3] = {1}; // expected-note{{Array at the right-hand side of subtraction}} + long long la2[3] = {1}; // expected-note{{Array at the left-hand side of subtraction}} char *pla1 = (char *)la1; char *pla2 = (char *)la2; d = pla1[1] - pla1[0]; @@ -117,6 +117,17 @@ void f7(int *p) { } void f8(int n) { - int a[10]; + int a[10] = {1}; int d = a[n] - a[0]; } + +int f9(const char *p1) { + const char *p2 = p1; + --p1; + ++p2; + return p1 - p2; // no-warning +} + +int f10(struct S *p1, struct S *p2) { + return &p1->c[5] - &p2->c[5]; // no-warning +} From ce80876f7364fba57de3cf5377d9f3673d6744b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20K=C3=A9ri?= <balazs.k...@ericsson.com> Date: Thu, 27 Jun 2024 10:26:20 +0200 Subject: [PATCH 2/2] improved note messages --- .../Checkers/PointerSubChecker.cpp | 17 ++++++++----- clang/test/Analysis/pointer-sub.c | 25 ++++++++++++++++++- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp index 63ed4df67d6d5..6025a3e810f41 100644 --- a/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/PointerSubChecker.cpp @@ -19,6 +19,7 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicExtent.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/FormatVariadic.h" using namespace clang; using namespace ento; @@ -175,12 +176,16 @@ void PointerSubChecker::checkPreStmt(const BinaryOperator *B, // do_something(&a.array[5] - &b.array[5]); // In this case don't emit notes. if (DiffDeclL != DiffDeclR) { - 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()}); + auto AddNote = [&R, &C](const ValueDecl *D, StringRef SideStr) { + if (D) { + std::string Msg = llvm::formatv( + "{0} at the {1}-hand side of subtraction", + D->getType()->isArrayType() ? "Array" : "Object", SideStr); + R->addNote(Msg, {D, C.getSourceManager()}); + } + }; + AddNote(DiffDeclL, "left"); + AddNote(DiffDeclR, "right"); } C.emitReport(std::move(R)); } diff --git a/clang/test/Analysis/pointer-sub.c b/clang/test/Analysis/pointer-sub.c index 404b8530b89c0..194c891889952 100644 --- a/clang/test/Analysis/pointer-sub.c +++ b/clang/test/Analysis/pointer-sub.c @@ -118,7 +118,7 @@ void f7(int *p) { void f8(int n) { int a[10] = {1}; - int d = a[n] - a[0]; + int d = a[n] - a[0]; // no-warning } int f9(const char *p1) { @@ -131,3 +131,26 @@ int f9(const char *p1) { int f10(struct S *p1, struct S *p2) { return &p1->c[5] - &p2->c[5]; // no-warning } + +struct S1 { + int a; + int b; // expected-note{{Object at the right-hand side of subtraction}} +}; + +int f11() { + struct S1 s; // expected-note{{Object at the left-hand side of subtraction}} + return (char *)&s - (char *)&s.b; // expected-warning{{Subtraction of two pointers that}} +} + +struct S2 { + char *p1; + char *p2; +}; + +void init_S2(struct S2 *); + +int f12() { + struct S2 s; + init_S2(&s); + return s.p1 - s.p2; // no-warning (pointers are unknown) +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits