Skip to content

Commit

Permalink
[lldb][TypeSystemClang] Add warning and defensive checks when ASTCont…
Browse files Browse the repository at this point in the history
…ext is not fully initialized (llvm#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 rust-lang#15, name = '<lldb.process.internal-state(pid=2712)>', stop reason = hit program assert
    frame rust-lang#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 rust-lang#15, name = '<lldb.process.internal-state(pid=2712)>', stop reason = hit program assert
    frame #0: 0x0000000180d08600 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x0000000180d40f50 libsystem_pthread.dylib`pthread_kill + 288
    frame rust-lang#2: 0x0000000180c4d908 libsystem_c.dylib`abort + 128
    frame rust-lang#3: 0x0000000180c4cc1c libsystem_c.dylib`__assert_rtn + 284
  * frame rust-lang#4: 0x000000010cdf3cdc liblldb.20.0.0git.dylib`DWARFASTParserClang::ExtractIntFromFormValue(lldb_private::CompilerType const&, lldb_private::plugin::dwarf::DWARFFormValue const&) const (.cold.1) + 
    frame rust-lang#5: 0x0000000109d30acc liblldb.20.0.0git.dylib`lldb_private::TypeSystemClang::GetBuiltinTypeForEncodingAndBitSize(lldb::Encoding, unsigned long) + 1188
    frame rust-lang#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
  • Loading branch information
Michael137 authored Oct 1, 2024
1 parent 47861fa commit a5f3a2a
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 4 deletions.
39 changes: 35 additions & 4 deletions lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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)
Expand Down
32 changes: 32 additions & 0 deletions lldb/unittests/Symbol/TestTypeSystemClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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());
Expand Down

0 comments on commit a5f3a2a

Please sign in to comment.