[PATCH] D38257: [OpenMP] Fix memory leak when translating arguments
This revision was automatically updated to reflect the committed changes. Closed by commit rL314328: [OpenMP] Fix memory leak when translating arguments (authored by Hahnfeld). Changed prior to commit: https://reviews.llvm.org/D38257?vs=116607&id=116844#toc Repository: rL LLVM https://reviews.llvm.org/D38257 Files: cfe/trunk/include/clang/Driver/ToolChain.h cfe/trunk/lib/Driver/Compilation.cpp cfe/trunk/lib/Driver/ToolChain.cpp cfe/trunk/test/Driver/openmp-offload-gpu.c Index: cfe/trunk/lib/Driver/ToolChain.cpp === --- cfe/trunk/lib/Driver/ToolChain.cpp +++ cfe/trunk/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 &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: cfe/trunk/lib/Driver/Compilation.cpp === --- cfe/trunk/lib/Driver/Compilation.cpp +++ cfe/trunk/lib/Driver/Compilation.cpp @@ -51,9 +51,10 @@ DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}]; if (!Entry) { +SmallVector 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: cfe/trunk/include/clang/Driver/ToolChain.h === --- cfe/trunk/include/clang/Driver/ToolChain.h +++ cfe/trunk/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 &AllocatedArgs) const; /// Choose a tool to use to handle the action \p JA. /// Index: cfe/trunk/test/Driver/openmp-offload-gpu.c === --- cfe/trunk/test/Driver/openmp-offload-gpu.c +++ cfe/trunk/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: cfe/trunk/lib/Driver/ToolChain.cpp === --- cfe/trunk/lib/Driver/ToolChain.cpp +++ cfe/trunk/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 &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: cfe/trunk/lib/Driver/Compilation.cpp === --- cfe/trunk/lib/Driver/Compilation.cpp +++ cfe/trunk/lib/Driver/Compilation.cpp @@ -51,9 +51,10 @@ DerivedArgList *&Entry = TCArgs[{TC, BoundArch, DeviceOffloadKind}]; if (!Entry) { +SmallVector AllocatedAr
[PATCH] D38257: [OpenMP] Fix memory leak when translating arguments
gtbercea accepted this revision. gtbercea added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D38257 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38257: [OpenMP] Fix memory leak when translating arguments
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 &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 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 &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 &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/Compilat