[Lldb-commits] [lldb] [lldb][DataFormatter] VectorType: fix format for arrays with size not a power-of-2 (PR #68907)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Fri Oct 13 11:29:33 PDT 2023
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/68907
>From 769cbd0a30f0903ff4b7263b7789fb28c49de1d6 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 12 Oct 2023 16:25:39 +0100
Subject: [PATCH 1/5] [lldb][DataFormatter][NFC] VectorType: remove redundant
parameter to helper function
---
lldb/source/DataFormatters/VectorType.cpp | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp
index 4afcfa2e8e490e0..7109708c1cc9605 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -169,14 +169,12 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
}
}
-static size_t CalculateNumChildren(
- CompilerType container_type, CompilerType element_type,
- lldb_private::ExecutionContextScope *exe_scope =
- nullptr // does not matter here because all we trade in are basic types
- ) {
+static size_t CalculateNumChildren(CompilerType container_type,
+ CompilerType element_type) {
std::optional<uint64_t> container_size =
- container_type.GetByteSize(exe_scope);
- std::optional<uint64_t> element_size = element_type.GetByteSize(exe_scope);
+ container_type.GetByteSize(/* exe_scope */ nullptr);
+ std::optional<uint64_t> element_size =
+ element_type.GetByteSize(/* exe_scope */ nullptr);
if (container_size && element_size && *element_size) {
if (*container_size % *element_size)
>From c3eeef2d4737e407ebd7bc9620f6460e1f45c654 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 12 Oct 2023 16:39:52 +0100
Subject: [PATCH 2/5] [lldb][DataFormatter][NFC] VectorType: clean up and
document helper function
In preparation for changing this function I cleaned
it up so the conditions under which the function
returns what are clearer.
---
lldb/source/DataFormatters/VectorType.cpp | 35 +++++++++++++++++++----
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp
index 7109708c1cc9605..e574a1e10866d27 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -169,19 +169,42 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
}
}
+/// \brief Returns the number of elements of 'container_type'
+/// as if its elements had type 'element_type'.
+///
+/// For example, a container of type
+/// `uint8_t __attribute__((vector_size(16)))` has 16 elements.
+/// But calling `CalculateNumChildren` with an 'element_type'
+/// of `float` (4-bytes) will return `4` because we are interpreting
+/// the byte-array as a `float32[]`.
+///
+/// \param[in] container_type The type of the container We
+/// are calculating the children of.
+///
+/// \param[in] element_type The type of elements we interpret
+/// container_type to contain for the purposes of calculating
+/// the number of children.
+///
+/// If size of the container is not a multiple of 'element_type'
+/// returns 0.
+///
+/// On error, returns 0.
static size_t CalculateNumChildren(CompilerType container_type,
CompilerType element_type) {
std::optional<uint64_t> container_size =
container_type.GetByteSize(/* exe_scope */ nullptr);
+ if (!container_size)
+ return 0;
+
std::optional<uint64_t> element_size =
element_type.GetByteSize(/* exe_scope */ nullptr);
+ if (!element_size || !*element_size)
+ return 0;
- if (container_size && element_size && *element_size) {
- if (*container_size % *element_size)
- return 0;
- return *container_size / *element_size;
- }
- return 0;
+ if (*container_size % *element_size)
+ return 0;
+
+ return *container_size / *element_size;
}
namespace lldb_private {
>From 9f80f21d9f26af2dc57025e704c41944773b6c25 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 12 Oct 2023 17:04:55 +0100
Subject: [PATCH 3/5] [lldb][DataFormatter] Fix GetNumChildren for VectorType
formatter
To get the number of children for a VectorType (i.e.,
a type declared with a `vector_size`/`ext_vector_type` attribute)
LLDB previously did following calculation:
1. Get byte-size of the vector container from Clang (`getTypeInfo`).
2. Get byte-size of the element type we want to interpret the array as.
(e.g., sometimes we want to interpret an `unsigned char vec[16]`
as a `float32[]`).
3. `numChildren = containerSize / reinterpretedElementSize`
However, for step 1, clang will return us the *aligned* container byte-size.
So for a type such as `float __attribute__((ext_vector_type(3)))`
(which is an array of 3 4-byte floats), clang will round up the
bit-width of the array to `16`.
(see [here](https://github.com/llvm/llvm-project/blob/ab6a66dbec61654d0962f6abf6d6c5b776937584/clang/lib/AST/ASTContext.cpp#L1987-L1992))
This means that for vectors where the size isn't a power-of-2, LLDB
will miscalculate the number of elements.
**Solution**
This patch changes step 1 such that we calculate the container size
as `numElementsInSource * byteSizeOfElement`. This mimicks the typical
`sizeof(array) / sizeof(array[0])` calculation in C.
---
lldb/source/DataFormatters/VectorType.cpp | 33 ++++++++++++-------
.../vector-types/TestVectorTypesFormatting.py | 7 ++++
.../data-formatter/vector-types/main.cpp | 4 ++-
3 files changed, 31 insertions(+), 13 deletions(-)
diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp
index e574a1e10866d27..93eea9507242a9a 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -169,8 +169,9 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
}
}
-/// \brief Returns the number of elements of 'container_type'
-/// as if its elements had type 'element_type'.
+/// \brief Returns the number of elements stored in a container
+/// (with element type 'container_elem_type') as if it had elements
+/// of type 'element_type'.
///
/// For example, a container of type
/// `uint8_t __attribute__((vector_size(16)))` has 16 elements.
@@ -178,8 +179,11 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
/// of `float` (4-bytes) will return `4` because we are interpreting
/// the byte-array as a `float32[]`.
///
-/// \param[in] container_type The type of the container We
-/// are calculating the children of.
+/// \param[in] container_elem_type The type of the elements stored
+/// in the container we are calculating the children of.
+///
+/// \param[in] num_elements Number of 'container_elem_type's our
+/// container stores.
///
/// \param[in] element_type The type of elements we interpret
/// container_type to contain for the purposes of calculating
@@ -189,22 +193,25 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
/// returns 0.
///
/// On error, returns 0.
-static size_t CalculateNumChildren(CompilerType container_type,
+static size_t CalculateNumChildren(CompilerType container_elem_type,
+ uint64_t num_elements,
CompilerType element_type) {
- std::optional<uint64_t> container_size =
- container_type.GetByteSize(/* exe_scope */ nullptr);
- if (!container_size)
+ std::optional<uint64_t> container_elem_size =
+ container_elem_type.GetByteSize(/* exe_scope */ nullptr);
+ if (!container_elem_size)
return 0;
+ auto container_size = *container_elem_size * num_elements;
+
std::optional<uint64_t> element_size =
element_type.GetByteSize(/* exe_scope */ nullptr);
if (!element_size || !*element_size)
return 0;
- if (*container_size % *element_size)
+ if (container_size % *element_size)
return 0;
- return *container_size / *element_size;
+ return container_size / *element_size;
}
namespace lldb_private {
@@ -242,11 +249,13 @@ class VectorTypeSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
m_parent_format = m_backend.GetFormat();
CompilerType parent_type(m_backend.GetCompilerType());
CompilerType element_type;
- parent_type.IsVectorType(&element_type);
+ uint64_t num_elements;
+ parent_type.IsVectorType(&element_type, &num_elements);
m_child_type = ::GetCompilerTypeForFormat(
m_parent_format, element_type,
parent_type.GetTypeSystem().GetSharedPointer());
- m_num_children = ::CalculateNumChildren(parent_type, m_child_type);
+ m_num_children =
+ ::CalculateNumChildren(element_type, num_elements, m_child_type);
m_item_format = GetItemFormatForFormat(m_parent_format, m_child_type);
return false;
}
diff --git a/lldb/test/API/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py b/lldb/test/API/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py
index 4103d62878c70f3..1839c28aeb29fc6 100644
--- a/lldb/test/API/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py
+++ b/lldb/test/API/functionalities/data-formatter/vector-types/TestVectorTypesFormatting.py
@@ -86,3 +86,10 @@ def cleanup():
v.SetFormat(lldb.eFormatVectorOfFloat32)
oldValueAgain = v.GetChildAtIndex(0).GetValue()
self.assertEqual(oldValue, oldValueAgain, "same format but different values")
+
+ # Test formatter for vector types whose size is not a power-of-2
+ f3 = self.frame().FindVariable("f3")
+ self.assertEqual(f3.GetNumChildren(), 3)
+ self.assertEqual(f3.GetChildAtIndex(0).GetData().float[0], 1.25)
+ self.assertEqual(f3.GetChildAtIndex(1).GetData().float[0], 2.50)
+ self.assertEqual(f3.GetChildAtIndex(2).GetData().float[0], 2.50)
diff --git a/lldb/test/API/functionalities/data-formatter/vector-types/main.cpp b/lldb/test/API/functionalities/data-formatter/vector-types/main.cpp
index ef0a227560bc253..7f2309e776bc27e 100644
--- a/lldb/test/API/functionalities/data-formatter/vector-types/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/vector-types/main.cpp
@@ -1,8 +1,10 @@
typedef float float4 __attribute__((ext_vector_type(4)));
-typedef unsigned char vec __attribute__((ext_vector_type(16)));
+typedef unsigned char vec __attribute__((ext_vector_type(16)));
+typedef float float3 __attribute__((ext_vector_type(3)));
int main() {
float4 f4 = {1.25, 1.25, 2.50, 2.50};
vec v = (vec)f4;
+ float3 f3 = f4.gba;
return 0; // break here
}
>From d0d6425021e1666b92b1c96accebf9edf1d217b6 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 13 Oct 2023 19:04:42 +0100
Subject: [PATCH 4/5] fixup! fix doxygen comment and use std::optional
---
lldb/source/DataFormatters/VectorType.cpp | 29 ++++++++++++-----------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp
index 93eea9507242a9a..4b534224d5384d3 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -169,9 +169,9 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
}
}
-/// \brief Returns the number of elements stored in a container
-/// (with element type 'container_elem_type') as if it had elements
-/// of type 'element_type'.
+/// The number of elements stored in a container (with element
+/// type 'container_elem_type') as if it had elements of type
+/// 'element_type'.
///
/// For example, a container of type
/// `uint8_t __attribute__((vector_size(16)))` has 16 elements.
@@ -189,27 +189,28 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
/// container_type to contain for the purposes of calculating
/// the number of children.
///
-/// If size of the container is not a multiple of 'element_type'
-/// returns 0.
-///
-/// On error, returns 0.
-static size_t CalculateNumChildren(CompilerType container_elem_type,
- uint64_t num_elements,
- CompilerType element_type) {
+/// \returns The number of elements stored in a container of
+/// type 'element_type'. Returns a std::nullopt if the
+/// size of the container is not a multiple of 'element_type'
+/// or if an error occurs.
+static std::optional<size_t> CalculateNumChildren(
+ CompilerType container_elem_type,
+ uint64_t num_elements,
+ CompilerType element_type) {
std::optional<uint64_t> container_elem_size =
container_elem_type.GetByteSize(/* exe_scope */ nullptr);
if (!container_elem_size)
- return 0;
+ return {};
auto container_size = *container_elem_size * num_elements;
std::optional<uint64_t> element_size =
element_type.GetByteSize(/* exe_scope */ nullptr);
if (!element_size || !*element_size)
- return 0;
+ return {};
if (container_size % *element_size)
- return 0;
+ return {};
return container_size / *element_size;
}
@@ -255,7 +256,7 @@ class VectorTypeSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
m_parent_format, element_type,
parent_type.GetTypeSystem().GetSharedPointer());
m_num_children =
- ::CalculateNumChildren(element_type, num_elements, m_child_type);
+ ::CalculateNumChildren(element_type, num_elements, m_child_type).value_or(0);
m_item_format = GetItemFormatForFormat(m_parent_format, m_child_type);
return false;
}
>From bdcad25470bf3390f3783b2c9725f4f87accf090 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 13 Oct 2023 19:29:09 +0100
Subject: [PATCH 5/5] fixup! fix doxygen
---
lldb/source/DataFormatters/VectorType.cpp | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/lldb/source/DataFormatters/VectorType.cpp b/lldb/source/DataFormatters/VectorType.cpp
index 4b534224d5384d3..57dae0b2c71f0fa 100644
--- a/lldb/source/DataFormatters/VectorType.cpp
+++ b/lldb/source/DataFormatters/VectorType.cpp
@@ -169,8 +169,8 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
}
}
-/// The number of elements stored in a container (with element
-/// type 'container_elem_type') as if it had elements of type
+/// Calculates the number of elements stored in a container (with
+/// element type 'container_elem_type') as if it had elements of type
/// 'element_type'.
///
/// For example, a container of type
@@ -193,10 +193,9 @@ static lldb::Format GetItemFormatForFormat(lldb::Format format,
/// type 'element_type'. Returns a std::nullopt if the
/// size of the container is not a multiple of 'element_type'
/// or if an error occurs.
-static std::optional<size_t> CalculateNumChildren(
- CompilerType container_elem_type,
- uint64_t num_elements,
- CompilerType element_type) {
+static std::optional<size_t>
+CalculateNumChildren(CompilerType container_elem_type, uint64_t num_elements,
+ CompilerType element_type) {
std::optional<uint64_t> container_elem_size =
container_elem_type.GetByteSize(/* exe_scope */ nullptr);
if (!container_elem_size)
@@ -256,7 +255,8 @@ class VectorTypeSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
m_parent_format, element_type,
parent_type.GetTypeSystem().GetSharedPointer());
m_num_children =
- ::CalculateNumChildren(element_type, num_elements, m_child_type).value_or(0);
+ ::CalculateNumChildren(element_type, num_elements, m_child_type)
+ .value_or(0);
m_item_format = GetItemFormatForFormat(m_parent_format, m_child_type);
return false;
}
More information about the lldb-commits
mailing list