Confirmed, after r368534 the Chromium builds looks good again. (r368528 without r368534 regressed something else, but r368534 fixed things. I'm assuming you don't need a repro for the breakage in r368528 without r368534.)
Your warnings even found a bug: https://bugs.chromium.org/p/openscreen/issues/detail?id=63 Thanks! :) On Sun, Aug 11, 2019 at 3:18 PM Gábor Horváth <xazax....@gmail.com> wrote: > This should be fixed now. > > On Sat, Aug 10, 2019, 8:08 PM Gábor Horváth <xazax....@gmail.com> wrote: > >> 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
