Hi Jordan,
Thank you for your comments. I have changed the check to use
RecursiveASTVisitor. However i'm not sure what i should write as category in
bugreport. You mentioned adding a new category in CommonBugCategories for
SIzeofOnExpression, should i do that or is what i have enough for now?
I have provided a new diff.
//Anders
From: Jordan Rose [mailto:[email protected]]
Sent: den 25 september 2013 04:01
To: Anders Rönnholm
Cc: [email protected]
Subject: Re: [PATCH] [StaticAnalyzer] New checker StringPlusChar
I was sort of right: we have a warning -Wstring-plus-int for adding an integer
(or character literal) to a string literal...but nothing for variables.
Nevertheless, this is a syntactic check rather than a path-sensitive one, so
I'd recommend writing it using checkASTCodeBody instead. You can take a look at
CheckSizeofPointer.cpp to see how this works, though I'd recommend using the
slightly-more-powerful RecursiveASTVisitor (overriding VisitBinaryOperator)
rather than building your own recursion with StmtVisitor.
Jordan
On Sep 19, 2013, at 9:23 , Jordan Rose
<[email protected]<mailto:[email protected]>> wrote:
> I could have sworn we had a check or warning for this already, but I don't
> see it. Hopefully I'll get a chance to review this soon; thanks, Anders!
>
> Jordan
>
>
> On Sep 19, 2013, at 7:46 , Anders Rönnholm
> <[email protected]<mailto:[email protected]>> wrote:
>
>> Hi,
>>
>> This is a new check i want to get reviewed. It checks for char literal
>> additions on strings.
>>
>> Example:
>> char* str;
>> str = str + 'c';
>>
>> This is a suspicious pointer arithmetic, probably string addition was
>> intended.
>>
>> Thanks,
>> Anders
>>
>> .......................................................................................................................
>> Anders Rönnholm Senior Engineer
>> Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden
>>
>> Mobile: +46 (0)70 912 42 54
>> E-mail:
>> [email protected]<mailto:[email protected]>
>>
>> www.evidente.se<http://www.evidente.se>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> [email protected]<mailto:[email protected]>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
Index: lib/StaticAnalyzer/Checkers/StringPlusCharChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/StringPlusCharChecker.cpp (revision 0)
+++ lib/StaticAnalyzer/Checkers/StringPlusCharChecker.cpp (working copy)
@@ -0,0 +1,89 @@
+//== StringPlusCharChecker.cpp - string plus char checker ------------------==//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+///
+/// \file
+/// \brief This defines StringPlusCharChecker, a check that
+/// checks additions of chars to string pointers.
+///
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class StringPlusCharVisitor
+ : public RecursiveASTVisitor<StringPlusCharVisitor> {
+public:
+ explicit StringPlusCharVisitor(BugReporter &br, AnalysisDeclContext *ac)
+ : BR(br), AC(ac) {}
+
+ bool VisitBinaryOperator(BinaryOperator *B) {
+ // Only check add operators.
+ if (B->getOpcode() != BO_Add)
+ return false;
+
+ const DeclRefExpr *StringRefExpr =
+ dyn_cast<DeclRefExpr>(B->getLHS()->IgnoreImpCasts());
+ const CharacterLiteral *CharExpr =
+ dyn_cast<CharacterLiteral>(B->getRHS()->IgnoreImpCasts());
+ if (!CharExpr || !StringRefExpr)
+ return false;
+
+ // Return if not a PointerType.
+ if (!StringRefExpr->getType()->isAnyPointerType())
+ return false;
+
+ // Only report if it is a string or string ptr.
+ const ASTContext &Context = BR.getContext();
+ if (!(StringRefExpr->getType() ==
+ Context.getPointerType(Context.getConstType(Context.CharTy)) ||
+ StringRefExpr->getType() == Context.getPointerType(Context.CharTy)))
+ return false;
+
+ PathDiagnosticLocation ELoc =
+ PathDiagnosticLocation::createBegin(B, BR.getSourceManager(), AC);
+ BR.EmitBasicReport(AC->getDecl(), "Unusual pointer arithmetic",
+ "String plus char",
+ "Unusual pointer arithmetic. A character literal is "
+ "added to a value of type char*",
+ ELoc);
+ return false;
+ }
+
+private:
+ BugReporter &BR;
+ AnalysisDeclContext *AC;
+};
+}
+
+namespace {
+class StringPlusCharChecker : public Checker<check::ASTCodeBody> {
+public:
+ void checkASTCodeBody(const Decl *D, AnalysisManager &mgr,
+ BugReporter &BR) const;
+};
+} // end anonymous namespace
+
+void StringPlusCharChecker::checkASTCodeBody(const Decl *D,
+ AnalysisManager &mgr,
+ BugReporter &BR) const {
+
+ StringPlusCharVisitor Visitor(BR, mgr.getAnalysisDeclContext(D));
+ Visitor.TraverseDecl(const_cast<Decl *>(D));
+}
+
+void ento::registerStringPlusCharChecker(CheckerManager &mgr) {
+ mgr.registerChecker<StringPlusCharChecker>();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt (revision 191652)
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt (working copy)
@@ -60,6 +61,7 @@
SimpleStreamChecker.cpp
StackAddrEscapeChecker.cpp
StreamChecker.cpp
+ StringPlusCharChecker.cpp
TaintTesterChecker.cpp
TraversalChecker.cpp
UndefBranchChecker.cpp
Index: lib/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- lib/StaticAnalyzer/Checkers/Checkers.td (revision 191652)
+++ lib/StaticAnalyzer/Checkers/Checkers.td (working copy)
@@ -116,6 +116,10 @@
HelpText<"Warn about unintended use of sizeof() on pointer expressions">,
DescFile<"CheckSizeofPointer.cpp">;
+def StringPlusCharChecker : Checker<"StringPlusChar">,
+ HelpText<"Warn about string plus char">,
+ DescFile<"StringPlusCharChecker.cpp">;
+
} // end "alpha.core"
//===----------------------------------------------------------------------===//
Index: test/Analysis/string-plus-char-checks.c
===================================================================
--- test/Analysis/string-plus-char-checks.c (revision 0)
+++ test/Analysis/string-plus-char-checks.c (working copy)
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.core.StringPlusChar -verify %s
+
+void checkStringPointerPlusChar() {
+ char* str;
+ char *str2 = str + 'c'; // expected-warning {{Unusual pointer arithmetic. A character literal is added to a value of type char*}}
+}
+
+void checkConstStringPointerPlusChar() {
+ const char* str;
+ const char *str2 = str + 'c'; // expected-warning {{Unusual pointer arithmetic. A character literal is added to a value of type char*}}
+}
+
+void checkCharPlusStringPointer() {
+ char* str = "";
+ str = 'c' + str;
+} // no-warning
+
+void checkIntPointerPlusChar() {
+ int* i = 0;
+ i = i + 'c';
+} // no-warning
Index: test/Analysis/string-plus-char-checks.cpp
===================================================================
--- test/Analysis/string-plus-char-checks.cpp (revision 0)
+++ test/Analysis/string-plus-char-checks.cpp (working copy)
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.core.StringPlusChar -verify %s
+class A {
+public:
+ A(): str() { }
+ A(const char *p) { }
+ A(char *p) : str(p + 'a') { } // expected-warning {{Unusual pointer arithmetic. A character literal is added to a value of type char*}}
+ A& operator+(const char *p) { return *this; }
+ A& operator+(char ch) { return *this; }
+ char * str;
+};
+
+void checkStringPointerPlusChar(const char *str) {
+ A a = str + 'a'; // expected-warning {{Unusual pointer arithmetic. A character literal is added to a value of type char*}}
+}
+
+void checkClassOperatorPlusChar(const char *str) {
+ A a;
+ a = a + str + 'b';
+} // no-warning
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
