https://github.com/NagyDonat created
https://github.com/llvm/llvm-project/pull/147536
This commit removes the test file test-member-invalidation.cpp which was
recently introduced in 39bc0529b018a89b4b6a21aaabe240cd3a65c44d by splitting
off a test case from new.cpp.
In that commit I preserved that test in a slightly modified setting to
demonstrate that it wasn't broken by the change; but in this separate commit
I'm removing it because I don't think that it "deserves a place" among our
tests.
The primary issue is that this test examines the values of data members of a
deleted object -- which is irrelevant, because code that relies on the value of
these members should be reported as a use-after-free bug. (In fact,
cplusplus.NewDelete reports it as a use-after-free bug, and the checker family
`MallocChecker` sinks the execution path even if that particular frontend is
not enabled.)
Moreover, a comment claimed that this tests "Invalidate Region even in case of
default destructor" while in fact it tested a situaton where the destructor is
a plain declared-but-not-defined method. The invalidation of `this` is done by
the conservative evaluation, and we don't need this overcomplicated test to
validate that very basic behavior.
From c6abdfa0d4b3d491d426fe8075f5f5326f96abcd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Don=C3=A1t=20Nagy?= <donat.n...@ericsson.com>
Date: Tue, 8 Jul 2025 15:58:17 +0200
Subject: [PATCH] [analyzer][NFC] Remove irrelevant overcomplicated test
This commit removes the test file test-member-invalidation.cpp which was
recently introduced in 39bc0529b018a89b4b6a21aaabe240cd3a65c44d by
splitting off a test case from new.cpp.
In that commit I preserved that test in a slightly modified setting to
demonstrate that it wasn't broken by the change; but in this separate
commit I'm removing it because I don't think that it "deserves a place"
among our tests.
The primary issue is that this test examines the values of data members
of a deleted object -- which is irrelevant, because code that relies on
the value of these members should be reported as a use-after-free bug.
(In fact, cplusplus.NewDelete reports it as a use-after-free bug, and
the checker family `MallocChecker` sinks the execution path even if that
particular frontend is not enabled.)
Moreover, a comment claimed that this tests "Invalidate Region even in
case of default destructor" while in fact it tested a situaton where the
destructor is a plain declared-but-not-defined method. The invalidation
of `this` is done by the conservative evaluation, and we don't need this
overcomplicated test to validate that very basic behavior.
---
clang/test/Analysis/new.cpp | 5 --
.../Analysis/test-member-invalidation.cpp | 47 -------------------
2 files changed, 52 deletions(-)
delete mode 100644 clang/test/Analysis/test-member-invalidation.cpp
diff --git a/clang/test/Analysis/new.cpp b/clang/test/Analysis/new.cpp
index 8439a4e55d812..15c27e7d01308 100644
--- a/clang/test/Analysis/new.cpp
+++ b/clang/test/Analysis/new.cpp
@@ -326,8 +326,3 @@ void testArrayDestr() {
delete[] p;
clang_analyzer_warnIfReached(); // no-warning
}
-
-// See also test-member-invalidation.cpp which validates that calling an
-// unknown destructor invalidates the members of an object. This behavior
-// cannot be tested in this file because here `MallocChecker.cpp` sinks
-// execution paths that refer to members of a deleted object.
diff --git a/clang/test/Analysis/test-member-invalidation.cpp
b/clang/test/Analysis/test-member-invalidation.cpp
deleted file mode 100644
index cbff3986325df..0000000000000
--- a/clang/test/Analysis/test-member-invalidation.cpp
+++ /dev/null
@@ -1,47 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-std=c++11 -verify=expected,nosink -analyzer-config eagerly-assume=false %s
-// RUN: %clang_analyze_cc1
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -std=c++11
-verify=expected,sink -analyzer-config eagerly-assume=false %s
-
-// This test validates that calling an unknown destructor invalidates the
-// members of an object. This was originally a part of the test file `new.cpp`,
-// but was split off into a separate file because the checker family
-// implemented in `MallocChecker.cpp` (which is activated via unix.Malloc in
-// `new.cpp` sinks all execution paths that refer to members of a deleted
object.
-
-void clang_analyzer_eval(bool);
-
-// Invalidate Region even in case of default destructor
-class InvalidateDestTest {
-public:
- int x;
- int *y;
- ~InvalidateDestTest();
-};
-
-int test_member_invalidation() {
-
- //test invalidation of member variable
- InvalidateDestTest *test = new InvalidateDestTest();
- test->x = 5;
- int *k = &(test->x);
- clang_analyzer_eval(*k == 5); // expected-warning{{TRUE}}
- delete test;
- clang_analyzer_eval(*k == 5); // nosink-warning{{UNKNOWN}}
-
- //test invalidation of member pointer
- int localVar = 5;
- test = new InvalidateDestTest();
- test->y = &localVar;
- delete test;
- clang_analyzer_eval(localVar == 5); // nosink-warning{{UNKNOWN}}
-
- // Test aray elements are invalidated.
- int Var1 = 5;
- int Var2 = 5;
- InvalidateDestTest *a = new InvalidateDestTest[2];
- a[0].y = &Var1;
- a[1].y = &Var2;
- delete[] a;
- clang_analyzer_eval(Var1 == 5); // nosink-warning{{UNKNOWN}}
- clang_analyzer_eval(Var2 == 5); // nosink-warning{{UNKNOWN}}
- return 0;
-}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
