Skip to content

Commit

Permalink
[lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantA…
Browse files Browse the repository at this point in the history
…rrayType (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)
  • Loading branch information
Michael137 authored and banach-space committed Aug 7, 2024
1 parent 616bfc9 commit 0b964a1
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 35 deletions.
10 changes: 9 additions & 1 deletion lldb/include/lldb/Symbol/SymbolFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
12 changes: 6 additions & 6 deletions lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
50 changes: 26 additions & 24 deletions lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion lldb/test/API/lang/c/struct_types/main.c
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// clang-format off
struct things_to_sum {
int a;
int b;
Expand All @@ -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 = {}"])
Expand Down
80 changes: 80 additions & 0 deletions lldb/test/Shell/SymbolFile/DWARF/vla.cpp
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit 0b964a1

Please sign in to comment.