Skip to content

Commit

Permalink
[c++] Use memcpy to type pun; fix alias violations
Browse files Browse the repository at this point in the history
We now use memcpy to pun from one type to another, avoiding aliasing
violations as well as alignment errors. x86-like chips never failed due
to the alignment errors, but others, like some ARM chips, did. This
should also fix emscripten cross-compilation.

Fixes #305
  • Loading branch information
chwarr committed Mar 22, 2017
1 parent 7bffdde commit 1ba29cd
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 78 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ different versioning scheme, following the Haskell community's
* Eliminate need for warning suppression on MSVC14 via warning.h in Bond
itself. warning.h is still in place on MSVC12; furthermore, we don't alter
warning.h for now as it may be depended upon by application code.
* Avoid unaligned memory access on non-x86/x64 platforms.
[Issue #305](https://github.com/Microsoft/bond/issues/305)
* Improve compliance with strict-aliasing rules.
* Bond now builds on Clang/GCC with `-fstrict-aliasing`.

## C# ###

Expand Down
3 changes: 3 additions & 0 deletions cmake/Config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ add_definitions (-D_ENABLE_ATOMIC_ALIGNMENT_FIX)

cxx_add_compile_options(Clang
-fPIC
-fstrict-aliasing
--std=c++11
-Wall
-Werror
Expand All @@ -122,6 +123,7 @@ cxx_add_compile_options(Clang

cxx_add_compile_options(AppleClang
-fPIC
-fstrict-aliasing
--std=c++11
-Wall
-Werror
Expand All @@ -130,6 +132,7 @@ cxx_add_compile_options(AppleClang

cxx_add_compile_options(GNU
-fPIC
-fstrict-aliasing
--std=c++11
-Wall
-Werror
Expand Down
12 changes: 10 additions & 2 deletions cpp/inc/bond/protocol/compact_binary.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include <bond/stream/output_counter.h>
#include <boost/call_traits.hpp>
#include <boost/noncopyable.hpp>
#include <boost/static_assert.hpp>
#include <cstring>

/*
Expand Down Expand Up @@ -224,14 +226,18 @@ class CompactBinaryReader

if (id == (0x07 << 5))
{
// ID is in (0xff, 0xffff] and is in the next two bytes
_input.Read(id);
}
else if (id == (0x06 << 5))
{
_input.Read(reinterpret_cast<uint8_t&>(id));
// ID is in (5, 0xff] and is in the next one byte
_input.Read(raw);
id = static_cast<uint16_t>(raw);
}
else
{
// ID is in [0, 5] and was in the byte we already read
id >>= 5;
}
}
Expand Down Expand Up @@ -310,7 +316,9 @@ class CompactBinaryReader
Read(T& value)
{
BOOST_STATIC_ASSERT(sizeof(value) == sizeof(int32_t));
Read(*reinterpret_cast<int32_t*>(&value));
int32_t raw;
Read(raw);
std::memcpy(&value, &raw, sizeof(raw));
}


Expand Down
13 changes: 7 additions & 6 deletions cpp/inc/bond/protocol/random_protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
#include <bond/core/containers.h>
#include <bond/core/blob.h>
#include <bond/core/traits.h>

#include <cstring>
#include <boost/static_assert.hpp>
#include <cstring>

namespace bond
{
Expand Down Expand Up @@ -116,6 +115,8 @@ class RandomProtocolReader

void Read(double& value)
{
BOOST_STATIC_ASSERT(sizeof(double) == sizeof(uint64_t));

uint8_t sign;
uint8_t exponent;
uint32_t mantissa;
Expand All @@ -129,12 +130,13 @@ class RandomProtocolReader
exponent = 0x80;

uint64_t bits = ((uint64_t)(sign) << 63) | ((uint64_t)(exponent) << (52 + 3)) | (uint64_t)mantissa;

*reinterpret_cast<uint64_t*>(&value) = bits;
std::memcpy(&value, &bits, sizeof(bits));
}

void Read(float& value)
{
BOOST_STATIC_ASSERT(sizeof(float) == sizeof(uint32_t));

uint8_t sign;
uint8_t exponent;
uint16_t mantissa;
Expand All @@ -148,8 +150,7 @@ class RandomProtocolReader
exponent = 0x80;

uint32_t bits = ((uint32_t)(sign) << 31) | ((uint32_t)(exponent) << 23) | (uint32_t)mantissa;

*reinterpret_cast<uint32_t*>(&value) = bits;
std::memcpy(&value, &bits, sizeof(bits));
}

template <typename T>
Expand Down
38 changes: 22 additions & 16 deletions cpp/inc/bond/stream/input_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@

#include <bond/core/blob.h>
#include <bond/core/exception.h>
#include <bond/core/traits.h>
#include <boost/static_assert.hpp>
#include <cstring>

namespace bond
{
Expand All @@ -21,7 +23,7 @@ struct VariableUnsignedUnchecked
{
T byte = *p++;
value |= (byte & 0x7f) << Shift;

if (byte >= 0x80)
VariableUnsignedUnchecked<T, Shift + 7>::Read(p, value);
}
Expand All @@ -36,7 +38,7 @@ struct VariableUnsignedUnchecked<T, 0>
{
T byte = *p++;
value = (byte & 0x7f);

if (byte >= 0x80)
VariableUnsignedUnchecked<T, 7>::Read(p, value);
}
Expand Down Expand Up @@ -84,14 +86,14 @@ struct VariableUnsignedUnchecked<uint64_t, 56>
}

/// @brief Memory backed input stream
class InputBuffer
class InputBuffer
{
public:
/// @brief Default constructor
InputBuffer()
: _pointer()
{}

/// @brief Construct from a blob
///
/// InputBuffer makes a copy of the blob. Assuming that the blob was
Expand All @@ -106,7 +108,7 @@ class InputBuffer
///
/// Pointer(s) to the memory buffer may be held by the objects deserialized
/// from the stream. It is the application's responsibility to manage
/// the lifetime of the memory buffer appropriately.
/// the lifetime of the memory buffer appropriately.
InputBuffer(const void* buffer, uint32_t length)
: _blob(buffer, length),
_pointer()
Expand All @@ -126,38 +128,42 @@ class InputBuffer
{
EofException(sizeof(uint8_t));
}

value = static_cast<const uint8_t>(_blob.content()[_pointer++]);
}


template <typename T>
void Read(T& value)
{
BOOST_STATIC_ASSERT(bond::is_arithmetic<T>::value || bond::is_enum<T>::value);

if (sizeof(T) > _blob.length() - _pointer)
{
EofException(sizeof(T));
}

value = *reinterpret_cast<const T*>(_blob.content() + _pointer);

const void* const src = _blob.content() + _pointer;
std::memcpy(&value, src, sizeof(T));

_pointer += sizeof(T);
}


void Read(void *buffer, uint32_t size)
{
if (size > _blob.length() - _pointer)
{
EofException(size);
}

::memcpy(buffer, _blob.content() + _pointer, size);
const void* const src = _blob.content() + _pointer;
std::memcpy(buffer, src, size);

_pointer += size;
}


void Read(blob& blob, uint32_t size)
{
if (size > _blob.length() - _pointer)
Expand All @@ -170,14 +176,14 @@ class InputBuffer
_pointer += size;
}

void Skip(uint32_t size)

void Skip(uint32_t size)
{
if (size > _blob.length() - _pointer)
{
return;
}

_pointer += size;
}

Expand All @@ -202,7 +208,7 @@ class InputBuffer
GenericReadVariableUnsigned(*this, value);
}
}

protected:
void EofException(uint32_t size) const
{
Expand Down
Loading

0 comments on commit 1ba29cd

Please sign in to comment.