From a5f3a2a8414a16077ca9a5544d30dd44b30b901e Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Tue, 1 Oct 2024 10:57:20 +0100 Subject: [PATCH] [lldb][TypeSystemClang] Add warning and defensive checks when ASTContext is not fully initialized (#110481) As this comment around target initialization implies: ``` // This can be NULL if we don't know anything about the architecture or if // the target for an architecture isn't enabled in the llvm/clang that we // built ``` There are cases where we might fail to call `InitBuiltinTypes` when creating the backing `ASTContext` for a `TypeSystemClang`. If that happens, the builtins `QualType`s, e.g., `VoidPtrTy`/`IntTy`/etc., are not initialized and dereferencing them as we do in `GetBuiltinTypeForEncodingAndBitSize` (and other places) will lead to nullptr-dereferences. Example backtrace: ``` (lldb) run Assertion failed: (!isNull() && "Cannot retrieve a NULL type pointer"), function getCommonPtr, file Type.h, line 958. Process 2680 stopped * thread #15, name = '', stop reason = hit program assert frame #4: 0x000000010cdf3cdc liblldb.20.0.0git.dylib`DWARFASTParserClang::ExtractIntFromFormValue(lldb_private::CompilerType const&, lldb_private::plugin::dwarf::DWARFFormValue const&) const (.cold.1) + liblldb.20.0.0git.dylib`DWARFASTParserClang::ParseObjCMethod(lldb_private::ObjCLanguage::MethodName const&, lldb_private::plugin::dwarf::DWARFDIE const&, lldb_private::CompilerType, ParsedDWARFTypeAttributes , bool) (.cold.1): -> 0x10cdf3cdc <+0>: stp x29, x30, [sp, #-0x10]! 0x10cdf3ce0 <+4>: mov x29, sp 0x10cdf3ce4 <+8>: adrp x0, 545 0x10cdf3ce8 <+12>: add x0, x0, #0xa25 ; "ParseObjCMethod" Target 0: (lldb) stopped. (lldb) bt * thread #15, name = '', stop reason = hit program assert frame #0: 0x0000000180d08600 libsystem_kernel.dylib`__pthread_kill + 8 frame #1: 0x0000000180d40f50 libsystem_pthread.dylib`pthread_kill + 288 frame #2: 0x0000000180c4d908 libsystem_c.dylib`abort + 128 frame #3: 0x0000000180c4cc1c libsystem_c.dylib`__assert_rtn + 284 * frame #4: 0x000000010cdf3cdc liblldb.20.0.0git.dylib`DWARFASTParserClang::ExtractIntFromFormValue(lldb_private::CompilerType const&, lldb_private::plugin::dwarf::DWARFFormValue const&) const (.cold.1) + frame #5: 0x0000000109d30acc liblldb.20.0.0git.dylib`lldb_private::TypeSystemClang::GetBuiltinTypeForEncodingAndBitSize(lldb::Encoding, unsigned long) + 1188 frame #6: 0x0000000109aaaed4 liblldb.20.0.0git.dylib`DynamicLoaderMacOS::NotifyBreakpointHit(void*, lldb_private::StoppointCallbackContext*, unsigned long long, unsigned long long) + 384 ``` This patch adds a one-time user-visible warning for when we fail to initialize the AST to indicate that initialization went wrong for the given target. Additionally, we add checks for whether one of the `ASTContext` `QualType`s is invalid before dereferencing any builtin types. The warning would look as follows: ``` (lldb) target create "a.out" Current executable set to 'a.out' (arm64). (lldb) b main warning: Failed to initialize builtin ASTContext types for target 'some-unknown-triple'. Printing variables may behave unexpectedly. Breakpoint 1: where = a.out`main + 8 at stepping.cpp:5:14, address = 0x0000000100003f90 ``` rdar://134869779 --- .../TypeSystem/Clang/TypeSystemClang.cpp | 39 +++++++++++++++++-- lldb/unittests/Symbol/TestTypeSystemClang.cpp | 32 +++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 264b2e84114077..7097ab2dcb230b 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -54,6 +54,7 @@ #include "Plugins/ExpressionParser/Clang/ClangUserExpression.h" #include "Plugins/ExpressionParser/Clang/ClangUtil.h" #include "Plugins/ExpressionParser/Clang/ClangUtilityFunction.h" +#include "lldb/Core/Debugger.h" #include "lldb/Core/DumpDataExtractor.h" #include "lldb/Core/Module.h" #include "lldb/Core/PluginManager.h" @@ -697,10 +698,20 @@ void TypeSystemClang::CreateASTContext() { TargetInfo *target_info = getTargetInfo(); if (target_info) m_ast_up->InitBuiltinTypes(*target_info); - else if (auto *log = GetLog(LLDBLog::Expressions)) - LLDB_LOG(log, - "Failed to initialize builtin ASTContext types for target '{0}'", - m_target_triple); + else { + std::string err = + llvm::formatv( + "Failed to initialize builtin ASTContext types for target '{0}'. " + "Printing variables may behave unexpectedly.", + m_target_triple) + .str(); + + LLDB_LOG(GetLog(LLDBLog::Expressions), err.c_str()); + + static std::once_flag s_uninitialized_target_warning; + Debugger::ReportWarning(std::move(err), /*debugger_id=*/std::nullopt, + &s_uninitialized_target_warning); + } GetASTMap().Insert(m_ast_up.get(), this); @@ -749,6 +760,10 @@ CompilerType TypeSystemClang::GetBuiltinTypeForEncodingAndBitSize(Encoding encoding, size_t bit_size) { ASTContext &ast = getASTContext(); + + if (!ast.VoidPtrTy) + return {}; + switch (encoding) { case eEncodingInvalid: if (QualTypeMatchesBitSize(bit_size, ast, ast.VoidPtrTy)) @@ -891,6 +906,9 @@ CompilerType TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize( llvm::StringRef type_name, uint32_t dw_ate, uint32_t bit_size) { ASTContext &ast = getASTContext(); + if (!ast.VoidPtrTy) + return {}; + switch (dw_ate) { default: break; @@ -2335,6 +2353,9 @@ CompilerType TypeSystemClang::GetIntTypeFromBitSize(size_t bit_size, bool is_signed) { clang::ASTContext &ast = getASTContext(); + if (!ast.VoidPtrTy) + return {}; + if (is_signed) { if (bit_size == ast.getTypeSize(ast.SignedCharTy)) return GetType(ast.SignedCharTy); @@ -2376,6 +2397,9 @@ CompilerType TypeSystemClang::GetIntTypeFromBitSize(size_t bit_size, } CompilerType TypeSystemClang::GetPointerSizedIntType(bool is_signed) { + if (!getASTContext().VoidPtrTy) + return {}; + return GetIntTypeFromBitSize( getASTContext().getTypeSize(getASTContext().VoidPtrTy), is_signed); } @@ -7453,6 +7477,13 @@ clang::FieldDecl *TypeSystemClang::AddFieldToRecordType( clang::Expr *bit_width = nullptr; if (bitfield_bit_size != 0) { + if (clang_ast.IntTy.isNull()) { + LLDB_LOG( + GetLog(LLDBLog::Expressions), + "{0} failed: builtin ASTContext types have not been initialized"); + return nullptr; + } + llvm::APInt bitfield_bit_size_apint(clang_ast.getTypeSize(clang_ast.IntTy), bitfield_bit_size); bit_width = new (clang_ast) diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp index 7d64e1cdd56f64..0733e42bb46331 100644 --- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -13,6 +13,7 @@ #include "lldb/Core/Declaration.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" +#include "lldb/lldb-enumerations.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/ExprCXX.h" @@ -231,6 +232,37 @@ TEST_F(TestTypeSystemClang, TestBuiltinTypeForEncodingAndBitSize) { VerifyEncodingAndBitSize(*m_ast, eEncodingIEEE754, 64); } +TEST_F(TestTypeSystemClang, TestBuiltinTypeForEmptyTriple) { + // Test that we can access type-info of builtin Clang AST + // types without crashing even when the target triple is + // empty. + + TypeSystemClang ast("empty triple AST", llvm::Triple{}); + + // This test only makes sense if the builtin ASTContext types were + // not initialized. + ASSERT_TRUE(ast.getASTContext().VoidPtrTy.isNull()); + + EXPECT_FALSE(ast.GetBuiltinTypeByName(ConstString("int")).IsValid()); + EXPECT_FALSE(ast.GetBuiltinTypeForDWARFEncodingAndBitSize( + "char", dwarf::DW_ATE_signed_char, 8) + .IsValid()); + EXPECT_FALSE(ast.GetBuiltinTypeForEncodingAndBitSize(lldb::eEncodingUint, 8) + .IsValid()); + EXPECT_FALSE(ast.GetPointerSizedIntType(/*is_signed=*/false)); + EXPECT_FALSE(ast.GetIntTypeFromBitSize(8, /*is_signed=*/false)); + + CompilerType record_type = ast.CreateRecordType( + nullptr, OptionalClangModuleID(), lldb::eAccessPublic, "Record", + llvm::to_underlying(clang::TagTypeKind::Struct), + lldb::eLanguageTypeC_plus_plus, std::nullopt); + TypeSystemClang::StartTagDeclarationDefinition(record_type); + EXPECT_EQ(ast.AddFieldToRecordType(record_type, "field", record_type, + eAccessPublic, /*bitfield_bit_size=*/8), + nullptr); + TypeSystemClang::CompleteTagDeclarationDefinition(record_type); +} + TEST_F(TestTypeSystemClang, TestDisplayName) { TypeSystemClang ast("some name", llvm::Triple()); EXPECT_EQ("some name", ast.getDisplayName());