Skip to content

[lldb] Provide TrackingOutputBufferDeleter for custom unique_ptr deleter #142815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 5, 2025

Conversation

rupprecht
Copy link
Collaborator

Suggested in #142676 as a way to automatically free the buffer in tests

@rupprecht rupprecht requested a review from Michael137 June 4, 2025 16:56
@rupprecht rupprecht requested a review from JDevlieghere as a code owner June 4, 2025 16:57
@llvmbot llvmbot added the lldb label Jun 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 4, 2025

@llvm/pr-subscribers-lldb

Author: Jordan Rupprecht (rupprecht)

Changes

Suggested in #142676 as a way to automatically free the buffer in tests


Full diff: https://github.com/llvm/llvm-project/pull/142815.diff

2 Files Affected:

  • (modified) lldb/include/lldb/Core/DemangledNameInfo.h (+20)
  • (modified) lldb/unittests/Core/MangledTest.cpp (+36-36)
diff --git a/lldb/include/lldb/Core/DemangledNameInfo.h b/lldb/include/lldb/Core/DemangledNameInfo.h
index 4b5ba5e42b3b1..a3edb3e034eb1 100644
--- a/lldb/include/lldb/Core/DemangledNameInfo.h
+++ b/lldb/include/lldb/Core/DemangledNameInfo.h
@@ -13,6 +13,7 @@
 #include "llvm/Demangle/Utility.h"
 
 #include <cstddef>
+#include <cstdlib>
 #include <utility>
 
 namespace lldb_private {
@@ -159,4 +160,23 @@ struct TrackingOutputBuffer : public llvm::itanium_demangle::OutputBuffer {
 };
 } // namespace lldb_private
 
+/// Custom deleter to use with unique_ptr.
+///
+/// Usage:
+/// \code{.cpp}
+///
+/// auto OB =
+///     std::unique_ptr<TrackingOutputBuffer, TrackingOutputBufferDeleter>(
+///         new TrackingOutputBuffer());
+///
+/// \endcode
+struct TrackingOutputBufferDeleter {
+  void operator()(TrackingOutputBuffer *TOB) {
+    if (!TOB)
+      return;
+    std::free(TOB->getBuffer());
+    delete TOB;
+  }
+};
+
 #endif // LLDB_CORE_DEMANGLEDNAMEINFO_H
diff --git a/lldb/unittests/Core/MangledTest.cpp b/lldb/unittests/Core/MangledTest.cpp
index e9dfd05459642..42e732b79dedf 100644
--- a/lldb/unittests/Core/MangledTest.cpp
+++ b/lldb/unittests/Core/MangledTest.cpp
@@ -26,6 +26,8 @@
 
 #include "gtest/gtest.h"
 
+#include <memory>
+
 using namespace lldb;
 using namespace lldb_private;
 
@@ -589,25 +591,25 @@ TEST_P(DemanglingPartsTestFixture, DemanglingParts) {
 
   ASSERT_NE(nullptr, Root);
 
-  TrackingOutputBuffer OB;
-  Root->print(OB);
-  auto demangled = std::string_view(OB);
+  auto OB = std::unique_ptr<TrackingOutputBuffer, TrackingOutputBufferDeleter>(
+      new TrackingOutputBuffer());
+  Root->print(*OB);
+  auto demangled = std::string_view(*OB);
 
-  ASSERT_EQ(OB.NameInfo.hasBasename(), valid_basename);
+  ASSERT_EQ(OB->NameInfo.hasBasename(), valid_basename);
 
-  EXPECT_EQ(OB.NameInfo.BasenameRange, info.BasenameRange);
-  EXPECT_EQ(OB.NameInfo.ScopeRange, info.ScopeRange);
-  EXPECT_EQ(OB.NameInfo.ArgumentsRange, info.ArgumentsRange);
-  EXPECT_EQ(OB.NameInfo.QualifiersRange, info.QualifiersRange);
+  EXPECT_EQ(OB->NameInfo.BasenameRange, info.BasenameRange);
+  EXPECT_EQ(OB->NameInfo.ScopeRange, info.ScopeRange);
+  EXPECT_EQ(OB->NameInfo.ArgumentsRange, info.ArgumentsRange);
+  EXPECT_EQ(OB->NameInfo.QualifiersRange, info.QualifiersRange);
 
   auto get_part = [&](const std::pair<size_t, size_t> &loc) {
     return demangled.substr(loc.first, loc.second - loc.first);
   };
 
-  EXPECT_EQ(get_part(OB.NameInfo.BasenameRange), basename);
-  EXPECT_EQ(get_part(OB.NameInfo.ScopeRange), scope);
-  EXPECT_EQ(get_part(OB.NameInfo.QualifiersRange), qualifiers);
-  std::free(OB.getBuffer());
+  EXPECT_EQ(get_part(OB->NameInfo.BasenameRange), basename);
+  EXPECT_EQ(get_part(OB->NameInfo.ScopeRange), scope);
+  EXPECT_EQ(get_part(OB->NameInfo.QualifiersRange), qualifiers);
 }
 
 INSTANTIATE_TEST_SUITE_P(DemanglingPartsTests, DemanglingPartsTestFixture,
@@ -635,36 +637,35 @@ TEST_P(DemanglingInfoCorrectnessTestFixutre, Correctness) {
 
   ASSERT_NE(nullptr, Root);
 
-  TrackingOutputBuffer OB;
-  Root->print(OB);
+  auto OB = std::unique_ptr<TrackingOutputBuffer, TrackingOutputBufferDeleter>(
+      new TrackingOutputBuffer());
+  Root->print(*OB);
 
   // Filter out cases which would never show up in frames. We only care about
   // function names.
   if (Root->getKind() !=
           llvm::itanium_demangle::Node::Kind::KFunctionEncoding &&
-      Root->getKind() != llvm::itanium_demangle::Node::Kind::KDotSuffix) {
-    std::free(OB.getBuffer());
+      Root->getKind() != llvm::itanium_demangle::Node::Kind::KDotSuffix)
     return;
-  }
 
-  ASSERT_TRUE(OB.NameInfo.hasBasename());
-
-  auto tracked_name = llvm::StringRef(OB);
-
-  auto return_left = tracked_name.slice(0, OB.NameInfo.ScopeRange.first);
-  auto scope = tracked_name.slice(OB.NameInfo.ScopeRange.first,
-                                  OB.NameInfo.ScopeRange.second);
-  auto basename = tracked_name.slice(OB.NameInfo.BasenameRange.first,
-                                     OB.NameInfo.BasenameRange.second);
-  auto template_args = tracked_name.slice(OB.NameInfo.BasenameRange.second,
-                                          OB.NameInfo.ArgumentsRange.first);
-  auto args = tracked_name.slice(OB.NameInfo.ArgumentsRange.first,
-                                 OB.NameInfo.ArgumentsRange.second);
-  auto return_right = tracked_name.slice(OB.NameInfo.ArgumentsRange.second,
-                                         OB.NameInfo.QualifiersRange.first);
-  auto qualifiers = tracked_name.slice(OB.NameInfo.QualifiersRange.first,
-                                       OB.NameInfo.QualifiersRange.second);
-  auto suffix = tracked_name.slice(OB.NameInfo.QualifiersRange.second,
+  ASSERT_TRUE(OB->NameInfo.hasBasename());
+
+  auto tracked_name = llvm::StringRef(*OB);
+
+  auto return_left = tracked_name.slice(0, OB->NameInfo.ScopeRange.first);
+  auto scope = tracked_name.slice(OB->NameInfo.ScopeRange.first,
+                                  OB->NameInfo.ScopeRange.second);
+  auto basename = tracked_name.slice(OB->NameInfo.BasenameRange.first,
+                                     OB->NameInfo.BasenameRange.second);
+  auto template_args = tracked_name.slice(OB->NameInfo.BasenameRange.second,
+                                          OB->NameInfo.ArgumentsRange.first);
+  auto args = tracked_name.slice(OB->NameInfo.ArgumentsRange.first,
+                                 OB->NameInfo.ArgumentsRange.second);
+  auto return_right = tracked_name.slice(OB->NameInfo.ArgumentsRange.second,
+                                         OB->NameInfo.QualifiersRange.first);
+  auto qualifiers = tracked_name.slice(OB->NameInfo.QualifiersRange.first,
+                                       OB->NameInfo.QualifiersRange.second);
+  auto suffix = tracked_name.slice(OB->NameInfo.QualifiersRange.second,
                                    llvm::StringRef::npos);
 
   auto reconstructed_name =
@@ -672,7 +673,6 @@ TEST_P(DemanglingInfoCorrectnessTestFixutre, Correctness) {
                        return_right, qualifiers, suffix);
 
   EXPECT_EQ(reconstructed_name, demangled);
-  std::free(OB.getBuffer());
 }
 
 INSTANTIATE_TEST_SUITE_P(

/// new TrackingOutputBuffer());
///
/// \endcode
struct TrackingOutputBufferDeleter {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just put it into the test file for now? And put it back into the header once we actually need it elsewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@rupprecht rupprecht merged commit ae8ac77 into llvm:main Jun 5, 2025
10 checks passed
rorth pushed a commit to rorth/llvm-project that referenced this pull request Jun 11, 2025
…r deleter (llvm#142815)

Suggested in llvm#142676 as a way to automatically free the buffer in tests
DhruvSrivastavaX pushed a commit to DhruvSrivastavaX/lldb-for-aix that referenced this pull request Jun 12, 2025
…r deleter (llvm#142815)

Suggested in llvm#142676 as a way to automatically free the buffer in tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants