Merge topic 'export-repeat'

8affe9aa33 export: Fix use-after-free on multiple calls overwriting same FILE

Acked-by: Kitware Robot <[email protected]>
Merge-request: !4494

Source/cmExportCommand.cxx

@@ -198,7 +198,6 @@ bool cmExportCommand(std::vector<std::string> const& args,
   } else {
     ebfg->SetTargets(targets);
   }
-  mf.AddExportBuildFileGenerator(ebfg.get());
   ebfg->SetExportOld(arguments.ExportOld);
 
   // Compute the set of configurations exported.
@@ -211,10 +210,11 @@ bool cmExportCommand(std::vector<std::string> const& args,
     ebfg->AddConfiguration(ct);
   }
   if (exportSet != nullptr) {
-    gg->AddBuildExportExportSet(std::move(ebfg));
+    gg->AddBuildExportExportSet(ebfg.get());
   } else {
-    gg->AddBuildExportSet(std::move(ebfg));
+    gg->AddBuildExportSet(ebfg.get());
   }
+  mf.AddExportBuildFileGenerator(std::move(ebfg));
 
   return true;
 }

Source/cmGlobalGenerator.cxx

@@ -262,17 +262,16 @@ void cmGlobalGenerator::ResolveLanguageCompiler(const std::string& lang,
   }
 }
 
-void cmGlobalGenerator::AddBuildExportSet(
-  std::unique_ptr<cmExportBuildFileGenerator> gen)
+void cmGlobalGenerator::AddBuildExportSet(cmExportBuildFileGenerator* gen)
 {
-  this->BuildExportSets[gen->GetMainExportFileName()] = std::move(gen);
+  this->BuildExportSets[gen->GetMainExportFileName()] = gen;
 }
 
 void cmGlobalGenerator::AddBuildExportExportSet(
-  std::unique_ptr<cmExportBuildFileGenerator> gen)
+  cmExportBuildFileGenerator* gen)
 {
-  this->BuildExportExportSets[gen->GetMainExportFileName()] = gen.get();
-  this->AddBuildExportSet(std::move(gen));
+  this->BuildExportExportSets[gen->GetMainExportFileName()] = gen;
+  this->AddBuildExportSet(gen);
 }
 
 bool cmGlobalGenerator::GenerateImportFile(const std::string& file)
@@ -283,7 +282,7 @@ bool cmGlobalGenerator::GenerateImportFile(const std::string& file)
 
     if (!this->ConfigureDoneCMP0026AndCMP0024) {
       for (const auto& m : this->Makefiles) {
-        m->RemoveExportBuildFileGeneratorCMP0024(it->second.get());
+        m->RemoveExportBuildFileGeneratorCMP0024(it->second);
       }
     }
 
@@ -1317,7 +1316,7 @@ cmExportBuildFileGenerator* cmGlobalGenerator::GetExportedTargetsFile(
   const std::string& filename) const
 {
   auto const it = this->BuildExportSets.find(filename);
-  return it == this->BuildExportSets.end() ? nullptr : it->second.get();
+  return it == this->BuildExportSets.end() ? nullptr : it->second;
 }
 
 void cmGlobalGenerator::AddCMP0042WarnTarget(const std::string& target)
@@ -1353,9 +1352,9 @@ bool cmGlobalGenerator::CheckALLOW_DUPLICATE_CUSTOM_TARGETS() const
 void cmGlobalGenerator::ComputeBuildFileGenerators()
 {
   for (unsigned int i = 0; i < this->LocalGenerators.size(); ++i) {
-    std::vector<cmExportBuildFileGenerator*> gens =
+    std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const& gens =
       this->Makefiles[i]->GetExportBuildFileGenerators();
-    for (cmExportBuildFileGenerator* g : gens) {
+    for (std::unique_ptr<cmExportBuildFileGenerator> const& g : gens) {
       g->Compute(this->LocalGenerators[i].get());
     }
   }

Source/cmGlobalGenerator.h

@@ -463,13 +463,12 @@ public:
 
   void ProcessEvaluationFiles();
 
-  std::map<std::string, std::unique_ptr<cmExportBuildFileGenerator>>&
-  GetBuildExportSets()
+  std::map<std::string, cmExportBuildFileGenerator*>& GetBuildExportSets()
   {
     return this->BuildExportSets;
   }
-  void AddBuildExportSet(std::unique_ptr<cmExportBuildFileGenerator>);
-  void AddBuildExportExportSet(std::unique_ptr<cmExportBuildFileGenerator>);
+  void AddBuildExportSet(cmExportBuildFileGenerator* gen);
+  void AddBuildExportExportSet(cmExportBuildFileGenerator* gen);
   bool IsExportedTargetsFile(const std::string& filename) const;
   bool GenerateImportFile(const std::string& file);
   cmExportBuildFileGenerator* GetExportedTargetsFile(
@@ -580,8 +579,7 @@ protected:
   std::set<std::string> InstallComponents;
   // Sets of named target exports
   cmExportSetMap ExportSets;
-  std::map<std::string, std::unique_ptr<cmExportBuildFileGenerator>>
-    BuildExportSets;
+  std::map<std::string, cmExportBuildFileGenerator*> BuildExportSets;
   std::map<std::string, cmExportBuildFileGenerator*> BuildExportExportSets;
 
   std::map<std::string, std::string> AliasTargets;

Source/cmMakefile.cxx

@@ -32,6 +32,7 @@
 #include "cmCustomCommandLines.h"
 #include "cmExecutionStatus.h"
 #include "cmExpandedCommandArgument.h" // IWYU pragma: keep
+#include "cmExportBuildFileGenerator.h"
 #include "cmFileLockPool.h"
 #include "cmFunctionBlocker.h"
 #include "cmGeneratedFileStream.h"
@@ -813,7 +814,7 @@ cmMakefile::GetEvaluationFiles() const
   return this->EvaluationFiles;
 }
 
-std::vector<cmExportBuildFileGenerator*>
+std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const&
 cmMakefile::GetExportBuildFileGenerators() const
 {
   return this->ExportBuildFileGenerators;
@@ -822,16 +823,21 @@ cmMakefile::GetExportBuildFileGenerators() const
 void cmMakefile::RemoveExportBuildFileGeneratorCMP0024(
   cmExportBuildFileGenerator* gen)
 {
-  auto it = std::find(this->ExportBuildFileGenerators.begin(),
-                      this->ExportBuildFileGenerators.end(), gen);
+  auto it =
+    std::find_if(this->ExportBuildFileGenerators.begin(),
+                 this->ExportBuildFileGenerators.end(),
+                 [gen](std::unique_ptr<cmExportBuildFileGenerator> const& p) {
+                   return p.get() == gen;
+                 });
   if (it != this->ExportBuildFileGenerators.end()) {
     this->ExportBuildFileGenerators.erase(it);
   }
 }
 
-void cmMakefile::AddExportBuildFileGenerator(cmExportBuildFileGenerator* gen)
+void cmMakefile::AddExportBuildFileGenerator(
+  std::unique_ptr<cmExportBuildFileGenerator> gen)
 {
-  this->ExportBuildFileGenerators.push_back(gen);
+  this->ExportBuildFileGenerators.emplace_back(std::move(gen));
 }
 
 namespace {

Source/cmMakefile.h

@@ -954,10 +954,11 @@ public:
   const std::vector<std::unique_ptr<cmGeneratorExpressionEvaluationFile>>&
   GetEvaluationFiles() const;
 
-  std::vector<cmExportBuildFileGenerator*> GetExportBuildFileGenerators()
-    const;
+  std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const&
+  GetExportBuildFileGenerators() const;
   void RemoveExportBuildFileGeneratorCMP0024(cmExportBuildFileGenerator* gen);
-  void AddExportBuildFileGenerator(cmExportBuildFileGenerator* gen);
+  void AddExportBuildFileGenerator(
+    std::unique_ptr<cmExportBuildFileGenerator> gen);
 
   // Maintain a stack of package roots to allow nested PACKAGE_ROOT_PATH
   // searches
@@ -1062,7 +1063,8 @@ private:
   mutable cmsys::RegularExpression cmNamedCurly;
 
   std::vector<cmMakefile*> UnConfiguredDirectories;
-  std::vector<cmExportBuildFileGenerator*> ExportBuildFileGenerators;
+  std::vector<std::unique_ptr<cmExportBuildFileGenerator>>
+    ExportBuildFileGenerators;
 
   std::vector<std::unique_ptr<cmGeneratorExpressionEvaluationFile>>
     EvaluationFiles;

Tests/RunCMake/export/Repeat.cmake

@@ -0,0 +1,5 @@
+add_library(foo INTERFACE)
+export(TARGETS foo FILE foo.cmake)
+export(TARGETS foo FILE foo.cmake)
+add_subdirectory(Repeat)
+include(CMakePackageConfigHelpers)

Tests/RunCMake/export/Repeat/CMakeLists.txt

@@ -0,0 +1,2 @@
+add_library(bar INTERFACE)
+export(TARGETS bar FILE ${CMAKE_BINARY_DIR}/foo.cmake)

Tests/RunCMake/export/RunCMakeTest.cmake

@@ -2,6 +2,7 @@ include(RunCMake)
 
 run_cmake(CustomTarget)
 run_cmake(Empty)
+run_cmake(Repeat)
 run_cmake(TargetNotFound)
 run_cmake(AppendExport)
 run_cmake(OldIface)