invalid read on pointer returned from get()

Question

New to C++ and trying to understand object lifetimes and smart pointers.

Why does the result of creating a raw pointer from a unique_ptr with get() in this fashion:

auto w = (std::make_unique<Wall>()).get();

result in a pointer that passes a null check but causes an invalid read when used (and according to valgrind looks like caused by a double delete in the Wall destructor?) but when created in a two step fashion:

auto wall = std::make_unique<Wall>();
auto w = wall.get();  

present no such problems?

So I have two questions:

(1) why does the first way result in a raw pointer causing an invalid read and the second way doesn't?

(2) why does the invalid read pointer pass a null check?

#include <iostream>
#include <memory>
#include <string>

class Object {
public:
    virtual ~Object() = 0;
    virtual void talk() {}
};

Object::~Object() {}

class Wall: public Object {
public:
    virtual ~Wall() {}
    virtual void talk() { std::cout << "I'm a wall." << std::endl; }
};

int main(int argc, char** argv) {

    //auto wall = std::make_unique<Wall>();
    //auto w = str.get();  
    auto w = (std::make_unique<Wall>()).get();

    if (!w) 
        std::cout << "null pointer..." << std::endl;

    w->talk();
}  

valgrind output:

    ==21668== Memcheck, a memory error detector
    ==21668== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
    ==21668== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
    ==21668== Command: ./wall
    ==21668== 
    ==21668== Invalid read of size 8
    ==21668==    at 0x402452: main (main.cpp:33)
    ==21668==  Address 0x5ab6c80 is 0 bytes inside a block of size 8 free'd
    ==21668==    at 0x4C2F24B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==21668==    by 0x402548: Wall::~Wall() (main.cpp:15)
    ==21668==    by 0x402955: std::default_delete<Wall>::operator()(Wall*) const (unique_ptr.h:76)
    ==21668==    by 0x40269C: std::unique_ptr<Wall, std::default_delete<Wall> >::~unique_ptr() (unique_ptr.h:236)
    ==21668==    by 0x40242A: main (main.cpp:27)
    ==21668==  Block was alloc'd at
    ==21668==    at 0x4C2E0EF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==21668==    by 0x4025CD: _ZSt11make_uniqueI4WallIEENSt9_MakeUniqIT_E15__single_objectEDpOT0_ (unique_ptr.h:765)
    ==21668==    by 0x40240E: main (main.cpp:27)

Answer 1

In

auto w = (std::make_unique<Wall>()).get();

the anonymous std::make_unique<Wall>() only exists until the end of the line. Then it goes out of scope and is destroyed, taking the contained pointer with it.

This deletes the pointer out from under w without w being aware. w passes the null check because it is not null, but where it points must not be used.

In your second example

auto wall = std::make_unique<Wall>();
auto w = wall.get();  

wall maintains the unique_ptr until wall goes out of scope, which is hopefully long enough to make use of w.

My suggestion is to discard w and use wall throughout. This way there is no chance for a dangling pointer.

#include <iostream>
#include <memory>
#include <string>

class Object {
public:
    virtual ~Object() = 0;
    virtual void talk() {}
};

Object::~Object() {}

class Wall: public Object {
public:
    virtual ~Wall() {}
    virtual void talk() { std::cout << "I'm a wall." << std::endl; }
};

int main(int argc, char** argv) {

    auto wall = std::make_unique<Wall>();

    if (!wall)
        std::cout << "null pointer..." << std::endl;

    wall->talk();
    wall.reset();
    if (!wall)
        std::cout << "null pointer..." << std::endl;
}

Expected output:

I'm a wall.
null pointer...