[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

@steakhal should the release notes page be updated to add this?

I think this could be beneficial for the users to be aware of that change.

If yes, who is responsible for doing that? Developers? (me) or someone else is 
taking care of reviewing the list of changes since latest releases?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

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

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Thanks @steakhal for the proposal :) but I pushed it myself: 
https://github.com/llvm/llvm-project/commit/ce97312d109b21acb97d3ea243e214f20bd87cfc

I used to have svn access, and Chris kindly give me write access to the git 
repository.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

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

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce97312d109b: Implement BufferOverlap check for 
sprint/snprintf (authored by ArnaudBienner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c

Index: clang/test/Analysis/buffer-overlap.c
===
--- /dev/null
+++ clang/test/Analysis/buffer-overlap.c
@@ -0,0 +1,98 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+
+// This provides us with four possible sprintf() definitions.
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+typedef typeof(sizeof(int)) size_t;
+
+#ifdef VARIANT
+
+#define __sprintf_chk BUILTIN(__sprintf_chk)
+#define __snprintf_chk BUILTIN(__snprintf_chk)
+int __sprintf_chk (char * __restrict str, int flag, size_t os,
+const char * __restrict fmt, ...);
+int __snprintf_chk (char * __restrict str, size_t len, int flag, size_t os,
+const char * __restrict fmt, ...);
+
+#define sprintf(str, ...) __sprintf_chk(str, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+#define snprintf(str, len, ...) __snprintf_chk(str, len, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+
+#else /* VARIANT */
+
+#define sprintf BUILTIN(sprintf)
+int sprintf(char *restrict buffer, const char *restrict format, ... );
+int snprintf(char *restrict buffer, size_t bufsz,
+ const char *restrict format, ... );
+#endif /* VARIANT */
+
+void test_sprintf1() {
+  char a[4] = {0};
+  sprintf(a, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf4() {
+  char a[4] = {0};
+  sprintf(a, "%d", 42); // no-warning
+}
+
+void test_sprintf5() {
+  char a[4] = {0};
+  char b[4] = {0};
+  sprintf(a, "%s", b); // no-warning
+}
+
+void test_snprintf1() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf2() {
+  char a[4] = {0};
+  snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf5() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d", 42); // no-warning
+}
+
+void test_snprintf6() {
+  char a[4] = {0};
+  char b[4] = {0};
+  snprintf(a, sizeof(a), "%s", b); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -175,6 +176,8 @@
std::bind(::evalMemcmp, _1, _2, _3, CK_Regular)},
   {{CDF_MaybeBuiltin, {"bzero"}, 2}, ::evalBzero},
   {{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, ::evalBzero},
+  {{CDF_MaybeBuiltin, {"sprintf"}, 2}, ::evalSprintf},
+  {{CDF_MaybeBuiltin, {"snprintf"}, 2}, ::evalSnprintf},
   };
 
   // These require a bit of special handling.
@@ -228,6 +231,11 @@
   void evalMemset(CheckerContext , const CallExpr *CE) const;
   void evalBzero(CheckerContext , const CallExpr *CE) const;
 
+  void evalSprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSnprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSprintfCommon(CheckerContext , const CallExpr *CE, bool IsBounded,
+ bool IsBuiltin) const;
+
   // Utility methods
   std::pair
   static 

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 526743.
ArnaudBienner added a comment.

- clang-format fix


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c

Index: clang/test/Analysis/buffer-overlap.c
===
--- /dev/null
+++ clang/test/Analysis/buffer-overlap.c
@@ -0,0 +1,98 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+
+// This provides us with four possible sprintf() definitions.
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+typedef typeof(sizeof(int)) size_t;
+
+#ifdef VARIANT
+
+#define __sprintf_chk BUILTIN(__sprintf_chk)
+#define __snprintf_chk BUILTIN(__snprintf_chk)
+int __sprintf_chk (char * __restrict str, int flag, size_t os,
+const char * __restrict fmt, ...);
+int __snprintf_chk (char * __restrict str, size_t len, int flag, size_t os,
+const char * __restrict fmt, ...);
+
+#define sprintf(str, ...) __sprintf_chk(str, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+#define snprintf(str, len, ...) __snprintf_chk(str, len, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+
+#else /* VARIANT */
+
+#define sprintf BUILTIN(sprintf)
+int sprintf(char *restrict buffer, const char *restrict format, ... );
+int snprintf(char *restrict buffer, size_t bufsz,
+ const char *restrict format, ... );
+#endif /* VARIANT */
+
+void test_sprintf1() {
+  char a[4] = {0};
+  sprintf(a, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf4() {
+  char a[4] = {0};
+  sprintf(a, "%d", 42); // no-warning
+}
+
+void test_sprintf5() {
+  char a[4] = {0};
+  char b[4] = {0};
+  sprintf(a, "%s", b); // no-warning
+}
+
+void test_snprintf1() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf2() {
+  char a[4] = {0};
+  snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf5() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d", 42); // no-warning
+}
+
+void test_snprintf6() {
+  char a[4] = {0};
+  char b[4] = {0};
+  snprintf(a, sizeof(a), "%s", b); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -175,6 +176,8 @@
std::bind(::evalMemcmp, _1, _2, _3, CK_Regular)},
   {{CDF_MaybeBuiltin, {"bzero"}, 2}, ::evalBzero},
   {{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, ::evalBzero},
+  {{CDF_MaybeBuiltin, {"sprintf"}, 2}, ::evalSprintf},
+  {{CDF_MaybeBuiltin, {"snprintf"}, 2}, ::evalSnprintf},
   };
 
   // These require a bit of special handling.
@@ -228,6 +231,11 @@
   void evalMemset(CheckerContext , const CallExpr *CE) const;
   void evalBzero(CheckerContext , const CallExpr *CE) const;
 
+  void evalSprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSnprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSprintfCommon(CheckerContext , const CallExpr *CE, bool IsBounded,
+ bool IsBuiltin) const;
+
   // Utility methods
   std::pair
   static assumeZero(CheckerContext ,
@@ -2352,6 +2360,51 @@
   C.addTransition(State);
 }
 
+void 

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 526463.
ArnaudBienner added a comment.

- Code review comments: change back the string buffer check + run clang format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c

Index: clang/test/Analysis/buffer-overlap.c
===
--- /dev/null
+++ clang/test/Analysis/buffer-overlap.c
@@ -0,0 +1,98 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+
+// This provides us with four possible sprintf() definitions.
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+typedef typeof(sizeof(int)) size_t;
+
+#ifdef VARIANT
+
+#define __sprintf_chk BUILTIN(__sprintf_chk)
+#define __snprintf_chk BUILTIN(__snprintf_chk)
+int __sprintf_chk (char * __restrict str, int flag, size_t os,
+const char * __restrict fmt, ...);
+int __snprintf_chk (char * __restrict str, size_t len, int flag, size_t os,
+const char * __restrict fmt, ...);
+
+#define sprintf(str, ...) __sprintf_chk(str, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+#define snprintf(str, len, ...) __snprintf_chk(str, len, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+
+#else /* VARIANT */
+
+#define sprintf BUILTIN(sprintf)
+int sprintf(char *restrict buffer, const char *restrict format, ... );
+int snprintf(char *restrict buffer, size_t bufsz,
+ const char *restrict format, ... );
+#endif /* VARIANT */
+
+void test_sprintf1() {
+  char a[4] = {0};
+  sprintf(a, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf4() {
+  char a[4] = {0};
+  sprintf(a, "%d", 42); // no-warning
+}
+
+void test_sprintf5() {
+  char a[4] = {0};
+  char b[4] = {0};
+  sprintf(a, "%s", b); // no-warning
+}
+
+void test_snprintf1() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf2() {
+  char a[4] = {0};
+  snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf5() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d", 42); // no-warning
+}
+
+void test_snprintf6() {
+  char a[4] = {0};
+  char b[4] = {0};
+  snprintf(a, sizeof(a), "%s", b); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -175,6 +176,8 @@
std::bind(::evalMemcmp, _1, _2, _3, CK_Regular)},
   {{CDF_MaybeBuiltin, {"bzero"}, 2}, ::evalBzero},
   {{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, ::evalBzero},
+  {{CDF_MaybeBuiltin, {"sprintf"}, 2}, ::evalSprintf},
+  {{CDF_MaybeBuiltin, {"snprintf"}, 2}, ::evalSnprintf},
   };
 
   // These require a bit of special handling.
@@ -228,6 +231,11 @@
   void evalMemset(CheckerContext , const CallExpr *CE) const;
   void evalBzero(CheckerContext , const CallExpr *CE) const;
 
+  void evalSprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSnprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSprintfCommon(CheckerContext , const CallExpr *CE, bool IsBounded,
+ bool IsBuiltin) const;
+
   // Utility methods
   std::pair
   static assumeZero(CheckerContext ,

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

@steakhal did you get a chance to look at my comment?

I would really love to see this merged upstream if you think this could be a 
beneficial change :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

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

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Giving this a second thought, I feel like the initial check:

  if (!ArgExpr->getType()->isAnyPointerType() ||
  !ArgExpr->getType()->getPointeeType()->isAnyCharacterType())

is better than the new one. To me it reads like "expr type is a pointer and it 
points to character type" which is more understandable IMHO.

If you're worried about the expression being a bit long, I could move type to a 
temp variable:

  if (const QualType type = ArgExpr->getType();
  !type->isAnyPointerType() ||
  !type->getPointeeType()->isAnyCharacterType())

Though I'm not sure this is really more readable.

What do you think?

Any other suggestion/comment about this patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

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

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Thanks for the review :)

I implemented the suggested changes.

Just one question: `PointeeTy.isNull()`: is this guaranteed to always work even 
though `getType()->isAnyPointerType() == false`?

I'm assuming yes since the tests still pass, but wanted to confirm this is the 
best way to do this check for `char*` arguments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

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

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 522738.
ArnaudBienner added a comment.

- Code review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c

Index: clang/test/Analysis/buffer-overlap.c
===
--- /dev/null
+++ clang/test/Analysis/buffer-overlap.c
@@ -0,0 +1,98 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+
+// This provides us with four possible sprintf() definitions.
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+typedef typeof(sizeof(int)) size_t;
+
+#ifdef VARIANT
+
+#define __sprintf_chk BUILTIN(__sprintf_chk)
+#define __snprintf_chk BUILTIN(__snprintf_chk)
+int __sprintf_chk (char * __restrict str, int flag, size_t os,
+const char * __restrict fmt, ...);
+int __snprintf_chk (char * __restrict str, size_t len, int flag, size_t os,
+const char * __restrict fmt, ...);
+
+#define sprintf(str, ...) __sprintf_chk(str, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+#define snprintf(str, len, ...) __snprintf_chk(str, len, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+
+#else /* VARIANT */
+
+#define sprintf BUILTIN(sprintf)
+int sprintf(char *restrict buffer, const char *restrict format, ... );
+int snprintf(char *restrict buffer, size_t bufsz,
+ const char *restrict format, ... );
+#endif /* VARIANT */
+
+void test_sprintf1() {
+  char a[4] = {0};
+  sprintf(a, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_sprintf4() {
+  char a[4] = {0};
+  sprintf(a, "%d", 42); // no-warning
+}
+
+void test_sprintf5() {
+  char a[4] = {0};
+  char b[4] = {0};
+  sprintf(a, "%s", b); // no-warning
+}
+
+void test_snprintf1() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf2() {
+  char a[4] = {0};
+  snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{Arguments must not be overlapping buffers}}
+}
+
+void test_snprintf5() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d", 42); // no-warning
+}
+
+void test_snprintf6() {
+  char a[4] = {0};
+  char b[4] = {0};
+  snprintf(a, sizeof(a), "%s", b); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -175,6 +176,8 @@
std::bind(::evalMemcmp, _1, _2, _3, CK_Regular)},
   {{CDF_MaybeBuiltin, {"bzero"}, 2}, ::evalBzero},
   {{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, ::evalBzero},
+  {{CDF_MaybeBuiltin, {"sprintf"}, 2}, ::evalSprintf},
+  {{CDF_MaybeBuiltin, {"snprintf"}, 2}, ::evalSnprintf},
   };
 
   // These require a bit of special handling.
@@ -228,6 +231,11 @@
   void evalMemset(CheckerContext , const CallExpr *CE) const;
   void evalBzero(CheckerContext , const CallExpr *CE) const;
 
+  void evalSprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSnprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSprintfCommon(CheckerContext , const CallExpr *CE, bool IsBounded,
+ bool IsBuiltin) const;
+
   // Utility methods
   std::pair
   static assumeZero(CheckerContext ,
@@ -2352,6 +2360,50 @@
   C.addTransition(State);
 }
 

[PATCH] D150531: Fix start index for sprintf ovlerap check + tests

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner abandoned this revision.
ArnaudBienner added a comment.

Sorry about creating a new patch instead of updating the existing one.
Closing that one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150531/new/

https://reviews.llvm.org/D150531

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

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 522020.
ArnaudBienner added a comment.

Updating D150430 : Implement BufferOverlap 
check for sprint/snprintf


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c

Index: clang/test/Analysis/buffer-overlap.c
===
--- /dev/null
+++ clang/test/Analysis/buffer-overlap.c
@@ -0,0 +1,76 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+
+// This provides us with four possible sprintf() definitions.
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+typedef typeof(sizeof(int)) size_t;
+
+#ifdef VARIANT
+
+#define __sprintf_chk BUILTIN(__sprintf_chk)
+#define __snprintf_chk BUILTIN(__snprintf_chk)
+int __sprintf_chk (char * __restrict str, int flag, size_t os,
+const char * __restrict fmt, ...);
+int __snprintf_chk (char * __restrict str, size_t len, int flag, size_t os,
+const char * __restrict fmt, ...);
+
+#define sprintf(str, ...) __sprintf_chk(str, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+#define snprintf(str, len, ...) __snprintf_chk(str, len, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+
+#else /* VARIANT */
+
+#define sprintf BUILTIN(sprintf)
+int sprintf(char *restrict buffer, const char *restrict format, ... );
+int snprintf(char *restrict buffer, size_t bufsz,
+ const char *restrict format, ... );
+#endif /* VARIANT */
+
+void test_sprintf1() {
+  char a[4] = {0};
+  sprintf(a, "%d/%s", 1, a); // expected-warning{{overlapping}}
+}
+
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf1() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf2() {
+  char a[4] = {0};
+  snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{overlapping}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -175,6 +176,8 @@
std::bind(::evalMemcmp, _1, _2, _3, CK_Regular)},
   {{CDF_MaybeBuiltin, {"bzero"}, 2}, ::evalBzero},
   {{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, ::evalBzero},
+  {{CDF_MaybeBuiltin, {"sprintf"}}, ::evalSprintf},
+  {{CDF_MaybeBuiltin, {"snprintf"}}, ::evalSnprintf},
   };
 
   // These require a bit of special handling.
@@ -228,6 +231,11 @@
   void evalMemset(CheckerContext , const CallExpr *CE) const;
   void evalBzero(CheckerContext , const CallExpr *CE) const;
 
+  void evalSprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSnprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSprintfCommon(CheckerContext , const CallExpr *CE, bool IsBounded,
+ bool IsBuiltin) const;
+
   // Utility methods
   std::pair
   static assumeZero(CheckerContext ,
@@ -2352,6 +2360,61 @@
   C.addTransition(State);
 }
 
+void CStringChecker::evalSprintf(CheckerContext , const CallExpr *CE) const {
+  CurrentFunctionDescription = "'sprintf'";
+  bool IsBI = CE->getBuiltinCallee() == Builtin::BI__builtin___sprintf_chk;
+  evalSprintfCommon(C, CE, /* IsBounded */ false, IsBI);
+}
+
+void CStringChecker::evalSnprintf(CheckerContext , const CallExpr *CE) const {
+  CurrentFunctionDescription = "'snprintf'";
+  bool IsBI = CE->getBuiltinCallee() == Builtin::BI__builtin___snprintf_chk;
+  evalSprintfCommon(C, CE, /* IsBounded */ true, IsBI);
+}
+
+void 

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 522019.
ArnaudBienner added a comment.

Updating D150430 : Implement BufferOverlap 
check for sprint/snprintf


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c


Index: clang/test/Analysis/buffer-overlap.c
===
--- clang/test/Analysis/buffer-overlap.c
+++ clang/test/Analysis/buffer-overlap.c
@@ -45,6 +45,16 @@
   sprintf(a, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
 
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{overlapping}}
+}
+
 void test_snprintf1() {
   char a[4] = {0};
   snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{overlapping}}
@@ -54,3 +64,13 @@
   char a[4] = {0};
   snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{overlapping}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2380,9 +2380,9 @@
   // Check that the source and destination do not overlap.
   // Iterate over CE->getNumArgs(), skipping all parameters which are not 
format
   // arguments
-  // For sprintf case, it starts at index 3:
+  // For sprintf case, it starts at position 3/index 2:
   // sprintf(char *buffer, const char* format, ... /* format arguments */);
-  unsigned int format_arguments_start_idx = 3;
+  unsigned int format_arguments_start_idx = 2;
   // snprintf case: one extra extra arguments for size
   // int snprintf(char *buffer, size_t bufsz, const char *format,
   //  ... /* format arguments */);


Index: clang/test/Analysis/buffer-overlap.c
===
--- clang/test/Analysis/buffer-overlap.c
+++ clang/test/Analysis/buffer-overlap.c
@@ -45,6 +45,16 @@
   sprintf(a, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
 
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{overlapping}}
+}
+
 void test_snprintf1() {
   char a[4] = {0};
   snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{overlapping}}
@@ -54,3 +64,13 @@
   char a[4] = {0};
   snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{overlapping}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2380,9 +2380,9 @@
   // Check that the source and destination do not overlap.
   // Iterate over CE->getNumArgs(), skipping all parameters which are not format
   // arguments
-  // For sprintf case, it starts at index 3:
+  // For sprintf case, it starts at position 3/index 2:
   // sprintf(char *buffer, const char* format, ... /* format arguments */);
-  unsigned int format_arguments_start_idx = 3;
+  unsigned int format_arguments_start_idx = 2;
   // snprintf case: one extra extra arguments for size
   // int snprintf(char *buffer, size_t bufsz, const char *format,
   //  ... /* format arguments */);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 522018.
ArnaudBienner added a comment.

Fix start index for sprintf ovlerap check + tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c


Index: clang/test/Analysis/buffer-overlap.c
===
--- clang/test/Analysis/buffer-overlap.c
+++ clang/test/Analysis/buffer-overlap.c
@@ -45,6 +45,16 @@
   sprintf(a, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
 
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{overlapping}}
+}
+
 void test_snprintf1() {
   char a[4] = {0};
   snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{overlapping}}
@@ -54,3 +64,13 @@
   char a[4] = {0};
   snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{overlapping}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2380,9 +2380,9 @@
   // Check that the source and destination do not overlap.
   // Iterate over CE->getNumArgs(), skipping all parameters which are not 
format
   // arguments
-  // For sprintf case, it starts at index 3:
+  // For sprintf case, it starts at position 3/index 2:
   // sprintf(char *buffer, const char* format, ... /* format arguments */);
-  unsigned int format_arguments_start_idx = 3;
+  unsigned int format_arguments_start_idx = 2;
   // snprintf case: one extra extra arguments for size
   // int snprintf(char *buffer, size_t bufsz, const char *format,
   //  ... /* format arguments */);


Index: clang/test/Analysis/buffer-overlap.c
===
--- clang/test/Analysis/buffer-overlap.c
+++ clang/test/Analysis/buffer-overlap.c
@@ -45,6 +45,16 @@
   sprintf(a, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
 
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{overlapping}}
+}
+
 void test_snprintf1() {
   char a[4] = {0};
   snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{overlapping}}
@@ -54,3 +64,13 @@
   char a[4] = {0};
   snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{overlapping}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2380,9 +2380,9 @@
   // Check that the source and destination do not overlap.
   // Iterate over CE->getNumArgs(), skipping all parameters which are not format
   // arguments
-  // For sprintf case, it starts at index 3:
+  // For sprintf case, it starts at position 3/index 2:
   // sprintf(char *buffer, const char* format, ... /* format arguments */);
-  unsigned int format_arguments_start_idx = 3;
+  unsigned int format_arguments_start_idx = 2;
   // snprintf case: one extra extra arguments for size
   // int snprintf(char *buffer, size_t bufsz, const char *format,
   //  ... /* format arguments */);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150531: Fix start index for sprintf ovlerap check + tests

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner created this revision.
Herald added subscribers: steakhal, martong, arphaman.
Herald added a reviewer: NoQ.
Herald added a project: All.
ArnaudBienner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150531

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c


Index: clang/test/Analysis/buffer-overlap.c
===
--- clang/test/Analysis/buffer-overlap.c
+++ clang/test/Analysis/buffer-overlap.c
@@ -45,6 +45,16 @@
   sprintf(a, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
 
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{overlapping}}
+}
+
 void test_snprintf1() {
   char a[4] = {0};
   snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{overlapping}}
@@ -54,3 +64,13 @@
   char a[4] = {0};
   snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{overlapping}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2380,9 +2380,9 @@
   // Check that the source and destination do not overlap.
   // Iterate over CE->getNumArgs(), skipping all parameters which are not 
format
   // arguments
-  // For sprintf case, it starts at index 3:
+  // For sprintf case, it starts at position 3/index 2:
   // sprintf(char *buffer, const char* format, ... /* format arguments */);
-  unsigned int format_arguments_start_idx = 3;
+  unsigned int format_arguments_start_idx = 2;
   // snprintf case: one extra extra arguments for size
   // int snprintf(char *buffer, size_t bufsz, const char *format,
   //  ... /* format arguments */);


Index: clang/test/Analysis/buffer-overlap.c
===
--- clang/test/Analysis/buffer-overlap.c
+++ clang/test/Analysis/buffer-overlap.c
@@ -45,6 +45,16 @@
   sprintf(a, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
 
+void test_sprintf2() {
+  char a[4] = {0};
+  sprintf(a, "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_sprintf3() {
+  char a[4] = {0};
+  sprintf(a, "%s/%s", a, a); // expected-warning{{overlapping}}
+}
+
 void test_snprintf1() {
   char a[4] = {0};
   snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{overlapping}}
@@ -54,3 +64,13 @@
   char a[4] = {0};
   snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{overlapping}}
 }
+
+void test_snprintf3() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s", a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf4() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%s/%s", a, a); // expected-warning{{overlapping}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2380,9 +2380,9 @@
   // Check that the source and destination do not overlap.
   // Iterate over CE->getNumArgs(), skipping all parameters which are not format
   // arguments
-  // For sprintf case, it starts at index 3:
+  // For sprintf case, it starts at position 3/index 2:
   // sprintf(char *buffer, const char* format, ... /* format arguments */);
-  unsigned int format_arguments_start_idx = 3;
+  unsigned int format_arguments_start_idx = 2;
   // snprintf case: one extra extra arguments for size
   // int snprintf(char *buffer, size_t bufsz, const char *format,
   //  ... /* format arguments */);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2385
+  // sprintf(char *buffer, const char* format, ... /* format arguments */);
+  unsigned int format_arguments_start_idx = 3;
+  // snprintf case: one extra extra arguments for size

Just realized this should be 2 (position 3, but index 2).

Will upload a newer version of the patch with this change, and new tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150430/new/

https://reviews.llvm.org/D150430

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

[PATCH] D150430: Implement BufferOverlap check for sprint/snprintf

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner created this revision.
Herald added subscribers: steakhal, martong.
Herald added a reviewer: NoQ.
Herald added a project: All.
ArnaudBienner edited the summary of this revision.
ArnaudBienner added a subscriber: dergachev.a.
ArnaudBienner published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I discussed about this with you @dergachev.a a long time ago on the mailing 
list ("static or dynamic code analysis for undefined behavior in sprintf") and 
finally took the time to implement something :)

Let me know what you think about this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150430

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/buffer-overlap.c

Index: clang/test/Analysis/buffer-overlap.c
===
--- /dev/null
+++ clang/test/Analysis/buffer-overlap.c
@@ -0,0 +1,56 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+//
+// RUN: %clang_analyze_cc1 -verify %s -DVARIANT -DUSE_BUILTINS \
+// RUN:   -analyzer-checker=alpha.unix.cstring.BufferOverlap
+
+// This provides us with four possible sprintf() definitions.
+
+#ifdef USE_BUILTINS
+#define BUILTIN(f) __builtin_##f
+#else /* USE_BUILTINS */
+#define BUILTIN(f) f
+#endif /* USE_BUILTINS */
+
+typedef typeof(sizeof(int)) size_t;
+
+#ifdef VARIANT
+
+#define __sprintf_chk BUILTIN(__sprintf_chk)
+#define __snprintf_chk BUILTIN(__snprintf_chk)
+int __sprintf_chk (char * __restrict str, int flag, size_t os,
+const char * __restrict fmt, ...);
+int __snprintf_chk (char * __restrict str, size_t len, int flag, size_t os,
+const char * __restrict fmt, ...);
+
+#define sprintf(str, ...) __sprintf_chk(str, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+#define snprintf(str, len, ...) __snprintf_chk(str, len, 0, __builtin_object_size(str, 0), __VA_ARGS__)
+
+#else /* VARIANT */
+
+#define sprintf BUILTIN(sprintf)
+int sprintf(char *restrict buffer, const char *restrict format, ... );
+int snprintf(char *restrict buffer, size_t bufsz,
+ const char *restrict format, ... );
+#endif /* VARIANT */
+
+void test_sprintf1() {
+  char a[4] = {0};
+  sprintf(a, "%d/%s", 1, a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf1() {
+  char a[4] = {0};
+  snprintf(a, sizeof(a), "%d/%s", 1, a); // expected-warning{{overlapping}}
+}
+
+void test_snprintf2() {
+  char a[4] = {0};
+  snprintf(a+1, sizeof(a)-1, "%d/%s", 1, a); // expected-warning{{overlapping}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -12,6 +12,7 @@
 //===--===//
 
 #include "InterCheckerAPI.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -175,6 +176,8 @@
std::bind(::evalMemcmp, _1, _2, _3, CK_Regular)},
   {{CDF_MaybeBuiltin, {"bzero"}, 2}, ::evalBzero},
   {{CDF_MaybeBuiltin, {"explicit_bzero"}, 2}, ::evalBzero},
+  {{CDF_MaybeBuiltin, {"sprintf"}}, ::evalSprintf},
+  {{CDF_MaybeBuiltin, {"snprintf"}}, ::evalSnprintf},
   };
 
   // These require a bit of special handling.
@@ -228,6 +231,11 @@
   void evalMemset(CheckerContext , const CallExpr *CE) const;
   void evalBzero(CheckerContext , const CallExpr *CE) const;
 
+  void evalSprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSnprintf(CheckerContext , const CallExpr *CE) const;
+  void evalSprintfCommon(CheckerContext , const CallExpr *CE, bool IsBounded,
+ bool IsBuiltin) const;
+
   // Utility methods
   std::pair
   static assumeZero(CheckerContext ,
@@ -2352,6 +2360,61 @@
   C.addTransition(State);
 }
 
+void CStringChecker::evalSprintf(CheckerContext , const CallExpr *CE) const {
+  CurrentFunctionDescription = "'sprintf'";
+  bool IsBI = CE->getBuiltinCallee() == Builtin::BI__builtin___sprintf_chk;
+  evalSprintfCommon(C, CE, /* IsBounded */ false, IsBI);
+}
+
+void CStringChecker::evalSnprintf(CheckerContext , const CallExpr *CE) const {
+  CurrentFunctionDescription = "'snprintf'";
+  bool IsBI = CE->getBuiltinCallee() == Builtin::BI__builtin___snprintf_chk;
+  evalSprintfCommon(C, CE, /* IsBounded */ true, IsBI);
+}
+
+void CStringChecker::evalSprintfCommon(CheckerContext , const CallExpr *CE,
+   bool IsBounded, bool 

[PATCH] D56366: New warning call-to-virtual-from-ctor-dtor when calling a virtual function from a constructor or a destructor

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a subscriber: xbolva00.
ArnaudBienner added a comment.

Thanks @xbolva00  for adding reviewers and @rjmccall for reviewing!

@rjmccall as you might remember, we had a discussion on the mailing list 
("Warning when calling virtual functions from constructor/desctructor?") and 
from what I understand the overall feeling was that this patch/warning won't be 
accepted until we move forward with your proposal of having a "-Wversion=" flag 
to allow deactivate new warnings when upgrading clang, but enable them by 
default otherwise.
Have you made any progress on that? Or do you think the warning can be 
implemented anyway? (maybe off by default?)

Just want to avoid spending our time reviewing/doing changes on a patch that 
won't be accepted in the end :)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56366/new/

https://reviews.llvm.org/D56366



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

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added inline comments.



Comment at: lib/Sema/SemaChecking.cpp:11103
+
+  if (auto *BO = dyn_cast(E)) {
+BinaryOperator::Opcode Opc = BO->getOpcode();

Shouldn't that be "const auto*" instead?
I'm surprised dyn_cast casts away const qualifier, but FWIW having the returned 
pointer being const makes clearer the variable pointed by this pointer won't be 
modified.
But maybe this was intended to not make the code too verbose?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63082/new/

https://reviews.llvm.org/D63082



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

[PATCH] D56366: New warning call-to-virtual-from-ctor-dtor when calling a virtual function from a constructor or a destructor

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner created this revision.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D56366

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaOverload.cpp
  test/Analysis/ctor.mm
  test/Analysis/dtor.cpp
  test/Analysis/virtualcall.cpp
  test/SemaCXX/warn-virtual-call-from-ctor-dtor.cpp

Index: test/SemaCXX/warn-virtual-call-from-ctor-dtor.cpp
===
--- /dev/null
+++ test/SemaCXX/warn-virtual-call-from-ctor-dtor.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -Wcall-to-virtual-from-ctor-dtor
+struct A {
+  A() { f(); } // expected-warning {{call to virtual member function 'f' from constructor of 'A' will not call overridden implementations}} expected-note {{qualify the call to silence this warning}}
+  ~A() { f(); } // expected-warning {{call to virtual member function 'f' from destructor of 'A' will not call overridden implementations}} expected-note {{qualify the call to silence this warning}}
+
+  virtual void f() {}
+};
+
+// Warns in classes inheriting from A too
+struct B : A {
+  B() { f(); } // expected-warning {{call to virtual member function 'f' from constructor of 'B' will not call overridden implementations}} expected-note {{qualify the call to silence this warning}}
+  ~B() { f(); } // expected-warning {{call to virtual member function 'f' from destructor of 'B' will not call overridden implementations}} expected-note {{qualify the call to silence this warning}}
+  void f() {}
+};
+
+// Don't warn if the class if final
+struct C final : A {
+  C() { f(); }
+  ~C() { f(); }
+};
+
+// Don't warn (or note) when calling the function on a pointer.
+struct D : A {
+  A *a;
+  D() { a->f(); };
+  ~D() { a->f(); };
+};
+
+// Don't warn if the call is fully qualified.
+struct E {
+  virtual void f() = 0;
+  E() {
+E::f();
+  }
+};
Index: test/Analysis/virtualcall.cpp
===
--- test/Analysis/virtualcall.cpp
+++ test/Analysis/virtualcall.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-output=text -verify -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-output=text -Wno-call-to-virtual-from-ctor-dtor -verify -std=c++11 %s
 
-// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:PureOnly=true -DPUREONLY=1 -analyzer-output=text -verify -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=optin.cplusplus.VirtualCall -analyzer-store region -analyzer-config optin.cplusplus.VirtualCall:PureOnly=true -DPUREONLY=1 -analyzer-output=text -Wno-call-to-virtual-from-ctor-dtor -verify -std=c++11 %s
 
 #include "virtualcall.h"
 
Index: test/Analysis/dtor.cpp
===
--- test/Analysis/dtor.cpp
+++ test/Analysis/dtor.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection,cplusplus -analyzer-config c++-inlining=destructors -Wno-null-dereference -Wno-inaccessible-base -verify -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection,cplusplus -analyzer-config c++-inlining=destructors -Wno-null-dereference -Wno-inaccessible-base -Wno-call-to-virtual-from-ctor-dtor -verify -analyzer-config eagerly-assume=false %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_checkInlined(bool);
Index: test/Analysis/ctor.mm
===
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -1,7 +1,7 @@
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -DI386 -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-config c++-inlining=constructors -Wno-null-dereference -std=c++11 -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -DI386 -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-config c++-inlining=constructors -Wno-null-dereference -std=c++11 -verify -DTEST_INLINABLE_ALLOCATORS -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin12 -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-config c++-inlining=constructors -Wno-null-dereference -std=c++11 -verify -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin12 -analyzer-checker=core,debug.ExprInspection -fobjc-arc -analyzer-config c++-inlining=constructors -Wno-null-dereference -std=c++11 -verify -DTEST_INLINABLE_ALLOCATORS -analyzer-config eagerly-assume=false %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -DI386 -analyzer-checker=core,debug.ExprInspection -fobjc-arc 

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

OK, thanks Serge! :)

For the record, I updated the patch one last time after it was accepted to 
remove my change to constant-expression-cxx1y.cpp since someone else did the 
same change in a separate commit meanwhile.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 180075.
ArnaudBienner added a comment.
Herald added a reviewer: serge-sans-paille.

Make -Wstring-plus-int warns even if when the result is not out of bounds


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382

Files:
  bindings/python/tests/cindex/test_diagnostics.py
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/string-plus-int.cpp


Index: test/SemaCXX/string-plus-int.cpp
===
--- test/SemaCXX/string-plus-int.cpp
+++ test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
   consumeChar(*(5 + "foo"));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
 
   consume(L"foo" + 5);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
 
   // Should not warn.
   consume(&("foo"[3]));
   consume(&("foo"[index]));
   consume(&("foo"[kMyEnum]));
-  consume("foo" + kMySmallEnum);
-  consume(kMySmallEnum + "foo");
 
-  consume(L"foo" + 2);
-
-  consume("foo" + 3);  // Points at the \0
-  consume("foo" + 4);  // Points 1 past the \0, which is legal too.
-  consume("\pfoo" + 4);  // Pascal strings don't have a trailing \0, but they
- // have a leading length byte, so this is fine too.
 
   consume("foo" + kMyOperatorOverloadedEnum);
   consume(kMyOperatorOverloadedEnum + "foo");
-
-  #define A "foo"
-  #define B "bar"
-  consume(A B + sizeof(A) - 1);
 }
 
 template 
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9143,16 +9143,6 @@
   if (!IsStringPlusInt || IndexExpr->isValueDependent())
 return;
 
-  Expr::EvalResult Result;
-  if (IndexExpr->EvaluateAsInt(Result, Self.getASTContext())) {
-llvm::APSInt index = Result.Val.getInt();
-unsigned StrLenWithNull = StrExpr->getLength() + 1;
-if (index.isNonNegative() &&
-index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull),
-  index.isUnsigned()))
-  return;
-  }
-
   SourceRange DiagRange(LHSExpr->getBeginLoc(), RHSExpr->getEndLoc());
   Self.Diag(OpLoc, diag::warn_string_plus_int)
   << DiagRange << IndexExpr->IgnoreImpCasts()->getType();
Index: bindings/python/tests/cindex/test_diagnostics.py

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Fixed two tests broken by this change:

- test_diagnostics.py: AFAICT we are not testing all warnings here, but just 
that warnings are emitted correctly. The "+1" didn't seem to be useful, since 
the warning triggered was about the const char* being converted to an int (and 
this warning still applies)
- test/SemaCXX/constant-expression-cxx1y.cpp: is a false positive for 
-Wstring-plus-int so use the preferred, equivalent syntax

@thakis: do those additional changes look OK to you? Or do you want someone 
else to review those?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 178624.
ArnaudBienner added a comment.
Herald added a subscriber: arphaman.

Update tests broken by this change


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382

Files:
  bindings/python/tests/cindex/test_diagnostics.py
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/constant-expression-cxx1y.cpp
  test/SemaCXX/string-plus-int.cpp

Index: test/SemaCXX/string-plus-int.cpp
===
--- test/SemaCXX/string-plus-int.cpp
+++ test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
   consumeChar(*(5 + "foo"));  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
 
   consume(L"foo" + 5);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{to a string does not append to the string}} expected-note {{use array indexing to silence this warning}}
 
   // Should not warn.
   consume(&("foo"[3]));
   consume(&("foo"[index]));
   consume(&("foo"[kMyEnum]));
-  consume("foo" + kMySmallEnum);
-  consume(kMySmallEnum + "foo");
 
-  consume(L"foo" + 2);
-
-  consume("foo" + 3);  // Points at the \0
-  consume("foo" + 4);  // Points 1 past the \0, which is legal too.
-  consume("\pfoo" + 4);  // Pascal strings don't have a trailing \0, but they
- // have a leading length byte, so this is fine too.
 
   consume("foo" + kMyOperatorOverloadedEnum);
   consume(kMyOperatorOverloadedEnum + "foo");
-
-  #define A "foo"
-  #define B "bar"
-  consume(A B + sizeof(A) - 1);
 }
 
 template 
Index: test/SemaCXX/constant-expression-cxx1y.cpp
===
--- test/SemaCXX/constant-expression-cxx1y.cpp
+++ test/SemaCXX/constant-expression-cxx1y.cpp
@@ -444,7 +444,7 @@
   static_assert(test_bounds("foo", 0)[0] == 'f', "");
   static_assert(test_bounds("foo", 3)[0] == 0, "");
   static_assert(test_bounds("foo", 4)[-3] == 'o', "");
-  static_assert(test_bounds("foo" + 4, -4)[0] == 'f', "");
+  static_assert(test_bounds(&"foo"[4], -4)[0] == 'f', "");
   static_assert(test_bounds("foo", 5) != 0, ""); // expected-error {{constant}} expected-note {{call}}
   static_assert(test_bounds("foo", -1) != 0, ""); // expected-error {{constant}} expected-note {{call}}
   static_assert(test_bounds("foo", 1000) != 0, ""); // expected-error {{constant}} expected-note {{call}}
Index: lib/Sema/SemaExpr.cpp

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner reopened this revision.
ArnaudBienner added a comment.
This revision is now accepted and ready to land.

Reopening since this broke some tests.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Really sorry about breaking the build :( Thanks for reverting it.
Actually, I think we could fix that test by removing the " +1": AFAICT this 
test is checking that warnings are correctly emitted from clang, but not 
testing all the warnings. So the conversion from const char* to int is enough: 
no need to have an extra  +1 at the end.
I will update my patch to update the test accordingly.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Thanks for the archeological work and finding the conversation related to the 
initial patch :)
Interesting that the last case you mentioned is exactly the bug I had in my 
project (though in your case this would have been intended).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

So I built Firefox with my patched version of clang and the only place it found 
a warning was in ffmpeg [1], a case you already reported in [2] when you 
implemented that patch, so no new positive AFAICT.
Do you also want to try on Chromium and see how it goes? Or is Firefox enough? 
(it's a pretty big C++ codebase, so I would assume that's enough)

[1]: mozilla-central/media/ffvpx/libavutil/utils.c:73:42: warning: adding 
'unsigned long' to a string does not append to the string [-Wstring-plus-int]
278:13.00 return LICENSE_PREFIX FFMPEG_LICENSE + sizeof(LICENSE_PREFIX) - 1;
[2]: http://comments.gmane.org/gmane.comp.compilers.clang.scm/47203


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

I found the commit: [1] which links to a related discussion [2] but there isn't 
much details. I wasn't sure if there was other discussions.
I will try to build Firefox with this change, and see how it goes (just need to 
find some time to do it).
I'll keep you posted.

[1]: 
https://github.com/llvm-mirror/clang/commit/1cb2d742eb6635aeab6132ee5f0b5781d39487d7
[2]: http://comments.gmane.org/gmane.comp.compilers.clang.scm/47203


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Nico: I've added you as reviewer since you originally implemented this warning 
(thanks for this BTW :) as said, it's really helpful), but feel free to 
delegate to someone else.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

I found this warning to be really useful but was confused that it is not always 
triggered.
I initially discovered -Wstring-plus-char (and fixed some issues in the 
projects I work on where sometimes developers didn't realized the "+" wasn't 
doing to do concatenation because it was applied on char and char*).
Then I also activated -Werror=string-plus-int to reduce the risk of developers 
doing mistakes in our codebase.

Turns out that because we had some buggy code not catch by this warning for 
this reason: the result of the concatenation was not out of bounds, so the 
warning wasn't triggered.

I understand that point of view, but IMHO this warning should be activated even 
though, like for -Wstring-plus-char: it's easy to fix the warning by having a 
nicer, more readable code, and IMO code raising this warning is likely to 
indicate an issue in the code.
Having developers accidentally concatenating string and integer can happen 
(even more if when not concatenating literals directly).
But I've found a bug in our codebase even more tricky: when concatenating enum 
and literals char* strings:
We had a code like this (using Qt framework):

  QString path = QStandardPaths::StandardLocation(QStandardPaths::DataLocation) 
+ "/filename.ext";

The developer was trying to use QStandardPaths::standardLocations [1] static 
function (mind the lowercase s and the extra s at the end) which takes an enum 
of type QStandardPaths::StandardLocation. Similar name (so easy to do a typo)  
but very different meanings.
So instead of getting a string object and concatenating it with some literal 
strings, path was set to a truncated version of "/filename.ext".
This kind of bugs is hard to catch during code review process (and wasn't in my 
case).

Long story, but I think it's interesting to illustrate the need to have this 
warning applied to more cases.

[1]: http://doc.qt.io/qt-5/qstandardpaths.html#standardLocations


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55382/new/

https://reviews.llvm.org/D55382



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner created this revision.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D55382

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/string-plus-int.cpp


Index: test/SemaCXX/string-plus-int.cpp
===
--- test/SemaCXX/string-plus-int.cpp
+++ test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + index);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume("foo" + kMyEnum);  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume("foo" + kMySmallEnum); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   consume(5 + "foo");  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(index + "foo");  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
   consume(kMyEnum + "foo");  // expected-warning {{adding 'MyEnum' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
+  consume(kMySmallEnum + "foo"); // expected-warning {{adding 'MyEnum' to a 
string does not append to the string}} expected-note {{use array indexing to 
silence this warning}}
 
   // FIXME: suggest replacing with "foo"[5]
   consumeChar(*("foo" + 5));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
   consumeChar(*(5 + "foo"));  // expected-warning {{adding 'int' to a string 
does not append to the string}} expected-note {{use array indexing to silence 
this warning}}
 
   consume(L"foo" + 5);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume(L"foo" + 2); // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  consume("foo" + 3);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("foo" + 4);  // expected-warning {{adding 'int' to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
+  consume("\pfoo" + 4);  // expected-warning {{adding 'int' to a string does 
not append to the string}} expected-note {{use array indexing to silence this 
warning}}
+
+  #define A "foo"
+  #define B "bar"
+  consume(A B + sizeof(A) - 1); // expected-warning {{to a string does not 
append to the string}} expected-note {{use array indexing to silence this 
warning}}
 
   // Should not warn.
   consume(&("foo"[3]));
   consume(&("foo"[index]));
   consume(&("foo"[kMyEnum]));
-  consume("foo" + kMySmallEnum);
-  consume(kMySmallEnum + "foo");
 
-  consume(L"foo" + 2);
-
-  consume("foo" + 3);  // Points at the \0
-  consume("foo" + 4);  // Points 1 past the \0, which is legal too.
-  consume("\pfoo" + 4);  // Pascal strings don't have a trailing \0, but they
- // have a leading length byte, so this is fine too.
 
   consume("foo" + kMyOperatorOverloadedEnum);
   consume(kMyOperatorOverloadedEnum + "foo");
-
-  #define A "foo"
-  #define B "bar"
-  consume(A B + sizeof(A) - 1);
 }
 
 template 
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -9133,16 +9133,6 @@
   if (!IsStringPlusInt || IndexExpr->isValueDependent())
 return;
 
-  Expr::EvalResult Result;
-  if (IndexExpr->EvaluateAsInt(Result, Self.getASTContext())) {
-llvm::APSInt index = Result.Val.getInt();
-unsigned StrLenWithNull = StrExpr->getLength() + 1;
-if (index.isNonNegative() &&
-index <= llvm::APSInt(llvm::APInt(index.getBitWidth(), StrLenWithNull),
-  index.isUnsigned()))
-  return;
-  }
-
   SourceRange DiagRange(LHSExpr->getBeginLoc(), RHSExpr->getEndLoc());
   Self.Diag(OpLoc, diag::warn_string_plus_int)
   << DiagRange << IndexExpr->IgnoreImpCasts()->getType();


Index: test/SemaCXX/string-plus-int.cpp
===
--- test/SemaCXX/string-plus-int.cpp
+++ test/SemaCXX/string-plus-int.cpp
@@ -31,37 +31,36 @@
   consume("foo" + 5);  // expected-warning {{adding 'int' to a string does not append to the 

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

Thanks for the review! :)
Could someone please land the patch for me? I don't have commit access.


Repository:
  rC Clang

https://reviews.llvm.org/D53807



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

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a comment.

@rsmith do you have any thoughts about this change?


Repository:
  rC Clang

https://reviews.llvm.org/D53807



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

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner added a reviewer: davide.
ArnaudBienner added a comment.

Hi Davide,

I see you the last person who updated the test file related to this feature.
Would you feel comfortable reviewing my patch?

This is my first clang patch, so I apologize in advance if I missed obvious 
things :)


Repository:
  rC Clang

https://reviews.llvm.org/D53807



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

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner updated this revision to Diff 171502.
ArnaudBienner added a comment.

Update tests


Repository:
  rC Clang

https://reviews.llvm.org/D53807

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  test/Misc/warning-flags.c
  test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp


Index: test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp
===
--- test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp
+++ test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -fsyntax-only -verify 
-Wcall-to-pure-virtual-from-ctor-dtor
 struct A {
   A() { f(); } // expected-warning {{call to pure virtual member function 'f' 
has undefined behavior; overrides of 'f' in subclasses are not available in the 
constructor of 'A'}}
   ~A() { f(); } // expected-warning {{call to pure virtual member function 'f' 
has undefined behavior; overrides of 'f' in subclasses are not available in the 
destructor of 'A'}}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (76):
+CHECK: Warnings without flags (75):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -40,7 +40,6 @@
 CHECK-NEXT:   warn_arcmt_nsalloc_realloc
 CHECK-NEXT:   warn_asm_label_on_auto_decl
 CHECK-NEXT:   warn_c_kext
-CHECK-NEXT:   warn_call_to_pure_virtual_member_function_from_ctor_dtor
 CHECK-NEXT:   warn_call_wrong_number_of_arguments
 CHECK-NEXT:   warn_case_empty_range
 CHECK-NEXT:   warn_char_constant_too_large
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1656,7 +1656,7 @@
 def warn_call_to_pure_virtual_member_function_from_ctor_dtor : Warning<
   "call to pure virtual member function %0 has undefined behavior; "
   "overrides of %0 in subclasses are not available in the "
-  "%select{constructor|destructor}1 of %2">;
+  "%select{constructor|destructor}1 of %2">, 
InGroup;
 
 def select_special_member_kind : TextSubstitution<
   "%select{default constructor|copy constructor|move constructor|"
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -285,6 +285,7 @@
 def FlagEnum : DiagGroup<"flag-enum">;
 def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>;
 def InfiniteRecursion : DiagGroup<"infinite-recursion">;
+def PureVirtualCallFromCtorDtor: 
DiagGroup<"call-to-pure-virtual-from-ctor-dtor">;
 def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">;
 def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">;
 def : DiagGroup<"import">;


Index: test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp
===
--- test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp
+++ test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify
+// RUN: %clang_cc1 %s -fsyntax-only -verify -Wcall-to-pure-virtual-from-ctor-dtor
 struct A {
   A() { f(); } // expected-warning {{call to pure virtual member function 'f' has undefined behavior; overrides of 'f' in subclasses are not available in the constructor of 'A'}}
   ~A() { f(); } // expected-warning {{call to pure virtual member function 'f' has undefined behavior; overrides of 'f' in subclasses are not available in the destructor of 'A'}}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (76):
+CHECK: Warnings without flags (75):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -40,7 +40,6 @@
 CHECK-NEXT:   warn_arcmt_nsalloc_realloc
 CHECK-NEXT:   warn_asm_label_on_auto_decl
 CHECK-NEXT:   warn_c_kext
-CHECK-NEXT:   warn_call_to_pure_virtual_member_function_from_ctor_dtor
 CHECK-NEXT:   warn_call_wrong_number_of_arguments
 CHECK-NEXT:   warn_case_empty_range
 CHECK-NEXT:   warn_char_constant_too_large
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ 

[PATCH] D53807: Create a diagnostic group for warn_call_to_pure_virtual_member_function_from_ctor_dtor, so it can be turned into an error using Werror

[Arnaud Bienner via Phabricator via cfe-commits]

ArnaudBienner created this revision.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D53807

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td


Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1656,7 +1656,7 @@
 def warn_call_to_pure_virtual_member_function_from_ctor_dtor : Warning<
   "call to pure virtual member function %0 has undefined behavior; "
   "overrides of %0 in subclasses are not available in the "
-  "%select{constructor|destructor}1 of %2">;
+  "%select{constructor|destructor}1 of %2">, 
InGroup;
 
 def select_special_member_kind : TextSubstitution<
   "%select{default constructor|copy constructor|move constructor|"
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -285,6 +285,7 @@
 def FlagEnum : DiagGroup<"flag-enum">;
 def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>;
 def InfiniteRecursion : DiagGroup<"infinite-recursion">;
+def PureVirtualCallFromCtorDtor: 
DiagGroup<"call-to-pure-virtual-from-ctor-dtor">;
 def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">;
 def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">;
 def : DiagGroup<"import">;


Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1656,7 +1656,7 @@
 def warn_call_to_pure_virtual_member_function_from_ctor_dtor : Warning<
   "call to pure virtual member function %0 has undefined behavior; "
   "overrides of %0 in subclasses are not available in the "
-  "%select{constructor|destructor}1 of %2">;
+  "%select{constructor|destructor}1 of %2">, InGroup;
 
 def select_special_member_kind : TextSubstitution<
   "%select{default constructor|copy constructor|move constructor|"
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -285,6 +285,7 @@
 def FlagEnum : DiagGroup<"flag-enum">;
 def IncrementBool : DiagGroup<"increment-bool", [DeprecatedIncrementBool]>;
 def InfiniteRecursion : DiagGroup<"infinite-recursion">;
+def PureVirtualCallFromCtorDtor: DiagGroup<"call-to-pure-virtual-from-ctor-dtor">;
 def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">;
 def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">;
 def : DiagGroup<"import">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits