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