Skip to content

Commit

Permalink
Add a verification mechanism to CompilerType.
Browse files Browse the repository at this point in the history
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

(cherry picked from commit ea96037)
  • Loading branch information
adrian-prantl committed Mar 11, 2020
1 parent 11a1904 commit 46a6e83
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 1 deletion.
11 changes: 10 additions & 1 deletion lldb/include/lldb/Symbol/CompilerType.h
Expand Up @@ -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) {}
Expand Down Expand Up @@ -375,6 +377,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;
};
Expand Down
5 changes: 5 additions & 0 deletions lldb/include/lldb/Symbol/TypeSystem.h
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions lldb/include/lldb/Symbol/TypeSystemClang.h
Expand Up @@ -463,6 +463,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;
Expand Down
6 changes: 6 additions & 0 deletions lldb/source/Symbol/CompilerType.cpp
Expand Up @@ -874,6 +874,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() &&
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/Symbol/TypeSystem.cpp
Expand Up @@ -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;
}
Expand Down
6 changes: 6 additions & 0 deletions lldb/source/Symbol/TypeSystemClang.cpp
Expand Up @@ -2531,6 +2531,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)));

Expand Down

0 comments on commit 46a6e83

Please sign in to comment.