From 1907413c920301e877b62026331424ced8901eaf Mon Sep 17 00:00:00 2001
From: Matteo Settenvini <matteo@member.fsf.org>
Date: Fri, 1 Apr 2016 14:08:48 +0200
Subject: [PATCH] Prevent SIGSEGV when ALIASing a target defined in another
 CMakeLists.txt

This commit reverts a67231ac, as it incorrectly assumed that aliases
can only be made for targets defined in the same
CMakeLists.txt. However, this is not the case e.g. for an alias made
in a subdirectory for a target in a top-level one.

There are some minor adjustements to take care of correct generation
of targets in cmLocalGenerator.cxx, and I factored out the map
definition in its own header, since it was used in multiple places in
the code.

I added a test to check for this corner case too.
---
 Source/cmAddExecutableCommand.cxx       |  2 +-
 Source/cmAddLibraryCommand.cxx          |  2 +-
 Source/cmGlobalGenerator.cxx            | 19 ++++++++-----------
 Source/cmGlobalGenerator.h              | 19 ++++++++++---------
 Source/cmLocalGenerator.cxx             | 16 ++++++----------
 Source/cmLocalGenerator.h               |  4 +++-
 Source/cmMakefile.cxx                   | 17 +++++++----------
 Source/cmMakefile.h                     | 19 ++++++-------------
 Source/cmTargetMap.h                    | 28 ++++++++++++++++++++++++++++
 Tests/AliasTarget/subdir/CMakeLists.txt |  4 ++++
 10 files changed, 74 insertions(+), 56 deletions(-)
 create mode 100644 Source/cmTargetMap.h

diff --git a/Source/cmAddExecutableCommand.cxx b/Source/cmAddExecutableCommand.cxx
index a84bb9d..47f6592 100644
--- a/Source/cmAddExecutableCommand.cxx
+++ b/Source/cmAddExecutableCommand.cxx
@@ -192,7 +192,7 @@ bool cmAddExecutableCommand
       this->SetError(e.str());
       return false;
       }
-    this->Makefile->AddAlias(exename, aliasedName);
+    this->Makefile->AddAlias(exename, aliasedTarget);
     return true;
     }
 
diff --git a/Source/cmAddLibraryCommand.cxx b/Source/cmAddLibraryCommand.cxx
index 5296cbb..e0adee3 100644
--- a/Source/cmAddLibraryCommand.cxx
+++ b/Source/cmAddLibraryCommand.cxx
@@ -314,7 +314,7 @@ bool cmAddLibraryCommand
       this->SetError(e.str());
       return false;
       }
-    this->Makefile->AddAlias(libName, aliasedName);
+    this->Makefile->AddAlias(libName, aliasedTarget);
     return true;
     }
 
diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 1d0ade4..27963b1 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -2195,10 +2195,9 @@ cmGlobalGenerator::FindLocalGenerator(const std::string& start_dir) const
 }
 
 //----------------------------------------------------------------------------
-void cmGlobalGenerator::AddAlias(const std::string& name,
-                                 std::string const& tgtName)
+void cmGlobalGenerator::AddAlias(const std::string& name, cmTarget *tgt)
 {
-  this->AliasTargets[name] = tgtName;
+  this->AliasTargets[name] = tgt;
 }
 
 //----------------------------------------------------------------------------
@@ -2225,7 +2224,7 @@ void cmGlobalGenerator::IndexGeneratorTarget(cmGeneratorTarget* gt)
 
 cmTarget* cmGlobalGenerator::FindTargetImpl(std::string const& name) const
 {
-  TargetMap::const_iterator i = this->TargetSearchIndex.find(name);
+  cmTargetMap::const_iterator i = this->TargetSearchIndex.find(name);
   if (i != this->TargetSearchIndex.end())
     {
     return i->second;
@@ -2252,11 +2251,10 @@ cmGlobalGenerator::FindTarget(const std::string& name,
 {
   if (!excludeAliases)
     {
-    std::map<std::string, std::string>::const_iterator ai =
-        this->AliasTargets.find(name);
+    cmTargetMap::const_iterator ai = this->AliasTargets.find(name);
     if (ai != this->AliasTargets.end())
       {
-      return this->FindTargetImpl(ai->second);
+      return ai->second;
       }
     }
   return this->FindTargetImpl(name);
@@ -2265,11 +2263,10 @@ cmGlobalGenerator::FindTarget(const std::string& name,
 cmGeneratorTarget*
 cmGlobalGenerator::FindGeneratorTarget(const std::string& name) const
 {
-  std::map<std::string, std::string>::const_iterator ai =
-      this->AliasTargets.find(name);
-  if (ai != this->AliasTargets.end())
+  cmTargetMap::const_iterator i = this->AliasTargets.find(name);
+  if (i != this->AliasTargets.end())
     {
-    return this->FindGeneratorTargetImpl(ai->second);
+    return this->FindGeneratorTargetImpl(i->second->GetName());
     }
   return this->FindGeneratorTargetImpl(name);
 }
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 14c7d67..a556766 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -22,6 +22,7 @@
 #include "cmGeneratorTarget.h"
 #include "cmGeneratorExpression.h"
 #include "cmState.h"
+#include "cmTargetMap.h"
 
 #if defined(CMAKE_BUILD_WITH_CMAKE)
 # include "cmFileLockPool.h"
@@ -247,7 +248,7 @@ public:
 
   cmGeneratorTarget* FindGeneratorTarget(const std::string& name) const;
 
-  void AddAlias(const std::string& name, const std::string& tgtName);
+  void AddAlias(const std::string& name, cmTarget *tgt);
   bool IsAlias(const std::string& name) const;
 
   /** Determine if a name resolves to a framework on disk or a built target
@@ -415,7 +416,8 @@ protected:
   std::map<std::string, cmExportBuildFileGenerator*> BuildExportSets;
   std::map<std::string, cmExportBuildFileGenerator*> BuildExportExportSets;
 
-  std::map<std::string, std::string> AliasTargets;
+  // All targets in the entire project.
+  cmTargetMap AliasTargets;
 
   cmTarget* FindTargetImpl(std::string const& name) const;
 
@@ -428,23 +430,22 @@ protected:
 
 private:
 
+  // Map efficiently from target name to cmTarget instance.
+  // Do not use this structure for looping over all targets.
+  // It contains both normal and globally visible imported targets.
+  cmTargetMap TargetSearchIndex;
+
 #if defined(CMAKE_BUILD_WITH_CMAKE)
 # ifdef CMake_HAVE_CXX11_UNORDERED_MAP
-  typedef std::unordered_map<std::string, cmTarget*> TargetMap;
   typedef std::unordered_map<std::string, cmGeneratorTarget*>
     GeneratorTargetMap;
 # else
-  typedef cmsys::hash_map<std::string, cmTarget*> TargetMap;
   typedef cmsys::hash_map<std::string, cmGeneratorTarget*> GeneratorTargetMap;
 # endif
 #else
-  typedef std::map<std::string,cmTarget *> TargetMap;
   typedef std::map<std::string,cmGeneratorTarget *> GeneratorTargetMap;
 #endif
-  // Map efficiently from target name to cmTarget instance.
-  // Do not use this structure for looping over all targets.
-  // It contains both normal and globally visible imported targets.
-  TargetMap TargetSearchIndex;
+
   GeneratorTargetMap GeneratorTargetSearchIndex;
 
   cmMakefile* TryCompileOuterMakefile;
diff --git a/Source/cmLocalGenerator.cxx b/Source/cmLocalGenerator.cxx
index 1be39a9..2aab31d 100644
--- a/Source/cmLocalGenerator.cxx
+++ b/Source/cmLocalGenerator.cxx
@@ -485,23 +485,19 @@ private:
   std::string Name;
 };
 
-cmGeneratorTarget* cmLocalGenerator::FindGeneratorTarget(
-    const std::string& name) const
+cmGeneratorTarget* cmLocalGenerator::FindGeneratorTarget(const std::string& name) const
 {
-  std::map<std::string, std::string>::const_iterator i =
-      this->AliasTargets.find(name);
+  const std::string *namePtr = &name;
+  cmTargetMap::const_iterator i = this->AliasTargets.find(name);
   if (i != this->AliasTargets.end())
     {
-    std::vector<cmGeneratorTarget*>::const_iterator ai =
-        std::find_if(this->GeneratorTargets.begin(),
-                     this->GeneratorTargets.end(),
-                     NamedGeneratorTargetFinder(i->second));
-    return *ai;
+    namePtr = &i->second->GetName();
     }
+    
   std::vector<cmGeneratorTarget*>::const_iterator ti =
       std::find_if(this->GeneratorTargets.begin(),
                    this->GeneratorTargets.end(),
-                   NamedGeneratorTargetFinder(name));
+                   NamedGeneratorTargetFinder(*namePtr));
   if ( ti != this->GeneratorTargets.end() )
     {
     return *ti;
diff --git a/Source/cmLocalGenerator.h b/Source/cmLocalGenerator.h
index 68e7667..825b7df 100644
--- a/Source/cmLocalGenerator.h
+++ b/Source/cmLocalGenerator.h
@@ -16,6 +16,7 @@
 #include "cmState.h"
 #include "cmake.h"
 #include "cmOutputConverter.h"
+#include "cmTargetMap.h"
 
 class cmMakefile;
 class cmGlobalGenerator;
@@ -382,7 +383,8 @@ protected:
   std::vector<cmGeneratorTarget*> GeneratorTargets;
   std::vector<cmGeneratorTarget*> ImportedGeneratorTargets;
   std::vector<cmGeneratorTarget*> OwnedImportedGeneratorTargets;
-  std::map<std::string, std::string> AliasTargets;
+
+  cmTargetMap AliasTargets;
 
   bool EmitUniversalBinaryFlags;
 
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index aa6f7c8..b1166b0 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -1761,7 +1761,7 @@ std::vector<cmTarget*> cmMakefile::GetImportedTargets() const
 {
   std::vector<cmTarget*> tgts;
   tgts.reserve(this->ImportedTargets.size());
-  for (TargetMap::const_iterator it = this->ImportedTargets.begin();
+  for (cmTargetMap::const_iterator it = this->ImportedTargets.begin();
        it != this->ImportedTargets.end(); ++it)
     {
     tgts.push_back(it->second);
@@ -2039,11 +2039,10 @@ void cmMakefile::AddGlobalLinkInformation(const std::string& name,
 }
 
 
-void cmMakefile::AddAlias(const std::string& lname,
-                          std::string const& tgtName)
+void cmMakefile::AddAlias(const std::string& lname, cmTarget *tgt)
 {
-  this->AliasTargets[lname] = tgtName;
-  this->GetGlobalGenerator()->AddAlias(lname, tgtName);
+  this->AliasTargets[lname] = tgt;
+  this->GetGlobalGenerator()->AddAlias(lname, tgt);
 }
 
 cmTarget* cmMakefile::AddLibrary(const std::string& lname,
@@ -4031,12 +4030,10 @@ cmTarget* cmMakefile::FindTarget(const std::string& name,
 {
   if (!excludeAliases)
     {
-    std::map<std::string, std::string>::const_iterator i =
-        this->AliasTargets.find(name);
+    cmTargetMap::const_iterator i = this->AliasTargets.find(name);
     if (i != this->AliasTargets.end())
       {
-      cmTargets::iterator ai = this->Targets.find(i->second);
-      return &ai->second;
+      return i->second;
       }
     }
   cmTargets::iterator i = this->Targets.find( name );
@@ -4207,7 +4204,7 @@ cmTarget* cmMakefile::FindTargetToUse(const std::string& name,
 {
   // Look for an imported target.  These take priority because they
   // are more local in scope and do not have to be globally unique.
-  TargetMap::const_iterator
+  cmTargetMap::const_iterator
     imported = this->ImportedTargets.find(name);
   if(imported != this->ImportedTargets.end())
     {
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 362ea75..37cf67a 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -21,6 +21,7 @@
 #include "cmake.h"
 #include "cmState.h"
 #include "cmAlgorithms.h"
+#include "cmTargetMap.h"
 
 #if defined(CMAKE_BUILD_WITH_CMAKE)
 #include "cmSourceGroup.h"
@@ -283,7 +284,7 @@ public:
   cmTarget* AddLibrary(const std::string& libname, cmState::TargetType type,
                   const std::vector<std::string> &srcs,
                   bool excludeFromAll = false);
-  void AddAlias(const std::string& libname, const std::string& tgt);
+  void AddAlias(const std::string& libname, cmTarget *tgt);
 
 #if defined(CMAKE_BUILD_WITH_CMAKE)
   /**
@@ -397,7 +398,7 @@ public:
                             bool excludeAliases = false) const;
   bool IsAlias(const std::string& name) const;
 
-  std::map<std::string, std::string> GetAliasTargets() const
+  const cmTargetMap& GetAliasTargets() const
   {
     return this->AliasTargets;
   }
@@ -784,16 +785,8 @@ protected:
 
   // libraries, classes, and executables
   mutable cmTargets Targets;
-#if defined(CMAKE_BUILD_WITH_CMAKE)
-#ifdef CMake_HAVE_CXX11_UNORDERED_MAP
-  typedef std::unordered_map<std::string, cmTarget*> TargetMap;
-#else
-  typedef cmsys::hash_map<std::string, cmTarget*> TargetMap;
-#endif
-#else
-  typedef std::map<std::string, cmTarget*> TargetMap;
-#endif
-  std::map<std::string, std::string> AliasTargets;
+
+  cmTargetMap AliasTargets;
   std::vector<cmSourceFile*> SourceFiles;
 
   // Tests
@@ -869,7 +862,7 @@ private:
   friend class cmParseFileScope;
 
   std::vector<cmTarget*> ImportedTargetsOwned;
-  TargetMap ImportedTargets;
+  cmTargetMap ImportedTargets;
 
   // Internal policy stack management.
   void PushPolicy(bool weak = false,
diff --git a/Source/cmTargetMap.h b/Source/cmTargetMap.h
new file mode 100644
index 0000000..e179be2
--- /dev/null
+++ b/Source/cmTargetMap.h
@@ -0,0 +1,28 @@
+/*============================================================================
+  CMake - Cross Platform Makefile Generator
+  Copyright 2000-2009 Kitware, Inc., Insight Software Consortium
+
+  Distributed under the OSI-approved BSD License (the "License");
+  see accompanying file Copyright.txt for details.
+
+  This software is distributed WITHOUT ANY WARRANTY; without even the
+  implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+  See the License for more information.
+============================================================================*/
+
+#ifndef cmTargetMap_h
+#define cmTargetMap_h
+
+#if defined(CMAKE_BUILD_WITH_CMAKE)
+# ifdef CMake_HAVE_CXX11_UNORDERED_MAP
+#  include <unordered_map>
+  typedef std::unordered_map<std::string, cmTarget*> cmTargetMap;
+# else
+#  include <cmsys/hash_map.hxx>
+  typedef cmsys::hash_map<std::string, cmTarget*> cmTargetMap;
+# endif
+#else
+  typedef std::map<std::string,cmTarget *> cmTargetMap;
+#endif
+
+#endif
diff --git a/Tests/AliasTarget/subdir/CMakeLists.txt b/Tests/AliasTarget/subdir/CMakeLists.txt
index 8c84aea..bf9e768 100644
--- a/Tests/AliasTarget/subdir/CMakeLists.txt
+++ b/Tests/AliasTarget/subdir/CMakeLists.txt
@@ -1,3 +1,7 @@
 
 add_library(tgt STATIC empty.cpp)
 add_library(Sub::tgt ALIAS tgt)
+
+# foo comes from the top-level CMakeLists.txt
+add_library(Top::foo ALIAS foo)
+get_target_property(a_prop Top::foo INTERFACE_SYSTEM_INCLUDE_DIRECTORIES)
-- 
2.7.4