Charusso updated this revision to Diff 232203.
Charusso edited the summary of this revision.
Charusso added a comment.
- Add more tests.
- Let us specify the descendant as allowing equality of symbols.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70805/new/
https://reviews.llvm.org/D70805
Files:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h
clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
clang/test/Analysis/cert/str31-c-fp-suppression.cpp
clang/test/Analysis/sval-has-desc.c
Index: clang/test/Analysis/sval-has-desc.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/sval-has-desc.c
@@ -0,0 +1,23 @@
+// RUN: %clang_analyze_cc1 \
+// RUN: -analyzer-checker=core,unix,debug.ExprInspection \
+// RUN: -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+void clang_analyzer_hasDescendantTrue(size_t, const char *);
+void clang_analyzer_hasDescendantFalse(const char *, size_t);
+void clang_analyzer_hasDescendantEquals(const char *, const char *);
+
+void test_desc(const char *src) {
+ size_t size = strlen(src);
+ clang_analyzer_hasDescendantTrue(size, src); // expected-warning {{TRUE}}
+}
+
+void test_non_desc(const char *src) {
+ size_t size = strlen(src);
+ clang_analyzer_hasDescendantFalse(src, size); // expected-warning {{FALSE}}
+}
+
+void test_equality_is_desc(const char *src) {
+ clang_analyzer_hasDescendantEquals(src, src); // expected-warning {{TRUE}}
+}
Index: clang/test/Analysis/cert/str31-c-fp-suppression.cpp
===================================================================
--- clang/test/Analysis/cert/str31-c-fp-suppression.cpp
+++ clang/test/Analysis/cert/str31-c-fp-suppression.cpp
@@ -71,7 +71,6 @@
free(dest); // no-warning
}
-// FIXME: Suppress that with 'SValVisitor' or something similar.
void test_complex_size_false_positive(const char *src) {
char *dest;
size_t size;
@@ -80,8 +79,7 @@
dest = (char *)malloc(size * 2 + 2);
strcpy(dest, &src[13]);
- free(dest);
- // expected-warning@-1 {{'dest' is not null-terminated}}
+ free(dest); // no-warning
}
void test_complex_size_wrong_size(const char *src) {
@@ -93,8 +91,7 @@
dest = (char *)malloc(size * 2);
strcpy(dest, &src[13]);
- free(dest);
- // expected-warning@-1 {{'dest' is not null-terminated}}
+ free(dest); // no-warning
}
void test_char_by_char(const char *src, size_t size) {
Index: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
@@ -27,6 +27,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h"
#include "llvm/ADT/Optional.h"
#include <utility>
@@ -218,17 +219,11 @@
DefinedOrUnknownSVal SrcSize = getDynamicSize(State, SrcMR, SVB);
DefinedOrUnknownSVal DestSize = getDynamicSize(State, DestMR, SVB);
- // 'strlen(src) + integer' is most likely fine.
- // FIXME: Use the 'SValVisitor' to catch every such constructs of the symbol.
// FIXME: We cannot catch every '+ integer' part at the moment so we do not
// check that property for now.
- if (const SymExpr *SE = DestSize.getAsSymExpr())
- for (const auto &Sym : SE->symbols())
- if (const auto *SIE = dyn_cast<SymIntExpr>(Sym))
- if (SIE->getOpcode() == BO_Add)
- if (const auto *SM = dyn_cast<SymbolMetadata>(SIE->getLHS()))
- if (SM->getRegion() == SrcMR)
- return;
+ SValHasDescendant HasDescendantSrc(SrcMR);
+ if (HasDescendantSrc.Visit(DestSize))
+ return;
// 'StringRegion' returns the size with the null-terminator.
if (const llvm::APSInt *SrcSizeInt = SVB.getKnownValue(State, SrcSize))
Index: clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
@@ -14,6 +14,7 @@
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicSize.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/ScopedPrinter.h"
@@ -41,6 +42,7 @@
void analyzerWarnOnDeadSymbol(const CallExpr *CE, CheckerContext &C) const;
void analyzerDump(const CallExpr *CE, CheckerContext &C) const;
void analyzerExplain(const CallExpr *CE, CheckerContext &C) const;
+ void analyzerHasDescendant(const CallExpr *CE, CheckerContext &C) const;
void analyzerPrintState(const CallExpr *CE, CheckerContext &C) const;
void analyzerGetExtent(const CallExpr *CE, CheckerContext &C) const;
void analyzerHashDump(const CallExpr *CE, CheckerContext &C) const;
@@ -83,6 +85,7 @@
.Case("clang_analyzer_warnOnDeadSymbol",
&ExprInspectionChecker::analyzerWarnOnDeadSymbol)
.StartsWith("clang_analyzer_explain", &ExprInspectionChecker::analyzerExplain)
+ .StartsWith("clang_analyzer_hasDescendant", &ExprInspectionChecker::analyzerHasDescendant)
.StartsWith("clang_analyzer_dump", &ExprInspectionChecker::analyzerDump)
.Case("clang_analyzer_getExtent", &ExprInspectionChecker::analyzerGetExtent)
.Case("clang_analyzer_printState",
@@ -206,6 +209,21 @@
reportBug(Ex.Visit(V), C);
}
+void ExprInspectionChecker::analyzerHasDescendant(const CallExpr *CE,
+ CheckerContext &C) const {
+
+ if (CE->getNumArgs() < 2) {
+ reportBug("Missing argument for descending", C);
+ return;
+ }
+
+ SVal LhsV = C.getSVal(CE->getArg(0));
+ SVal RhsV = C.getSVal(CE->getArg(1));
+ SValHasDescendant Desc(RhsV);
+
+ reportBug(Desc.Visit(LhsV) ? "TRUE" : "FALSE", C);
+}
+
void ExprInspectionChecker::analyzerDump(const CallExpr *CE,
CheckerContext &C) const {
if (CE->getNumArgs() == 0) {
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h
===================================================================
--- /dev/null
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h
@@ -0,0 +1,108 @@
+//===- SValHasDescendant.h - Symbolic value traversing ----------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines SValHasDescendant which returns whether the given
+// symbol or region has a descendant symbol.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SVALHASDESCENDANT_H
+#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SVALHASDESCENDANT_H
+
+#include "clang/AST/DeclCXX.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/SValVisitor.h"
+
+namespace clang {
+namespace ento {
+
+class SValHasDescendant : public FullSValVisitor<SValHasDescendant, bool> {
+ Optional<SVal> StoredV;
+ const MemRegion *StoredMR;
+
+public:
+ SValHasDescendant(SVal V) : StoredV(V) {}
+ SValHasDescendant(const MemRegion *MR) : StoredMR(MR) { assert(MR); }
+
+ bool VisitLocMemRegionVal(loc::MemRegionVal V) {
+ if (StoredV)
+ if (const auto MRV = StoredV->getAs<loc::MemRegionVal>())
+ if (V == MRV)
+ return true;
+
+ return Visit(V.getRegion());
+ }
+
+ bool VisitNonLocSymbolVal(nonloc::SymbolVal V) {
+ if (StoredV && *StoredV == V)
+ return true;
+
+ return Visit(V.getSymbol());
+ }
+
+ bool VisitNonLocLazyCompoundVal(nonloc::LazyCompoundVal V) {
+ if (StoredV && *StoredV == V)
+ return true;
+
+ return Visit(V.getRegion());
+ }
+
+ bool VisitSymbolRegionValue(const SymbolRegionValue *S) {
+ return Visit(S->getRegion());
+ }
+
+ bool VisitSymbolExtent(const SymbolExtent *S) {
+ return Visit(S->getRegion());
+ }
+
+ bool VisitSymbolMetadata(const SymbolMetadata *S) {
+ return Visit(S->getRegion());
+ }
+
+ bool VisitSymIntExpr(const SymIntExpr *S) { return Visit(S->getLHS()); }
+
+ // TODO: IntSymExpr doesn't appear in practice.
+ // Add the relevant code once it does.
+
+ bool VisitSymSymExpr(const SymSymExpr *S) {
+ if (Visit(S->getLHS()))
+ return true;
+
+ return Visit(S->getRHS());
+ }
+
+ // TODO: SymbolCast doesn't appear in practice.
+ // Add the relevant code once it does.
+
+ bool VisitSymbolicRegion(const SymbolicRegion *SR) {
+ if (SR == StoredMR)
+ return true;
+
+ if (StoredV)
+ if (auto MRV = StoredV->getAs<loc::MemRegionVal>())
+ if (MRV->getRegion() == SR)
+ return true;
+
+ return Visit(SR->getSymbol());
+ }
+
+ bool VisitMemRegion(const MemRegion *MR) {
+ if (StoredV)
+ if (auto MRV = StoredV->getAs<loc::MemRegionVal>())
+ if (MRV->getRegion() == MR)
+ return true;
+
+ return StoredMR && StoredMR == MR;
+ }
+
+ bool VisitSVal(SVal V) { return StoredV && *StoredV == V; }
+};
+
+} // end namespace ento
+} // end namespace clang
+
+#endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SVALHASDESCENDANT_H
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
