Skip to content

Commit

Permalink
[lldb] Fix unaligned load in DataExtractor
Browse files Browse the repository at this point in the history
Somehow UBSan would only report the unaligned load in TestLinuxCore.py
when running the tests with reproducers. This patch fixes the issue by
using a memcpy in the GetDouble and the GetFloat method.

Differential revision: https://reviews.llvm.org/D83256
  • Loading branch information
JDevlieghere committed Jul 7, 2020
1 parent 7fc279c commit 5e9b16b
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 33 deletions.
17 changes: 17 additions & 0 deletions lldb/include/lldb/Utility/DataExtractor.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
#ifndef LLDB_UTILITY_DATAEXTRACTOR_H
#define LLDB_UTILITY_DATAEXTRACTOR_H

#include "lldb/Utility/Endian.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-forward.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/DataExtractor.h"
#include "llvm/Support/SwapByteOrder.h"

#include <cassert>
#include <stdint.h>
Expand Down Expand Up @@ -979,6 +981,21 @@ class DataExtractor {
}

protected:
template <typename T> T Get(lldb::offset_t *offset_ptr, T fail_value) const {
constexpr size_t src_size = sizeof(T);
T val = fail_value;

const T *src = static_cast<const T *>(GetData(offset_ptr, src_size));
if (!src)
return val;

memcpy(&val, src, src_size);
if (m_byte_order != endian::InlHostByteOrder())
llvm::sys::swapByteOrder(val);

return val;
}

// Member variables
const uint8_t *m_start; ///< A pointer to the first byte of data.
const uint8_t
Expand Down
35 changes: 2 additions & 33 deletions lldb/source/Utility/DataExtractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

#include "lldb/Utility/DataBuffer.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/Endian.h"
#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Stream.h"
Expand Down Expand Up @@ -624,41 +623,11 @@ int64_t DataExtractor::GetMaxS64Bitfield(offset_t *offset_ptr, size_t size,
}

float DataExtractor::GetFloat(offset_t *offset_ptr) const {
typedef float float_type;
float_type val = 0.0;
const size_t src_size = sizeof(float_type);
const float_type *src =
static_cast<const float_type *>(GetData(offset_ptr, src_size));
if (src) {
if (m_byte_order != endian::InlHostByteOrder()) {
const uint8_t *src_data = reinterpret_cast<const uint8_t *>(src);
uint8_t *dst_data = reinterpret_cast<uint8_t *>(&val);
for (size_t i = 0; i < sizeof(float_type); ++i)
dst_data[sizeof(float_type) - 1 - i] = src_data[i];
} else {
val = *src;
}
}
return val;
return Get<float>(offset_ptr, 0.0f);
}

double DataExtractor::GetDouble(offset_t *offset_ptr) const {
typedef double float_type;
float_type val = 0.0;
const size_t src_size = sizeof(float_type);
const float_type *src =
static_cast<const float_type *>(GetData(offset_ptr, src_size));
if (src) {
if (m_byte_order != endian::InlHostByteOrder()) {
const uint8_t *src_data = reinterpret_cast<const uint8_t *>(src);
uint8_t *dst_data = reinterpret_cast<uint8_t *>(&val);
for (size_t i = 0; i < sizeof(float_type); ++i)
dst_data[sizeof(float_type) - 1 - i] = src_data[i];
} else {
val = *src;
}
}
return val;
return Get<double>(offset_ptr, 0.0);
}

long double DataExtractor::GetLongDouble(offset_t *offset_ptr) const {
Expand Down
102 changes: 102 additions & 0 deletions lldb/unittests/Utility/DataExtractorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -299,3 +299,105 @@ TEST(DataExtractorTest, GetULEB128_bit63) {
EXPECT_EQ(expected, BE.GetULEB128(&offset));
EXPECT_EQ(9U, offset);
}

TEST(DataExtractorTest, GetFloat) {
float expected = 4.0f;
lldb::offset_t offset;

{
uint8_t buffer[] = {0x00, 0x00, 0x80, 0x40};
DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
sizeof(void *));

offset = 0;
EXPECT_DOUBLE_EQ(expected, LE.GetFloat(&offset));
EXPECT_EQ(4U, offset);
}

{
uint8_t buffer[] = {0x40, 0x80, 0x00, 0x00};
DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
sizeof(void *));
offset = 0;
EXPECT_DOUBLE_EQ(expected, BE.GetFloat(&offset));
EXPECT_EQ(4U, offset);
}
}

TEST(DataExtractorTest, GetFloatUnaligned) {
float expected = 4.0f;
lldb::offset_t offset;

{
uint8_t buffer[] = {0x00, 0x00, 0x00, 0x80, 0x40};
DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
sizeof(void *));

offset = 1;
EXPECT_DOUBLE_EQ(expected, LE.GetFloat(&offset));
EXPECT_EQ(5U, offset);
}

{
uint8_t buffer[] = {0x00, 0x40, 0x80, 0x00, 0x00};
DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
sizeof(void *));
offset = 1;
EXPECT_DOUBLE_EQ(expected, BE.GetFloat(&offset));
EXPECT_EQ(5U, offset);
}
}

TEST(DataExtractorTest, GetDouble) {
if (sizeof(double) != 8)
return;

double expected = 4.0f;
lldb::offset_t offset;

{
uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
sizeof(void *));

offset = 0;
EXPECT_DOUBLE_EQ(expected, LE.GetDouble(&offset));
EXPECT_EQ(8U, offset);
}

{
uint8_t buffer[] = {0x40, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
sizeof(void *));
offset = 0;
EXPECT_DOUBLE_EQ(expected, BE.GetDouble(&offset));
EXPECT_EQ(8U, offset);
}
}

TEST(DataExtractorTest, GetDoubleUnaligned) {
if (sizeof(double) != 8)
return;

float expected = 4.0f;
lldb::offset_t offset;

{
uint8_t buffer[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x40};
DataExtractor LE(buffer, sizeof(buffer), lldb::eByteOrderLittle,
sizeof(void *));

offset = 1;
EXPECT_DOUBLE_EQ(expected, LE.GetDouble(&offset));
EXPECT_EQ(9U, offset);
}

{
uint8_t buffer[] = {0x00, 0x40, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
DataExtractor BE(buffer, sizeof(buffer), lldb::eByteOrderBig,
sizeof(void *));
offset = 1;
EXPECT_DOUBLE_EQ(expected, BE.GetDouble(&offset));
EXPECT_EQ(9U, offset);
}
}

0 comments on commit 5e9b16b

Please sign in to comment.