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

Reply via email to