r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes
Gábor Horváth via cfe-commits
cfe-commits at lists.llvm.org
Sun Aug 11 17:00:31 PDT 2019
Great news!
Thanks for all the feedback and patience! :)
On Sun, Aug 11, 2019, 4:35 PM Nico Weber <thakis at 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.hun at gmail.com> wrote:
>
>> This should be fixed now.
>>
>> On Sat, Aug 10, 2019, 8:08 PM Gábor Horváth <xazax.hun at 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 <thakis at 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 at 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 at lists.llvm.org
>>>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190811/12c942d5/attachment.html>
More information about the cfe-commits
mailing list