2

I have question about new placement of array in c++. below code is a sample code that i made.

#include <vector>
#include <iostream>
class Point
{
    int x,y;
    public:
        Point():x(0), y(0){std::cout<<"Point() : "<<this<<std::endl;}
        void print(){std::cout<<x<<":"<<y<<std::endl;}
        Point(int a, int b) :x(a), y(b){std::cout<<"Point(int,int) : value & addr "<<a<<":"<<b<<" ~ "<<this<<std::endl;}
        ~Point(){std::cout<<"~Point() : "<<this<<" "<<x<<":"<<y<<std::endl;}
};

int main()
{
    // multiple allocation
    void* mem_ptr_arr = operator new(sizeof(Point)*3);
    for(int i=0; i<3; i++)
        new( mem_ptr_arr+sizeof(Point)*i ) Point(i,i);

    Point* ref_ptr_arr = static_cast<Point*>(mem_ptr_arr);
    // delete process
    for(int i=0; i<3; i++)
        (ref_ptr_arr+i)->~Point();
    operator delete(ref_ptr_arr);
    
    Point* new_ptr = new Point[3]{};
    delete[] new_ptr;

    return 0;
}

I want to duplicate functionality of new and delete operation. So break down each operation like below

  1. new -> operator new + new(some_ptr) Constructor
  2. delete -> Obj.~Destructor + delete(some_ptr) My question is, Is it correct usage of new placement of array(ref_ptr_arr)? When i debug some memory, they do not use same heap address after delete previous pointer.
πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
ALPHA
  • 79
  • 7
  • 1
    It seems unnecessary? You have to ensure that the argument to placement new is sufficiently large **and correctly aligned**. – Quimby Oct 04 '22 at 09:21
  • 2
    Destruction should happen in reverse order (last -> first). Otherwise, seems okay to me. Re alignment: It is valid if the alignment of Point is less or equal to ```__STDCPP_DEFAULT_NEW_ALIGNMENT__``` – Homer512 Oct 04 '22 at 09:25
  • 2
    IIRC, this is illegal until C++20: `ref_ptr_arr+i` since you are performing pointer arithmetic but there is no array of type `Point[]` there. But in real world it works, otherwise we wouldn't be able to implement `std::vector` (there are language-lawyer discussions about its implementability). – Daniel Langr Oct 04 '22 at 09:26
  • 3
    Fwiw, `mem_ptr_arr+sizeof(Point)*i` isn't standard compliant anyway. Some toolchains expose pointer arithmetic on `void*` (e.g. gnu), but they're the non-standard exception; not the rule. – WhozCraig Oct 04 '22 at 09:27
  • 1
    Also note that ```(sizeof(Point)*N)``` can wrap-around. If ```N``` is controlled by the user, that is a security issue because it can lead to smaller allocations, followed by the initialization writing beyond the end. The real allocator (just like ```calloc```) guard against this. – Homer512 Oct 04 '22 at 09:29
  • Thank you for advice. It really helpful to me. Avoiding memory arithmetic in my code, Use stl or array can help to meet C++ standard like below? `Point* ptr_arr = static_cast(mem_ptr_arr);` `new(&ptr_arr[i]) Point(i,i);` – ALPHA Oct 04 '22 at 09:44
  • Plus, @Homer512 can you explain what is wrap-around more detail? Thank you! – ALPHA Oct 04 '22 at 09:49
  • 1
    See bugs like these: https://nvd.nist.gov/vuln/detail/CVE-2021-26461 Basically, if ```sizeof(Point)``` is 8, I can set ```N``` to ```2**62 + 1```. The multiplication ```(2**62 + 1) * 8``` will exceed 64 bit and then wrap-around to 8. But your initialization loop will still try to access all ```(2**62 + 1)``` elements, writing beyond the end of the actual allocation. – Homer512 Oct 04 '22 at 09:56
  • 1
    See also https://stackoverflow.com/questions/199333/how-do-i-detect-unsigned-integer-overflow – Homer512 Oct 04 '22 at 09:57
  • Thank you @Homer512! really good example to understand for me – ALPHA Oct 04 '22 at 09:59
  • 2
    @Homer512 You should have specified that this was an off-topic issue. There is no `N` in OP's code so that comment was a bit misleading. – Daniel Langr Oct 04 '22 at 10:00
  • 1
    @DanielLangr I don't consider this off-topic since the presented code, like most code on SO, is a watered down toy example rather than the real deal. And messing up the multiplication is a common error as shown by the numerous CVEs I could quote on the issue. – Homer512 Oct 04 '22 at 10:06

1 Answers1

2

I would simplify this way: First, just allocate the array with a low level function like malloc() or mmap(), brk() and dispose of it accordingly. It helps to keep the two worlds separated.

Second, when calling placement new you do not necessarily need to take that pointer if you already have it.

And last it looks like you are doing void pointer arithmetic and that was banned.

    // multiple allocation
    Point* points = (Point*)std::malloc(sizeof(Point)*3);
    for(int i=0; i<3; i++)
        new ( &points[i] ) Point(i,i);

    // delete process
    for(int i=0; i<3; i++)
        points[i].~Point();
    std::free( points );
Something Something
  • 3,999
  • 1
  • 6
  • 21
  • Note that if you use `malloc` and `free` (which are not part of C++), the possible class-provided `new` and `delete` operators will be ignored. – Daniel Langr Oct 04 '22 at 09:51
  • 1
    `malloc` and `free` are part of the standard C++. See draft C++ standard item 20.10.12 https://isocpp.org/files/papers/N4860.pdf – Something Something Oct 04 '22 at 17:27
  • Previously, I used `Points* point = (Point*)new char[sizeof(Point)*3]` for this, and then I don't have to touch malloc. – Mooing Duck Oct 04 '22 at 17:51
  • @MooingDuck malloc/free are actually unusual to be used with placement new, especially in my industry. We typically would create memory with mass allocation calls like `mmap()` or `sbrk()` – Something Something Oct 04 '22 at 18:04
  • 2
    *"when calling placement new you do not necessarily need to take that pointer"* It's better to use the returned pointer. Under C++17 rules, using the old pointer instead of the returned one required `std::launder`. I'm not sure if C++20 with its implicit lifetime rules changes this or not. – HolyBlackCat Oct 04 '22 at 18:08
  • @HolyBlackCat I can see your point but every other alternative I can think of makes this code much more complex and obscure. I think clarity is important and UB here is a non-issue. – Something Something Oct 04 '22 at 21:09