Skip to content

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Oct 6, 2023

While working on #68377 inspecting Allocate() calls, I found out that there are couple of places where we forget to use placement-new to create objects in the allocated memory.

@Endilll Endilll added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Oct 6, 2023
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Oct 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2023

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Changes

While working on #68377 inspecting Allocate() calls, I found out that there are couple of places where we forget to use placement-new to create objects in the allocated memory.


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

3 Files Affected:

  • (modified) clang/lib/AST/DeclCXX.cpp (+2-1)
  • (modified) clang/lib/AST/DeclObjC.cpp (+2-2)
  • (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+5-2)
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 42bab4ed51b7290..a92b788366434ce 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1484,7 +1484,8 @@ void CXXRecordDecl::setCaptures(ASTContext &Context,
     if (Captures[I].isExplicit())
       ++Data.NumExplicitCaptures;
 
-    *ToCapture++ = Captures[I];
+    new (ToCapture) LambdaCapture(Captures[I]);
+    ToCapture++;
   }
 
   if (!lambdaIsDefaultConstructibleAndAssignable())
diff --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp
index e934a81d086e3c0..e1eef2dbd9c3d28 100644
--- a/clang/lib/AST/DeclObjC.cpp
+++ b/clang/lib/AST/DeclObjC.cpp
@@ -931,8 +931,8 @@ void ObjCMethodDecl::setParamsAndSelLocs(ASTContext &C,
   unsigned Size = sizeof(ParmVarDecl *) * NumParams +
                   sizeof(SourceLocation) * SelLocs.size();
   ParamsAndSelLocs = C.Allocate(Size);
-  std::copy(Params.begin(), Params.end(), getParams());
-  std::copy(SelLocs.begin(), SelLocs.end(), getStoredSelLocs());
+  std::uninitialized_copy(Params.begin(), Params.end(), getParams());
+  std::uninitialized_copy(SelLocs.begin(), SelLocs.end(), getStoredSelLocs());
 }
 
 void ObjCMethodDecl::getSelectorLocs(
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index d553b3c6d78dedc..6a2f607d916c472 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2002,13 +2002,16 @@ void ASTDeclReader::ReadCXXDefinitionData(
       case LCK_StarThis:
       case LCK_This:
       case LCK_VLAType:
-        *ToCapture++ = Capture(Loc, IsImplicit, Kind, nullptr,SourceLocation());
+        new (ToCapture)
+            Capture(Loc, IsImplicit, Kind, nullptr, SourceLocation());
+        ToCapture++;
         break;
       case LCK_ByCopy:
       case LCK_ByRef:
         auto *Var = readDeclAs<VarDecl>();
         SourceLocation EllipsisLoc = readSourceLocation();
-        *ToCapture++ = Capture(Loc, IsImplicit, Kind, Var, EllipsisLoc);
+        new (ToCapture) Capture(Loc, IsImplicit, Kind, Var, EllipsisLoc);
+        ToCapture++;
         break;
       }
     }

@Endilll Endilll removed clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Oct 6, 2023
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the cleanup!

@AaronBallman AaronBallman merged commit 99e6ef3 into llvm:main Oct 6, 2023
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Oct 12, 2023
Local branch amd-gfx f039005 Merged main:ff843c00ce1d into amd-gfx:8bf801a67373
Remote branch main 99e6ef3 [clang][NFC] Add missing placement-new after Allocate() calls (llvm#68382)
@Endilll Endilll deleted the add-missing-placement-new branch April 6, 2024 04:21
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"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants