26

I've been trying to figure out how to access a mapped buffer from C++17 without invoking undefined behavior. For this example, I'll use a buffer returned by Vulkan's vkMapMemory.

So, according to N4659 (the final C++17 working draft), section [intro.object] (emphasis added):

The constructs in a C++ program create, destroy, refer to, access, and manipulate objects. An object is created by a definition (6.1), by a new-expression (8.3.4), when implicitly changing the active member of a union (12.3), or when a temporary object is created (7.4, 15.2).

These are, apparently, the only valid ways to create a C++ object. So let's say we get a void* pointer to a mapped region of host-visible (and coherent) device memory (assuming, of course, that all the required arguments have valid values and the call succeeds, and the returned block of memory is of sufficient size and properly aligned):

void* ptr{};
vkMapMemory(device, memory, offset, size, flags, &ptr);
assert(ptr != nullptr);

Now, I wish to access this memory as a float array. The obvious thing to do would be to static_cast the pointer and go on my merry way as follows:

volatile float* float_array = static_cast<volatile float*>(ptr);

(The volatile is included since this is mapped as coherent memory, and thus may be written by the GPU at any point). However, a float array doesn't technically exist in that memory location, at least not in the sense of the quoted excerpt, and thus accessing the memory through such a pointer would be undefined behavior. Therefore, according to my understanding, I'm left with two options:

1. memcpy the data

It should always be possible to use a local buffer, cast it to std::byte* and memcpy the representation over to the mapped region. The GPU will interpret it as instructed in the shaders (in this case, as an array of 32-bit float) and thus problem solved. However, this requires extra memory and extra copies, so I would prefer to avoid this.

2. placement-new the array

It appears that section [new.delete.placement] doesn't impose any restrictions on how the placement address is obtained (it need not be a safely-derived pointer regardless of the implementation's pointer safety). It should, therefore, be possible to create a valid float array via placement-new as follows:

volatile float* float_array = new (ptr) volatile float[sizeInFloats];

The pointer float_array should now be safe to access (within the bounds of the array, or one-past).


So, my questions are the following:

  1. Is the simple static_cast indeed undefined behavior?
  2. Is this placement-new usage well-defined?
  3. Is this technique applicable to similar situations, such as accessing memory-mapped hardware?

As a side note, I've never had an issue by simply casting the returned pointer, I'm just trying to figure out what the proper way to do this would be, according to the letter of the standard.

curiousguy
  • 8,038
  • 2
  • 40
  • 58
Socrates Zouras
  • 271
  • 2
  • 7
  • 3
    Beware that placement new with an array type seems to have implementation specified memory overhead. [Related question](https://stackoverflow.com/questions/8720425/array-placement-new-requires-unspecified-overhead-in-the-buffer). – François Andrieux Nov 16 '18 at 15:36
  • 3
    You might be interested in the ["bless" proposal](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0593r2.html). I want to say it includes this use case, but I admit I'm not 100% sure. – chris Nov 16 '18 at 15:55
  • 3
    *"I'm just trying to figure out what the proper way to do this would be"* To be honest, I don't see the point in doing this. Since your compiler has no way to know how `vkMapMemory` works, it has to assume that the `float`s were created properly, and the UB in this case can't have any consequences. – HolyBlackCat Nov 16 '18 at 16:10
  • @HolyBlackCat That's a dangerous game to play. Empirically, it probably works right now, but would you really bet against the entirity of conceivable "as-if" optimizations that this cannot turn out bad? – Max Langhof Nov 16 '18 at 17:36
  • 1
    @HolyBlackCat: just because something is undefined by the C++ spec, that does not mean the compiler cannot define some meaning of its own for it -- EVERY compiler (and every OS) extends the spec to provide additional functionality, so you need to check those specifications. If the OP was using `mmap`, I would refer to the POSIX spec that defines what that is, but I have no idea where vkMapMemory comes from. Presumably there is a standard somewhere that defines what it does. – Chris Dodd Nov 17 '18 at 20:47
  • IMO this is outside the scope of what the standard covers – M.M Nov 18 '18 at 08:16
  • "Now, I wish to access this memory as a float array." Do you really need to access this as an array? Isn't it enough, if you can put floats in there? Ask this, because the accepted answer doesn't give a float array either. And you can memcpy floats one-by-one into the mapped area, there is no need for a separate large local buffer. That would be the only standard conformant way to do this, as far as I know (and you have to be careful with pointer addition, as supposedly `ptr` doesn't point to an actual C++ array). – geza Nov 18 '18 at 19:27
  • **[basic.life]**: "The lifetime of an object of type T begins when: — storage with the proper alignment and size for type T is obtained, and — if the object has non-trivial initialization, its initialization is complete." Since `float` array has trivial initialization, the second requirement does not apply. Float array objects begin their lifetime once storage has been allocated. – Raymond Chen Nov 20 '18 at 04:57

3 Answers3

9

Short answer

As per the Standard, everything involving hardware-mapped memory is undefined behavior since that concept does not exist for the abstract machine. You should refer to your implementation manual.


Long answer

Even though hardware-mapped memory is undefined behavior by the Standard, we can imagine any sane implementation providing some obeys common rules. Some constructs are then more undefined behavior than others (whatever that means).

Is the simple static_cast indeed undefined behavior?

volatile float* float_array = static_cast<volatile float*>(ptr);

Yes, this is undefined behavior and have been discussed many times on StackOverflow.

Is this placement-new usage well-defined?

volatile float* float_array = new (ptr) volatile float[N];

No, even though this looks well defined, this is implementation dependent. As it happens, operator ::new[] is allowed to reserve some overhead1, 2, and you cannot know how much unless you check your toolchain documentation. As a consequence, ::new (dst) T[N] requires an unknown amount of memory greater or equal to N*sizeof T and any dst you allocate might be too small, involving buffer overflow.

How to proceed, then?

A solution would be to manually build a sequence of floats:

auto p = static_cast<volatile float*>(ptr);
for (std::size_t n = 0 ; n < N; ++n) {
    ::new (p+n) volatile float;
}

Or equivalently, relying on the Standard Library:

#include <memory>
auto p = static_cast<volatile float*>(ptr);
std::uninitialized_default_construct(p, p+N);

This constructs contiguously N uninitialized volatile float objects at the memory pointed to by ptr. This means you must initialize those before reading them; reading an uninitialized object is undefined behavior.

Is this technique applicable to similar situations, such as accessing memory-mapped hardware?

No, again this is really implementation-defined. We can only assume your implementation took reasonable choices, but you should check what its documentation says.

Community
  • 1
  • 1
YSC
  • 38,212
  • 9
  • 96
  • 149
  • Comments are not for extended discussion; this conversation has been [moved to chat](https://chat.stackoverflow.com/rooms/183950/discussion-on-answer-by-ysc-how-to-properly-access-mapped-memory-without-undefin). –  Nov 20 '18 at 11:57
4

The C++ spec has no concept of mapped memory, so everything to do with it is undefined behavior as far as the C++ spec is concerned. So you need to look to the particular implementation (compiler and operating system) that you are using to see what is defined and what you can do safely.

On most systems, mapping will return memory that came from somewhere else, and may (or may not) have been initialized in a way that is compatible with some specific type. In general, if the memory was originally written as float values of the correct, supported form, then you can safely cast the pointer to a float * and access it that way. But you do need to know how the memory being mapped was originally written.

Chris Dodd
  • 119,907
  • 13
  • 134
  • 226
  • @curiousguy: Almost correct, so no C++ compiler is required to support it. It does have a concept of the 'compilation unit' and more than one may be combined in an implementation-defined way, so that is at least a nod to separate compilation in the standard, if only in its absence. – Chris Dodd Nov 22 '18 at 00:45
  • You mean "translation unit" (TU). And historically pure separate compilation (compilation of each TU to executable code then linking) was not possible in std C++. – curiousguy Nov 22 '18 at 06:08
-3

C++ is compatible with C, and manipulating raw memory is kind-of-what C was perfect for. So don't worry, C++ is perfectly capable of doing what you want.

  • Edit:- follow this link for the simple answer for C/C++ compatibilty. -

In your example, you do not need to call new at all! To explain...

Not all objects in C++ require construction. These are known as PoD (plain-old-data) types. They are

1) Basic types (floats/ints/enums,etc).
2) All Pointers, but not smart pointers. 3) Arrays of PoD types.
4) Structures that contain only basic types, or other PoD types.
...
5) A class too can be a PoD-type, but the convention is that anything declared "class" should never be relied on to be PoD.

You can test whether a type is PoD using a standard function library object.

Now the only thing that is undefined about casting a pointer to a PoD-type, is that the contents of the structure are not set by anything, so you should treat them as "write-only" values. In your case you may have written to them from the "device" and so initializing them will destroy those values. (The correct cast is a "reinterpret_cast" by the way)

You are right to worry about alignment issues, but you are wrong to think this is something the C++ code can fix. Alignment is a property of the memory, not a language feature. To align the memory, you must make sure the "offset" is always a multiple of the "alignas" of your structure. On x64/x86, getting this wrong will not create any problems, only slow down the access to your memory. On other systems it can cause a fatal exception.
On the other hand, your memory is not "volatile", it is accessed by another thread. This thread may be on another device, but it is another thread. You need to use thread-safe memory. In C++, this is provided by atomic variables. However, an "atomic" is not a PoD object! You should use a memory fence instead. These primitives force the memory to be read from and to memory. The volatile keyword does this too, but the compiler is allowed to reorder volatile writes and this may cause unexpected results.

Finally, if you want your code to be "modern C++" style, you should do the following.
1) Declare your custom PoD structure to represent your data layout. You can use static_assert(std::is_pod<MyType>::value). This will warn you if the structure is not compatible.
2) Declare a pointer to your type. (In this case only, do not use a smart pointer, unless there is a way to "free" the memory which makes sense)
3) Only allocate memory via a call which returns this pointer type. This function needs to
a) Initialize your pointer type with the result of your call to the Vulkan API.
b) Use an in-place new on the pointer - this is not required if you only write to the data - but is good practice. If you want to default values, initialize them in your structure declaration. If you want to keep the values, simply don't give them default values, and the in-place new will do nothing.

Use an "acquire" fence before reading the memory, a "release" fence after writing. Vulcan may provide a specific mechanism for this, I don't know. It is normal though for all synchronization primitives (such as mutex lock/unlock) to imply a memory fence, so you may get away without this step.

  • Comments are not for extended discussion; this conversation has been [moved to chat](https://chat.stackoverflow.com/rooms/184043/discussion-on-answer-by-user10668284-how-to-properly-access-mapped-memory-withou). –  Nov 21 '18 at 15:45