C++ return reference declared in function scope

Question

Possible Duplicate:
Can a local variable's memory be accessed outside its scope?

I saw a function when doing some code review.

wchar_t* GetString(HINSTANCE hInstance, UINT SID)
{
    wchar_t buf[2048] = {0}; 
    LoadStringW(hInstance, SID, buf, sizeof(buf)/sizeof(wchar_t));
    return &buf[0];
}

void SomeWork()
{
    std::wstring str( GetString(hInst, 123) );
}

I thought buf should be destroyed right after function return, so the pointer &buf[0] might be invalid. But it seems work fine, how does it work? And is it a fine design? Thanks.

In silico · Accepted Answer · 2012-09-05T07:19:09.170

I thought buf should be destroyed right after function return, so the pointer &buf[0] might be invalid.

Your thoughts are 100% correct.

But it seems work fine, how does it work?

It "works" because the part of the call stack that holds the buf array just happened to not have its contents written over by the time you actually use it.

But this is undefined behavior, and undefined behavior means anything can happen, and that can include "summoning an Armageddon" and/or "working just fine". You just got lucky this time.

And is it a fine design?

No, it's a terrible design. Fortunately it has a simple fix: just return the std::wstring itself.

std::wstring GetString(HINSTANCE hInstance, UINT SID) 
{ 
    wchar_t buf[2048] = {0};  
    LoadStringW(hInstance, SID, buf, sizeof(buf)/sizeof(wchar_t)); 
    return std::wstring(buf);
} 

void SomeWork()   
{   
    std::wstring str = GetString(hInst, 123);   
}

All non-stupid C++ compilers will optimize away the temporaries in this case, so this code has no performance penalty in practice. In fact, the Visual C++ compilers will optimize this case even when you turn off all optimizations.

This specific optimization is called return value optimization (RVO). If your C++ compiler doesn't do RVO even when set at its highest optimization level, get another one.

score 3 · Answer 2 · answered Sep 05 '12 at 07:04

3

No. This is serious defect. Returning pointer or reference to local object is undefined behavior. It means everything could happen, including starting WWIII. In your case undefined behavior is just the same like it "works fine". By luck.

answered Sep 05 '12 at 07:04

Rost

8,779
28
50

score 1 · Answer 3 · answered Sep 05 '12 at 07:05

Theoretically speaking, this is undefined behavior, which means anything could happen, including sometimes working (apparently) correctly. However, being undefined means there's no way to guarantee that it will always work.

The reason why it appears to work now is because the memory modified by GetString has not been modified by any other code being executed by the time you got to read it. In other words, you got lucky.

score 1 · Answer 4 · answered Sep 05 '12 at 07:13

I thought buf should be destroyed right after function return, so the pointer &buf[0] might be invalid.

That's correct. The lifetime of buf is over; its memory might or might not have been made inaccessible or reused for a different object; and any access to it gives undefined behavoiur.

But it seems work fine, how does it work?

Because, on many platforms, automatic variables are stored on a stack, and a function's stack memory does not become inaccessible when the function returns. This means that (invalid dangling references to) the variables will often still appear to have their old values, until you call another function an the memory is reused.

And is it a fine design?

Certainly not. You are relying on undefined behaviour, and your code could stop appearing to work for many reasons - changing platform or compiler, adding another function call to your code, or the shifting phase of the moon.

I would return a std::wstring.

C++ return reference declared in function scope

4 Answers4