5

I work on a medium-sized open source project which requires interpreting raw bytes as different types. This is performed by creative use of reinterpret casting. However, in a simple test case, compiling with GCC 10 or above with optimisation levels above O1 the test fails. Lower versions of GCC as well as Clang do not show this issue.

We managed to write a small reproducible sample as follows. This takes a uint64_t and gives back a pointer to an array of uint32_t containing the data from the uint64_t. The expected result is that bar() returns the number 1 as a uint32_t.

#include <cstddef>
#include <cstdint>

struct RegisterValue {
    size_t bytes = 8;
    alignas(8) char localValue[16] = {42};
    void set(uint64_t value) {
        uint64_t* view = reinterpret_cast<uint64_t*>(this->localValue);
        view[0] = value;
    }
    uint32_t* getAsVector() {
        if (bytes <= 16) {
            return reinterpret_cast<uint32_t*>(this->localValue);
        } else {
            static uint32_t t = {0xdeadbeef};
            return &t;
        }
    }
};
extern "C" uint32_t bar() {
    RegisterValue v;
    v.set(0x0000000200000001);
    return v.getAsVector()[0];
}

You can play around with the code in compiler explorer.

Essentially, all the reinterpret_casting is being aggressively optimised by GCC resulting in assembly which simply returns an immediate. This is expected. However, at lower optimisation levels (O1) this returns the expected result (1), but at higher levels (O2, O3) this incorrectly returns 42. What makes this more confusing is almost any change to the source (e.g. removing the if/else leaving the first return in getAsVector()) causes the expected behaviour.

This sounds like UB but we have read CPP Reference: reinterpret_cast and can't confidently say whether this is UB or not. If it is, then we need to update our implementation, if it isn't, this would suggest a compiler bug.

Any insight would be helpful.

8
  • 1
    "...simply returns an immediate" what does that mean? immediate? Commented Oct 17, 2024 at 11:25
  • 2
    I am not a C++ expert, but in C this would be a clear strict aliasing violation. I would assume that C++ has inherited these rules, because they are linked to what compiler optimizations are possible. Commented Oct 17, 2024 at 11:40
  • 2
    @463035818_is_not_an_ai reverseengineering.stackexchange.com/q/17671/24377 Commented Oct 17, 2024 at 11:54
  • 3
    The standard way to get bytes in and out of integers is to use memcpy. The compiler knows about this and will optimize it. See stackoverflow.com/a/70138631/17398063 Commented Oct 17, 2024 at 12:02
  • 1
    @463035818_is_not_an_ai the answer Weijun Zhou gave is correct. You can see what I mean through the following link godbolt.org/z/1eYs4orKc Commented Oct 17, 2024 at 14:17

2 Answers 2

4

Yes, what you are doing is Undefined Behavior, type aliasing violation, because you access your localValue through uint64_t*. According to reinterpret_cast, uint64_t* is type-accessible through char*, but not vice-versa. First part can be fixed with std::memcpy:

void set(uint64_t value) {
    std::memcpy(localValue, &value, sizeof(value));
}

but I don't have any straightforward idea for uint32_t* getAsVector() without introducing additional class fields to hold the results. You could either return const char* and let the user handle uint32 conversion, or return a std::vector<std::uint32_t>.

Sign up to request clarification or add additional context in comments.

1 Comment

Thank you. This was one of our solutions but we came to the same conclusion where getAsVector would need more fields or would need to return the data in a different way. Not the end of the world, but would require extensive updating across the code base and slightly more annoying interaction with the class.
0

TL;DR: first part of the answer contains an issue due to the switch between 32 and 64 bits (thanks to @PasserBy for its valuable comments). An updated snippet is provided at the end.

It is indeed UB.

Beyond the magic numbers for size and alignment (which are more a design issue) you have:

alignas(8) char localValue[16] = {42};

At this point only an array of initialized char exists.

Then

uint32_t* getAsVector() {
        if (bytes <= 16) {
            return reinterpret_cast<uint32_t*>(this->localValue);
        }...

This implicitely creates (see below about implicit lifetime types) an array of std::uint32_t at the storage location but it's value maybe still be indeterminate. Indeed, if called before any set call only char objects have been initialized. Evaluation of indeterminate value is UB (see https://timsong-cpp.github.io/cppwp/n4950/basic.indet#2, thanks to @user17732522).

Besides, the correct way to tell the compiler that it is actually looking at an object of a different type is with std::launder. Notice that this does not help starting object lifetime, it only tells that we are effectively pointing to an object that exists (it helps the compiler to perform better and more accurate optimizations):

... std::launder(reinterpret_cast<uint32_t*>(this->localValue));

Even if you use your setter:

void set(uint64_t value) {
        uint64_t* view = reinterpret_cast<uint64_t*>(this->localValue);
        view[0] = value;
    }

This implicitely creates an array of std::uint64_t at the storage location and you're initializing its value (yet the std::launder is also missing).

But the issue is that, in the getter, you're looking for std::uint32_t, not std::uint64_t and you initialized none.

Besides, when switching between one type of array and the other, your also triggering UB by trying to reference objects of unrelated types at the same location. It's just not allowed. I've opened another question to check if it possible to "convey" a value representation from a type to another one.

The only ways to map a byte-level value representation would be explicitly with calls to std::memcpy, std::bit_cast or std::start_lifetime_as for instance (see below) but they are some pitfalls too.

<original imperfect answer, improved below> The correct way to go would be

alignas(std::uint64_t) alignas(std::uint32_t) char localValue[2*sizeof(std::uint64_t)];

(If I'm getting your intent right).

Then, in setter:

std::uint64_t* view = std::launder(reinterpret_cast<uint64_t*>(this->localValue));

provided that std::uint64_t is indeed implemented as a scalar type (which should be and can be check with is_scalar).

Why is it so: scalar type are implicit lifetime types which means, basically, that they are implicitly created (starting their lifetime) with some operations such as the creation of an array of bytes.
Besides, if I get it write, the std::uint64_t will live as long as its storage (third bullet of end of lifetime).

the std::launder is necessary to explicit to the compiler that an std::uint64 _t effectively exists at the given address.

Also, in getter:

std::uint32_t array[4] = std::launder(reinterpret_cast<uint32_t*>(this->localValue)); // actual size depends on the need but must match storage size
return array;

The same arguments as above apply (an array is also implicit lifetime type).

Another issue with your code is that you may have an indeterminate value when getting the buffer as an std::uint32 _t without having usedset beforehand, because nothing guarantees that the value representation will be kept.

If you want 42 to be usable as a default initialisation value, you may need a std::start_lifetime_as (c++23) to keep the value representation from the initialization. In this case, it will work because value representation of integers is well defined in c++. It won't be so straightforward for other types.
This also validate the switching between array of 32 bits and single 64 bits.

TWIST: to this day, std::start_lifetime_as is not supported: https://en.cppreference.com/w/cpp/compiler_support/23

As a work around you can initialize it with a constexpr static version of set. Maybe a lambda will do also.

<the issue in this answer is that I set 64 bits integer and read uninitialized 32 bits ones> </original imperfect answer, improved below>


This is the improved answer:

#include <array>
#include <cstddef>
#include <cstdint>
#include <new>

struct S {
    alignas(std::uint64_t) alignas(std::uint32_t)
        std::array<std::byte, sizeof(std::uint64_t)> localValue = []() {
            std::array<std::byte, sizeof(std::uint64_t)> init;
            std::uint32_t* val =
                std::launder(reinterpret_cast<std::uint32_t*>(init.data()));
            val[0] = 0;
            val[1] = 42;
            return init;
        }();

    void set(uint64_t value) {
        std::uint32_t* view =
            std::launder(reinterpret_cast<std::uint32_t*>(localValue.data()));
        // not proud of this but well defined IMHO :)
        view[0] = static_cast<std::uint32_t>(value >> 8u*sizeof(std::uint32_t));
        view[1] = static_cast<std::uint32_t>((value << 8u*sizeof(std::uint32_t)) >>
                                             8u*sizeof(std::uint32_t));
    }
    uint32_t* getAsVector() {
        return std::launder(
            reinterpret_cast<std::uint32_t*>(localValue.data()));
    }
};

LIVE

In this version arrays of 32 bits integer are guaranteed to live on every paths. Only 32 bits value representation are set.

23 Comments

@DanWeaver I missed a detail in you question: in fact you don't want a single 64 bit integer but 2 32 ones? Why is the setter 64 bits? My answer will have to be updated accordingly but the same reasoning will apply.
@PasserBy fixed, thx.
Apologies, my mistake, it only accepts parameter packs but not multiple arguments for some reason. You can write alignas(std::uint64_t) alignas(std::uint32_t) char ... though.
@PasserBy I saw and updated accordingly, I misread the doc also ;)
Even if set is called, the value is written as std::uint64_t and read as a std::uint32_t. That is UB without std::start_lifetime_as, or rather, performing lvalue-to-rvalue conversion in v.getAsVector()[0] will be UB.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.