Skip to content

[lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType #100710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also broken without this patch. E.g., accessing elements of a VLA with non-constant bounds will return garbage. My hunch is that we just calculate the stride incorrectly (or something related to calculating children count).


__builtin_debugtrap();
}
Loading