Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang] Diagnose dangling references in std::vector. #111753

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hokein
Copy link
Collaborator

@hokein hokein commented Oct 9, 2024

This is a follow-up to #108344.

The original bailout check was overly strict, causing it to miss cases like the vector(initializer_list, allocator) constructor. This patch relaxes the check to address that issue.

Fix #111680

@hokein hokein requested a review from usx95 October 9, 2024 19:39
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 9, 2024

@llvm/pr-subscribers-clang

Author: Haojian Wu (hokein)

Changes

This is a follow-up to #108344.

The original bailout check was overly strict, causing it to miss cases like the vector(initializer_list, allocator) constructor. This patch relaxes the check to address that issue.

Fix #111680


Full diff: https://github.com/llvm/llvm-project/pull/111753.diff

2 Files Affected:

  • (modified) clang/lib/Sema/CheckExprLifetime.cpp (+1-1)
  • (modified) clang/test/Sema/warn-lifetime-analysis-nocfg.cpp (+4-2)
diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp
index 009b8d000e6b0e..8a44a131c02cb4 100644
--- a/clang/lib/Sema/CheckExprLifetime.cpp
+++ b/clang/lib/Sema/CheckExprLifetime.cpp
@@ -404,7 +404,7 @@ shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) {
   if (LHSRecordDecl->hasAttr<PointerAttr>())
     return true;
 
-  if (Ctor->getConstructor()->getNumParams() != 1 ||
+  if (Ctor->getConstructor()->getNumParams() < 1 ||
       !isContainerOfPointer(LHSRecordDecl))
     return false;
 
diff --git a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
index 731639ab16a735..688f55edfe84df 100644
--- a/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
+++ b/clang/test/Sema/warn-lifetime-analysis-nocfg.cpp
@@ -164,14 +164,16 @@ template<typename T>
 struct initializer_list {
   const T* ptr; size_t sz;
 };
-template <typename T>
+template<typename T> class allocator {};
+template <typename T, typename Alloc = allocator<T>>
 struct vector {
   typedef __gnu_cxx::basic_iterator<T> iterator;
   iterator begin();
   iterator end();
   const T *data() const;
   vector();
-  vector(initializer_list<T> __l);
+  vector(initializer_list<T> __l,
+         const Alloc& alloc = Alloc());
 
   template<typename InputIterator>
 	vector(InputIterator first, InputIterator __last);

Copy link
Contributor

@usx95 usx95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the quick fix.

@@ -404,7 +404,7 @@ shouldTrackFirstArgumentForConstructor(const CXXConstructExpr *Ctor) {
if (LHSRecordDecl->hasAttr<PointerAttr>())
return true;

if (Ctor->getConstructor()->getNumParams() != 1 ||
if (Ctor->getConstructor()->getNumParams() < 1 ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we are checking for empty params. Let's replace this with Ctor->getConstructor()-> param_empty().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clang still doesn't catch dangling case of Container of pointer-like element
3 participants