https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/104799
>From 0c23c427f7154dabadbca65f64973ec2dbe365d9 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Mon, 19 Aug 2024 15:40:07 +0100
Subject: [PATCH 1/5] [lldb][ClangExpressionParser] Don't leak memory when
multiplexing ExternalASTSources
---
.../ExpressionParser/Clang/ASTUtils.cpp | 10 ++++--
.../Plugins/ExpressionParser/Clang/ASTUtils.h | 31 ++++++++++++++-----
.../Clang/ClangExpressionParser.cpp | 16 +++++-----
3 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index a95fce1c5aa966..c1d34c1d519a85 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,7 +8,10 @@
#include "ASTUtils.h"
-lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() = default;
+lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() {
+ if (m_owns_source)
+ m_Source->Release();
+}
void lldb_private::ExternalASTSourceWrapper::PrintStats() {
m_Source->PrintStats();
@@ -18,7 +21,10 @@ lldb_private::ASTConsumerForwarder::~ASTConsumerForwarder()
= default;
void lldb_private::ASTConsumerForwarder::PrintStats() { m_c->PrintStats(); }
-lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() = default;
+lldb_private::SemaSourceWithPriorities::~SemaSourceWithPriorities() {
+ for (auto * Source : Sources)
+ Source->Release();
+}
void lldb_private::SemaSourceWithPriorities::PrintStats() {
for (size_t i = 0; i < Sources.size(); ++i)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 79ed74b79f04fd..31345fe35e8621 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
#include "clang/Sema/MultiplexExternalSemaSource.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaConsumer.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include <optional>
namespace clang {
@@ -24,13 +25,17 @@ class Module;
namespace lldb_private {
-/// Wraps an ExternalASTSource into an ExternalSemaSource. Doesn't take
-/// ownership of the provided source.
+/// Wraps an ExternalASTSource into an ExternalSemaSource.
class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
ExternalASTSource *m_Source;
+
+ ///< If true, means that this class is responsible for
+ ///< decrementing the use count of m_Source.
+ bool m_owns_source = false;
public:
- ExternalASTSourceWrapper(ExternalASTSource *Source) : m_Source(Source) {
+ ExternalASTSourceWrapper(ExternalASTSource *Source, bool owns_source = false)
+ : m_Source(Source), m_owns_source(owns_source) {
assert(m_Source && "Can't wrap nullptr ExternalASTSource");
}
@@ -250,16 +255,26 @@ class SemaSourceWithPriorities : public
clang::ExternalSemaSource {
private:
/// The sources ordered in decreasing priority.
- llvm::SmallVector<clang::ExternalSemaSource *, 2> Sources;
+ llvm::SmallVector<clang::ExternalSemaSource*, 2>
+ Sources;
public:
/// Construct a SemaSourceWithPriorities with a 'high quality' source that
/// has the higher priority and a 'low quality' source that will be used
/// as a fallback.
- SemaSourceWithPriorities(clang::ExternalSemaSource &high_quality_source,
- clang::ExternalSemaSource &low_quality_source) {
- Sources.push_back(&high_quality_source);
- Sources.push_back(&low_quality_source);
+ ///
+ /// This class assumes shared ownership of the sources provided to it.
+ SemaSourceWithPriorities(
+ clang::ExternalSemaSource * high_quality_source,
+ clang::ExternalSemaSource * low_quality_source) {
+ assert(high_quality_source);
+ assert(low_quality_source);
+
+ high_quality_source->Retain();
+ low_quality_source->Retain();
+
+ Sources.push_back(high_quality_source);
+ Sources.push_back(low_quality_source);
}
~SemaSourceWithPriorities() override;
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index c441153d1efb05..b64613d214e157 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1221,18 +1221,20 @@ ClangExpressionParser::ParseInternal(DiagnosticManager
&diagnostic_manager,
decl_map->InstallCodeGenerator(&m_compiler->getASTConsumer());
decl_map->InstallDiagnosticManager(diagnostic_manager);
- clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
+ IntrusiveRefCntPtr<clang::ExternalASTSource> ast_source =
+ decl_map->CreateProxy();
if (ast_context.getExternalSource()) {
- auto module_wrapper =
+ auto *module_wrapper =
new ExternalASTSourceWrapper(ast_context.getExternalSource());
- auto ast_source_wrapper = new ExternalASTSourceWrapper(ast_source);
+ auto *ast_source_wrapper =
+ new ExternalASTSourceWrapper(ast_source.get(), true);
- auto multiplexer =
- new SemaSourceWithPriorities(*module_wrapper, *ast_source_wrapper);
- IntrusiveRefCntPtr<ExternalASTSource> Source(multiplexer);
- ast_context.setExternalSource(Source);
+ auto *multiplexer =
+ new SemaSourceWithPriorities(module_wrapper, ast_source_wrapper);
+
+ ast_context.setExternalSource(multiplexer);
} else {
ast_context.setExternalSource(ast_source);
}
>From 9ca4ebc63c723ef670698468095fbf53cb97d0f6 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Mon, 19 Aug 2024 16:31:52 +0100
Subject: [PATCH 2/5] fixup! remove redundant header
---
lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 31345fe35e8621..46ac5ee42a6dab 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,7 +14,6 @@
#include "clang/Sema/MultiplexExternalSemaSource.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaConsumer.h"
-#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include <optional>
namespace clang {
>From da394a71eb7ce9ab05a4adc763d28d1e49c07eac Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Mon, 19 Aug 2024 16:41:16 +0100
Subject: [PATCH 3/5] fixup! make ExternalASTSourceWrapper owning
---
.../Plugins/ExpressionParser/Clang/ASTUtils.cpp | 5 -----
lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h | 11 ++++-------
.../ExpressionParser/Clang/ClangExpressionParser.cpp | 4 ++--
3 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index c1d34c1d519a85..9619001be0d9af 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,11 +8,6 @@
#include "ASTUtils.h"
-lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() {
- if (m_owns_source)
- m_Source->Release();
-}
-
void lldb_private::ExternalASTSourceWrapper::PrintStats() {
m_Source->PrintStats();
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 46ac5ee42a6dab..4bad9ec8a5baf8 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -14,6 +14,7 @@
#include "clang/Sema/MultiplexExternalSemaSource.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaConsumer.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
#include <optional>
namespace clang {
@@ -26,15 +27,11 @@ namespace lldb_private {
/// Wraps an ExternalASTSource into an ExternalSemaSource.
class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
- ExternalASTSource *m_Source;
-
- ///< If true, means that this class is responsible for
- ///< decrementing the use count of m_Source.
- bool m_owns_source = false;
+ llvm::IntrusiveRefCntPtr<ExternalASTSource> m_Source;
public:
- ExternalASTSourceWrapper(ExternalASTSource *Source, bool owns_source = false)
- : m_Source(Source), m_owns_source(owns_source) {
+ explicit ExternalASTSourceWrapper(ExternalASTSource *Source)
+ : m_Source(Source) {
assert(m_Source && "Can't wrap nullptr ExternalASTSource");
}
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index b64613d214e157..ef8f64431f7300 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1221,7 +1221,7 @@ ClangExpressionParser::ParseInternal(DiagnosticManager
&diagnostic_manager,
decl_map->InstallCodeGenerator(&m_compiler->getASTConsumer());
decl_map->InstallDiagnosticManager(diagnostic_manager);
- IntrusiveRefCntPtr<clang::ExternalASTSource> ast_source =
+ clang::ExternalASTSource * ast_source =
decl_map->CreateProxy();
if (ast_context.getExternalSource()) {
@@ -1229,7 +1229,7 @@ ClangExpressionParser::ParseInternal(DiagnosticManager
&diagnostic_manager,
new ExternalASTSourceWrapper(ast_context.getExternalSource());
auto *ast_source_wrapper =
- new ExternalASTSourceWrapper(ast_source.get(), true);
+ new ExternalASTSourceWrapper(ast_source);
auto *multiplexer =
new SemaSourceWithPriorities(module_wrapper, ast_source_wrapper);
>From 856e4ae2a101dd4b0141413ce98f9f5481985c64 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Mon, 19 Aug 2024 16:41:26 +0100
Subject: [PATCH 4/5] fixup! clang-format
---
.../ExpressionParser/Clang/ClangExpressionParser.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
index ef8f64431f7300..d37787ebae26c3 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1221,15 +1221,13 @@ ClangExpressionParser::ParseInternal(DiagnosticManager
&diagnostic_manager,
decl_map->InstallCodeGenerator(&m_compiler->getASTConsumer());
decl_map->InstallDiagnosticManager(diagnostic_manager);
- clang::ExternalASTSource * ast_source =
- decl_map->CreateProxy();
+ clang::ExternalASTSource *ast_source = decl_map->CreateProxy();
if (ast_context.getExternalSource()) {
auto *module_wrapper =
new ExternalASTSourceWrapper(ast_context.getExternalSource());
- auto *ast_source_wrapper =
- new ExternalASTSourceWrapper(ast_source);
+ auto *ast_source_wrapper = new ExternalASTSourceWrapper(ast_source);
auto *multiplexer =
new SemaSourceWithPriorities(module_wrapper, ast_source_wrapper);
>From 86efdc138c8ac2db91a4a5a322b4fce5a5c9721d Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuc...@gmail.com>
Date: Mon, 19 Aug 2024 16:43:59 +0100
Subject: [PATCH 5/5] fixup! clang-format and add back destructor
---
lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp | 2 ++
lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h | 5 +++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
index 9619001be0d9af..edfea9776cfdbd 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.cpp
@@ -8,6 +8,8 @@
#include "ASTUtils.h"
+lldb_private::ExternalASTSourceWrapper::~ExternalASTSourceWrapper() = default;
+
void lldb_private::ExternalASTSourceWrapper::PrintStats() {
m_Source->PrintStats();
}
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index 4bad9ec8a5baf8..412cb194cc98a3 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -26,6 +26,8 @@ class Module;
namespace lldb_private {
/// Wraps an ExternalASTSource into an ExternalSemaSource.
+///
+/// Assumes shared ownership of the underlying source.
class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
llvm::IntrusiveRefCntPtr<ExternalASTSource> m_Source;
@@ -251,8 +253,7 @@ class SemaSourceWithPriorities : public
clang::ExternalSemaSource {
private:
/// The sources ordered in decreasing priority.
- llvm::SmallVector<clang::ExternalSemaSource*, 2>
- Sources;
+ llvm::SmallVector<clang::ExternalSemaSource *, 2> Sources;
public:
/// Construct a SemaSourceWithPriorities with a 'high quality' source that
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
