You are right!
I will look into this tomorrow. If you think this is urgent feel free to
revert the commits.
Cheers,
Gabor
On Sat, Aug 10, 2019, 6:41 PM Nico Weber <tha...@chromium.org> wrote:
> Hi Gabor,
>
> this makes clang warn on this program:
>
> $ cat test.cc
> #include <algorithm>
> #include <vector>
>
> struct S {
> bool operator==(const S &rhs) const;
> };
>
> const S &f(const std::vector<S> &v) {
> const auto &it = std::find(v.rbegin(), v.rend(), S());
> return *it;
> }
>
>
> $ out/gn/bin/clang -c test.cc -isysroot $(xcrun -show-sdk-path) -isystem
> libcxx/include/ -Wall
> test.cc:10:11: warning: returning reference to local temporary object
> [-Wreturn-stack-address]
> return *it;
> ^~
> test.cc:9:15: note: binding reference variable 'it' here
> const auto &it = std::find(v.rbegin(), v.rend(), S());
> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 1 warning generated.
>
>
> Is the warning correct here? The `const auto &it` is lifetime-extended to
> the end of the block, and *it should return a reference to an element in
> the vector. Since the vector's passed in, this should be fine. Or am I
> missing something?
>
> Thanks,
> Nico
>
> On Fri, Aug 9, 2019 at 11:15 AM Gabor Horvath via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: xazax
>> Date: Fri Aug 9 08:16:35 2019
>> New Revision: 368446
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=368446&view=rev
>> Log:
>> More warnings regarding gsl::Pointer and gsl::Owner attributes
>>
>> Differential Revision: https://reviews.llvm.org/D65120
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/test/Analysis/inner-pointer.cpp
>> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug 9 08:16:35 2019
>> @@ -6564,6 +6564,25 @@ template <typename T> static bool isReco
>> return false;
>> }
>>
>> +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
>> + if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
>> + if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()))
>> + return true;
>> + if (!Callee->getParent()->isInStdNamespace() ||
>> !Callee->getIdentifier())
>> + return false;
>> + if (!isRecordWithAttr<PointerAttr>(Callee->getThisObjectType()) &&
>> + !isRecordWithAttr<OwnerAttr>(Callee->getThisObjectType()))
>> + return false;
>> + if (!isRecordWithAttr<PointerAttr>(Callee->getReturnType()) &&
>> + !Callee->getReturnType()->isPointerType())
>> + return false;
>> + return llvm::StringSwitch<bool>(Callee->getName())
>> + .Cases("begin", "rbegin", "cbegin", "crbegin", true)
>> + .Cases("end", "rend", "cend", "crend", true)
>> + .Cases("c_str", "data", "get", true)
>> + .Default(false);
>> +}
>> +
>> static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
>> LocalVisitor Visit) {
>> auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
>> @@ -6577,10 +6596,9 @@ static void handleGslAnnotatedTypes(Indi
>> };
>>
>> if (auto *MCE = dyn_cast<CXXMemberCallExpr>(Call)) {
>> - const FunctionDecl *Callee = MCE->getDirectCallee();
>> - if (auto *Conv = dyn_cast_or_null<CXXConversionDecl>(Callee))
>> - if (isRecordWithAttr<PointerAttr>(Conv->getConversionType()))
>> - VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
>> + const auto *MD = cast_or_null<CXXMethodDecl>(MCE->getDirectCallee());
>> + if (MD && shouldTrackImplicitObjectArg(MD))
>> + VisitPointerArg(MD, MCE->getImplicitObjectArgument());
>> return;
>> }
>>
>>
>> Modified: cfe/trunk/test/Analysis/inner-pointer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inner-pointer.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Analysis/inner-pointer.cpp (original)
>> +++ cfe/trunk/test/Analysis/inner-pointer.cpp Fri Aug 9 08:16:35 2019
>> @@ -1,4 +1,5 @@
>> -// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
>> +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
>> +// RUN: -Wno-dangling -Wno-dangling-field -Wno-return-stack-address \
>> // RUN: %s -analyzer-output=text -verify
>>
>> #include "Inputs/system-header-simulator-cxx.h"
>>
>> Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
>> +++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Fri Aug 9
>> 08:16:35 2019
>> @@ -2,7 +2,6 @@
>> struct [[gsl::Owner(int)]] MyIntOwner {
>> MyIntOwner();
>> int &operator*();
>> - int *c_str() const;
>> };
>>
>> struct [[gsl::Pointer(int)]] MyIntPointer {
>> @@ -52,16 +51,6 @@ long *ownershipTransferToRawPointer() {
>> return t.releaseAsRawPointer(); // ok
>> }
>>
>> -int *danglingRawPtrFromLocal() {
>> - MyIntOwner t;
>> - return t.c_str(); // TODO
>> -}
>> -
>> -int *danglingRawPtrFromTemp() {
>> - MyIntPointer p;
>> - return p.toOwner().c_str(); // TODO
>> -}
>> -
>> struct Y {
>> int a[4];
>> };
>> @@ -103,6 +92,12 @@ MyIntPointer danglingGslPtrFromTemporary
>> return MyIntOwner{}; // expected-warning {{returning address of local
>> temporary object}}
>> }
>>
>> +MyIntOwner makeTempOwner();
>> +
>> +MyIntPointer danglingGslPtrFromTemporary2() {
>> + return makeTempOwner(); // expected-warning {{returning address of
>> local temporary object}}
>> +}
>> +
>> MyLongPointerFromConversion danglingGslPtrFromTemporaryConv() {
>> return MyLongOwnerWithConversion{}; // expected-warning {{returning
>> address of local temporary object}}
>> }
>> @@ -124,12 +119,52 @@ void initLocalGslPtrWithTempOwner() {
>> global2 = MyLongOwnerWithConversion{}; // TODO ?
>> }
>>
>> -struct IntVector {
>> - int *begin();
>> - int *end();
>> +namespace std {
>> +template <typename T>
>> +struct basic_iterator {};
>> +
>> +template <typename T>
>> +struct vector {
>> + typedef basic_iterator<T> iterator;
>> + iterator begin();
>> + T *data();
>> +};
>> +
>> +template<typename T>
>> +struct basic_string {
>> + const T *c_str() const;
>> +};
>> +
>> +template<typename T>
>> +struct unique_ptr {
>> + T *get() const;
>> };
>> +}
>>
>> void modelIterators() {
>> - int *it = IntVector{}.begin(); // TODO ?
>> + std::vector<int>::iterator it = std::vector<int>().begin(); //
>> expected-warning {{object backing the pointer will be destroyed at the end
>> of the full-expression}}
>> (void)it;
>> }
>> +
>> +std::vector<int>::iterator modelIteratorReturn() {
>> + return std::vector<int>().begin(); // expected-warning {{returning
>> address of local temporary object}}
>> +}
>> +
>> +const char *danglingRawPtrFromLocal() {
>> + std::basic_string<char> s;
>> + return s.c_str(); // expected-warning {{address of stack memory
>> associated with local variable 's' returned}}
>> +}
>> +
>> +const char *danglingRawPtrFromTemp() {
>> + return std::basic_string<char>().c_str(); // expected-warning
>> {{returning address of local temporary object}}
>> +}
>> +
>> +std::unique_ptr<int> getUniquePtr();
>> +
>> +int *danglingUniquePtrFromTemp() {
>> + return getUniquePtr().get(); // expected-warning {{returning address
>> of local temporary object}}
>> +}
>> +
>> +int *danglingUniquePtrFromTemp2() {
>> + return std::unique_ptr<int>().get(); // expected-warning {{returning
>> address of local temporary object}}
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
