Great news! Thanks for all the feedback and patience! :)
On Sun, Aug 11, 2019, 4:35 PM Nico Weber <tha...@chromium.org> wrote: > 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
