Author: Michael Buch Date: 2024-07-29T09:35:00+01:00 New Revision: d72c8b02802c87386f5db3c7de6c79e921618fa3
URL: https://github.com/llvm/llvm-project/commit/d72c8b02802c87386f5db3c7de6c79e921618fa3 DIFF: https://github.com/llvm/llvm-project/commit/d72c8b02802c87386f5db3c7de6c79e921618fa3.diff LOG: [lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (#100710) Depends on https://github.com/llvm/llvm-project/pull/100674 Currently, we treat VLAs declared as `int[]` and `int[0]` identically. I.e., we create them as `IncompleteArrayType`s. However, the `DW_AT_count` for `int[0]` *does* exist, and is retrievable without an execution context. This patch decouples the notion of "has 0 elements" from "has no known `DW_AT_count`". This aligns with how Clang represents `int[0]` in the AST (it treats it as a `ConstantArrayType` of 0 size). This issue was surfaced when adapting LLDB to https://github.com/llvm/llvm-project/issues/93069. There, the `__compressed_pair_padding` type has a `char[0]` member. If we previously got the `__compressed_pair_padding` out of a module (where clang represents `char[0]` as a `ConstantArrayType`), and try to merge the AST with one we got from DWARF (where LLDB used to represent `char[0]` as an `IncompleteArrayType`), the AST structural equivalence check fails, resulting in silent ASTImporter failures. This manifested in a failure in `TestNonModuleTypeSeparation.py`. **Implementation** 1. Adjust `ParseChildArrayInfo` to store the element counts of each VLA dimension as an `optional<uint64_t>`, instead of a regular `uint64_t`. So when we pass this down to `CreateArrayType`, we have a better heuristic for what is an `IncompleteArrayType`. 2. In `TypeSystemClang::GetBitSize`, if we encounter a `ConstantArrayType` simply return the size that it was created with. If we couldn't determine the authoritative bound from DWARF during parsing, we would've created an `IncompleteArrayType`. This ensures that `GetBitSize` on arrays with `DW_AT_count 0x0` returns `0` (whereas previously it would've returned a `std::nullopt`, causing that `FieldDecl` to just get dropped during printing) Added: lldb/test/Shell/SymbolFile/DWARF/vla.cpp Modified: lldb/include/lldb/Symbol/SymbolFile.h lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h lldb/test/API/lang/c/struct_types/main.c Removed: ################################################################################ diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index d20766788192f..8419495da73a2 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/lldb/include/lldb/Symbol/SymbolFile.h @@ -211,7 +211,15 @@ class SymbolFile : public PluginInterface { /// The characteristics of an array type. struct ArrayInfo { int64_t first_index = 0; - llvm::SmallVector<uint64_t, 1> element_orders; + + ///< Each entry belongs to a distinct DW_TAG_subrange_type. + ///< For multi-dimensional DW_TAG_array_types we would have + ///< an entry for each dimension. An entry represents the + ///< optional element count of the subrange. + /// + ///< The order of entries follows the order of the DW_TAG_subrange_type + ///< children of this DW_TAG_array_type. + llvm::SmallVector<std::optional<uint64_t>, 1> element_orders; uint32_t byte_stride = 0; uint32_t bit_stride = 0; }; diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp index 409e9bb37ab28..4ed523bbb9e76 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp @@ -37,7 +37,7 @@ DWARFASTParser::ParseChildArrayInfo(const DWARFDIE &parent_die, if (attributes.Size() == 0) continue; - uint64_t num_elements = 0; + std::optional<uint64_t> num_elements; uint64_t lower_bound = 0; uint64_t upper_bound = 0; bool upper_bound_valid = false; @@ -91,7 +91,7 @@ DWARFASTParser::ParseChildArrayInfo(const DWARFDIE &parent_die, } } - if (num_elements == 0) { + if (!num_elements || *num_elements == 0) { if (upper_bound_valid && upper_bound >= lower_bound) num_elements = upper_bound - lower_bound + 1; } diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp index 85c59a605c675..a4dcde1629fc2 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -1395,20 +1395,20 @@ DWARFASTParserClang::ParseArrayType(const DWARFDIE &die, uint64_t array_element_bit_stride = byte_stride * 8 + bit_stride; CompilerType clang_type; if (array_info && array_info->element_orders.size() > 0) { - uint64_t num_elements = 0; auto end = array_info->element_orders.rend(); for (auto pos = array_info->element_orders.rbegin(); pos != end; ++pos) { - num_elements = *pos; - clang_type = m_ast.CreateArrayType(array_element_type, num_elements, - attrs.is_vector); + clang_type = m_ast.CreateArrayType( + array_element_type, /*element_count=*/*pos, attrs.is_vector); + + uint64_t num_elements = pos->value_or(0); array_element_type = clang_type; array_element_bit_stride = num_elements ? array_element_bit_stride * num_elements : array_element_bit_stride; } } else { - clang_type = - m_ast.CreateArrayType(array_element_type, 0, attrs.is_vector); + clang_type = m_ast.CreateArrayType( + array_element_type, /*element_count=*/std::nullopt, attrs.is_vector); } ConstString empty_name; TypeSP type_sp = diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 0386e3b261c55..484ca04fe04c9 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -2233,30 +2233,31 @@ TypeSystemClang::CreateBlockPointerType(const CompilerType &function_type) { #pragma mark Array Types -CompilerType TypeSystemClang::CreateArrayType(const CompilerType &element_type, - size_t element_count, - bool is_vector) { - if (element_type.IsValid()) { - ASTContext &ast = getASTContext(); +CompilerType +TypeSystemClang::CreateArrayType(const CompilerType &element_type, + std::optional<size_t> element_count, + bool is_vector) { + if (!element_type.IsValid()) + return {}; - if (is_vector) { - return GetType(ast.getExtVectorType(ClangUtil::GetQualType(element_type), - element_count)); - } else { + ASTContext &ast = getASTContext(); - llvm::APInt ap_element_count(64, element_count); - if (element_count == 0) { - return GetType( - ast.getIncompleteArrayType(ClangUtil::GetQualType(element_type), - clang::ArraySizeModifier::Normal, 0)); - } else { - return GetType(ast.getConstantArrayType( - ClangUtil::GetQualType(element_type), ap_element_count, nullptr, - clang::ArraySizeModifier::Normal, 0)); - } - } - } - return CompilerType(); + // Unknown number of elements; this is an incomplete array + // (e.g., variable length array with non-constant bounds, or + // a flexible array member). + if (!element_count) + return GetType( + ast.getIncompleteArrayType(ClangUtil::GetQualType(element_type), + clang::ArraySizeModifier::Normal, 0)); + + if (is_vector) + return GetType(ast.getExtVectorType(ClangUtil::GetQualType(element_type), + *element_count)); + + llvm::APInt ap_element_count(64, *element_count); + return GetType(ast.getConstantArrayType(ClangUtil::GetQualType(element_type), + ap_element_count, nullptr, + clang::ArraySizeModifier::Normal, 0)); } CompilerType TypeSystemClang::CreateStructForIdentifier( @@ -4767,6 +4768,7 @@ TypeSystemClang::GetBitSize(lldb::opaque_compiler_type_t type, clang::QualType qual_type(GetCanonicalQualType(type)); const clang::Type::TypeClass type_class = qual_type->getTypeClass(); switch (type_class) { + case clang::Type::ConstantArray: case clang::Type::FunctionProto: case clang::Type::Record: return getASTContext().getTypeSize(qual_type); @@ -5457,9 +5459,9 @@ TypeSystemClang::GetNumChildren(lldb::opaque_compiler_type_t type, case clang::Type::IncompleteArray: if (auto array_info = GetDynamicArrayInfo(*this, GetSymbolFile(), qual_type, exe_ctx)) - // Only 1-dimensional arrays are supported. + // FIXME: Only 1-dimensional arrays are supported. num_children = array_info->element_orders.size() - ? array_info->element_orders.back() + ? array_info->element_orders.back().value_or(0) : 0; break; diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h index 70722eb375ab7..56a5c0a516706 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -498,7 +498,8 @@ class TypeSystemClang : public TypeSystem { // Array Types CompilerType CreateArrayType(const CompilerType &element_type, - size_t element_count, bool is_vector); + std::optional<size_t> element_count, + bool is_vector); // Enumeration Types CompilerType CreateEnumerationType(llvm::StringRef name, diff --git a/lldb/test/API/lang/c/struct_types/main.c b/lldb/test/API/lang/c/struct_types/main.c index e683c49108976..70217c57bec5f 100644 --- a/lldb/test/API/lang/c/struct_types/main.c +++ b/lldb/test/API/lang/c/struct_types/main.c @@ -1,3 +1,4 @@ +// clang-format off struct things_to_sum { int a; int b; @@ -18,7 +19,7 @@ int main (int argc, char const *argv[]) }; //% self.expect("frame variable pt.padding[0]", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["pt.padding[0] = "]) //% self.expect("frame variable pt.padding[1]", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["pt.padding[1] = "]) //% self.expect_expr("pt.padding[0]", result_type="char") - //% self.expect("image lookup -t point_tag", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ['padding[]']) + //% self.expect("image lookup -t point_tag", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ['padding[0]']) struct {} empty; //% self.expect("frame variable empty", substrs = ["empty = {}"]) diff --git a/lldb/test/Shell/SymbolFile/DWARF/vla.cpp b/lldb/test/Shell/SymbolFile/DWARF/vla.cpp new file mode 100644 index 0000000000000..344b100efd9f9 --- /dev/null +++ b/lldb/test/Shell/SymbolFile/DWARF/vla.cpp @@ -0,0 +1,80 @@ +// RUN: %clangxx_host -gdwarf -std=c++11 -o %t %s +// RUN: %lldb %t \ +// RUN: -o run \ +// RUN: -o "frame var --show-types f" \ +// RUN: -o "frame var vla0" \ +// RUN: -o "frame var fla0" \ +// RUN: -o "frame var fla1" \ +// RUN: -o "frame var vla01" \ +// RUN: -o "frame var vla10" \ +// RUN: -o "frame var vlaN" \ +// RUN: -o "frame var vlaNM" \ +// RUN: -o exit | FileCheck %s + +struct Foo { + static constexpr int n = 1; + int m_vlaN[n]; + + int m_vla0[0]; +}; + +int main() { + Foo f; + f.m_vlaN[0] = 60; + + // CHECK: (lldb) frame var --show-types f + // CHECK-NEXT: (Foo) f = { + // CHECK-NEXT: (int[1]) m_vlaN = { + // CHECK-NEXT: (int) [0] = 60 + // CHECK-NEXT: } + // CHECK-NEXT: (int[0]) m_vla0 = {} + // CHECK-NEXT: } + + int vla0[0] = {}; + + // CHECK: (lldb) frame var vla0 + // CHECK-NEXT: (int[0]) vla0 = {} + + int fla0[] = {}; + + // CHECK: (lldb) frame var fla0 + // CHECK-NEXT: (int[0]) fla0 = {} + + int fla1[] = {42}; + + // CHECK: (lldb) frame var fla1 + // CHECK-NEXT: (int[1]) fla1 = ([0] = 42) + + int vla01[0][1]; + + // CHECK: (lldb) frame var vla01 + // CHECK-NEXT: (int[0][1]) vla01 = {} + + int vla10[1][0]; + + // CHECK: (lldb) frame var vla10 + // CHECK-NEXT: (int[1][0]) vla10 = ([0] = int[0] + + int n = 3; + int vlaN[n]; + for (int i = 0; i < n; ++i) + vlaN[i] = -i; + + // CHECK: (lldb) frame var vlaN + // CHECK-NEXT: (int[]) vlaN = ([0] = 0, [1] = -1, [2] = -2) + + int m = 2; + int vlaNM[n][m]; + for (int i = 0; i < n; ++i) + for (int j = 0; j < m; ++j) + vlaNM[i][j] = i + j; + + // FIXME: multi-dimensional VLAs aren't well supported + // CHECK: (lldb) frame var vlaNM + // CHECK-NEXT: (int[][]) vlaNM = { + // CHECK-NEXT: [0] = ([0] = 0, [1] = 1, [2] = 1) + // CHECK-NEXT: [1] = ([0] = 1, [1] = 1, [2] = 2) + // CHECK-NEXT: } + + __builtin_debugtrap(); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
