[PATCH] D36672: [clang-tidy] readability-non-const-parameter: fixit on all function declarations

2017-09-08 Thread Anders Rönnholm via Phabricator via cfe-commits 
AndersRonnholm updated this revision to Diff 114346.
AndersRonnholm added a comment.
Herald added subscribers: xazax.hun, JDevlieghere.

Fixed comments


Repository:
  rL LLVM

https://reviews.llvm.org/D36672

Files:
  clang-tidy/readability/NonConstParameterCheck.cpp
  test/clang-tidy/readability-non-const-parameter.cpp


Index: test/clang-tidy/readability-non-const-parameter.cpp
===
--- test/clang-tidy/readability-non-const-parameter.cpp
+++ test/clang-tidy/readability-non-const-parameter.cpp
@@ -277,3 +277,26 @@
 int x = *p;
   }
 };
+
+int declarationFixit(int *i);
+// CHECK-FIXES: {{^}}int declarationFixit(const int *i);{{$}}
+int declarationFixit(int *i);
+// CHECK-FIXES: {{^}}int declarationFixit(const int *i);{{$}}
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'i' can be
+int declarationFixit(int *i) {
+  // CHECK-FIXES: {{^}}int declarationFixit(const int *i) {{{$}}
+  return *i;
+}
+
+
+class D {
+private:
+  int declarationFixit(int *i);
+  // CHECK-FIXES: {{^}}  int declarationFixit(const int *i);{{$}}
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:30: warning: pointer parameter 'i' can be
+int D::declarationFixit(int *i) {
+  // CHECK-FIXES: {{^}}int declarationFixit(const int *i) {{{$}}
+  return *i;
+}
Index: clang-tidy/readability/NonConstParameterCheck.cpp
===
--- clang-tidy/readability/NonConstParameterCheck.cpp
+++ clang-tidy/readability/NonConstParameterCheck.cpp
@@ -138,9 +138,20 @@
 if (!ParamInfo.CanBeConst)
   continue;
 
-diag(Par->getLocation(), "pointer parameter '%0' can be pointer to const")
-<< Par->getName()
-<< FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+auto D = diag(Par->getLocation(),
+  "pointer parameter '%0' can be pointer to const")
+ << Par->getName()
+ << FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+
+const DeclContext *Parent = Par->getParentFunctionOrMethod();
+const auto *FD = dyn_cast(Parent);
+while (FD) {
+  const auto ParDecl = FD->getParamDecl(Par->getFunctionScopeIndex());
+  if (Par != ParDecl)
+D << ParDecl->getName()
+  << FixItHint::CreateInsertion(ParDecl->getLocStart(), "const ");
+  FD = FD->getPreviousDecl();
+}
   }
 }
 


Index: test/clang-tidy/readability-non-const-parameter.cpp
===
--- test/clang-tidy/readability-non-const-parameter.cpp
+++ test/clang-tidy/readability-non-const-parameter.cpp
@@ -277,3 +277,26 @@
 int x = *p;
   }
 };
+
+int declarationFixit(int *i);
+// CHECK-FIXES: {{^}}int declarationFixit(const int *i);{{$}}
+int declarationFixit(int *i);
+// CHECK-FIXES: {{^}}int declarationFixit(const int *i);{{$}}
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'i' can be
+int declarationFixit(int *i) {
+  // CHECK-FIXES: {{^}}int declarationFixit(const int *i) {{{$}}
+  return *i;
+}
+
+
+class D {
+private:
+  int declarationFixit(int *i);
+  // CHECK-FIXES: {{^}}  int declarationFixit(const int *i);{{$}}
+};
+
+// CHECK-MESSAGES: :[[@LINE+1]]:30: warning: pointer parameter 'i' can be
+int D::declarationFixit(int *i) {
+  // CHECK-FIXES: {{^}}int declarationFixit(const int *i) {{{$}}
+  return *i;
+}
Index: clang-tidy/readability/NonConstParameterCheck.cpp
===
--- clang-tidy/readability/NonConstParameterCheck.cpp
+++ clang-tidy/readability/NonConstParameterCheck.cpp
@@ -138,9 +138,20 @@
 if (!ParamInfo.CanBeConst)
   continue;
 
-diag(Par->getLocation(), "pointer parameter '%0' can be pointer to const")
-<< Par->getName()
-<< FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+auto D = diag(Par->getLocation(),
+  "pointer parameter '%0' can be pointer to const")
+ << Par->getName()
+ << FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+
+const DeclContext *Parent = Par->getParentFunctionOrMethod();
+const auto *FD = dyn_cast(Parent);
+while (FD) {
+  const auto ParDecl = FD->getParamDecl(Par->getFunctionScopeIndex());
+  if (Par != ParDecl)
+D << ParDecl->getName()
+  << FixItHint::CreateInsertion(ParDecl->getLocStart(), "const ");
+  FD = FD->getPreviousDecl();
+}
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36672: [clang-tidy] readability-non-const-parameter: fixit on all function declarations

2017-09-08 Thread Anders Rönnholm via Phabricator via cfe-commits 
AndersRonnholm marked 2 inline comments as done.
AndersRonnholm added inline comments.



Comment at: clang-tidy/readability/NonConstParameterCheck.cpp:147
+if (const auto *Parent = Par->getParentFunctionOrMethod()) {
+  if (const auto *F = dyn_cast(Parent)) {
+const auto ParDecl =

aaron.ballman wrote:
> What if the parent is an `ObjCMethodDecl` instead?
I don't think this checker handles objective-c


Repository:
  rL LLVM

https://reviews.llvm.org/D36672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36672: [clang-tidy] readability-non-const-parameter: fixit on all function declarations

2017-12-06 Thread Anders Rönnholm via Phabricator via cfe-commits 
AndersRonnholm abandoned this revision.
AndersRonnholm added a comment.

Fixed by https://reviews.llvm.org/rL319021. At least for c/c++ not sure if it 
handles objective-c.


Repository:
  rL LLVM

https://reviews.llvm.org/D36672



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36670: misc-misplaced-widening-cast: fix assertion

2017-08-14 Thread Anders Rönnholm via Phabricator via cfe-commits 
AndersRonnholm created this revision.
AndersRonnholm added a project: clang-tools-extra.

Fixes assert reported in https://bugs.llvm.org/show_bug.cgi?id=33660


Repository:
  rL LLVM

https://reviews.llvm.org/D36670

Files:
  clang-tidy/misc/MisplacedWideningCastCheck.cpp
  test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp


Index: test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
===
--- test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
+++ test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
@@ -56,3 +56,9 @@
 return (long)a * 1000;
   }
 }
+
+// Shall not generate an assert. https://bugs.llvm.org/show_bug.cgi?id=33660
+template  class A {
+  enum Type {};
+  static char *m_fn1() { char p = (Type)(&p - m_fn1()); }
+};
Index: clang-tidy/misc/MisplacedWideningCastCheck.cpp
===
--- clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -192,6 +192,10 @@
   if (Calc->getLocStart().isMacroID())
 return;
 
+  if (Cast->isTypeDependent() || Cast->isValueDependent() ||
+  Calc->isTypeDependent() || Calc->isValueDependent())
+return;
+
   ASTContext &Context = *Result.Context;
 
   QualType CastType = Cast->getType();


Index: test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
===
--- test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
+++ test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
@@ -56,3 +56,9 @@
 return (long)a * 1000;
   }
 }
+
+// Shall not generate an assert. https://bugs.llvm.org/show_bug.cgi?id=33660
+template  class A {
+  enum Type {};
+  static char *m_fn1() { char p = (Type)(&p - m_fn1()); }
+};
Index: clang-tidy/misc/MisplacedWideningCastCheck.cpp
===
--- clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -192,6 +192,10 @@
   if (Calc->getLocStart().isMacroID())
 return;
 
+  if (Cast->isTypeDependent() || Cast->isValueDependent() ||
+  Calc->isTypeDependent() || Calc->isValueDependent())
+return;
+
   ASTContext &Context = *Result.Context;
 
   QualType CastType = Cast->getType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36672: readability-non-const-parameter: fixit on all function declarations

2017-08-14 Thread Anders Rönnholm via Phabricator via cfe-commits 
AndersRonnholm created this revision.

Fixes issue reported in https://bugs.llvm.org/show_bug.cgi?id=33219


Repository:
  rL LLVM

https://reviews.llvm.org/D36672

Files:
  clang-tidy/readability/NonConstParameterCheck.cpp
  test/clang-tidy/readability-non-const-parameter.cpp


Index: test/clang-tidy/readability-non-const-parameter.cpp
===
--- test/clang-tidy/readability-non-const-parameter.cpp
+++ test/clang-tidy/readability-non-const-parameter.cpp
@@ -277,3 +277,11 @@
 int x = *p;
   }
 };
+
+int declarationFixit(int *i);
+// CHECK-FIXES: {{^}}int declarationFixit(const int *i);{{$}}
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'i' can be
+int declarationFixit(int *i) {
+  // CHECK-FIXES: {{^}}int declarationFixit(const int *i) {{{$}}
+  return *i;
+}
Index: clang-tidy/readability/NonConstParameterCheck.cpp
===
--- clang-tidy/readability/NonConstParameterCheck.cpp
+++ clang-tidy/readability/NonConstParameterCheck.cpp
@@ -138,9 +138,20 @@
 if (!ParamInfo.CanBeConst)
   continue;
 
-diag(Par->getLocation(), "pointer parameter '%0' can be pointer to const")
-<< Par->getName()
-<< FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+auto D = diag(Par->getLocation(),
+  "pointer parameter '%0' can be pointer to const")
+ << Par->getName()
+ << FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+
+if (const auto *Parent = Par->getParentFunctionOrMethod()) {
+  if (const auto *F = dyn_cast(Parent)) {
+const auto ParDecl =
+F->getFirstDecl()->getParamDecl(Par->getFunctionScopeIndex());
+if (Par != ParDecl)
+  D << ParDecl->getName()
+<< FixItHint::CreateInsertion(ParDecl->getLocStart(), "const ");
+  }
+}
   }
 }
 


Index: test/clang-tidy/readability-non-const-parameter.cpp
===
--- test/clang-tidy/readability-non-const-parameter.cpp
+++ test/clang-tidy/readability-non-const-parameter.cpp
@@ -277,3 +277,11 @@
 int x = *p;
   }
 };
+
+int declarationFixit(int *i);
+// CHECK-FIXES: {{^}}int declarationFixit(const int *i);{{$}}
+// CHECK-MESSAGES: :[[@LINE+1]]:27: warning: pointer parameter 'i' can be
+int declarationFixit(int *i) {
+  // CHECK-FIXES: {{^}}int declarationFixit(const int *i) {{{$}}
+  return *i;
+}
Index: clang-tidy/readability/NonConstParameterCheck.cpp
===
--- clang-tidy/readability/NonConstParameterCheck.cpp
+++ clang-tidy/readability/NonConstParameterCheck.cpp
@@ -138,9 +138,20 @@
 if (!ParamInfo.CanBeConst)
   continue;
 
-diag(Par->getLocation(), "pointer parameter '%0' can be pointer to const")
-<< Par->getName()
-<< FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+auto D = diag(Par->getLocation(),
+  "pointer parameter '%0' can be pointer to const")
+ << Par->getName()
+ << FixItHint::CreateInsertion(Par->getLocStart(), "const ");
+
+if (const auto *Parent = Par->getParentFunctionOrMethod()) {
+  if (const auto *F = dyn_cast(Parent)) {
+const auto ParDecl =
+F->getFirstDecl()->getParamDecl(Par->getFunctionScopeIndex());
+if (Par != ParDecl)
+  D << ParDecl->getName()
+<< FixItHint::CreateInsertion(ParDecl->getLocStart(), "const ");
+  }
+}
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31650: [Analyzer] Detect when function pointer is freed

2017-04-04 Thread Anders Rönnholm via Phabricator via cfe-commits 
AndersRonnholm created this revision.

The MallocChecker does not currently detect freeing of function pointers.

Example code.

  void (*fnptr)(int);
  void freeIndirectFunctionPtr() {
void *p = (void*)fnptr;
free(p); // expected-warning {{Argument to free() points to a function 
pointer}}
  }


Repository:
  rL LLVM

https://reviews.llvm.org/D31650

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/malloc.c


Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1774,6 +1774,16 @@
return ok; // no warning
 }
 
+void (*fnptr)(int);
+void freeIndirectFunctionPtr() {
+  void *p = (void*)fnptr;
+  free(p); // expected-warning {{Argument to free() points to a function 
pointer}}
+}
+
+void freeFunctionPtr() {
+  free((void*)fnptr); // expected-warning {{Argument to free() is a function 
pointer}}
+}
+
 // 
 // False negatives.
 
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -392,6 +392,10 @@
   void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range,
   SymbolRef Sym) const;
 
+  void ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
+ SourceRange Range, const Expr *DeallocExpr,
+ const Expr *ArgExpr) const;
+
   /// Find the location of the allocation for Sym on the path leading to the
   /// exploded node N.
   LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
@@ -1516,6 +1520,12 @@
 }
   }
 
+  if (SymBase->getType()->isFunctionPointerType()) {
+ReportFunctionPointerFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr,
+  ArgExpr);
+return nullptr;
+  }
+
   ReleasedAllocated = (RsBase != nullptr) && (RsBase->isAllocated() ||
   RsBase->isAllocatedOfSizeZero());
 
@@ -1976,6 +1986,47 @@
   }
 }
 
+void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
+  SourceRange Range,
+  const Expr *DeallocExpr,
+  const Expr *ArgExpr) const {
+
+  if (!ChecksEnabled[CK_MallocChecker])
+return;
+
+  Optional CheckKind =
+  getCheckIfTracked(C, DeallocExpr);
+  if (!CheckKind.hasValue())
+return;
+
+  if (ExplodedNode *N = C.generateErrorNode()) {
+if (!BT_BadFree[*CheckKind])
+  BT_BadFree[*CheckKind].reset(
+  new BugType(CheckNames[*CheckKind], "Bad free", "Memory Error"));
+
+SmallString<100> Buf;
+llvm::raw_svector_ostream Os(Buf);
+
+const MemRegion *MR = ArgVal.getAsRegion();
+while (const ElementRegion *ER = dyn_cast_or_null(MR))
+  MR = ER->getSuperRegion();
+
+Os << "Argument to ";
+if (!printAllocDeallocName(Os, C, DeallocExpr))
+  Os << "deallocator";
+
+if (ArgExpr->IgnoreParenCasts()->getType()->isFunctionPointerType())
+  Os << " is a function pointer";
+else
+  Os << " points to a function pointer";
+
+auto R = llvm::make_unique(*BT_BadFree[*CheckKind], Os.str(), 
N);
+R->markInteresting(MR);
+R->addRange(Range);
+C.emitReport(std::move(R));
+  }
+}
+
 ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C,
   const CallExpr *CE,
   bool FreesOnFail,


Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1774,6 +1774,16 @@
return ok; // no warning
 }
 
+void (*fnptr)(int);
+void freeIndirectFunctionPtr() {
+  void *p = (void*)fnptr;
+  free(p); // expected-warning {{Argument to free() points to a function pointer}}
+}
+
+void freeFunctionPtr() {
+  free((void*)fnptr); // expected-warning {{Argument to free() is a function pointer}}
+}
+
 // 
 // False negatives.
 
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -392,6 +392,10 @@
   void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range,
   SymbolRef Sym) const;
 
+  void ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
+ SourceRange Range, const Expr *DeallocExpr,
+ const Expr *ArgExpr) const;
+
   /// Find the location of the allocation for Sym on the path leading to the
   /// e

[PATCH] D31650: [Analyzer] Detect when function pointer is freed

2017-04-20 Thread Anders Rönnholm via Phabricator via cfe-commits 
AndersRonnholm updated this revision to Diff 95929.
AndersRonnholm added a comment.

Updated after comments


Repository:
  rL LLVM

https://reviews.llvm.org/D31650

Files:
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  test/Analysis/malloc.c


Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1774,6 +1774,16 @@
return ok; // no warning
 }
 
+void (*fnptr)(int);
+void freeIndirectFunctionPtr() {
+  void *p = (void *)fnptr;
+  free(p); // expected-warning {{Argument to free() is a function pointer}}
+}
+
+void freeFunctionPtr() {
+  free((void *)fnptr); // expected-warning {{Argument to free() is a function 
pointer}}
+}
+
 // 
 // False negatives.
 
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -392,6 +392,9 @@
   void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range,
   SymbolRef Sym) const;
 
+  void ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
+ SourceRange Range, const Expr *FreeExpr) 
const;
+
   /// Find the location of the allocation for Sym on the path leading to the
   /// exploded node N.
   LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
@@ -1516,6 +1519,11 @@
 }
   }
 
+  if (SymBase->getType()->isFunctionPointerType()) {
+ReportFunctionPointerFree(C, ArgVal, ArgExpr->getSourceRange(), 
ParentExpr);
+return nullptr;
+  }
+
   ReleasedAllocated = (RsBase != nullptr) && (RsBase->isAllocated() ||
   RsBase->isAllocatedOfSizeZero());
 
@@ -1976,6 +1984,42 @@
   }
 }
 
+void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
+  SourceRange Range,
+  const Expr *FreeExpr) const {
+
+  if (!ChecksEnabled[CK_MallocChecker])
+return;
+
+  Optional CheckKind = getCheckIfTracked(C, 
FreeExpr);
+  if (!CheckKind.hasValue())
+return;
+
+  if (ExplodedNode *N = C.generateErrorNode()) {
+if (!BT_BadFree[*CheckKind])
+  BT_BadFree[*CheckKind].reset(
+  new BugType(CheckNames[*CheckKind], "Bad free", "Memory Error"));
+
+SmallString<100> Buf;
+llvm::raw_svector_ostream Os(Buf);
+
+const MemRegion *MR = ArgVal.getAsRegion();
+while (const ElementRegion *ER = dyn_cast_or_null(MR))
+  MR = ER->getSuperRegion();
+
+Os << "Argument to ";
+if (!printAllocDeallocName(Os, C, FreeExpr))
+  Os << "deallocator";
+
+Os << " is a function pointer";
+
+auto R = llvm::make_unique(*BT_BadFree[*CheckKind], Os.str(), 
N);
+R->markInteresting(MR);
+R->addRange(Range);
+C.emitReport(std::move(R));
+  }
+}
+
 ProgramStateRef MallocChecker::ReallocMem(CheckerContext &C,
   const CallExpr *CE,
   bool FreesOnFail,


Index: test/Analysis/malloc.c
===
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1774,6 +1774,16 @@
return ok; // no warning
 }
 
+void (*fnptr)(int);
+void freeIndirectFunctionPtr() {
+  void *p = (void *)fnptr;
+  free(p); // expected-warning {{Argument to free() is a function pointer}}
+}
+
+void freeFunctionPtr() {
+  free((void *)fnptr); // expected-warning {{Argument to free() is a function pointer}}
+}
+
 // 
 // False negatives.
 
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -392,6 +392,9 @@
   void ReportUseZeroAllocated(CheckerContext &C, SourceRange Range,
   SymbolRef Sym) const;
 
+  void ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
+ SourceRange Range, const Expr *FreeExpr) const;
+
   /// Find the location of the allocation for Sym on the path leading to the
   /// exploded node N.
   LeakInfo getAllocationSite(const ExplodedNode *N, SymbolRef Sym,
@@ -1516,6 +1519,11 @@
 }
   }
 
+  if (SymBase->getType()->isFunctionPointerType()) {
+ReportFunctionPointerFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr);
+return nullptr;
+  }
+
   ReleasedAllocated = (RsBase != nullptr) && (RsBase->isAllocated() ||
   RsBase->isAllocatedOfSizeZero());
 
@@ -1976,6 +1984,42 @@
   }
 }
 
+void MallocChecker::ReportFunctionPointerFree(CheckerContext &C, SVal ArgVal,
+