We had a similar problem with the analyzer, and you can get part of the way by overloading on & and &&. The problem with cast is that this is not valid:
Foo &F = cast<Foo>(getTemporary()); but this is use(cast<Foo>(getTemporary())) So…yeah. I don't think we have a good fix. Jordan On Nov 16, 2012, at 11:01 , David Blaikie <[email protected]> wrote: > On Fri, Nov 16, 2012 at 10:45 AM, Matthieu Monrocq > <[email protected]> wrote: >> >> >> On Fri, Nov 16, 2012 at 2:58 AM, David Blaikie <[email protected]> wrote: >>> >>> Can we warn about this? >>> >> >> Unfortunately no. I don't think so. It would require inspecting the body of >> `cast`... >> >> Argyrios implemented a whole class of warnings around this theme a while ago >> now, and as we drilled into it (and I believe you helped) we just decided >> that looking through functions was too difficult. As such: `T const& foo(T >> const& t) { return t; }` completely dulls out binding to temporary warnings >> => the function itself is correct, but its use may not. > > I'm a little confused now, TypeSourceInfo::getTypeLoc returns a > TypeLoc by /value/, doesn't it? So I'm not sure how casting that works > - the dynamic type is the same as the static type, yet we're still > able to cast to some derived type. (glancing at the doxygen though, > haven't looked at the code in detail) > >> However it seems to be that the problem is caused by the implementation of >> `cast` is wrong. >> >> template <class X, class Y> >> inline typename cast_retty<X, Y>::ret_type cast(const Y &Val) { >> assert(isa<X>(Val) && "cast<Ty>() argument of incompatible type!"); >> return cast_convert_val<X, Y, >> typename simplify_type<Y>::SimpleType>::doit(Val); >> } >> >> We accept a `const&`, allowing bind to temporary, but then the return type >> can be bound to X& which means the `const` qualifier was dropped on the >> floor. This is pretty nasty. > > Agreed, that's rough. > >> The culprit is actually just above: >> >> template<class To, class FromTy> struct cast_convert_val<To,FromTy,FromTy> { >> // This _is_ a simple type, just cast it. >> static typename cast_retty<To, FromTy>::ret_type doit(const FromTy &Val) { >> typename cast_retty<To, FromTy>::ret_type Res2 >> = (typename cast_retty<To, FromTy>::ret_type)const_cast<FromTy&>(Val); >> return Res2; >> } >> }; >> >> >> However, even overloading on const-ness would not make bind-to-const issues >> disappear. > > True, but it would pretty much fix this particular class of problems. > The person was probably trying to declare a local & just got a ref in > there by mistake. Less common that people declare locals const so the > accident would be found pretty consistently. > >> The only thing I can thing of is: >> => allow `cast` (and al) only on non-const reference >> => allow on non-const and const pointers >> >> which is the only way to circumvent bind-to-temporary situations I know of. >> >> I am not sure it's worth it. > > Probably not. Vaguely I'm wondering if we could have some > only-under-c++11 extra functionality to detect rvalue cases (do we > ever have dynamic type xvalues? I don't think so) & do something > different/break so we can catch them. > >> For detection I see only two possibilities: >> => Interprocedural (*) static analysis: when analyzing the function, note >> that its return may be an alias of the parameter; when analyzing the use, >> note that the statement is only valid if the return is not an alias of the >> temporary; cross-reference the two and warn about the incompatibility >> => Dynamic instrumentation: I wonder if Asan could catch this (maybe it does >> already). With the proper instrumentation the destructor of the temporary >> could trigger poisoning of the memory so that the next use cause an >> assertion. >> >> (*) Interprocedural in the general case, here it would not be. >> >> -- Matthieu >> >>> >>> On Thu, Nov 15, 2012 at 5:14 PM, Matt Beaumont-Gay <[email protected]> >>> wrote: >>>> Author: matthewbg >>>> Date: Thu Nov 15 19:14:52 2012 >>>> New Revision: 168124 >>>> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=168124&view=rev >>>> Log: >>>> Fix PR14321, a crash when Clang is built with GCC 4.7 at -O1 or greater. >>>> >>>> GCC 4.7 reuses stack slots fairly aggressively, which exposes more >>>> temporary >>>> lifetime bugs. >>>> >>>> No new test, this was caught by the existing >>>> CodeGenCXX/mangle-ms-templates.cpp. >>>> >>>> Modified: >>>> cfe/trunk/lib/AST/MicrosoftMangle.cpp >>>> >>>> Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=168124&r1=168123&r2=168124&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original) >>>> +++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Thu Nov 15 19:14:52 2012 >>>> @@ -373,7 +373,7 @@ >>>> dyn_cast<ClassTemplateSpecializationDecl>(ND)) { >>>> TypeSourceInfo *TSI = Spec->getTypeAsWritten(); >>>> if (TSI) { >>>> - TemplateSpecializationTypeLoc &TSTL = >>>> + TemplateSpecializationTypeLoc TSTL = >>>> cast<TemplateSpecializationTypeLoc>(TSI->getTypeLoc()); >>>> TemplateArgumentListInfo LI(TSTL.getLAngleLoc(), >>>> TSTL.getRAngleLoc()); >>>> for (unsigned i = 0, e = TSTL.getNumArgs(); i != e; ++i) >>>> >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> [email protected] >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
