NoQ created this revision.
An assertion failure was triggered in pr34779 by defining `strcpy` as `void
(unsigned char *, unsigned char *)`, while normally it is `char *(char *, char
*)`. The assertion is removed because normally we try not to crash in such
cases, but simply refuse to model. For now i did not try to actually model the
modified strcpy, because it requires more work (unhardcode `CharTy` in a lot of
places around the checker), and supporting other aspects of the non-standard
definition requires even more work (i.e. we try to bind the return value, need
to disable this mechanism when the return type is void), and if we try to model
other similar functions (are other string functions accepting unsigned chars as
well?) it'd bloat quite a bit.
So //for now the analyzer would not find C string errors// in code that defines
string functions in a non-standard manner. It simply tries to avoid crashing.
To think - maybe we should add more defensive checks (eg. into
`CallDescription`).
https://reviews.llvm.org/D39422
Files:
lib/StaticAnalyzer/Checkers/CStringChecker.cpp
test/Analysis/string-with-signedness.c
Index: test/Analysis/string-with-signedness.c
===================================================================
--- /dev/null
+++ test/Analysis/string-with-signedness.c
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -Wno-incompatible-library-redeclaration
-analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
+
+// expected-no-diagnostics
+
+void *strcpy(unsigned char *, unsigned char *);
+
+unsigned char a, b;
+void testUnsignedStrcpy() {
+ strcpy(&a, &b);
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -289,8 +289,8 @@
if (!ER)
return state;
- assert(ER->getValueType() == C.getASTContext().CharTy &&
- "CheckLocation should only be called with char* ElementRegions");
+ if (ER->getValueType() != C.getASTContext().CharTy)
+ return state;
// Get the size of the array.
const SubRegion *superReg = cast<SubRegion>(ER->getSuperRegion());
@@ -874,6 +874,8 @@
if (!ER)
return true; // cf top comment.
+ // FIXME: Does this crash when a non-standard definition
+ // of a library function is encountered?
assert(ER->getValueType() == C.getASTContext().CharTy &&
"IsFirstBufInBound should only be called with char* ElementRegions");
Index: test/Analysis/string-with-signedness.c
===================================================================
--- /dev/null
+++ test/Analysis/string-with-signedness.c
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -Wno-incompatible-library-redeclaration -analyzer-checker=core,unix.cstring,alpha.unix.cstring -verify %s
+
+// expected-no-diagnostics
+
+void *strcpy(unsigned char *, unsigned char *);
+
+unsigned char a, b;
+void testUnsignedStrcpy() {
+ strcpy(&a, &b);
+}
Index: lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -289,8 +289,8 @@
if (!ER)
return state;
- assert(ER->getValueType() == C.getASTContext().CharTy &&
- "CheckLocation should only be called with char* ElementRegions");
+ if (ER->getValueType() != C.getASTContext().CharTy)
+ return state;
// Get the size of the array.
const SubRegion *superReg = cast<SubRegion>(ER->getSuperRegion());
@@ -874,6 +874,8 @@
if (!ER)
return true; // cf top comment.
+ // FIXME: Does this crash when a non-standard definition
+ // of a library function is encountered?
assert(ER->getValueType() == C.getASTContext().CharTy &&
"IsFirstBufInBound should only be called with char* ElementRegions");
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
