int const& get_reference_warning()
{
int w = 0;
return w; // expected-warning{{reference to stack memory associated with local variable ‘w’ returned}}
}
int get_value();
int const& get_reference_no_warning() { return get_value(); } // Diagnosed by VC++ 2010 (haven’t tested on gcc), but unnoticed by Clang
int const& get_reference_no_warning_2()
{
int const& w2 = get_value(); // Correct, lifetime of the value is extended to the lifetime of the const reference
return w2; // Undefined behavior: goes unnoticed by Clang, VC++ 2010 and gcc 3.4.2
}
I compiled it with: clang.exe -fsyntax-only -Wall -pedantic test.cpp (on Windows, using PowerShell)
test.cpp(9) : warning: reference to stack memory associated with local variable ‘w’ returned
return w; // expected-warning{{reference to stack memory associated with local variable ‘w’ returned}}
^
1 warning generated.
As expected, the first return provoked a warning.
However in the two other cases, no warning is generated.
I was wondering if it would be possible for clang to diagnose those cases as well. I guess the third case is the trickier, though since the lifetime of the value is correctly extended, therre should be a way to detect that w2 is not just an ordinary const reference. I must admit I haven’t come over the Clang code base yet so it’s still a bit blurry to me (by the way if someone could kindly point me to some doc for developers…)
Should I file a bug ? (or perhaps one already exists…)
I'm not certain this is a bug. The 'const int&' causes the lifetime of the object returned from get_value() to be extended to its last use. Does this extend to the caller of get_reference_no_warning_2() since that also returns a const reference?
I haven’t got to generate an executable with Clang (issues with the linker, I am still trying to configure my environment…), on gcc I got a garbage value in the 3rd case.
In get_reference_no_warning_2, w2 is bound to a temporary which is
live for the lifetime of w2; returning a reference to the temporary
doesn't extend its lifetime.
I must admit I don’t know much about compilers, but naively I would say in order to implement this lifetime extension that the standard mandates, I would examine the signature of get_value(), which returns an int, and allocate enough space for an int on the stack (instead of a const reference) while treating it a const reference to an int for the purpose of compilation (it cannot be modified). That’s the only trick I can think of short of allocating this on the heap… and this would be a performance killer.
However the caller of get_reference_no_warning() or get_reference_no_warning_2() expects a const reference, and thus is unlikely to prepare the stack for a copy instead (just in case). While it could possibly be done with an inline function, it would get hairy as soon as the function is defined in another translation unit.
Therefore it seems that the proper thing to do would be to diagnose the issue (in both get_reference_no_warning cases) and let the developer fix them. I surmise it should be possible to detect them (since VC++ already detects the first case), but then, as I said, I am very naive about compilers yet.
I hope to look at this further during the week-end, guess it would be as good a way as any to try and understand how Clang work
For reference I fixed such an issue today in real code (compiled with gcc 3.4.2), and it had gone undiagnosed too (and generated some mess because it was a string instead of an int…).
Okay, with the week-end I have been able to look at this issue a bit more in-depth:
So, the first issue is:
struct S {
int array[32];
};
S value() { return S(); }
S const& return_temporary() {
return value();
}
The return statement yields the following AST:
However static DeclRefExpr* EvalVal(Expr *E) in SemaChecking.cpp (line 1979) does not handle CallExpr (or CXXOperatorCallExpr or CXXMemberCallExpr for that matter)
I was thinking about adding a case in the switch:
case Stmt::CallExprClass:
case Stmt::CXXMemberCallExprClass:
case Stmt::CXXOperatorCallExprClass: {
// Functions / Methods / Operators
// that return a pointer or prvalue
CallExpr* CE = cast(E);
if (!CE->getCallReturnType()->isReferenceType()) {
return CE;
}
return NULL;
}
since at the moment it’s not handled at all. However there are MANY possible kinds for Type, so I am quite puzzled when it comes to figuring out which ones are okay and which are not… Furthermore there is it seems the issue of typedef which might embed a reference within and that we’ll need to account for.
Regarding the second issue:
S const& return_bounded_temporary() {
S const& bind = value();
return bind;
}
This returns the following AST:
So there is no indication here that the reference variable has bounded a temporary at all, I would have expected something like CXXBindTemporaryExpr.
