atimin
atimin

Reputation: 549

Strange behaviour of std::make_shared

I have a very strange behaviour I can't understand.

This test passes:

CipString str = *std::make_shared<CipString>("Bye!").get();
EXPECT_EQ(static_cast<std::string>(str), "Bye!"); 

But this not:

CipString *str = std::make_shared<CipString>("Bye!").get();
EXPECT_EQ(static_cast<std::string>(*str), "Bye!");

I got an error:

Expected: static_cast(*str)

Which is: "p\x15\x97\x1"

To be equal to: "Bye!"

The CipString's code:

class CipString{
  public:

  CipString(const std::string& str) {
    length = str.size();
    string.reset(new uint8_t[length]);
    std::copy(str.begin(), str.end(), string.get());
  }

  operator std::string() const {
    std::string str("", length);
    std::copy(string.get(), string.get() + length, str.begin());
    return str;
  }

  uint16_t length; /**< Length of the String (16 bit value) */
  std::shared_ptr<uint8_t> string; /**< Pointer to the string data */
};

Upvotes: 1

Views: 765

Answers (3)

marcinj
marcinj

Reputation: 50036

You can rewrite this:

CipString str = *std::make_shared<CipString>("Bye!").get();
EXPECT_EQ(static_cast<std::string>(str), "Bye!"); 

as:

CipString str;
{
   auto sp = std::make_shared<CipString>("Bye!");
   str = *sp.get(); // here a copy is made (in original code copy initialization)
   // here sp will get destroyed
}
// here str contains correct "Bye!" value

And this has no Undefined Behaviour. On the other hand this code:

CipString *str = std::make_shared<CipString>("Bye!").get();

would be rewritten as:

CipString *str;
{
   auto sp = std::make_shared<CipString>("Bye!");
   str = sp.get(); // here a pointer is assigned to str, no copy is done as above
   // here sp will get destroyed, and also pointer to 
   //  which str points will be a dangling pointer
}
// here str should not be used - as it is UB

Upvotes: 0

Richard Hodges
Richard Hodges

Reputation: 69922

CipString str = *std::make_shared<CipString>("Bye!").get();
EXPECT_EQ(static_cast<std::string>(str), "Bye!"); 

decomposes to:

1) call make_shared to return a shared_ptr

2) get the address of the underlying object

3) construct the CipString with the pointer to the underlying object

4) destroy the shared_ptr

which is 'fine'

This this decomposes to:

CipString *str = std::make_shared<CipString>("Bye!").get();

1) construct a shared_ptr

2) get the address of the underlying object

3) create a temporary CipString from it and store the address in str

4) destroy the shared_ptr

5) destroy the temporary CipString (which str is pointing at)

which means that this:

EXPECT_EQ(static_cast<std::string>(*str), "Bye!");

becomes undefined behaviour

Upvotes: 0

rems4e
rems4e

Reputation: 3172

This line:

CipString *str = std::make_shared<CipString>("Bye!").get();

creates a shared_ptr that is destroyed just after the ;. str is a dangling pointer after that, and your test invokes undefined behavior by accessing freed memory.

You are basically asserting against garbage memory.

Upvotes: 8

Related Questions