aprantl created this revision.
aprantl added reviewers: kastiglione, jingham, labath.
Herald added a project: All.
aprantl requested review of this revision.
When a process gets restarted TypeSystem objects associated with it may get
deleted, and any CompilerType objects holding on to a reference to that type
system are a use-after-free in waiting. Because of the SBAPI, we don't have
tight control over where CompilerTypes go and when they are used. This is
particularly a problem in the Swift plugin, where the scratch TypeSystem can be
restarted while the process is still running. The Swift plugin has a lock to
prevent abuse, but where there's a lock there can be bugs.
To help diagnose these hard-to-debug problems (and because Halloween is right
around the corner) this patch introduces a global TypeSystem graveyard that
collects all dead TypeSystem pointers and checks against it in
CompilerType::IsValid(). This is intended as a bug-finding tool, which is why
this triggers lldbassert(). Compared to everything else LLDB is doing the extra
DenseMap lookup and lock should be negligible, which is why this feature is
turned on even in release mode.
rdar://101505232
https://reviews.llvm.org/D136650
Files:
lldb/include/lldb/Symbol/CompilerType.h
lldb/include/lldb/Symbol/TypeSystem.h
lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
lldb/source/Symbol/CompilerType.cpp
lldb/source/Symbol/TypeSystem.cpp
lldb/unittests/Symbol/TestTypeSystemClang.cpp
Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp
===================================================================
--- lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -29,6 +29,7 @@
void SetUp() override {
m_ast.reset(
new TypeSystemClang("test ASTContext", HostInfo::GetTargetTriple()));
+ GetTypeSystemGraveyard().Erase(m_ast.get());
}
void TearDown() override { m_ast.reset(); }
Index: lldb/source/Symbol/TypeSystem.cpp
===================================================================
--- lldb/source/Symbol/TypeSystem.cpp
+++ lldb/source/Symbol/TypeSystem.cpp
@@ -17,6 +17,12 @@
using namespace lldb_private;
using namespace lldb;
+TypeSystemSet &lldb_private::GetTypeSystemGraveyard() {
+ // Intentionally leaked to avoid C++ destructor ordering issue.
+ static TypeSystemSet *g_typesystem_graveyard = new TypeSystemSet();
+ return *g_typesystem_graveyard;
+}
+
/// A 64-bit SmallBitVector is only small up to 64-7 bits, and the
/// setBitsInMask interface wants to write full bytes.
static const size_t g_num_small_bitvector_bits = 64 - 8;
@@ -35,7 +41,11 @@
bool LanguageSet::Empty() const { return bitvector.none(); }
bool LanguageSet::operator[](unsigned i) const { return bitvector[i]; }
-TypeSystem::~TypeSystem() = default;
+TypeSystem::TypeSystem() {
+ // Remove this pointer from the graveyard in case it got reused.
+ GetTypeSystemGraveyard().Erase(this);
+}
+void TypeSystem::Finalize() { GetTypeSystemGraveyard().Insert(this, false); }
static lldb::TypeSystemSP CreateInstanceHelper(lldb::LanguageType language,
Module *module, Target *target) {
Index: lldb/source/Symbol/CompilerType.cpp
===================================================================
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -11,11 +11,13 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/StreamFile.h"
#include "lldb/Symbol/Type.h"
+#include "lldb/Symbol/TypeSystem.h"
#include "lldb/Target/ExecutionContext.h"
#include "lldb/Target/Process.h"
#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/Scalar.h"
#include "lldb/Utility/Stream.h"
#include "lldb/Utility/StreamString.h"
@@ -26,6 +28,17 @@
using namespace lldb;
using namespace lldb_private;
+bool CompilerType::IsValid() const {
+ if (!m_type || !m_type_system)
+ return false;
+ bool unused;
+ if (GetTypeSystemGraveyard().Lookup(m_type_system, unused)) {
+ lldbassert(false && "CompilerType belongs to deleted Typesystem");
+ return false;
+ }
+ return true;
+}
+
// Tests
bool CompilerType::IsAggregateType() const {
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===================================================================
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -552,7 +552,8 @@
}
TypeSystemClang::TypeSystemClang(llvm::StringRef name,
- llvm::Triple target_triple) {
+ llvm::Triple target_triple)
+ : TypeSystem() {
m_display_name = name.str();
if (!target_triple.str().empty())
SetTargetTriple(target_triple.str());
@@ -562,7 +563,8 @@
}
TypeSystemClang::TypeSystemClang(llvm::StringRef name,
- ASTContext &existing_ctxt) {
+ ASTContext &existing_ctxt)
+ : TypeSystem() {
m_display_name = name.str();
SetTargetTriple(existing_ctxt.getTargetInfo().getTriple().str());
@@ -661,6 +663,7 @@
m_diagnostics_engine_up.reset();
m_source_manager_up.reset();
m_language_options_up.reset();
+ TypeSystem::Finalize();
}
void TypeSystemClang::setSema(Sema *s) {
Index: lldb/include/lldb/Symbol/TypeSystem.h
===================================================================
--- lldb/include/lldb/Symbol/TypeSystem.h
+++ lldb/include/lldb/Symbol/TypeSystem.h
@@ -21,6 +21,7 @@
#include "llvm/Support/Error.h"
#include "lldb/Core/PluginInterface.h"
+#include "lldb/Core/ThreadSafeDenseMap.h"
#include "lldb/Expression/Expression.h"
#include "lldb/Symbol/CompilerDecl.h"
#include "lldb/Symbol/CompilerDeclContext.h"
@@ -74,7 +75,8 @@
class TypeSystem : public PluginInterface {
public:
// Constructors and Destructors
- ~TypeSystem() override;
+ TypeSystem();
+ ~TypeSystem() = default;
// LLVM RTTI support
virtual bool isA(const void *ClassID) const = 0;
@@ -87,7 +89,7 @@
// Free up any resources associated with this TypeSystem. Done before
// removing all the TypeSystems from the TypeSystemMap.
- virtual void Finalize() {}
+ virtual void Finalize();
virtual DWARFASTParser *GetDWARFParser() { return nullptr; }
virtual PDBASTParser *GetPDBParser() { return nullptr; }
@@ -558,6 +560,12 @@
llvm::Optional<CreateCallback> create_callback = llvm::None);
};
+using TypeSystemSet =
+ ThreadSafeDenseMap<TypeSystem *, bool, std::recursive_mutex>;
+/// A set of type systems that have been deleted and can no longer be accessed.
+/// The bool value is unused.
+TypeSystemSet &GetTypeSystemGraveyard();
+
} // namespace lldb_private
#endif // LLDB_SYMBOL_TYPESYSTEM_H
Index: lldb/include/lldb/Symbol/CompilerType.h
===================================================================
--- lldb/include/lldb/Symbol/CompilerType.h
+++ lldb/include/lldb/Symbol/CompilerType.h
@@ -66,10 +66,10 @@
/// Tests.
/// \{
explicit operator bool() const {
- return m_type != nullptr && m_type_system != nullptr;
+ return IsValid();
}
- bool IsValid() const { return m_type != nullptr && m_type_system != nullptr; }
+ bool IsValid() const;
bool IsArrayType(CompilerType *element_type = nullptr,
uint64_t *size = nullptr,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
