ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: steakhal, NoQ, martong, vsavchenko.
ASDenysPetrov added a project: clang.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus,
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
ASDenysPetrov requested review of this revision.
During pointer dereferencing CastRetrievedVal uses wrong type from the Store
after type punning. Namely, the pointer casts to another type and then assigns
with a value of one more another type. It produces NonLoc value when Loc is
expected.
Example for visibility:
void foo(char ***c, int *i) {
*(unsigned**)c = (unsigned*)i; // type punning
***c; // uses 'unsigned**' from the Store instead of 'char***'
}
Fixes: https://bugs.llvm.org/show_bug.cgi?id=37503
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89055
Files:
clang/lib/StaticAnalyzer/Core/Store.cpp
clang/test/Analysis/nonloc-as-loc-crash.c
clang/test/Analysis/string.c
clang/test/Analysis/svalbuilder-float-cast.c
Index: clang/test/Analysis/svalbuilder-float-cast.c
===================================================================
--- clang/test/Analysis/svalbuilder-float-cast.c
+++ clang/test/Analysis/svalbuilder-float-cast.c
@@ -4,13 +4,9 @@
void SymbolCast_of_float_type_aux(int *p) {
*p += 0;
- // FIXME: Ideally, all unknown values should be symbolicated.
- clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
-
+ clang_analyzer_denote(*p, "$x");
*p += 1;
- // This should NOT be (float)$x + 1. Symbol $x was never casted to float.
- // FIXME: Ideally, this should be $x + 1.
- clang_analyzer_express(*p); // expected-warning{{Not a symbol}}
+ clang_analyzer_express(*p); // expected-warning{{$x + 1}}
}
void SymbolCast_of_float_type() {
Index: clang/test/Analysis/string.c
===================================================================
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -363,6 +363,16 @@
strcpy(x, y); // no-warning
}
+// PR37503
+void *func_strcpy_no_assertion();
+char ***ptr_strcpy_no_assertion;
+void strcpy_no_assertion() {
+ *(unsigned char **)ptr_strcpy_no_assertion = (unsigned char
*)(func_strcpy_no_assertion());
+ char c;
+ strcpy(**ptr_strcpy_no_assertion, &c); // no-assertion
+}
+
+
//===----------------------------------------------------------------------===
// stpcpy()
//===----------------------------------------------------------------------===
Index: clang/test/Analysis/nonloc-as-loc-crash.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/nonloc-as-loc-crash.c
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void test(int *a, char ***b, float *c) {
+ *(unsigned char **)b = (unsigned char *)a;
+ if (**b == 0) // no-crash
+ ;
+
+ *(unsigned char **)b = (unsigned char *)c;
+ if (**b == 0) // no-crash
+ ;
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -426,12 +426,17 @@
// We might need to do that for non-void pointers as well.
// FIXME: We really need a single good function to perform casts for us
// correctly every time we need it.
- if (castTy->isPointerType() && !castTy->isVoidPointerType())
+ if (castTy->isPointerType() && !castTy->isVoidPointerType()) {
if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(V.getAsRegion())) {
- QualType sr = SR->getSymbol()->getType();
- if (!hasSameUnqualifiedPointeeType(sr, castTy))
- return loc::MemRegionVal(castRegion(SR, castTy));
+ QualType sr = SR->getSymbol()->getType();
+ if (!hasSameUnqualifiedPointeeType(sr, castTy))
+ return loc::MemRegionVal(castRegion(SR, castTy));
}
+ // Next fixes pointer dereference using type different from its initial one
+ // See PR37503 for details
+ if (const auto *SR = dyn_cast_or_null<ElementRegion>(V.getAsRegion()))
+ return loc::MemRegionVal(castRegion(SR, castTy));
+ }
return svalBuilder.dispatchCast(V, castTy);
}
Index: clang/test/Analysis/svalbuilder-float-cast.c
===================================================================
--- clang/test/Analysis/svalbuilder-float-cast.c
+++ clang/test/Analysis/svalbuilder-float-cast.c
@@ -4,13 +4,9 @@
void SymbolCast_of_float_type_aux(int *p) {
*p += 0;
- // FIXME: Ideally, all unknown values should be symbolicated.
- clang_analyzer_denote(*p, "$x"); // expected-warning{{Not a symbol}}
-
+ clang_analyzer_denote(*p, "$x");
*p += 1;
- // This should NOT be (float)$x + 1. Symbol $x was never casted to float.
- // FIXME: Ideally, this should be $x + 1.
- clang_analyzer_express(*p); // expected-warning{{Not a symbol}}
+ clang_analyzer_express(*p); // expected-warning{{$x + 1}}
}
void SymbolCast_of_float_type() {
Index: clang/test/Analysis/string.c
===================================================================
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -363,6 +363,16 @@
strcpy(x, y); // no-warning
}
+// PR37503
+void *func_strcpy_no_assertion();
+char ***ptr_strcpy_no_assertion;
+void strcpy_no_assertion() {
+ *(unsigned char **)ptr_strcpy_no_assertion = (unsigned char *)(func_strcpy_no_assertion());
+ char c;
+ strcpy(**ptr_strcpy_no_assertion, &c); // no-assertion
+}
+
+
//===----------------------------------------------------------------------===
// stpcpy()
//===----------------------------------------------------------------------===
Index: clang/test/Analysis/nonloc-as-loc-crash.c
===================================================================
--- /dev/null
+++ clang/test/Analysis/nonloc-as-loc-crash.c
@@ -0,0 +1,12 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+void test(int *a, char ***b, float *c) {
+ *(unsigned char **)b = (unsigned char *)a;
+ if (**b == 0) // no-crash
+ ;
+
+ *(unsigned char **)b = (unsigned char *)c;
+ if (**b == 0) // no-crash
+ ;
+}
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -426,12 +426,17 @@
// We might need to do that for non-void pointers as well.
// FIXME: We really need a single good function to perform casts for us
// correctly every time we need it.
- if (castTy->isPointerType() && !castTy->isVoidPointerType())
+ if (castTy->isPointerType() && !castTy->isVoidPointerType()) {
if (const auto *SR = dyn_cast_or_null<SymbolicRegion>(V.getAsRegion())) {
- QualType sr = SR->getSymbol()->getType();
- if (!hasSameUnqualifiedPointeeType(sr, castTy))
- return loc::MemRegionVal(castRegion(SR, castTy));
+ QualType sr = SR->getSymbol()->getType();
+ if (!hasSameUnqualifiedPointeeType(sr, castTy))
+ return loc::MemRegionVal(castRegion(SR, castTy));
}
+ // Next fixes pointer dereference using type different from its initial one
+ // See PR37503 for details
+ if (const auto *SR = dyn_cast_or_null<ElementRegion>(V.getAsRegion()))
+ return loc::MemRegionVal(castRegion(SR, castTy));
+ }
return svalBuilder.dispatchCast(V, castTy);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
