Skip to content

Commit e1a9c0d

Browse files
committed
[lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (llvm#100710)
Depends on llvm#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 llvm#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) (cherry picked from commit d72c8b0)
1 parent c0c3562 commit e1a9c0d

File tree

7 files changed

+127
-35
lines changed

7 files changed

+127
-35
lines changed

lldb/include/lldb/Symbol/SymbolFile.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,15 @@ class SymbolFile : public PluginInterface {
217217
/// The characteristics of an array type.
218218
struct ArrayInfo {
219219
int64_t first_index = 0;
220-
llvm::SmallVector<uint64_t, 1> element_orders;
220+
221+
///< Each entry belongs to a distinct DW_TAG_subrange_type.
222+
///< For multi-dimensional DW_TAG_array_types we would have
223+
///< an entry for each dimension. An entry represents the
224+
///< optional element count of the subrange.
225+
///
226+
///< The order of entries follows the order of the DW_TAG_subrange_type
227+
///< children of this DW_TAG_array_type.
228+
llvm::SmallVector<std::optional<uint64_t>, 1> element_orders;
221229
uint32_t byte_stride = 0;
222230
uint32_t bit_stride = 0;
223231
};

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ DWARFASTParser::ParseChildArrayInfo(const DWARFDIE &parent_die,
3737
if (attributes.Size() == 0)
3838
continue;
3939

40-
uint64_t num_elements = 0;
40+
std::optional<uint64_t> num_elements;
4141
uint64_t lower_bound = 0;
4242
uint64_t upper_bound = 0;
4343
bool upper_bound_valid = false;
@@ -91,7 +91,7 @@ DWARFASTParser::ParseChildArrayInfo(const DWARFDIE &parent_die,
9191
}
9292
}
9393

94-
if (num_elements == 0) {
94+
if (!num_elements || *num_elements == 0) {
9595
if (upper_bound_valid && upper_bound >= lower_bound)
9696
num_elements = upper_bound - lower_bound + 1;
9797
}

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,20 +1498,20 @@ DWARFASTParserClang::ParseArrayType(const DWARFDIE &die,
14981498
uint64_t array_element_bit_stride = byte_stride * 8 + bit_stride;
14991499
CompilerType clang_type;
15001500
if (array_info && array_info->element_orders.size() > 0) {
1501-
uint64_t num_elements = 0;
15021501
auto end = array_info->element_orders.rend();
15031502
for (auto pos = array_info->element_orders.rbegin(); pos != end; ++pos) {
1504-
num_elements = *pos;
1505-
clang_type = m_ast.CreateArrayType(array_element_type, num_elements,
1506-
attrs.is_vector);
1503+
clang_type = m_ast.CreateArrayType(
1504+
array_element_type, /*element_count=*/*pos, attrs.is_vector);
1505+
1506+
uint64_t num_elements = pos->value_or(0);
15071507
array_element_type = clang_type;
15081508
array_element_bit_stride = num_elements
15091509
? array_element_bit_stride * num_elements
15101510
: array_element_bit_stride;
15111511
}
15121512
} else {
1513-
clang_type =
1514-
m_ast.CreateArrayType(array_element_type, 0, attrs.is_vector);
1513+
clang_type = m_ast.CreateArrayType(
1514+
array_element_type, /*element_count=*/std::nullopt, attrs.is_vector);
15151515
}
15161516
ConstString empty_name;
15171517
TypeSP type_sp =

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2290,30 +2290,31 @@ TypeSystemClang::CreateBlockPointerType(const CompilerType &function_type) {
22902290

22912291
#pragma mark Array Types
22922292

2293-
CompilerType TypeSystemClang::CreateArrayType(const CompilerType &element_type,
2294-
size_t element_count,
2295-
bool is_vector) {
2296-
if (element_type.IsValid()) {
2297-
ASTContext &ast = getASTContext();
2293+
CompilerType
2294+
TypeSystemClang::CreateArrayType(const CompilerType &element_type,
2295+
std::optional<size_t> element_count,
2296+
bool is_vector) {
2297+
if (!element_type.IsValid())
2298+
return {};
22982299

2299-
if (is_vector) {
2300-
return GetType(ast.getExtVectorType(ClangUtil::GetQualType(element_type),
2301-
element_count));
2302-
} else {
2300+
ASTContext &ast = getASTContext();
23032301

2304-
llvm::APInt ap_element_count(64, element_count);
2305-
if (element_count == 0) {
2306-
return GetType(
2307-
ast.getIncompleteArrayType(ClangUtil::GetQualType(element_type),
2308-
clang::ArraySizeModifier::Normal, 0));
2309-
} else {
2310-
return GetType(ast.getConstantArrayType(
2311-
ClangUtil::GetQualType(element_type), ap_element_count, nullptr,
2312-
clang::ArraySizeModifier::Normal, 0));
2313-
}
2314-
}
2315-
}
2316-
return CompilerType();
2302+
// Unknown number of elements; this is an incomplete array
2303+
// (e.g., variable length array with non-constant bounds, or
2304+
// a flexible array member).
2305+
if (!element_count)
2306+
return GetType(
2307+
ast.getIncompleteArrayType(ClangUtil::GetQualType(element_type),
2308+
clang::ArraySizeModifier::Normal, 0));
2309+
2310+
if (is_vector)
2311+
return GetType(ast.getExtVectorType(ClangUtil::GetQualType(element_type),
2312+
*element_count));
2313+
2314+
llvm::APInt ap_element_count(64, *element_count);
2315+
return GetType(ast.getConstantArrayType(ClangUtil::GetQualType(element_type),
2316+
ap_element_count, nullptr,
2317+
clang::ArraySizeModifier::Normal, 0));
23172318
}
23182319

23192320
CompilerType TypeSystemClang::CreateStructForIdentifier(
@@ -4957,6 +4958,7 @@ TypeSystemClang::GetBitSize(lldb::opaque_compiler_type_t type,
49574958
clang::QualType qual_type(GetCanonicalQualType(type));
49584959
const clang::Type::TypeClass type_class = qual_type->getTypeClass();
49594960
switch (type_class) {
4961+
case clang::Type::ConstantArray:
49604962
case clang::Type::FunctionProto:
49614963
case clang::Type::Record:
49624964
return getASTContext().getTypeSize(qual_type);
@@ -5654,9 +5656,9 @@ TypeSystemClang::GetNumChildren(lldb::opaque_compiler_type_t type,
56545656
case clang::Type::IncompleteArray:
56555657
if (auto array_info =
56565658
GetDynamicArrayInfo(*this, GetSymbolFile(), qual_type, exe_ctx))
5657-
// Only 1-dimensional arrays are supported.
5659+
// FIXME: Only 1-dimensional arrays are supported.
56585660
num_children = array_info->element_orders.size()
5659-
? array_info->element_orders.back()
5661+
? array_info->element_orders.back().value_or(0)
56605662
: 0;
56615663
break;
56625664

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,8 @@ class TypeSystemClang : public TypeSystem {
535535
// Array Types
536536

537537
CompilerType CreateArrayType(const CompilerType &element_type,
538-
size_t element_count, bool is_vector);
538+
std::optional<size_t> element_count,
539+
bool is_vector);
539540

540541
// Enumeration Types
541542
clang::EnumDecl *CreateEnumerationDecl(

lldb/test/API/lang/c/struct_types/main.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// clang-format off
12
struct things_to_sum {
23
int a;
34
int b;
@@ -18,7 +19,7 @@ int main (int argc, char const *argv[])
1819
}; //% self.expect("frame variable pt.padding[0]", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["pt.padding[0] = "])
1920
//% self.expect("frame variable pt.padding[1]", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ["pt.padding[1] = "])
2021
//% self.expect_expr("pt.padding[0]", result_type="char")
21-
//% self.expect("image lookup -t point_tag", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ['padding[]'])
22+
//% self.expect("image lookup -t point_tag", DATA_TYPES_DISPLAYED_CORRECTLY, substrs = ['padding[0]'])
2223

2324
struct {} empty;
2425
//% self.expect("frame variable empty", substrs = ["empty = {}"])
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
// RUN: %clangxx_host -gdwarf -std=c++11 -o %t %s
2+
// RUN: %lldb %t \
3+
// RUN: -o run \
4+
// RUN: -o "frame var --show-types f" \
5+
// RUN: -o "frame var vla0" \
6+
// RUN: -o "frame var fla0" \
7+
// RUN: -o "frame var fla1" \
8+
// RUN: -o "frame var vla01" \
9+
// RUN: -o "frame var vla10" \
10+
// RUN: -o "frame var vlaN" \
11+
// RUN: -o "frame var vlaNM" \
12+
// RUN: -o exit | FileCheck %s
13+
14+
struct Foo {
15+
static constexpr int n = 1;
16+
int m_vlaN[n];
17+
18+
int m_vla0[0];
19+
};
20+
21+
int main() {
22+
Foo f;
23+
f.m_vlaN[0] = 60;
24+
25+
// CHECK: (lldb) frame var --show-types f
26+
// CHECK-NEXT: (Foo) f = {
27+
// CHECK-NEXT: (int[1]) m_vlaN = {
28+
// CHECK-NEXT: (int) [0] = 60
29+
// CHECK-NEXT: }
30+
// CHECK-NEXT: (int[0]) m_vla0 = {}
31+
// CHECK-NEXT: }
32+
33+
int vla0[0] = {};
34+
35+
// CHECK: (lldb) frame var vla0
36+
// CHECK-NEXT: (int[0]) vla0 = {}
37+
38+
int fla0[] = {};
39+
40+
// CHECK: (lldb) frame var fla0
41+
// CHECK-NEXT: (int[0]) fla0 = {}
42+
43+
int fla1[] = {42};
44+
45+
// CHECK: (lldb) frame var fla1
46+
// CHECK-NEXT: (int[1]) fla1 = ([0] = 42)
47+
48+
int vla01[0][1];
49+
50+
// CHECK: (lldb) frame var vla01
51+
// CHECK-NEXT: (int[0][1]) vla01 = {}
52+
53+
int vla10[1][0];
54+
55+
// CHECK: (lldb) frame var vla10
56+
// CHECK-NEXT: (int[1][0]) vla10 = ([0] = int[0]
57+
58+
int n = 3;
59+
int vlaN[n];
60+
for (int i = 0; i < n; ++i)
61+
vlaN[i] = -i;
62+
63+
// CHECK: (lldb) frame var vlaN
64+
// CHECK-NEXT: (int[]) vlaN = ([0] = 0, [1] = -1, [2] = -2)
65+
66+
int m = 2;
67+
int vlaNM[n][m];
68+
for (int i = 0; i < n; ++i)
69+
for (int j = 0; j < m; ++j)
70+
vlaNM[i][j] = i + j;
71+
72+
// FIXME: multi-dimensional VLAs aren't well supported
73+
// CHECK: (lldb) frame var vlaNM
74+
// CHECK-NEXT: (int[][]) vlaNM = {
75+
// CHECK-NEXT: [0] = ([0] = 0, [1] = 1, [2] = 1)
76+
// CHECK-NEXT: [1] = ([0] = 1, [1] = 1, [2] = 2)
77+
// CHECK-NEXT: }
78+
79+
__builtin_debugtrap();
80+
}

0 commit comments

Comments
 (0)