MaggieYi updated this revision to Diff 32671.
MaggieYi added a comment.
Hi Anna,
Many thanks for your comments. I have modified the patch to address your
comments. Please let me know what you think.
PS: If the updated patch looks good to you, could you please commit it for me
(as I do not have commit access) ?
Many thanks,
Ying Yi
http://reviews.llvm.org/D12163
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
test/Analysis/PR24184.cpp
test/Analysis/malloc.c
Index: test/Analysis/malloc.c
===================================================================
--- test/Analysis/malloc.c
+++ test/Analysis/malloc.c
@@ -1386,7 +1386,8 @@
int *s;
char *b = realloc(a->p, size);
char *m = realloc(a->p, size); // expected-warning {{Attempt to free released memory}}
- return a->p;
+ //PR24184: Object "a->p" is returned after being freed by calling "realloc".
+ return a->p; // expected-warning {{Use of memory after it is freed}}
}
// We should not warn in this case since the caller will presumably free a->p in all cases.
Index: test/Analysis/PR24184.cpp
===================================================================
--- /dev/null
+++ test/Analysis/PR24184.cpp
@@ -0,0 +1,97 @@
+// RUN: %clang_cc1 -w -analyze -analyzer-eagerly-assume -fcxx-exceptions -analyzer-checker=core -analyzer-checker=alpha.core.PointerArithm,alpha.core.CastToStruct -analyzer-max-loop 64 -verify %s
+// RUN: %clang_cc1 -w -analyze -analyzer-checker=core -analyzer-checker=cplusplus -fcxx-exceptions -analyzer-checker alpha.core.PointerArithm,alpha.core.CastToStruct -analyzer-max-loop 63 -verify %s
+
+// These tests used to hit an assertion in the bug report. Test case from http://llvm.org/PR24184.
+typedef struct {
+ int cbData;
+ unsigned pbData;
+} CRYPT_DATA_BLOB;
+
+typedef enum { DT_NONCE_FIXED } DATA_TYPE;
+int a;
+typedef int *vcreate_t(int *, DATA_TYPE, int, int);
+void fn1(unsigned, unsigned) {
+ char b = 0;
+ for (; 1; a++, &b + a * 0) // expected-warning{{Pointer arithmetic done on non-array variables means reliance on memory layout, which is dangerous}}
+ ;
+}
+
+vcreate_t fn2;
+struct A {
+ CRYPT_DATA_BLOB value;
+ int m_fn1() {
+ int c;
+ value.pbData == 0;
+ fn1(0, 0);
+ }
+};
+struct B {
+ A IkeHashAlg;
+ A IkeGType;
+ A NoncePhase1_r;
+};
+class C {
+ int m_fn2(B *);
+ void m_fn3(B *, int, int, int);
+};
+int C::m_fn2(B *p1) {
+ int *d;
+ int e = p1->IkeHashAlg.m_fn1();
+ unsigned f = p1->IkeGType.m_fn1(), h;
+ int g;
+ d = fn2(0, DT_NONCE_FIXED, (char)0, p1->NoncePhase1_r.value.cbData);
+ h = 0 | 0;
+ m_fn3(p1, 0, 0, 0);
+}
+
+// case 2:
+typedef struct {
+ int cbData;
+ unsigned char *pbData;
+} CRYPT_DATA_BLOB_1;
+typedef unsigned uint32_t;
+void fn1_1(void *p1, const void *p2) { p1 != p2; }
+
+void fn2_1(uint32_t *p1, unsigned char *p2, uint32_t p3) {
+ unsigned i = 0;
+ for (0; i < p3; i++)
+ fn1_1(p1 + i, p2 + i * 0); // expected-warning{{Pointer arithmetic done on non-array variables means reliance on memory layout, which is dangerous}}
+}
+
+struct A_1 {
+ CRYPT_DATA_BLOB_1 value;
+ uint32_t m_fn1() {
+ uint32_t a;
+ if (value.pbData)
+ fn2_1(&a, value.pbData, value.cbData);
+ return 0;
+ }
+};
+struct {
+ A_1 HashAlgId;
+} *b;
+void fn3() {
+ uint32_t c, d;
+ d = b->HashAlgId.m_fn1();
+ d << 0 | 0 | 0;
+ c = 0;
+ 0 | 1 << 0 | 0 && b;
+}
+
+// case 3:
+struct ST {
+ char c;
+};
+char *p;
+int foo1(ST);
+int foo2() {
+ ST *p1 = (ST *)(p); // expected-warning{{Casting a non-structure type to a structure type and accessing a field can lead to memory access errors or data corruption}}
+ while (p1->c & 0x0F || p1->c & 0x07)
+ p1 = p1 + foo1(*p1);
+}
+
+int foo3(int *node) {
+ int i = foo2();
+ if (i)
+ return foo2();
+}
\ No newline at end of file
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
===================================================================
--- include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
@@ -287,7 +287,10 @@
bool MarkAsSink,
ExplodedNode *P = nullptr,
const ProgramPointTag *Tag = nullptr) {
- if (!State || (State == Pred->getState() && !Tag && !MarkAsSink))
+ // It may not be safe to use the "Pred" node with no tag because the "Pred"
+ // node may be recycled in the reclamation function.
+ if (!State || (State == Pred->getState() && !Tag && !MarkAsSink &&
+ Pred->getLocation().getTag()))
return Pred;
Changed = true;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
