[llvm] [LLVM][Coverage][Unittest] Fix dangling reference in unittest (PR #147118)

[Tomohiro Kashiwada via llvm-commits]

https://github.com/kikairoya updated https://github.com/llvm/llvm-project/pull/147118

>From 7ecd8e7100bc713dfa782d94440b16790a2b9b5d Mon Sep 17 00:00:00 2001
From: kikairoya <kikairoya at gmail.com>
Date: Sun, 29 Jun 2025 10:50:49 +0900
Subject: [PATCH 1/2] [LLVM][Coverage][Unittest] Fix dangling reference in
 unittest

In loop of `writeAndReadCoverageRegions`, `OutputFunctions[I].Filenames` references to
contents of `Filenames` after returning from `readCoverageRegions` but `Filenames` will be
cleared in next call of `readCoverageRegions`, causes dangling reference.
The lifetime of the contents of `Filenames` must be equal or longer than `OutputFunctions[I]`,
thus it has been moved into `OutputFunctions[I]` (typed `OutputFunctionCoverageData`).
---
 .../ProfileData/CoverageMappingTest.cpp         | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index ec81e5f274efa..59c381a1b4e82 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -64,6 +64,7 @@ namespace {
 struct OutputFunctionCoverageData {
   StringRef Name;
   uint64_t Hash;
+  std::vector<std::string> FilenamesStorage;
   std::vector<StringRef> Filenames;
   std::vector<CounterMappingRegion> Regions;
   std::vector<CounterExpression> Expressions;
@@ -71,8 +72,10 @@ struct OutputFunctionCoverageData {
   OutputFunctionCoverageData() : Hash(0) {}
 
   OutputFunctionCoverageData(OutputFunctionCoverageData &&OFCD)
-      : Name(OFCD.Name), Hash(OFCD.Hash), Filenames(std::move(OFCD.Filenames)),
-        Regions(std::move(OFCD.Regions)) {}
+      : Name(OFCD.Name), Hash(OFCD.Hash),
+        FilenamesStorage(std::move(OFCD.FilenamesStorage)),
+        Filenames(std::move(OFCD.Filenames)), Regions(std::move(OFCD.Regions)) {
+  }
 
   OutputFunctionCoverageData(const OutputFunctionCoverageData &) = delete;
   OutputFunctionCoverageData &
@@ -135,7 +138,6 @@ struct InputFunctionCoverageData {
 struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
   bool UseMultipleReaders;
   StringMap<unsigned> Files;
-  std::vector<std::string> Filenames;
   std::vector<InputFunctionCoverageData> InputFunctions;
   std::vector<OutputFunctionCoverageData> OutputFunctions;
 
@@ -233,13 +235,10 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
 
   void readCoverageRegions(const std::string &Coverage,
                            OutputFunctionCoverageData &Data) {
-    // We will re-use the StringRef in duplicate tests, clear it to avoid
-    // clobber previous ones.
-    Filenames.clear();
-    Filenames.resize(Files.size() + 1);
+    Data.FilenamesStorage.resize(Files.size() + 1);
     for (const auto &E : Files)
-      Filenames[E.getValue()] = E.getKey().str();
-    ArrayRef<std::string> FilenameRefs = llvm::ArrayRef(Filenames);
+      Data.FilenamesStorage[E.getValue()] = E.getKey().str();
+    ArrayRef<std::string> FilenameRefs = llvm::ArrayRef(Data.FilenamesStorage);
     RawCoverageMappingReader Reader(Coverage, FilenameRefs, Data.Filenames,
                                     Data.Expressions, Data.Regions);
     EXPECT_THAT_ERROR(Reader.read(), Succeeded());

>From 32eeee9dd7e93822f995965e3e30d955ce1aa1e9 Mon Sep 17 00:00:00 2001
From: Tomohiro Kashiwada <kikairoya at gmail.com>
Date: Thu, 11 Sep 2025 18:52:08 +0900
Subject: [PATCH 2/2] add a comment to describe why +1

---
 llvm/unittests/ProfileData/CoverageMappingTest.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/llvm/unittests/ProfileData/CoverageMappingTest.cpp b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
index 59c381a1b4e82..b268aa7cdd057 100644
--- a/llvm/unittests/ProfileData/CoverageMappingTest.cpp
+++ b/llvm/unittests/ProfileData/CoverageMappingTest.cpp
@@ -235,6 +235,7 @@ struct CoverageMappingTest : ::testing::TestWithParam<std::tuple<bool, bool>> {
 
   void readCoverageRegions(const std::string &Coverage,
                            OutputFunctionCoverageData &Data) {
+    // +1 here since `Files` (filename to index map) uses 1-based index.
     Data.FilenamesStorage.resize(Files.size() + 1);
     for (const auto &E : Files)
       Data.FilenamesStorage[E.getValue()] = E.getKey().str();