[PATCH] D38257: [OpenMP] Fix memory leak when translating arguments

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld created this revision.

Parsing the argument after -Xopenmp-target allocates memory that needs
to be freed. Associate it with the final DerivedArgList after we know
which one will be used.


https://reviews.llvm.org/D38257

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/Compilation.cpp
  lib/Driver/ToolChain.cpp
  test/Driver/openmp-offload-gpu.c


Index: test/Driver/openmp-offload-gpu.c
===================================================================
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -2,9 +2,6 @@
 /// Perform several driver tests for OpenMP offloading
 ///
 
-// Until this test is stabilized on all local configurations.
-// UNSUPPORTED: linux
-
 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: powerpc-registered-target
Index: lib/Driver/ToolChain.cpp
===================================================================
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -800,9 +800,10 @@
   return VersionTuple();
 }
 
-llvm::opt::DerivedArgList *
-ToolChain::TranslateOpenMPTargetArgs(const llvm::opt::DerivedArgList &Args,
-    Action::OffloadKind DeviceOffloadKind) const {
+llvm::opt::DerivedArgList *ToolChain::TranslateOpenMPTargetArgs(
+    const llvm::opt::DerivedArgList &Args,
+    Action::OffloadKind DeviceOffloadKind,
+    SmallVector<llvm::opt::Arg *, 4> &AllocatedArgs) const {
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
     DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
     const OptTable &Opts = getDriver().getOpts();
@@ -854,6 +855,7 @@
       }
       XOpenMPTargetArg->setBaseArg(A);
       A = XOpenMPTargetArg.release();
+      AllocatedArgs.push_back(A);
       DAL->append(A);
       NewArgAdded = true;
     }
Index: lib/Driver/Compilation.cpp
===================================================================
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -51,9 +51,10 @@
 
   DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}];
   if (!Entry) {
+    SmallVector<Arg *, 4> AllocatedArgs;
     // Translate OpenMP toolchain arguments provided via the -Xopenmp-target 
flags.
-    DerivedArgList *OpenMPArgs = TC->TranslateOpenMPTargetArgs(*TranslatedArgs,
-        DeviceOffloadKind);
+    DerivedArgList *OpenMPArgs = TC->TranslateOpenMPTargetArgs(
+        *TranslatedArgs, DeviceOffloadKind, AllocatedArgs);
     if (!OpenMPArgs) {
       Entry = TC->TranslateArgs(*TranslatedArgs, BoundArch, DeviceOffloadKind);
     } else {
@@ -63,6 +64,11 @@
 
     if (!Entry)
       Entry = TranslatedArgs;
+
+    // Add allocated arguments to the final DAL.
+    for (auto ArgPtr : AllocatedArgs) {
+      Entry->AddSynthesizedArg(ArgPtr);
+    }
   }
 
   return *Entry;
Index: include/clang/Driver/ToolChain.h
===================================================================
--- include/clang/Driver/ToolChain.h
+++ include/clang/Driver/ToolChain.h
@@ -249,9 +249,10 @@
   ///
   /// \param DeviceOffloadKind - The device offload kind used for the
   /// translation.
-  virtual llvm::opt::DerivedArgList *
-  TranslateOpenMPTargetArgs(const llvm::opt::DerivedArgList &Args,
-      Action::OffloadKind DeviceOffloadKind) const;
+  virtual llvm::opt::DerivedArgList *TranslateOpenMPTargetArgs(
+      const llvm::opt::DerivedArgList &Args,
+      Action::OffloadKind DeviceOffloadKind,
+      SmallVector<llvm::opt::Arg *, 4> &AllocatedArgs) const;
 
   /// Choose a tool to use to handle the action \p JA.
   ///



Index: test/Driver/openmp-offload-gpu.c
===================================================================
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -2,9 +2,6 @@
 /// Perform several driver tests for OpenMP offloading
 ///
 
-// Until this test is stabilized on all local configurations.
-// UNSUPPORTED: linux
-
 // REQUIRES: clang-driver
 // REQUIRES: x86-registered-target
 // REQUIRES: powerpc-registered-target
Index: lib/Driver/ToolChain.cpp
===================================================================
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -800,9 +800,10 @@
   return VersionTuple();
 }
 
-llvm::opt::DerivedArgList *
-ToolChain::TranslateOpenMPTargetArgs(const llvm::opt::DerivedArgList &Args,
-    Action::OffloadKind DeviceOffloadKind) const {
+llvm::opt::DerivedArgList *ToolChain::TranslateOpenMPTargetArgs(
+    const llvm::opt::DerivedArgList &Args,
+    Action::OffloadKind DeviceOffloadKind,
+    SmallVector<llvm::opt::Arg *, 4> &AllocatedArgs) const {
   if (DeviceOffloadKind == Action::OFK_OpenMP) {
     DerivedArgList *DAL = new DerivedArgList(Args.getBaseArgs());
     const OptTable &Opts = getDriver().getOpts();
@@ -854,6 +855,7 @@
       }
       XOpenMPTargetArg->setBaseArg(A);
       A = XOpenMPTargetArg.release();
+      AllocatedArgs.push_back(A);
       DAL->append(A);
       NewArgAdded = true;
     }
Index: lib/Driver/Compilation.cpp
===================================================================
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -51,9 +51,10 @@
 
   DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}];
   if (!Entry) {
+    SmallVector<Arg *, 4> AllocatedArgs;
     // Translate OpenMP toolchain arguments provided via the -Xopenmp-target flags.
-    DerivedArgList *OpenMPArgs = TC->TranslateOpenMPTargetArgs(*TranslatedArgs,
-        DeviceOffloadKind);
+    DerivedArgList *OpenMPArgs = TC->TranslateOpenMPTargetArgs(
+        *TranslatedArgs, DeviceOffloadKind, AllocatedArgs);
     if (!OpenMPArgs) {
       Entry = TC->TranslateArgs(*TranslatedArgs, BoundArch, DeviceOffloadKind);
     } else {
@@ -63,6 +64,11 @@
 
     if (!Entry)
       Entry = TranslatedArgs;
+
+    // Add allocated arguments to the final DAL.
+    for (auto ArgPtr : AllocatedArgs) {
+      Entry->AddSynthesizedArg(ArgPtr);
+    }
   }
 
   return *Entry;
Index: include/clang/Driver/ToolChain.h
===================================================================
--- include/clang/Driver/ToolChain.h
+++ include/clang/Driver/ToolChain.h
@@ -249,9 +249,10 @@
   ///
   /// \param DeviceOffloadKind - The device offload kind used for the
   /// translation.
-  virtual llvm::opt::DerivedArgList *
-  TranslateOpenMPTargetArgs(const llvm::opt::DerivedArgList &Args,
-      Action::OffloadKind DeviceOffloadKind) const;
+  virtual llvm::opt::DerivedArgList *TranslateOpenMPTargetArgs(
+      const llvm::opt::DerivedArgList &Args,
+      Action::OffloadKind DeviceOffloadKind,
+      SmallVector<llvm::opt::Arg *, 4> &AllocatedArgs) const;
 
   /// Choose a tool to use to handle the action \p JA.
   ///

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits