1

I recently started learning about smart pointers and move semantics in C++. But I can't figure out why this code works. I have such code:

#include <iostream>
#include <memory>

using namespace std;

class Test
{
public:
    Test() 
    {
        cout << "Object created" << endl;
    }

    void testMethod()
    {
        cout << "Object existing" << endl;
    }

    ~Test() 
    {
        cout << "Object destroyed" << endl;
    }
};

int main(int argc, char *argv[])
{
    Test* testPtr = new Test{};
    {
        unique_ptr<Test> testSmartPtr(testPtr);
    }
    testPtr->testMethod();
    
    return 0;
}

My output is:

Object created
Object destroyed
Object existing

Why does row testPtr->testMethod() work? Doesn't unique_ptr delete the pointer assigned to it on destruction if the pointer is an lvalue?

Edit: I learned from the comments that this method doesn't check if the pointer exists. If so, is there a way to check if the pointer is valid?

Edit: I learned that I shouldn't do anything with invalid pointers. Thank you for all your answers and comments.

Edit: Even this code works:

#include <iostream>
#include <memory>

using namespace std;

class Test
{
public:
    Test(int num) :
        number{ num }
    {
        cout << "Object created" << endl;
    }

    void testMethod(int valueToAdd)
    {
        number += valueToAdd;
        cout << "Object current value: " << number << endl;
    }

    ~Test() 
    {
        cout << "Object destroyed" << endl;
    }

private:
    int number;
};

int main(int argc, char *argv[])
{
    Test* testPtr = new Test(42);
    {
        unique_ptr<Test> testSmartPtr(testPtr);
    }
    testPtr->testMethod(3);
    
    return 0;
}

I think it's because the compiler optimized it. Anyway, I really shouldn't do anything with invalid pointers.

Roman Leshchuk
  • 192
  • 1
  • 7
  • 1
    Undefined behavior. – Taekahn Aug 14 '22 at 18:33
  • 1
    When you have UB, anything might happen. That includes `testMethod()` working as expected - until your finals exam. – lorro Aug 14 '22 at 18:34
  • So this behavior is not expected by the standard? – Roman Leshchuk Aug 14 '22 at 18:36
  • 4
    you cannot check if a pointer is valid by dereferecing because doing so is undefined. – 463035818_is_not_an_ai Aug 14 '22 at 18:37
  • If so, is there a way to check if the pointer is valid? – Roman Leshchuk Aug 14 '22 at 18:38
  • 2
    Not in standard C++. That's why lifetime management is hard, and raw owning pointers are a bad idea. – StoryTeller - Unslander Monica Aug 14 '22 at 18:40
  • 1
    To check if an object is valid you can assign ownership of the object to a `std::shared_ptr` and have the `shared_ptr` give you a `std::weak_ptr` to the object. – user4581301 Aug 14 '22 at 18:54
  • 1
    To my surprise, it still works on 64bit mingw even after explicitly setting `testPtr = nullptr;` and even `memset(&testPtr, sizeof(testPtr), 0);` at the end of the anonymous block, and even if there was no raw pointer at all, and `unique_ptr testSmartPtr {std::move(testPtr)};` was used. – Dmytro Aug 14 '22 at 19:05
  • 1
    @Dmitry It is not surprising. While the abstract language specifies that the pointer is dereferenced when calling a member function like this, causing undefined behavior, straight-forward compilation of the code to a real architecture will simply result in a function call with the pointer being passed as argument. Nothing in the function body ever accesses a member and so the pointer does never need to be dereferenced on the hardware level. Its value becomes irrelevant. Nonetheless, the program has UB and is invalid. These assumptions about the compiler and hardware cannot be relied upon. – user17732522 Aug 14 '22 at 19:08
  • 1
    @user17732522 I tried replacing both with a shared_ptr(`shared_ptr testSmartPtr {std::move(testPtr)};`) and it still works despite ref_count being zero(due to `std::move`). It seems like the only way to avoid this kind of situation is vigilence and explicit if statement on a shared pointer's `use_count`. – Dmytro Aug 14 '22 at 19:15
  • 1
    @Dmitry One must read the specification of the tools one is using carefully. `operator*` and `operator->` of standard library smart pointers have preconditions that the pointer is not empty. The default-constructed state of these smart pointer types as well as there moved-from state are specified to be empty. Therefore using either `*` or `->` on it after having called `std::move` on the pointer is wrong and should be immediately considered a bug. More generally, the default assumption should be that a moved-from object is in unspecified state, so that most uses after would be wrong. – user17732522 Aug 14 '22 at 19:23
  • 1
    Crashed on my machine. **Undefined behavior**. If you are particularly unlucky, it may appear to work. If you are lucky, it will crash. – Eljay Aug 14 '22 at 19:33
  • 2
    @Dmitry "*To my surprise, it still works on 64bit mingw even after explicitly setting `testPtr = nullptr;` and even `memset(&testPtr, sizeof(testPtr), 0);` at the end of the anonymous block*" - see [Why does calling method through null pointer "work" in C++?](https://stackoverflow.com/questions/11320822/), and related questions. – Remy Lebeau Aug 14 '22 at 21:31
  • 1
    @RemyLebeau Oh right I forgot, C++ doesn't use the vtable for classes that do not have a virtual baseclass, making Test a subclass of ITest with virtual void testMethod() = 0; crashes as I expected(on my system, as it is still undefined behavior, dereferencing zero(vtable pointer) doesn't crash ALL systems, and C++ doesn't like talking about vtables). – Dmytro Aug 14 '22 at 22:31

1 Answers1

6

You do not need a std::unique_ptr to write code with the same issue

int main(int argc, char *argv[])
{
    Test* testPtr = new Test{};
    delete testPtr;
    testPtr->testMethod();       // UNDEFINED !!!
    return 0;
}

The output is the same as yours here https://godbolt.org/z/8bocKGj1M, but it could be something else entirely. The code has undefined behavior. You shall not dereference an invalid pointer.

If you had actually used members of the object in testMethod() some faulty output or a crash is more likely, but also not guaranteed. Looking ok is the worst incarnation of undefined behavior.

Your code demonstrates nicely why you should ban raw new completely. At least you should call new only as parameter to the smart pointers constructor, or even better, use std::make_unique. Its basically just a wrapper around a constructor call via new and its main purpose is to let you write code that is free of new:

int main(int argc, char *argv[])
{
    auto testPtr = std::make_unique<Test>();
    testPtr->testMethod();    
    return 0;
}

Even then you can access the raw pointer and do wrong stuff. Smart pointers help with ownership, but they are not fool-proof.

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185