[Lldb-commits] [lldb] ea96037 - Add a verification mechanism to CompilerType.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-03-11T12:43:32-07:00
New Revision: ea960371861acad11b7018a5e280ae7a41ab9c02

URL: https://github.com/llvm/llvm-project/commit/ea960371861acad11b7018a5e280ae7a41ab9c02
DIFF: https://github.com/llvm/llvm-project/commit/ea960371861acad11b7018a5e280ae7a41ab9c02.diff

LOG: Add a verification mechanism to CompilerType.

Badly-written code can combine an unrelated TypeSystem and opaque type
pointer into a CompilerType. This is particularly an issue in
swift-lldb. This patch adds an assertion mechanism that catches these
kinds of mistakes early. Because this is an assertion-only code path
there is not cost for release builds.

Differential Revision: https://reviews.llvm.org/D76011

Added: 
    

Modified: 
    lldb/include/lldb/Symbol/CompilerType.h
    lldb/include/lldb/Symbol/TypeSystem.h
    lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
    lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
    lldb/source/Symbol/CompilerType.cpp
    lldb/source/Symbol/TypeSystem.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Symbol/CompilerType.h b/lldb/include/lldb/Symbol/CompilerType.h
index 0d536e090d20..0d1b00a1b26c 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -39,7 +39,9 @@ class CompilerType {
   ///
   /// \see lldb_private::TypeSystemClang::GetType(clang::QualType)
   CompilerType(TypeSystem *type_system, lldb::opaque_compiler_type_t type)
-      : m_type(type), m_type_system(type_system) {}
+      : m_type(type), m_type_system(type_system) {
+    assert(Verify() && "verification failed");
+  }
 
   CompilerType(const CompilerType &rhs)
       : m_type(rhs.m_type), m_type_system(rhs.m_type_system) {}
@@ -380,6 +382,13 @@ class CompilerType {
   }
 
 private:
+#ifndef NDEBUG
+  /// If the type is valid, ask the TypeSystem to verify the integrity
+  /// of the type to catch CompilerTypes that mix and match invalid
+  /// TypeSystem/Opaque type pairs.
+  bool Verify() const;
+#endif
+
   lldb::opaque_compiler_type_t m_type = nullptr;
   TypeSystem *m_type_system = nullptr;
 };

diff  --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h
index 37c87a4bd025..a84b9a1c441c 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -129,6 +129,11 @@ class TypeSystem : public PluginInterface {
                                               void *other_opaque_decl_ctx) = 0;
 
   // Tests
+#ifndef NDEBUG
+  /// Verify the integrity of the type to catch CompilerTypes that mix
+  /// and match invalid TypeSystem/Opaque type pairs.
+  virtual bool Verify(lldb::opaque_compiler_type_t type) = 0;
+#endif
 
   virtual bool IsArrayType(lldb::opaque_compiler_type_t type,
                            CompilerType *element_type, uint64_t *size,

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 47f1a852289a..42655ad6cc24 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2532,6 +2532,12 @@ ConvertAccessTypeToObjCIvarAccessControl(AccessType access) {
 
 // Tests
 
+#ifndef NDEBUG
+bool TypeSystemClang::Verify(lldb::opaque_compiler_type_t type) {
+  return !type || llvm::isa<clang::Type>(GetQualType(type).getTypePtr());
+}
+#endif
+
 bool TypeSystemClang::IsAggregateType(lldb::opaque_compiler_type_t type) {
   clang::QualType qual_type(RemoveWrappingTypes(GetCanonicalQualType(type)));
 

diff  --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 75e749f5783f..e22635dce9d0 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -495,6 +495,10 @@ class TypeSystemClang : public TypeSystem {
 
   // Tests
 
+#ifndef NDEBUG
+  bool Verify(lldb::opaque_compiler_type_t type) override;
+#endif
+  
   bool IsArrayType(lldb::opaque_compiler_type_t type,
                    CompilerType *element_type, uint64_t *size,
                    bool *is_incomplete) override;

diff  --git a/lldb/source/Symbol/CompilerType.cpp b/lldb/source/Symbol/CompilerType.cpp
index 1507f2c3121b..6c150988a012 100644
--- a/lldb/source/Symbol/CompilerType.cpp
+++ b/lldb/source/Symbol/CompilerType.cpp
@@ -865,6 +865,12 @@ bool CompilerType::GetValueAsScalar(const lldb_private::DataExtractor &data,
   return false;
 }
 
+#ifndef NDEBUG
+bool CompilerType::Verify() const {
+  return !IsValid() || m_type_system->Verify(m_type);
+}
+#endif
+
 bool lldb_private::operator==(const lldb_private::CompilerType &lhs,
                               const lldb_private::CompilerType &rhs) {
   return lhs.GetTypeSystem() == rhs.GetTypeSystem() &&

diff  --git a/lldb/source/Symbol/TypeSystem.cpp b/lldb/source/Symbol/TypeSystem.cpp
index b0131e988363..fd5b9613f717 100644
--- a/lldb/source/Symbol/TypeSystem.cpp
+++ b/lldb/source/Symbol/TypeSystem.cpp
@@ -70,6 +70,10 @@ lldb::TypeSystemSP TypeSystem::CreateInstance(lldb::LanguageType language,
   return CreateInstanceHelper(language, nullptr, target);
 }
 
+#ifndef NDEBUG
+bool TypeSystem::Verify(lldb::opaque_compiler_type_t type) { return true; }
+#endif
+
 bool TypeSystem::IsAnonymousType(lldb::opaque_compiler_type_t type) {
   return false;
 }