0

I am developing an in-memory database, and my system needs a large array of std::atomic_int objects that roughly act as locks for database records. Now I would prefer to allocate these locks using VM system calls such as mmap on Unix-like systems and VirtualAlloc on Win32/64. There are several reasons for this, and just one of them is not having to explicitly initialise memory (i.e., memory allocated by the VM syscalls is guaranteed to be zeroed by the OS). So, I would essentially like to do this:

#include <sys/mman.h>
#include <atomic>

// ...
size_t numberOfLocks = ... some large number ...;
std::atomic_int* locks = reinterpret_cast<std::atomic_int*>(mmap(0, numberOfLocks * sizeof(std::atomic_int), PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANON, -1, 0));
// ... use locks[i].load() or locks[i].store() as with any memory order as appropriate

My main question is whether this code is safe. I'd intuitively expect the code to work on any reasonable platform with a modern compiler: mmap is guaranteed to return memory aligned to the VM page boundary so any alignment requirements of std::atomic_int should be honoured, and the constructor of std::atomic_int does not initialise the value so there is no danger of not invoking the constructor as long reads and writes are implemented in a reasonable way (e.g., using the __atomic_* builtins of GCC and clang).

However, I can imagine that this code is not necessarily safe according to the C++ standard — am I right in thinking that? If that is correct, is there any check so that, if the code successfully compiles on the target platform (i.e., if the implementation of std::atomic_int is what I expect it to be), then everything works as I expect?

Related to that, I expect the following code, where std::atomic_int is not property aligned, to break on x86:

uint8_t* region = reinterpret_cast<uint8_t*>(mmap(...));
std::atomic_int* lock = reinterpret_cast<std::atomic_int*>(region + 1);
lock->store(42, std::memory_order_relaxed);

The reason why I think this should not work is because a reasonable implementation of std::atomic_int::store with std::memory_order_relaxed on x86 is just a normal move, which is guaranteed to be atomic only for word-aligned accesses. If I am right about this, is there anything I can add to the code to safeguard against such situations and possibly detect such issues at compile time?

Boris
  • 57
  • 1
  • 9
  • You can find a lot of questions on SO about `mmap` and the C++ standard. In short it's UB because the constructor of the `std::atomic_int` has never been called (by this program) and therefore `locks` is a dangling pointer. One of those questions is [this](https://stackoverflow.com/questions/45310822), but it's not very convincing as a dupe target. – nwp Dec 04 '18 at 14:28
  • There is work being done in that direction and you can read about the current situation in [the paper about `std::bless`](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0593r2.html). – nwp Dec 04 '18 at 14:31
  • Thank you, this is very interesting. I can't say I completely understand the reasons for all this complexity, but I'll keep reading. But still, regardless of the standard, is it realistic to expect this to work on most modern compilers (MSVC, GCC, clang) and platforms (x86_64 and ARM in particular)? – Boris Dec 04 '18 at 14:40
  • I would expect it to work for a while in practice, but you can never be sure what an optimizer might do with UB. Consider [`std::launder`](https://en.cppreference.com/w/cpp/utility/launder)ing the pointer first which may or may not avoid undesirable code-gen. – nwp Dec 04 '18 at 14:46

1 Answers1

0

It is safe as mmap allocates memory suitably aligned for any built-in and SIMD type.

Make sure you call std::uninitialized_default_construct_n (or your own pre-C++17 equivalent) to satisfy the requirement of C++ standard that the constructor must be called, and std::destroy_n to invoke destructors after use. These calls compile into 0 instructions because the default constructor and destructor of std::atomic<> are trivial (do nothing):

size_t numberOfLocks = ... some large number ...;
auto* locks = static_cast<std::atomic_int*>(mmap(0, numberOfLocks * sizeof(std::atomic_int), ...));

// initialize
std::uninitialized_default_construct_n(locks, numberOfLocks); 
// ... use ...
// uninitialize
std::destroy_n(locks, numberOfLocks);
Maxim Egorushkin
  • 131,725
  • 17
  • 180
  • 271
  • 1
    Doesn't that make the whole point of `mmap` moot? I though you wanted to read the memory of the file you mapped in, not simply overwrite it by placement-newing the elements into it. As far as I know with `mmap` you either have an uninitialized read or throw away the data, so it's either UB or useless. – nwp Dec 04 '18 at 15:27
  • @nwp Here `mmap` is used for anonymous memory allocation, not mapping a file. `mmap` always zeros-out memory so that no information leaks from other processes that used the memory previously - basic security feature. – Maxim Egorushkin Dec 04 '18 at 15:28
  • Thank you! Out of curiosity: what can go wrong if I don't use `std::uninitialized_default_construct` or an equivalent? I understand that there is no performance penalty for doing so -- I just wish to understand what rule in the C++ standard I'm breaking if I'm not calling this. – Boris Dec 04 '18 at 15:38
  • @Boris I cannot think of anything going wrong here in practice if you don't invoke the constructors and destructors because they are trivial. – Maxim Egorushkin Dec 04 '18 at 15:44