[Lldb-commits] [lldb] [lldb] Fix misaligned pointer UB in DataExtractor::GetU16/U32/U64 array reads (PR #213038)
Yao Qi via lldb-commits
lldb-commits at lists.llvm.org
Sat Aug 1 06:12:36 PDT 2026
https://github.com/qiyao updated https://github.com/llvm/llvm-project/pull/213038
>From 90e562be4f7d5cf62842d33251c3f30fdf03355f Mon Sep 17 00:00:00 2001
From: Yao Qi <yao_qi at apple.com>
Date: Sat, 4 Jul 2026 00:08:39 +0100
Subject: [PATCH 1/5] [lldb] Fix misaligned pointer UB in
DataExtractor::GetU16/U32/U64 array reads
The multi-value overloads `DataExtractor::GetU16`/`GetU32`/`GetU64(offset_ptr,
dst, count)` extract `count` consecutive integers from the buffer. When the
data byte order differs from the host, they walked the source and destination
through typed pointers (`const uint16_t *`/`uint32_t *`/`uint64_t *`).
`GetData()` hands back a pointer to an arbitrary byte offset within the
underlying buffer, and the caller's destination is an arbitrary `void *`, so
neither is guaranteed to be aligned for the wider integer type. Casting such a
byte-offset pointer to a wider typed pointer, advancing it, and loading or
storing through it is undefined behavior even when the individual accesses go
through `memcpy`-based helpers, because the typed pointer itself is required to
be aligned.
Compiling with UBSan's alignment check (`-fsanitize=alignment`) turns this UB
into a runtime diagnostic, for example:
```
runtime error: store to misaligned address 0x... for type 'uint16_t',
which requires 2 byte alignment
```
Read the source and write the destination through `uint8_t` byte pointers and
copy each swapped value with `memcpy`, so no over-aligned typed pointer is ever
formed. The behavior is otherwise unchanged.
This was surfaced by `lldb-target-fuzzer` fuzzer. It is a sanitizer-only
diagnostic: no crash occurs without UBSan on the platforms LLDB targets, and the
fix is a pure refactor of the read loop, so no separate unit test is added.
---
lldb/source/Utility/DataExtractor.cpp | 66 ++++++++++++++++-----------
1 file changed, 39 insertions(+), 27 deletions(-)
diff --git a/lldb/source/Utility/DataExtractor.cpp b/lldb/source/Utility/DataExtractor.cpp
index f6251d0679b41..6cdd89e106aaf 100644
--- a/lldb/source/Utility/DataExtractor.cpp
+++ b/lldb/source/Utility/DataExtractor.cpp
@@ -400,17 +400,21 @@ uint64_t DataExtractor::GetU64_unchecked(offset_t *offset_ptr) const {
void *DataExtractor::GetU16(offset_t *offset_ptr, void *void_dst,
uint32_t count) const {
const size_t src_size = sizeof(uint16_t) * count;
- const uint16_t *src =
- static_cast<const uint16_t *>(GetData(offset_ptr, src_size));
+ // GetData() returns a pointer to an arbitrary byte offset within the
+ // underlying buffer, so it carries no guarantee of being aligned for a
+ // uint16_t. Both the source and the caller-supplied destination are
+ // therefore treated as raw bytes: reading or writing a uint16_t through a
+ // typed pointer that is not 2-byte aligned is undefined behavior (and is
+ // flagged by UBSan's alignment check). Access each element through a byte
+ // pointer and copy it with memcpy, which imposes no alignment requirement.
+ const uint8_t *src =
+ static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
if (src) {
if (m_byte_order != endian::InlHostByteOrder()) {
- uint16_t *dst_pos = static_cast<uint16_t *>(void_dst);
- uint16_t *dst_end = dst_pos + count;
- const uint16_t *src_pos = src;
- while (dst_pos < dst_end) {
- *dst_pos = ReadSwapInt16(src_pos);
- ++dst_pos;
- ++src_pos;
+ uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
+ for (uint32_t i = 0; i < count; ++i) {
+ uint16_t value = ReadSwapInt16(src, i * sizeof(uint16_t));
+ memcpy(dst_pos + i * sizeof(uint16_t), &value, sizeof(value));
}
} else {
memcpy(void_dst, src, src_size);
@@ -449,17 +453,21 @@ uint32_t DataExtractor::GetU32(offset_t *offset_ptr) const {
void *DataExtractor::GetU32(offset_t *offset_ptr, void *void_dst,
uint32_t count) const {
const size_t src_size = sizeof(uint32_t) * count;
- const uint32_t *src =
- static_cast<const uint32_t *>(GetData(offset_ptr, src_size));
+ // GetData() returns a pointer to an arbitrary byte offset within the
+ // underlying buffer, so it carries no guarantee of being aligned for a
+ // uint32_t. Both the source and the caller-supplied destination are
+ // therefore treated as raw bytes: reading or writing a uint32_t through a
+ // typed pointer that is not 4-byte aligned is undefined behavior (and is
+ // flagged by UBSan's alignment check). Access each element through a byte
+ // pointer and copy it with memcpy, which imposes no alignment requirement.
+ const uint8_t *src =
+ static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
if (src) {
if (m_byte_order != endian::InlHostByteOrder()) {
- uint32_t *dst_pos = static_cast<uint32_t *>(void_dst);
- uint32_t *dst_end = dst_pos + count;
- const uint32_t *src_pos = src;
- while (dst_pos < dst_end) {
- *dst_pos = ReadSwapInt32(src_pos);
- ++dst_pos;
- ++src_pos;
+ uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
+ for (uint32_t i = 0; i < count; ++i) {
+ uint32_t value = ReadSwapInt32(src, i * sizeof(uint32_t));
+ memcpy(dst_pos + i * sizeof(uint32_t), &value, sizeof(value));
}
} else {
memcpy(void_dst, src, src_size);
@@ -497,17 +505,21 @@ uint64_t DataExtractor::GetU64(offset_t *offset_ptr) const {
void *DataExtractor::GetU64(offset_t *offset_ptr, void *void_dst,
uint32_t count) const {
const size_t src_size = sizeof(uint64_t) * count;
- const uint64_t *src =
- static_cast<const uint64_t *>(GetData(offset_ptr, src_size));
+ // GetData() returns a pointer to an arbitrary byte offset within the
+ // underlying buffer, so it carries no guarantee of being aligned for a
+ // uint64_t. Both the source and the caller-supplied destination are
+ // therefore treated as raw bytes: reading or writing a uint64_t through a
+ // typed pointer that is not 8-byte aligned is undefined behavior (and is
+ // flagged by UBSan's alignment check). Access each element through a byte
+ // pointer and copy it with memcpy, which imposes no alignment requirement.
+ const uint8_t *src =
+ static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
if (src) {
if (m_byte_order != endian::InlHostByteOrder()) {
- uint64_t *dst_pos = static_cast<uint64_t *>(void_dst);
- uint64_t *dst_end = dst_pos + count;
- const uint64_t *src_pos = src;
- while (dst_pos < dst_end) {
- *dst_pos = ReadSwapInt64(src_pos);
- ++dst_pos;
- ++src_pos;
+ uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
+ for (uint32_t i = 0; i < count; ++i) {
+ uint64_t value = ReadSwapInt64(src, i * sizeof(uint64_t));
+ memcpy(dst_pos + i * sizeof(uint64_t), &value, sizeof(value));
}
} else {
memcpy(void_dst, src, src_size);
>From c43a0f1f623a0b794c3d1e0c929a322cc6941739 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao_qi at apple.com>
Date: Fri, 31 Jul 2026 07:47:47 +0100
Subject: [PATCH 2/5] [lldb] Trim DataExtractor::GetU16/U32/U64 alignment
comments
Address review feedback that the alignment comments added in the previous
commit are too verbose and duplicated across all three array overloads.
Keep one concise comment on `GetU16` stating the general principle (the
source and destination are only byte-aligned, so access them through byte
pointers and `memcpy`). Replace the duplicated blocks in `GetU32` and
`GetU64` with a short back-reference. The bug-fix narrative stays in the
previous commit's message. No functional change.
---
lldb/source/Utility/DataExtractor.cpp | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)
diff --git a/lldb/source/Utility/DataExtractor.cpp b/lldb/source/Utility/DataExtractor.cpp
index 6cdd89e106aaf..2df5791ff3488 100644
--- a/lldb/source/Utility/DataExtractor.cpp
+++ b/lldb/source/Utility/DataExtractor.cpp
@@ -400,13 +400,9 @@ uint64_t DataExtractor::GetU64_unchecked(offset_t *offset_ptr) const {
void *DataExtractor::GetU16(offset_t *offset_ptr, void *void_dst,
uint32_t count) const {
const size_t src_size = sizeof(uint16_t) * count;
- // GetData() returns a pointer to an arbitrary byte offset within the
- // underlying buffer, so it carries no guarantee of being aligned for a
- // uint16_t. Both the source and the caller-supplied destination are
- // therefore treated as raw bytes: reading or writing a uint16_t through a
- // typed pointer that is not 2-byte aligned is undefined behavior (and is
- // flagged by UBSan's alignment check). Access each element through a byte
- // pointer and copy it with memcpy, which imposes no alignment requirement.
+ // GetData() and void_dst are only byte-aligned, so read and write through
+ // byte pointers and memcpy each swapped value to avoid forming a misaligned
+ // typed pointer.
const uint8_t *src =
static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
if (src) {
@@ -453,13 +449,7 @@ uint32_t DataExtractor::GetU32(offset_t *offset_ptr) const {
void *DataExtractor::GetU32(offset_t *offset_ptr, void *void_dst,
uint32_t count) const {
const size_t src_size = sizeof(uint32_t) * count;
- // GetData() returns a pointer to an arbitrary byte offset within the
- // underlying buffer, so it carries no guarantee of being aligned for a
- // uint32_t. Both the source and the caller-supplied destination are
- // therefore treated as raw bytes: reading or writing a uint32_t through a
- // typed pointer that is not 4-byte aligned is undefined behavior (and is
- // flagged by UBSan's alignment check). Access each element through a byte
- // pointer and copy it with memcpy, which imposes no alignment requirement.
+ // Byte-aligned access only; see GetU16.
const uint8_t *src =
static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
if (src) {
@@ -505,13 +495,7 @@ uint64_t DataExtractor::GetU64(offset_t *offset_ptr) const {
void *DataExtractor::GetU64(offset_t *offset_ptr, void *void_dst,
uint32_t count) const {
const size_t src_size = sizeof(uint64_t) * count;
- // GetData() returns a pointer to an arbitrary byte offset within the
- // underlying buffer, so it carries no guarantee of being aligned for a
- // uint64_t. Both the source and the caller-supplied destination are
- // therefore treated as raw bytes: reading or writing a uint64_t through a
- // typed pointer that is not 8-byte aligned is undefined behavior (and is
- // flagged by UBSan's alignment check). Access each element through a byte
- // pointer and copy it with memcpy, which imposes no alignment requirement.
+ // Byte-aligned access only; see GetU16.
const uint8_t *src =
static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
if (src) {
>From 440e1a1274b6a36addd41da52d10ebc14ee876f6 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao_qi at apple.com>
Date: Fri, 31 Jul 2026 21:40:00 +0100
Subject: [PATCH 3/5] [lldb] Replace the ReadSwapInt16/32/64 helpers with a
ReadSwap<T> template
`DataExtractor.cpp` carries six file-local byte-swapping read helpers:
`ReadSwapInt16`, `ReadSwapInt32` and `ReadSwapInt64`, each in a `(ptr, offset)`
and a `(ptr)` flavour. All six spell out the same three lines, `memcpy` into a
local of the right width, `llvm::byteswap`, return the value.
Replace them with a single `ReadSwap<T>` whose offset defaults to 0, and name
the width at the call sites instead. `memcpy` is what lets the helper read from
a pointer that is not aligned for `T`, so the template takes a `const uint8_t *`
and advances it as bytes. Keeping the parameter byte-typed, as the removed
helpers were, also keeps `offset` unambiguously a byte count: passing a wider
pointer together with an element index is a compile error rather than a silent
read at the wrong offset. No functional change.
---
lldb/source/Utility/DataExtractor.cpp | 62 +++++++--------------------
1 file changed, 16 insertions(+), 46 deletions(-)
diff --git a/lldb/source/Utility/DataExtractor.cpp b/lldb/source/Utility/DataExtractor.cpp
index 2df5791ff3488..a65cc508673f6 100644
--- a/lldb/source/Utility/DataExtractor.cpp
+++ b/lldb/source/Utility/DataExtractor.cpp
@@ -67,43 +67,13 @@ static inline uint16_t ReadInt16(const void *ptr) {
return value;
}
-static inline uint16_t ReadSwapInt16(const unsigned char *ptr,
- offset_t offset) {
- uint16_t value;
- memcpy(&value, ptr + offset, 2);
- return llvm::byteswap<uint16_t>(value);
-}
-
-static inline uint32_t ReadSwapInt32(const unsigned char *ptr,
- offset_t offset) {
- uint32_t value;
- memcpy(&value, ptr + offset, 4);
- return llvm::byteswap<uint32_t>(value);
-}
-
-static inline uint64_t ReadSwapInt64(const unsigned char *ptr,
- offset_t offset) {
- uint64_t value;
- memcpy(&value, ptr + offset, 8);
- return llvm::byteswap<uint64_t>(value);
-}
-
-static inline uint16_t ReadSwapInt16(const void *ptr) {
- uint16_t value;
- memcpy(&value, ptr, 2);
- return llvm::byteswap<uint16_t>(value);
-}
-
-static inline uint32_t ReadSwapInt32(const void *ptr) {
- uint32_t value;
- memcpy(&value, ptr, 4);
- return llvm::byteswap<uint32_t>(value);
-}
-
-static inline uint64_t ReadSwapInt64(const void *ptr) {
- uint64_t value;
- memcpy(&value, ptr, 8);
- return llvm::byteswap<uint64_t>(value);
+/// Read a byte-swapped \c T from \a ptr + \a offset, which need not be aligned
+/// for \c T.
+template <typename T>
+static inline T ReadSwap(const uint8_t *ptr, offset_t offset = 0) {
+ T value;
+ memcpy(&value, ptr + offset, sizeof(T));
+ return llvm::byteswap<T>(value);
}
static inline uint64_t ReadMaxInt64(const uint8_t *data, size_t byte_size,
@@ -354,7 +324,7 @@ uint16_t DataExtractor::GetU16(offset_t *offset_ptr) const {
static_cast<const uint8_t *>(GetData(offset_ptr, sizeof(val)));
if (data) {
if (m_byte_order != endian::InlHostByteOrder())
- val = ReadSwapInt16(data);
+ val = ReadSwap<uint16_t>(data);
else
val = ReadInt16(data);
}
@@ -366,7 +336,7 @@ uint16_t DataExtractor::GetU16_unchecked(offset_t *offset_ptr) const {
if (m_byte_order == endian::InlHostByteOrder())
val = ReadInt16(m_start, *offset_ptr);
else
- val = ReadSwapInt16(m_start, *offset_ptr);
+ val = ReadSwap<uint16_t>(m_start, *offset_ptr);
*offset_ptr += sizeof(val);
return val;
}
@@ -376,7 +346,7 @@ uint32_t DataExtractor::GetU32_unchecked(offset_t *offset_ptr) const {
if (m_byte_order == endian::InlHostByteOrder())
val = ReadInt32(m_start, *offset_ptr);
else
- val = ReadSwapInt32(m_start, *offset_ptr);
+ val = ReadSwap<uint32_t>(m_start, *offset_ptr);
*offset_ptr += sizeof(val);
return val;
}
@@ -386,7 +356,7 @@ uint64_t DataExtractor::GetU64_unchecked(offset_t *offset_ptr) const {
if (m_byte_order == endian::InlHostByteOrder())
val = ReadInt64(m_start, *offset_ptr);
else
- val = ReadSwapInt64(m_start, *offset_ptr);
+ val = ReadSwap<uint64_t>(m_start, *offset_ptr);
*offset_ptr += sizeof(val);
return val;
}
@@ -409,7 +379,7 @@ void *DataExtractor::GetU16(offset_t *offset_ptr, void *void_dst,
if (m_byte_order != endian::InlHostByteOrder()) {
uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
for (uint32_t i = 0; i < count; ++i) {
- uint16_t value = ReadSwapInt16(src, i * sizeof(uint16_t));
+ uint16_t value = ReadSwap<uint16_t>(src, i * sizeof(uint16_t));
memcpy(dst_pos + i * sizeof(uint16_t), &value, sizeof(value));
}
} else {
@@ -432,7 +402,7 @@ uint32_t DataExtractor::GetU32(offset_t *offset_ptr) const {
static_cast<const uint8_t *>(GetData(offset_ptr, sizeof(val)));
if (data) {
if (m_byte_order != endian::InlHostByteOrder()) {
- val = ReadSwapInt32(data);
+ val = ReadSwap<uint32_t>(data);
} else {
memcpy(&val, data, 4);
}
@@ -456,7 +426,7 @@ void *DataExtractor::GetU32(offset_t *offset_ptr, void *void_dst,
if (m_byte_order != endian::InlHostByteOrder()) {
uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
for (uint32_t i = 0; i < count; ++i) {
- uint32_t value = ReadSwapInt32(src, i * sizeof(uint32_t));
+ uint32_t value = ReadSwap<uint32_t>(src, i * sizeof(uint32_t));
memcpy(dst_pos + i * sizeof(uint32_t), &value, sizeof(value));
}
} else {
@@ -479,7 +449,7 @@ uint64_t DataExtractor::GetU64(offset_t *offset_ptr) const {
static_cast<const uint8_t *>(GetData(offset_ptr, sizeof(val)));
if (data) {
if (m_byte_order != endian::InlHostByteOrder()) {
- val = ReadSwapInt64(data);
+ val = ReadSwap<uint64_t>(data);
} else {
memcpy(&val, data, 8);
}
@@ -502,7 +472,7 @@ void *DataExtractor::GetU64(offset_t *offset_ptr, void *void_dst,
if (m_byte_order != endian::InlHostByteOrder()) {
uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
for (uint32_t i = 0; i < count; ++i) {
- uint64_t value = ReadSwapInt64(src, i * sizeof(uint64_t));
+ uint64_t value = ReadSwap<uint64_t>(src, i * sizeof(uint64_t));
memcpy(dst_pos + i * sizeof(uint64_t), &value, sizeof(value));
}
} else {
>From 4a5dc83ce8cef3e2361b503f9a17114569047fbc Mon Sep 17 00:00:00 2001
From: Yao Qi <yao_qi at apple.com>
Date: Fri, 31 Jul 2026 22:26:27 +0100
Subject: [PATCH 4/5] [lldb] Fold DataExtractor::GetU16/U32/U64 array reads
into a GetUInt<T> helper
The multi-value overloads `GetU16`, `GetU32` and `GetU64(offset_ptr, dst,
count)` are three copies of the same byte-wise read loop, differing only in the
integer width. Turn them into one-line forwarders to a file-local `GetUInt<T>`,
which fetches `sizeof(T) * count` bytes and then either copies them straight
through, when the data's byte order already matches the host's, or walks them
element by element through `ReadSwap<T>`.
Both the buffer returned by `GetData()` and the caller's destination are only
byte-aligned, so `GetUInt` transfers every value with `memcpy` and never forms a
pointer to an over-aligned type. That is what the alignment comment is about,
and it is now stated once rather than three times.
Drop the block comments above the three overloads. They repeated the Doxygen
that `DataExtractor.h` already carries, and the one on `GetU64` described a
`bool` return the function does not have. No functional change.
---
lldb/source/Utility/DataExtractor.cpp | 112 ++++++++------------------
1 file changed, 33 insertions(+), 79 deletions(-)
diff --git a/lldb/source/Utility/DataExtractor.cpp b/lldb/source/Utility/DataExtractor.cpp
index a65cc508673f6..cef2e9e53b77d 100644
--- a/lldb/source/Utility/DataExtractor.cpp
+++ b/lldb/source/Utility/DataExtractor.cpp
@@ -90,6 +90,33 @@ static inline uint64_t ReadMaxInt64(const uint8_t *data, size_t byte_size,
return res;
}
+/// Implements DataExtractor::GetU16/GetU32/GetU64(offset_ptr, dst, count); see
+/// DataExtractor.h for the contract.
+///
+/// \a data's buffer and \a dst are only byte-aligned, so every value is
+/// transferred with memcpy: loading or storing a \c T through a pointer that is
+/// not sizeof(T)-aligned is undefined behavior, and trips UBSan.
+template <typename T>
+static void *GetUInt(const DataExtractor &data, offset_t *offset_ptr, void *dst,
+ uint32_t count) {
+ const size_t src_size = sizeof(T) * count;
+ const uint8_t *src =
+ static_cast<const uint8_t *>(data.GetData(offset_ptr, src_size));
+ if (!src)
+ return nullptr;
+
+ if (data.GetByteOrder() == endian::InlHostByteOrder()) {
+ memcpy(dst, src, src_size);
+ } else {
+ uint8_t *dst_bytes = static_cast<uint8_t *>(dst);
+ for (uint32_t i = 0; i < count; ++i) {
+ T value = ReadSwap<T>(src, i * sizeof(T));
+ memcpy(dst_bytes + i * sizeof(T), &value, sizeof(T));
+ }
+ }
+ return dst;
+}
+
DataExtractor::DataExtractor()
: m_byte_order(endian::InlHostByteOrder()), m_addr_size(sizeof(void *)),
m_data_sp() {}
@@ -361,35 +388,9 @@ uint64_t DataExtractor::GetU64_unchecked(offset_t *offset_ptr) const {
return val;
}
-// Extract "count" uint16_t values from the binary data and update the offset
-// pointed to by "offset_ptr". The extracted data is copied into "dst".
-//
-// RETURNS the non-nullptr buffer pointer upon successful extraction of
-// all the requested bytes, or nullptr when the data is not available in the
-// buffer due to being out of bounds, or insufficient data.
-void *DataExtractor::GetU16(offset_t *offset_ptr, void *void_dst,
+void *DataExtractor::GetU16(offset_t *offset_ptr, void *dst,
uint32_t count) const {
- const size_t src_size = sizeof(uint16_t) * count;
- // GetData() and void_dst are only byte-aligned, so read and write through
- // byte pointers and memcpy each swapped value to avoid forming a misaligned
- // typed pointer.
- const uint8_t *src =
- static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
- if (src) {
- if (m_byte_order != endian::InlHostByteOrder()) {
- uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
- for (uint32_t i = 0; i < count; ++i) {
- uint16_t value = ReadSwap<uint16_t>(src, i * sizeof(uint16_t));
- memcpy(dst_pos + i * sizeof(uint16_t), &value, sizeof(value));
- }
- } else {
- memcpy(void_dst, src, src_size);
- }
- // Return a non-nullptr pointer to the converted data as an indicator of
- // success
- return void_dst;
- }
- return nullptr;
+ return GetUInt<uint16_t>(*this, offset_ptr, dst, count);
}
// Extract a single uint32_t from the data and update the offset pointed to by
@@ -410,33 +411,9 @@ uint32_t DataExtractor::GetU32(offset_t *offset_ptr) const {
return val;
}
-// Extract "count" uint32_t values from the binary data and update the offset
-// pointed to by "offset_ptr". The extracted data is copied into "dst".
-//
-// RETURNS the non-nullptr buffer pointer upon successful extraction of
-// all the requested bytes, or nullptr when the data is not available in the
-// buffer due to being out of bounds, or insufficient data.
-void *DataExtractor::GetU32(offset_t *offset_ptr, void *void_dst,
+void *DataExtractor::GetU32(offset_t *offset_ptr, void *dst,
uint32_t count) const {
- const size_t src_size = sizeof(uint32_t) * count;
- // Byte-aligned access only; see GetU16.
- const uint8_t *src =
- static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
- if (src) {
- if (m_byte_order != endian::InlHostByteOrder()) {
- uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
- for (uint32_t i = 0; i < count; ++i) {
- uint32_t value = ReadSwap<uint32_t>(src, i * sizeof(uint32_t));
- memcpy(dst_pos + i * sizeof(uint32_t), &value, sizeof(value));
- }
- } else {
- memcpy(void_dst, src, src_size);
- }
- // Return a non-nullptr pointer to the converted data as an indicator of
- // success
- return void_dst;
- }
- return nullptr;
+ return GetUInt<uint32_t>(*this, offset_ptr, dst, count);
}
// Extract a single uint64_t from the data and update the offset pointed to by
@@ -457,32 +434,9 @@ uint64_t DataExtractor::GetU64(offset_t *offset_ptr) const {
return val;
}
-// GetU64
-//
-// Get multiple consecutive 64 bit values. Return true if the entire read
-// succeeds and increment the offset pointed to by offset_ptr, else return
-// false and leave the offset pointed to by offset_ptr unchanged.
-void *DataExtractor::GetU64(offset_t *offset_ptr, void *void_dst,
+void *DataExtractor::GetU64(offset_t *offset_ptr, void *dst,
uint32_t count) const {
- const size_t src_size = sizeof(uint64_t) * count;
- // Byte-aligned access only; see GetU16.
- const uint8_t *src =
- static_cast<const uint8_t *>(GetData(offset_ptr, src_size));
- if (src) {
- if (m_byte_order != endian::InlHostByteOrder()) {
- uint8_t *dst_pos = static_cast<uint8_t *>(void_dst);
- for (uint32_t i = 0; i < count; ++i) {
- uint64_t value = ReadSwap<uint64_t>(src, i * sizeof(uint64_t));
- memcpy(dst_pos + i * sizeof(uint64_t), &value, sizeof(value));
- }
- } else {
- memcpy(void_dst, src, src_size);
- }
- // Return a non-nullptr pointer to the converted data as an indicator of
- // success
- return void_dst;
- }
- return nullptr;
+ return GetUInt<uint64_t>(*this, offset_ptr, dst, count);
}
uint32_t DataExtractor::GetMaxU32(offset_t *offset_ptr,
>From fabc4a77ce0a1999048fe21c5c0ca9c2adb1b676 Mon Sep 17 00:00:00 2001
From: Yao Qi <yao_qi at apple.com>
Date: Sat, 1 Aug 2026 08:43:17 +0100
Subject: [PATCH 5/5] [lldb] Add unit tests for the DataExtractor
GetU16/U32/U64 array overloads
The multi-value overloads `GetU16`, `GetU32` and `GetU64(offset_ptr, dst,
count)` had no unit test coverage, even though they are the only place in
`DataExtractor` that walks a buffer element by element and byte-swaps as it
copies. Cover all three widths for both byte orders, plus the out-of-bounds
path, which has to return `nullptr` and leave the offset unmodified.
Every read starts at an odd offset and lands in a destination that starts one
byte past an 8-byte boundary, so both the source and the destination are
misaligned for the type being extracted. That is the configuration in which
reading or writing through a typed pointer would be undefined behavior, so a
regression fails the value checks and also trips UBSan's alignment check in a
sanitizer build.
The expected values do not depend on the host byte order. A `DataExtractor`
converts from the data's byte order to the host's, so the little- and big-endian
cases together always exercise both the straight-copy and the byte-swapping
path, whichever way round the host is.
---
lldb/unittests/Utility/DataExtractorTest.cpp | 167 +++++++++++++++++++
1 file changed, 167 insertions(+)
diff --git a/lldb/unittests/Utility/DataExtractorTest.cpp b/lldb/unittests/Utility/DataExtractorTest.cpp
index 536e69755d17b..8b6270b7dc0d7 100644
--- a/lldb/unittests/Utility/DataExtractorTest.cpp
+++ b/lldb/unittests/Utility/DataExtractorTest.cpp
@@ -10,8 +10,55 @@
#include "lldb/Utility/DataExtractor.h"
+#include <cstdint>
+#include <cstring>
+
using namespace lldb_private;
+namespace {
+/// Destination for the multi-value GetU16/GetU32/GetU64 overloads. The usable
+/// region starts one byte into an 8-byte aligned object, so its address is
+/// misaligned for every type extracted into it below.
+struct alignas(8) UnalignedDest {
+ uint8_t pad;
+ uint8_t data[24];
+};
+
+/// Byte written over an UnalignedDest before each extraction, so that a write
+/// past the values that were asked for is visible afterwards.
+constexpr uint8_t g_fill = 0xAA;
+
+/// Read element \a i of type \c T out of \a dest without forming a misaligned
+/// pointer to \c T.
+template <typename T> T GetElement(const UnalignedDest &dest, size_t i) {
+ T value;
+ memcpy(&value, dest.data + i * sizeof(T), sizeof(T));
+ return value;
+}
+
+/// Offset of the first byte of \a dest at or after \a from that no longer holds
+/// g_fill, or sizeof(UnalignedDest::data) if the fill is still intact. Lets a
+/// test assert that an extraction wrote no further than it was asked to.
+size_t FirstWrittenByte(const UnalignedDest &dest, size_t from) {
+ for (size_t i = from; i < sizeof(dest.data); ++i)
+ if (dest.data[i] != g_fill)
+ return i;
+ return sizeof(dest.data);
+}
+
+/// Source for the GetU16/GetU32/GetU64 array tests. Reads start at offset 1, so
+/// they are misaligned for every type extracted, as is the UnalignedDest they
+/// are copied into.
+///
+/// The expected values do not depend on the host byte order. A DataExtractor
+/// converts from the data's byte order to the host's, so the little- and
+/// big-endian cases together always exercise both the straight-copy and the
+/// byte-swapping path, whichever way round the host is.
+alignas(8) constexpr uint8_t g_buffer[] = {0x00, 0x01, 0x23, 0x45, 0x67, 0x89,
+ 0xab, 0xcd, 0xef, 0x11, 0x22, 0x33,
+ 0x44, 0x55, 0x66, 0x77, 0x88};
+} // namespace
+
TEST(DataExtractorTest, GetBitfield) {
uint8_t buffer[] = {0x01, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF};
DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
@@ -401,3 +448,123 @@ TEST(DataExtractorTest, GetDoubleUnaligned) {
EXPECT_EQ(9U, offset);
}
}
+
+TEST(DataExtractorTest, GetU16Array) {
+ DataExtractor LE(g_buffer, sizeof(g_buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+ DataExtractor BE(g_buffer, sizeof(g_buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+ UnalignedDest dest;
+ lldb::offset_t offset;
+
+ memset(&dest, g_fill, sizeof(dest));
+ offset = 1;
+ ASSERT_EQ(static_cast<void *>(dest.data), LE.GetU16(&offset, dest.data, 4));
+ EXPECT_EQ(9U, offset);
+ EXPECT_EQ(0x2301U, GetElement<uint16_t>(dest, 0));
+ EXPECT_EQ(0x6745U, GetElement<uint16_t>(dest, 1));
+ EXPECT_EQ(0xab89U, GetElement<uint16_t>(dest, 2));
+ EXPECT_EQ(0xefcdU, GetElement<uint16_t>(dest, 3));
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 4 * sizeof(uint16_t)));
+
+ memset(&dest, g_fill, sizeof(dest));
+ offset = 1;
+ ASSERT_EQ(static_cast<void *>(dest.data), BE.GetU16(&offset, dest.data, 4));
+ EXPECT_EQ(9U, offset);
+ EXPECT_EQ(0x0123U, GetElement<uint16_t>(dest, 0));
+ EXPECT_EQ(0x4567U, GetElement<uint16_t>(dest, 1));
+ EXPECT_EQ(0x89abU, GetElement<uint16_t>(dest, 2));
+ EXPECT_EQ(0xcdefU, GetElement<uint16_t>(dest, 3));
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 4 * sizeof(uint16_t)));
+}
+
+TEST(DataExtractorTest, GetU32Array) {
+ DataExtractor LE(g_buffer, sizeof(g_buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+ DataExtractor BE(g_buffer, sizeof(g_buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+ UnalignedDest dest;
+ lldb::offset_t offset;
+
+ memset(&dest, g_fill, sizeof(dest));
+ offset = 1;
+ ASSERT_EQ(static_cast<void *>(dest.data), LE.GetU32(&offset, dest.data, 2));
+ EXPECT_EQ(9U, offset);
+ EXPECT_EQ(0x67452301U, GetElement<uint32_t>(dest, 0));
+ EXPECT_EQ(0xefcdab89U, GetElement<uint32_t>(dest, 1));
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 2 * sizeof(uint32_t)));
+
+ memset(&dest, g_fill, sizeof(dest));
+ offset = 1;
+ ASSERT_EQ(static_cast<void *>(dest.data), BE.GetU32(&offset, dest.data, 2));
+ EXPECT_EQ(9U, offset);
+ EXPECT_EQ(0x01234567U, GetElement<uint32_t>(dest, 0));
+ EXPECT_EQ(0x89abcdefU, GetElement<uint32_t>(dest, 1));
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 2 * sizeof(uint32_t)));
+}
+
+TEST(DataExtractorTest, GetU64Array) {
+ DataExtractor LE(g_buffer, sizeof(g_buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+ DataExtractor BE(g_buffer, sizeof(g_buffer), lldb::eByteOrderBig,
+ sizeof(void *));
+ UnalignedDest dest;
+ lldb::offset_t offset;
+
+ memset(&dest, g_fill, sizeof(dest));
+ offset = 1;
+ ASSERT_EQ(static_cast<void *>(dest.data), LE.GetU64(&offset, dest.data, 2));
+ EXPECT_EQ(17U, offset);
+ EXPECT_EQ(0xefcdab8967452301ULL, GetElement<uint64_t>(dest, 0));
+ EXPECT_EQ(0x8877665544332211ULL, GetElement<uint64_t>(dest, 1));
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 2 * sizeof(uint64_t)));
+
+ memset(&dest, g_fill, sizeof(dest));
+ offset = 1;
+ ASSERT_EQ(static_cast<void *>(dest.data), BE.GetU64(&offset, dest.data, 2));
+ EXPECT_EQ(17U, offset);
+ EXPECT_EQ(0x0123456789abcdefULL, GetElement<uint64_t>(dest, 0));
+ EXPECT_EQ(0x1122334455667788ULL, GetElement<uint64_t>(dest, 1));
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 2 * sizeof(uint64_t)));
+}
+
+// The three tests below ask for one value more than the buffer holds. That has
+// to fail, leave the offset unmodified and write nothing at all. The byte order
+// does not matter: the bounds check rejects the read before any swapping
+// happens.
+
+TEST(DataExtractorTest, GetU16ArrayOutOfBounds) {
+ DataExtractor data(g_buffer, sizeof(g_buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+ UnalignedDest dest;
+ memset(&dest, g_fill, sizeof(dest));
+
+ lldb::offset_t offset = 1;
+ EXPECT_EQ(nullptr, data.GetU16(&offset, dest.data, 9));
+ EXPECT_EQ(1U, offset);
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 0));
+}
+
+TEST(DataExtractorTest, GetU32ArrayOutOfBounds) {
+ DataExtractor data(g_buffer, sizeof(g_buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+ UnalignedDest dest;
+ memset(&dest, g_fill, sizeof(dest));
+
+ lldb::offset_t offset = 1;
+ EXPECT_EQ(nullptr, data.GetU32(&offset, dest.data, 5));
+ EXPECT_EQ(1U, offset);
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 0));
+}
+
+TEST(DataExtractorTest, GetU64ArrayOutOfBounds) {
+ DataExtractor data(g_buffer, sizeof(g_buffer), lldb::eByteOrderLittle,
+ sizeof(void *));
+ UnalignedDest dest;
+ memset(&dest, g_fill, sizeof(dest));
+
+ lldb::offset_t offset = 1;
+ EXPECT_EQ(nullptr, data.GetU64(&offset, dest.data, 3));
+ EXPECT_EQ(1U, offset);
+ EXPECT_EQ(sizeof(dest.data), FirstWrittenByte(dest, 0));
+}
More information about the lldb-commits
mailing list