[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add warning and defensive checks when ASTContext is not fully initialized (PR #110481)

Michael Buch via lldb-commits lldb-commits at lists.llvm.org
Mon Sep 30 06:12:21 PDT 2024


https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/110481

>From 0669f9fbeddaf81edfca7e81f4883a1b95a780f6 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 30 Sep 2024 10:53:47 +0100
Subject: [PATCH 1/5] [lldb][TypeSystemClang] Add warning and defensive checks
 when ASTContext is not fully initialized

---
 .../TypeSystem/Clang/TypeSystemClang.cpp      | 28 ++++++++++++++++---
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index b0f49ebf2d2cbb..2f423269fecd2d 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,19 @@ 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 +759,11 @@ CompilerType
 TypeSystemClang::GetBuiltinTypeForEncodingAndBitSize(Encoding encoding,
                                                      size_t bit_size) {
   ASTContext &ast = getASTContext();
+  if (!ast.VoidPtrTy) {
+    LLDB_LOG(GetLog(LLDBLog::Expressions), "{0} failed: builtin types on ASTContext were not initialized properly.", __func__);
+    return {};
+  }
+
   switch (encoding) {
   case eEncodingInvalid:
     if (QualTypeMatchesBitSize(bit_size, ast, ast.VoidPtrTy))
@@ -891,6 +906,11 @@ CompilerType TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
     llvm::StringRef type_name, uint32_t dw_ate, uint32_t bit_size) {
   ASTContext &ast = getASTContext();
 
+  if (!ast.VoidPtrTy) {
+    LLDB_LOG(GetLog(LLDBLog::Expressions), "{0} failed: builtin types on ASTContext were not initialized properly.", __func__);
+    return {};
+  }
+
   switch (dw_ate) {
   default:
     break;

>From 46c4015cd76709bae4b9e38eff312ad58225b62a Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 30 Sep 2024 11:16:06 +0100
Subject: [PATCH 2/5] fixup! format

---
 lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 2f423269fecd2d..9ba5d0a79445e8 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -759,6 +759,7 @@ CompilerType
 TypeSystemClang::GetBuiltinTypeForEncodingAndBitSize(Encoding encoding,
                                                      size_t bit_size) {
   ASTContext &ast = getASTContext();
+
   if (!ast.VoidPtrTy) {
     LLDB_LOG(GetLog(LLDBLog::Expressions), "{0} failed: builtin types on ASTContext were not initialized properly.", __func__);
     return {};

>From caed9b07141176ab202d8c95bf0ffa4f59dce1d3 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 30 Sep 2024 11:16:31 +0100
Subject: [PATCH 3/5] fixup! clang-format

---
 .../Plugins/TypeSystem/Clang/TypeSystemClang.cpp    | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 9ba5d0a79445e8..02d6675ddc811e 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -709,7 +709,8 @@ void TypeSystemClang::CreateASTContext() {
     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);
+    Debugger::ReportWarning(std::move(err), /*debugger_id=*/std::nullopt,
+                            &s_uninitialized_target_warning);
   }
 
   GetASTMap().Insert(m_ast_up.get(), this);
@@ -761,7 +762,10 @@ TypeSystemClang::GetBuiltinTypeForEncodingAndBitSize(Encoding encoding,
   ASTContext &ast = getASTContext();
 
   if (!ast.VoidPtrTy) {
-    LLDB_LOG(GetLog(LLDBLog::Expressions), "{0} failed: builtin types on ASTContext were not initialized properly.", __func__);
+    LLDB_LOG(GetLog(LLDBLog::Expressions),
+             "{0} failed: builtin types on ASTContext were not initialized "
+             "properly.",
+             __func__);
     return {};
   }
 
@@ -908,7 +912,10 @@ CompilerType TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
   ASTContext &ast = getASTContext();
 
   if (!ast.VoidPtrTy) {
-    LLDB_LOG(GetLog(LLDBLog::Expressions), "{0} failed: builtin types on ASTContext were not initialized properly.", __func__);
+    LLDB_LOG(GetLog(LLDBLog::Expressions),
+             "{0} failed: builtin types on ASTContext were not initialized "
+             "properly.",
+             __func__);
     return {};
   }
 

>From 34c93ec9a1e22c6623f8106e1e6f5163804ba087 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 30 Sep 2024 12:52:44 +0100
Subject: [PATCH 4/5] fixup! add tests; add more checks

---
 .../TypeSystem/Clang/TypeSystemClang.cpp      | 16 ++++++++++++++
 lldb/unittests/Symbol/TestTypeSystemClang.cpp | 22 +++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 02d6675ddc811e..79752e115fec0d 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2363,6 +2363,14 @@ CompilerType TypeSystemClang::GetIntTypeFromBitSize(size_t bit_size,
                                                     bool is_signed) {
   clang::ASTContext &ast = getASTContext();
 
+  if (!ast.VoidPtrTy) {
+    LLDB_LOG(GetLog(LLDBLog::Expressions),
+             "{0} failed: builtin types on ASTContext were not initialized "
+             "properly.",
+             __func__);
+    return {};
+  }
+
   if (is_signed) {
     if (bit_size == ast.getTypeSize(ast.SignedCharTy))
       return GetType(ast.SignedCharTy);
@@ -2404,6 +2412,14 @@ CompilerType TypeSystemClang::GetIntTypeFromBitSize(size_t bit_size,
 }
 
 CompilerType TypeSystemClang::GetPointerSizedIntType(bool is_signed) {
+  if (!getASTContext().VoidPtrTy) {
+    LLDB_LOG(GetLog(LLDBLog::Expressions),
+             "{0} failed: builtin types on ASTContext were not initialized "
+             "properly.",
+             __func__);
+    return {};
+  }
+
   return GetIntTypeFromBitSize(
       getASTContext().getTypeSize(getASTContext().VoidPtrTy), is_signed);
 }
diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index 7d64e1cdd56f64..62564f15b3ede8 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,27 @@ TEST_F(TestTypeSystemClang, TestBuiltinTypeForEncodingAndBitSize) {
   VerifyEncodingAndBitSize(*m_ast, eEncodingIEEE754, 64);
 }
 
+TEST_F(TestTypeSystemClang, TestBuiltinTypeForEmptyTriple) {
+  // TODO:
+  // * docs
+  // * use all the relevant APIs that dereference builtins
+
+  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));
+}
+
 TEST_F(TestTypeSystemClang, TestDisplayName) {
   TypeSystemClang ast("some name", llvm::Triple());
   EXPECT_EQ("some name", ast.getDisplayName());

>From b7c358d40be57f96723abb34905a33eb31303d55 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Mon, 30 Sep 2024 14:09:47 +0100
Subject: [PATCH 5/5] fixup! remove logs; add more checks

---
 .../TypeSystem/Clang/TypeSystemClang.cpp      | 37 ++++++-------------
 lldb/unittests/Symbol/TestTypeSystemClang.cpp |  6 +--
 2 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 79752e115fec0d..bfc7d517432b2e 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -761,13 +761,8 @@ TypeSystemClang::GetBuiltinTypeForEncodingAndBitSize(Encoding encoding,
                                                      size_t bit_size) {
   ASTContext &ast = getASTContext();
 
-  if (!ast.VoidPtrTy) {
-    LLDB_LOG(GetLog(LLDBLog::Expressions),
-             "{0} failed: builtin types on ASTContext were not initialized "
-             "properly.",
-             __func__);
+  if (!ast.VoidPtrTy)
     return {};
-  }
 
   switch (encoding) {
   case eEncodingInvalid:
@@ -911,13 +906,8 @@ CompilerType TypeSystemClang::GetBuiltinTypeForDWARFEncodingAndBitSize(
     llvm::StringRef type_name, uint32_t dw_ate, uint32_t bit_size) {
   ASTContext &ast = getASTContext();
 
-  if (!ast.VoidPtrTy) {
-    LLDB_LOG(GetLog(LLDBLog::Expressions),
-             "{0} failed: builtin types on ASTContext were not initialized "
-             "properly.",
-             __func__);
+  if (!ast.VoidPtrTy)
     return {};
-  }
 
   switch (dw_ate) {
   default:
@@ -2363,13 +2353,8 @@ CompilerType TypeSystemClang::GetIntTypeFromBitSize(size_t bit_size,
                                                     bool is_signed) {
   clang::ASTContext &ast = getASTContext();
 
-  if (!ast.VoidPtrTy) {
-    LLDB_LOG(GetLog(LLDBLog::Expressions),
-             "{0} failed: builtin types on ASTContext were not initialized "
-             "properly.",
-             __func__);
+  if (!ast.VoidPtrTy)
     return {};
-  }
 
   if (is_signed) {
     if (bit_size == ast.getTypeSize(ast.SignedCharTy))
@@ -2412,13 +2397,8 @@ CompilerType TypeSystemClang::GetIntTypeFromBitSize(size_t bit_size,
 }
 
 CompilerType TypeSystemClang::GetPointerSizedIntType(bool is_signed) {
-  if (!getASTContext().VoidPtrTy) {
-    LLDB_LOG(GetLog(LLDBLog::Expressions),
-             "{0} failed: builtin types on ASTContext were not initialized "
-             "properly.",
-             __func__);
+  if (!getASTContext().VoidPtrTy)
     return {};
-  }
 
   return GetIntTypeFromBitSize(
       getASTContext().getTypeSize(getASTContext().VoidPtrTy), is_signed);
@@ -7497,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)
@@ -8525,7 +8512,7 @@ bool TypeSystemClang::CompleteTagDeclarationDefinition(
   /// TODO This really needs to be fixed.
 
   QualType integer_type(enum_decl->getIntegerType());
-  if (!integer_type.isNull()) {
+  if (!integer_type.isNull() && !ast.IntTy.isNull()) {
     unsigned NumPositiveBits = 1;
     unsigned NumNegativeBits = 0;
 
diff --git a/lldb/unittests/Symbol/TestTypeSystemClang.cpp b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
index 62564f15b3ede8..065c35e7d27376 100644
--- a/lldb/unittests/Symbol/TestTypeSystemClang.cpp
+++ b/lldb/unittests/Symbol/TestTypeSystemClang.cpp
@@ -233,9 +233,9 @@ TEST_F(TestTypeSystemClang, TestBuiltinTypeForEncodingAndBitSize) {
 }
 
 TEST_F(TestTypeSystemClang, TestBuiltinTypeForEmptyTriple) {
-  // TODO:
-  // * docs
-  // * use all the relevant APIs that dereference builtins
+  // 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{});
 



More information about the lldb-commits mailing list