From: Gabor Horvath Date: Tue, 3 Sep 2019 16:17:24 +0000 (+0000) Subject: [LifetimeAnalysis] Fix some false positives X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=89e632ecc72fc21e447a9c772bb57eabf364c0fa;p=clang [LifetimeAnalysis] Fix some false positives Differential Revision: https://reviews.llvm.org/D66806 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@370773 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index 51e9f3d632..ff9b225207 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -6775,6 +6775,10 @@ static bool shouldTrackFirstArgument(const FunctionDecl *FD) { static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call, LocalVisitor Visit) { auto VisitPointerArg = [&](const Decl *D, Expr *Arg) { + // We are not interested in the temporary base objects of gsl Pointers: + // Temp().ptr; // Here ptr might not dangle. + if (isa(Arg->IgnoreImpCasts())) + return; Path.push_back({IndirectLocalPathEntry::GslPointerInit, Arg, D}); if (Arg->isGLValue()) visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, @@ -7295,6 +7299,8 @@ static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) { for (auto It = Path.rbegin(), End = Path.rend(); It != End; ++It) { if (It->Kind == IndirectLocalPathEntry::VarInit) continue; + if (It->Kind == IndirectLocalPathEntry::AddressOf) + continue; return It->Kind == IndirectLocalPathEntry::GslPointerInit; } return false; diff --git a/test/Sema/warn-lifetime-analysis-nocfg.cpp b/test/Sema/warn-lifetime-analysis-nocfg.cpp index ce633e649f..8ba7686944 100644 --- a/test/Sema/warn-lifetime-analysis-nocfg.cpp +++ b/test/Sema/warn-lifetime-analysis-nocfg.cpp @@ -7,13 +7,21 @@ struct [[gsl::Owner(int)]] MyIntOwner { struct [[gsl::Pointer(int)]] MyIntPointer { MyIntPointer(int *p = nullptr); // Conversion operator and constructor conversion will result in two - // different ASTs. The former is tested with another owner and + // different ASTs. The former is tested with another owner and // pointer type. MyIntPointer(const MyIntOwner &); int &operator*(); MyIntOwner toOwner(); }; +struct MySpecialIntPointer : MyIntPointer { +}; + +// We did see examples in the wild when a derived class changes +// the ownership model. So we have a test for it. +struct [[gsl::Owner(int)]] MyOwnerIntPointer : MyIntPointer { +}; + struct [[gsl::Pointer(long)]] MyLongPointerFromConversion { MyLongPointerFromConversion(long *p = nullptr); long &operator*(); @@ -56,21 +64,21 @@ struct Y { }; void dangligGslPtrFromTemporary() { - MyIntPointer p = Y{}.a; // expected-warning {{temporary whose address is used as value of local variable 'p' will be destroyed at the end of the full-expression}} + MyIntPointer p = Y{}.a; // TODO (void)p; } struct DanglingGslPtrField { - MyIntPointer p; // expected-note 2{{pointer member declared here}} + MyIntPointer p; // expected-note {{pointer member declared here}} MyLongPointerFromConversion p2; // expected-note {{pointer member declared here}} - DanglingGslPtrField(int i) : p(&i) {} // expected-warning {{initializing pointer member 'p' with the stack address of parameter 'i'}} + DanglingGslPtrField(int i) : p(&i) {} // TODO DanglingGslPtrField() : p2(MyLongOwnerWithConversion{}) {} // expected-warning {{initializing pointer member 'p2' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}} DanglingGslPtrField(double) : p(MyIntOwner{}) {} // expected-warning {{initializing pointer member 'p' to point to a temporary object whose lifetime is shorter than the lifetime of the constructed object}} }; MyIntPointer danglingGslPtrFromLocal() { int j; - return &j; // expected-warning {{address of stack memory associated with local variable 'j' returned}} + return &j; // TODO } MyIntPointer returningLocalPointer() { @@ -124,6 +132,7 @@ template struct basic_iterator { basic_iterator operator++(); T& operator*() const; + T* operator->() const; }; template @@ -141,6 +150,12 @@ typename remove_reference::type &&move(T &&t) noexcept; template auto data(const C &c) -> decltype(c.data()); +template +auto begin(C &c) -> decltype(c.begin()); + +template +T *begin(T (&array)[N]); + template struct vector { typedef __gnu_cxx::basic_iterator iterator; @@ -158,6 +173,8 @@ struct basic_string_view { template struct basic_string { + basic_string(); + basic_string(const T *); const T *c_str() const; operator basic_string_view () const; }; @@ -188,8 +205,23 @@ struct any {}; template T any_cast(const any& operand); + +template +struct reference_wrapper { + template + reference_wrapper(U &&); +}; + +template +reference_wrapper ref(T& t) noexcept; } +struct Unannotated { + typedef std::vector::iterator iterator; + iterator begin(); + operator iterator() const; +}; + void modelIterators() { std::vector::iterator it = std::vector().begin(); // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} (void)it; @@ -298,3 +330,123 @@ void handleGslPtrInitsThroughReference2() { const std::vector &v = getVec(); const int *val = v.data(); // Ok, it is lifetime extended. } + +void handleTernaryOperator(bool cond) { + std::basic_string def; + std::basic_string_view v = cond ? def : ""; // expected-warning {{object backing the pointer will be destroyed at the end of the full-expression}} +} + +std::reference_wrapper danglingPtrFromNonOwnerLocal() { + int i = 5; + return i; // TODO +} + +std::reference_wrapper danglingPtrFromNonOwnerLocal2() { + int i = 5; + return std::ref(i); // TODO +} + +std::reference_wrapper danglingPtrFromNonOwnerLocal3() { + int i = 5; + return std::reference_wrapper(i); // TODO +} + +std::reference_wrapper danglingPtrFromNonOwnerLocal4() { + Unannotated i; + return std::reference_wrapper(i); // TODO +} + +std::reference_wrapper danglingPtrFromNonOwnerLocal5() { + Unannotated i; + return std::ref(i); // TODO +} + +int *returnPtrToLocalArray() { + int a[5]; + return std::begin(a); // TODO +} + +struct ptr_wrapper { + std::vector::iterator member; +}; + +ptr_wrapper getPtrWrapper(); + +std::vector::iterator returnPtrFromWrapper() { + ptr_wrapper local = getPtrWrapper(); + return local.member; +} + +std::vector::iterator returnPtrFromWrapperThroughRef() { + ptr_wrapper local = getPtrWrapper(); + ptr_wrapper &local2 = local; + return local2.member; +} + +std::vector::iterator returnPtrFromWrapperThroughRef2() { + ptr_wrapper local = getPtrWrapper(); + std::vector::iterator &local2 = local.member; + return local2; +} + +void checkPtrMemberFromAggregate() { + std::vector::iterator local = getPtrWrapper().member; // OK. +} + +std::vector::iterator doNotInterferWithUnannotated() { + Unannotated value; + // Conservative choice for now. Probably not ok, but we do not warn. + return std::begin(value); +} + +std::vector::iterator doNotInterferWithUnannotated2() { + Unannotated value; + return value; +} + +std::vector::iterator supportDerefAddrofChain(int a, std::vector::iterator value) { + switch (a) { + default: + return value; + case 1: + return *&value; + case 2: + return *&*&value; + case 3: + return *&*&*&value; + } +} + +int &supportDerefAddrofChain2(int a, std::vector::iterator value) { + switch (a) { + default: + return *value; + case 1: + return **&value; + case 2: + return **&*&value; + case 3: + return **&*&*&value; + } +} + +int *supportDerefAddrofChain3(int a, std::vector::iterator value) { + switch (a) { + default: + return &*value; + case 1: + return &*&*value; + case 2: + return &*&**&value; + case 3: + return &*&**&*&value; + } +} + +MyIntPointer handleDerivedToBaseCast1(MySpecialIntPointer ptr) { + return ptr; +} + +MyIntPointer handleDerivedToBaseCast2(MyOwnerIntPointer ptr) { + return ptr; // expected-warning {{address of stack memory associated with parameter 'ptr' returned}} +}