Refactor generation of shared library flags

CMAKE_SHARED_LIBRARY_<lang>_FLAGS has flags on various platforms for a
variety of purposes that are correlated with shared libraries but not
exclusive to them.  Refactor generation of these flags to use new
purpose-specific platform variables

  CMAKE_<lang>_COMPILE_OPTIONS_DLL
  CMAKE_<lang>_COMPILE_OPTIONS_PIC
  CMAKE_<lang>_COMPILE_OPTIONS_PIE

Activate the DLL flags specifically for shared libraries.  Add a new
POSITION_INDEPENDENT_CODE target property to activate PIC/PIE flags, and
default to true for shared libraries to preserve default behavior.
Initialize the new property from CMAKE_POSITION_INDEPENDENT_CODE to
allow easy global configuration in projects.

Although the default behavior is unchanged by this refactoring, the new
approach ignores CMAKE_SHARED_LIBRARY_<lang>_FLAGS completely.  We must
leave it set in case projects reference the value.  Furthermore, if a
project modifies CMAKE_SHARED_LIBRARY_<lang>_FLAGS it expects the new
value to be used.  Add policy CMP0018 to handle compatibility with
projects that modify this platform variable.

Add a PositionIndependentCode test on platforms where we can get
meaningful results.

Source/cmCoreTryCompile.cxx

@@ -281,6 +281,10 @@ int cmCoreTryCompile::TryCompileCode(std::vector<std::string> const& argv)
       flag += this->Makefile->GetSafeDefinition("CMAKE_OSX_DEPLOYMENT_TARGET");
       cmakeFlags.push_back(flag);
       }
+    if(this->Makefile->GetDefinition("CMAKE_POSITION_INDEPENDENT_CODE")!=0)
+      {
+      fprintf(fout, "SET(CMAKE_POSITION_INDEPENDENT_CODE \"ON\")\n");
+      }
 
     /* Use a random file name to avoid rapid creation and deletion
        of the same executable name (some filesystems fail on that).  */

Source/cmDocumentVariables.cxx

@@ -1350,6 +1350,14 @@ void cmDocumentVariables::DefineVariables(cmake* cm)
      "See that target property for additional information.",
      false,
      "Variables that Control the Build");
+  cm->DefineProperty
+    ("CMAKE_POSITION_INDEPENDENT_FLAGS", cmProperty::VARIABLE,
+     "Default value for POSITION_INDEPENDENT_CODE of targets.",
+     "This variable is used to initialize the "
+     "POSITION_INDEPENDENT_CODE property on all the targets. "
+     "See that target property for additional information.",
+     false,
+     "Variables that Control the Build");
 
 //   Variables defined when the a language is enabled These variables will
 // also be defined whenever CMake has loaded its support for compiling (LANG)

Source/cmGlobalGenerator.cxx

@@ -586,6 +586,16 @@ cmGlobalGenerator::EnableLanguage(std::vector<std::string>const& languages,
           }
         } // end if in try compile
       } // end need test language
+    // Store the shared library flags so that we can satisfy CMP0018
+    std::string sharedLibFlagsVar = "CMAKE_SHARED_LIBRARY_";
+    sharedLibFlagsVar += lang;
+    sharedLibFlagsVar += "_FLAGS";
+    const char* sharedLibFlags =
+      mf->GetSafeDefinition(sharedLibFlagsVar.c_str());
+    if (sharedLibFlags)
+      {
+      this->LanguageToOriginalSharedLibFlags[lang] = sharedLibFlags;
+      }
     } // end for each language
 
   // Now load files that can override any settings on the platform or for
@@ -2106,6 +2116,17 @@ cmGlobalGenerator::GenerateRuleFile(std::string const& output) const
   return ruleFile;
 }
 
+//----------------------------------------------------------------------------
+std::string cmGlobalGenerator::GetSharedLibFlagsForLanguage(
+                                                        std::string const& l)
+{
+  if(this->LanguageToOriginalSharedLibFlags.count(l) > 0)
+    {
+    return this->LanguageToOriginalSharedLibFlags[l];
+    }
+  return "";
+}
+
 //----------------------------------------------------------------------------
 void cmGlobalGenerator::AppendDirectoryForConfig(const char*, const char*,
                                                  const char*, std::string&)

Source/cmGlobalGenerator.h

@@ -277,6 +277,8 @@ public:
       i.e. "Can I build Debug and Release in the same tree?" */
   virtual bool IsMultiConfig() { return false; }
 
+  std::string GetSharedLibFlagsForLanguage(std::string const& lang);
+
   /** Generate an <output>.rule file path for a given command output.  */
   virtual std::string GenerateRuleFile(std::string const& output) const;
 
@@ -357,6 +359,7 @@ private:
   std::map<cmStdString, cmStdString> LanguageToOutputExtension;
   std::map<cmStdString, cmStdString> ExtensionToLanguage;
   std::map<cmStdString, int> LanguageToLinkerPreference;
+  std::map<cmStdString, cmStdString> LanguageToOriginalSharedLibFlags;
 
   // Record hashes for rules and outputs.
   struct RuleHash { char Data[32]; };

Source/cmGlobalXCodeGenerator.cxx

@@ -1594,14 +1594,14 @@ void cmGlobalXCodeGenerator::CreateBuildSettings(cmTarget& target,
     if(strcmp(lang, "CXX") == 0)
       {
       this->CurrentLocalGenerator->AddLanguageFlags(cflags, "C", configName);
-      this->CurrentLocalGenerator->AddSharedFlags(cflags, lang, shared);
+      this->CurrentLocalGenerator->AddCMP0018Flags(cflags, &target, "C");
       }
 
     // Add language-specific flags.
     this->CurrentLocalGenerator->AddLanguageFlags(flags, lang, configName);
 
     // Add shared-library flags if needed.
-    this->CurrentLocalGenerator->AddSharedFlags(flags, lang, shared);
+    this->CurrentLocalGenerator->AddCMP0018Flags(flags, &target, lang);
     }
   else if(binary)
   {

Source/cmLocalGenerator.cxx

@@ -1549,12 +1549,6 @@ void cmLocalGenerator::GetTargetFlags(std::string& linkLibs,
         return;
         }
       this->AddLanguageFlags(flags, linkLanguage, buildType.c_str());
-      std::string sharedFlagsVar = "CMAKE_SHARED_LIBRARY_";
-      sharedFlagsVar += linkLanguage;
-      sharedFlagsVar += "_FLAGS";
-      flags += " ";
-      flags += this->Makefile->GetSafeDefinition(sharedFlagsVar.c_str());
-      flags += " ";
       cmOStringStream linklibs;
       this->OutputLinkLibraries(linklibs, target, false);
       linkLibs = linklibs.str();
@@ -1962,6 +1956,111 @@ void cmLocalGenerator::AddSharedFlags(std::string& flags,
     }
 }
 
+//----------------------------------------------------------------------------
+void cmLocalGenerator::AddCMP0018Flags(std::string &flags, cmTarget* target,
+                                       std::string const& lang)
+{
+  int targetType = target->GetType();
+
+  bool shared = ((targetType == cmTarget::SHARED_LIBRARY) ||
+                  (targetType == cmTarget::MODULE_LIBRARY));
+
+  if (this->GetShouldUseOldFlags(shared, lang))
+    {
+    this->AddSharedFlags(flags, lang.c_str(), shared);
+    }
+  else
+    {
+    // Add position independendent flags, if needed.
+    if (target->GetPropertyAsBool("POSITION_INDEPENDENT_CODE"))
+      {
+      this->AddPositionIndependentFlags(flags, lang, targetType);
+      }
+    if (shared)
+      {
+      this->AppendFeatureOptions(flags, lang.c_str(), "DLL");
+      }
+    }
+}
+
+//----------------------------------------------------------------------------
+bool cmLocalGenerator::GetShouldUseOldFlags(bool shared,
+                                            const std::string &lang) const
+{
+  std::string originalFlags =
+    this->GlobalGenerator->GetSharedLibFlagsForLanguage(lang);
+  if (shared)
+    {
+    std::string flagsVar = "CMAKE_SHARED_LIBRARY_";
+    flagsVar += lang;
+    flagsVar += "_FLAGS";
+    const char* flags =
+        this->Makefile->GetSafeDefinition(flagsVar.c_str());
+
+    if (flags && flags != originalFlags)
+      {
+      switch (this->Makefile->GetPolicyStatus(cmPolicies::CMP0018))
+        {
+        case cmPolicies::WARN:
+        {
+          cmOStringStream e;
+          e << "Variable " << flagsVar << " has been modified. CMake "
+            "will ignore the POSITION_INDEPENDENT_CODE target property for "
+            "shared libraries and will use the " << flagsVar << " variable "
+            "instead.  This may cause errors if the original content of "
+            << flagsVar << " was removed.\n"
+            << this->Makefile->GetPolicies()->GetPolicyWarning(
+                                                      cmPolicies::CMP0018);
+
+          this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, e.str());
+          // fall through to OLD behaviour
+        }
+        case cmPolicies::OLD:
+          return true;
+        case cmPolicies::REQUIRED_IF_USED:
+        case cmPolicies::REQUIRED_ALWAYS:
+        case cmPolicies::NEW:
+        default:
+          return false;
+        }
+      }
+    }
+  return false;
+}
+
+//----------------------------------------------------------------------------
+void cmLocalGenerator::AddPositionIndependentFlags(std::string& flags,
+                                                   std::string const& lang,
+                                                   int targetType)
+{
+  const char* picFlags = 0;
+
+  if(targetType == cmTarget::EXECUTABLE)
+    {
+    std::string flagsVar = "CMAKE_";
+    flagsVar += lang;
+    flagsVar += "_COMPILE_OPTIONS_PIE";
+    picFlags = this->Makefile->GetSafeDefinition(flagsVar.c_str());
+    }
+  if (!picFlags)
+    {
+    std::string flagsVar = "CMAKE_";
+    flagsVar += lang;
+    flagsVar += "_COMPILE_OPTIONS_PIC";
+    picFlags = this->Makefile->GetSafeDefinition(flagsVar.c_str());
+    }
+  if (picFlags)
+    {
+    std::vector<std::string> options;
+    cmSystemTools::ExpandListArgument(picFlags, options);
+    for(std::vector<std::string>::const_iterator oi = options.begin();
+        oi != options.end(); ++oi)
+      {
+      this->AppendFlags(flags, this->EscapeForShell(oi->c_str()).c_str());
+      }
+    }
+}
+
 //----------------------------------------------------------------------------
 void cmLocalGenerator::AddConfigVariableFlags(std::string& flags,
                                               const char* var,

Source/cmLocalGenerator.h

@@ -140,7 +140,8 @@ public:
 
   void AddLanguageFlags(std::string& flags, const char* lang,
                         const char* config);
-  void AddSharedFlags(std::string& flags, const char* lang, bool shared);
+  void AddCMP0018Flags(std::string &flags, cmTarget* target,
+                       std::string const& lang);
   void AddConfigVariableFlags(std::string& flags, const char* var,
                               const char* config);
   ///! Append flags to a string.
@@ -425,6 +426,11 @@ protected:
 private:
   std::string ConvertToOutputForExistingCommon(const char* remote,
                                                std::string const& result);
+
+  void AddSharedFlags(std::string& flags, const char* lang, bool shared);
+  bool GetShouldUseOldFlags(bool shared, const std::string &lang) const;
+  void AddPositionIndependentFlags(std::string& flags, std::string const& l,
+                                   int targetType);
 };
 
 #endif

Source/cmMakefileTargetGenerator.cxx

@@ -245,9 +245,6 @@ std::string cmMakefileTargetGenerator::GetFlags(const std::string &l)
     std::string flags;
     const char *lang = l.c_str();
 
-    bool shared = ((this->Target->GetType() == cmTarget::SHARED_LIBRARY) ||
-                   (this->Target->GetType() == cmTarget::MODULE_LIBRARY));
-
     // Add language feature flags.
     this->AddFeatureFlags(flags, lang);
 
@@ -260,8 +257,7 @@ std::string cmMakefileTargetGenerator::GetFlags(const std::string &l)
       this->AddFortranFlags(flags);
       }
 
-    // Add shared-library flags if needed.
+    this->LocalGenerator->AddCMP0018Flags(flags, this->Target, lang);
-    this->LocalGenerator->AddSharedFlags(flags, lang, shared);
 
     // Add include directory flags.
     this->AddIncludeFlags(flags, lang);

Source/cmNinjaTargetGenerator.cxx

@@ -140,11 +140,8 @@ cmNinjaTargetGenerator::ComputeFlagsForObject(cmSourceFile *source,
   //   }
 
   // Add shared-library flags if needed.
-  {
+  this->LocalGenerator->AddCMP0018Flags(flags, this->Target,
-  bool shared = ((this->Target->GetType() == cmTarget::SHARED_LIBRARY) ||
+                                        language.c_str());
-                 (this->Target->GetType() == cmTarget::MODULE_LIBRARY));
-  this->GetLocalGenerator()->AddSharedFlags(flags, language.c_str(), shared);
-  }
 
   // TODO: Handle response file.
   // Add include directory flags.

Source/cmPolicies.cxx

@@ -463,6 +463,34 @@ cmPolicies::cmPolicies()
     "The OLD behaviour is to always prefer files from CMAKE_MODULE_PATH over "
     "files from the CMake modules directory.",
     2,8,4,0, cmPolicies::WARN);
+
+    this->DefinePolicy(
+    CMP0018, "CMP0018",
+    "Ignore CMAKE_SHARED_LIBRARY_<Lang>_FLAGS variable.",
+    "CMake 2.8.8 and lower compiled sources in SHARED and MODULE libraries "
+    "using the value of the undocumented CMAKE_SHARED_LIBRARY_<Lang>_FLAGS "
+    "platform variable.  The variable contained platform-specific flags "
+    "needed to compile objects for shared libraries.  Typically it included "
+    "a flag such as -fPIC for position independent code but also included "
+    "other flags needed on certain platforms.  CMake 2.8.9 and higher "
+    "prefer instead to use the POSITION_INDEPENDENT_CODE target property to "
+    "determine what targets should be position independent, and new "
+    "undocumented platform variables to select flags while ignoring "
+    "CMAKE_SHARED_LIBRARY_<Lang>_FLAGS completely."
+    "\n"
+    "The default for either approach produces identical compilation flags, "
+    "but if a project modifies CMAKE_SHARED_LIBRARY_<Lang>_FLAGS from its "
+    "original value this policy determines which approach to use."
+    "\n"
+    "The OLD behavior for this policy is to ignore the "
+    "POSITION_INDEPENDENT_CODE property for all targets and use the modified "
+    "value of CMAKE_SHARED_LIBRARY_<Lang>_FLAGS for SHARED and MODULE "
+    "libraries."
+    "\n"
+    "The NEW behavior for this policy is to ignore "
+    "CMAKE_SHARED_LIBRARY_<Lang>_FLAGS whether it is modified or not and "
+    "honor the POSITION_INDEPENDENT_CODE target property.",
+    2,8,9,0, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()

Source/cmPolicies.h

@@ -65,6 +65,9 @@ public:
     /// target_link_libraries() fails if only argument is not a target
     CMP0016,
     CMP0017, ///< Prefer files in CMAKE_ROOT when including from CMAKE_ROOT
+    CMP0018, ///< Ignore language flags for shared libs, and adhere to
+    /// POSITION_INDEPENDENT_CODE property and *_COMPILE_OPTIONS_PI{E,C}
+    /// instead.
 
     /** \brief Always the last entry.
      *

Source/cmTarget.cxx

@@ -755,6 +755,14 @@ void cmTarget::DefineProperties(cmake *cm)
      "A target property that can be set to override the prefix "
      "(such as \"lib\") on a library name.");
 
+  cm->DefineProperty
+    ("POSITION_INDEPENDENT_CODE", cmProperty::TARGET,
+     "Whether to create a position-independent target",
+     "The POSITION_INDEPENDENT_CODE property determines whether position "
+     "independent executables or shared libraries will be created.  "
+     "This property is true by default for SHARED and MODULE library "
+     "targets and false otherwise.");
+
   cm->DefineProperty
     ("POST_INSTALL_SCRIPT", cmProperty::TARGET,
      "Deprecated install support.",
@@ -1305,6 +1313,13 @@ void cmTarget::SetMakefile(cmMakefile* mf)
   this->SetProperty("INCLUDE_DIRECTORIES",
                     this->Makefile->GetProperty("INCLUDE_DIRECTORIES"));
 
+  if(this->TargetTypeValue == cmTarget::SHARED_LIBRARY
+      || this->TargetTypeValue == cmTarget::MODULE_LIBRARY)
+    {
+    this->SetProperty("POSITION_INDEPENDENT_CODE", "True");
+    }
+  this->SetPropertyDefault("POSITION_INDEPENDENT_CODE", 0);
+
   // Record current policies for later use.
   this->PolicyStatusCMP0003 =
     this->Makefile->GetPolicyStatus(cmPolicies::CMP0003);

Tests/CMakeLists.txt

@@ -315,6 +315,24 @@ IF(BUILD_TESTING)
 
   ADD_TEST_MACRO(Module.GenerateExportHeader GenerateExportHeader)
 
+  if (APPLE OR CMAKE_CXX_COMPILER_ID MATCHES "GNU")
+    include(CheckCXXCompilerFlag)
+    check_cxx_compiler_flag(-fPIE run_pic_test)
+  else()
+    if (CMAKE_CXX_COMPILER_ID MATCHES "PGI"
+        OR CMAKE_CXX_COMPILER_ID MATCHES "PathScale"
+        OR CMAKE_SYSTEM_NAME MATCHES "IRIX64"
+        OR CMAKE_CXX_COMPILER_ID MATCHES "Intel")
+      set(run_pic_test 0)
+    else()
+      set(run_pic_test 1)
+    endif()
+  endif()
+
+  if (run_pic_test)
+    ADD_TEST_MACRO(PositionIndependentTargets PositionIndependentTargets)
+  endif()
+
   ADD_TEST(LinkFlags-prepare
     ${CMAKE_CTEST_COMMAND} -C \${CTEST_CONFIGURATION_TYPE}
     --build-and-test

Tests/PositionIndependentTargets/CMakeLists.txt

@@ -0,0 +1,13 @@
+
+cmake_minimum_required(VERSION 2.8)
+
+project(PositionIndependentTargets)
+
+include(CheckCXXSourceCompiles)
+
+include_directories("${CMAKE_CURRENT_SOURCE_DIR}") # For pic_test.h
+
+add_subdirectory(global)
+add_subdirectory(targets)
+
+add_executable(PositionIndependentTargets main.cpp)

Tests/PositionIndependentTargets/global/CMakeLists.txt

@@ -0,0 +1,37 @@
+
+set(CMAKE_POSITION_INDEPENDENT_CODE True)
+
+add_executable(test_target_executable_global
+  "${CMAKE_CURRENT_SOURCE_DIR}/../pic_main.cpp"
+)
+
+add_library(test_target_shared_library_global
+  SHARED "${CMAKE_CURRENT_SOURCE_DIR}/../pic_lib.cpp"
+)
+set_target_properties(test_target_shared_library_global
+  PROPERTIES DEFINE_SYMBOL PIC_TEST_BUILD_DLL
+)
+
+add_library(test_target_static_library_global
+  STATIC "${CMAKE_CURRENT_SOURCE_DIR}/../pic_lib.cpp"
+)
+set_target_properties(test_target_static_library_global
+  PROPERTIES COMPILE_DEFINITIONS PIC_TEST_STATIC_BUILD
+)
+
+
+file(READ
+  "${CMAKE_CURRENT_SOURCE_DIR}/../pic_test.h"
+  PIC_HEADER_CONTENT
+)
+
+check_cxx_source_compiles(
+  "
+${PIC_HEADER_CONTENT}
+int main(int,char**) { return 0; }\n"
+  PIC_TRY_COMPILE_RESULT
+)
+
+if (NOT PIC_TRY_COMPILE_RESULT)
+  message(SEND_ERROR "TRY_COMPILE with content requiring __PIC__ failed. ${OUTPUT}")
+endif()

Tests/PositionIndependentTargets/main.cpp

@@ -0,0 +1,2 @@
+
+int main(int,char**) { return 0; }

Tests/PositionIndependentTargets/pic_lib.cpp

@@ -0,0 +1,12 @@
+
+#include "pic_test.h"
+
+class PIC_TEST_EXPORT Dummy
+{
+  int dummy();
+};
+
+int Dummy::dummy()
+{
+  return 0;
+}

Tests/PositionIndependentTargets/pic_main.cpp

@@ -0,0 +1,4 @@
+
+#include "pic_test.h"
+
+int main(int,char**) { return 0; }

Tests/PositionIndependentTargets/pic_test.h

@@ -0,0 +1,20 @@
+
+#if defined(__ELF__)
+#  if !defined(__PIC__)
+#    error "The POSITION_INDEPENDENT_CODE property should cause __PIC__ to be defined on ELF platforms."
+#  endif
+#endif
+
+#if defined(PIC_TEST_STATIC_BUILD)
+#  define PIC_TEST_EXPORT
+#else
+#  if defined(_WIN32) || defined(WIN32) /* Win32 version */
+#    ifdef PIC_TEST_BUILD_DLL
+#      define PIC_TEST_EXPORT __declspec(dllexport)
+#    else
+#      define PIC_TEST_EXPORT __declspec(dllimport)
+#    endif
+#  else
+#    define PIC_TEST_EXPORT
+#  endif
+#endif

Tests/PositionIndependentTargets/targets/CMakeLists.txt

@@ -0,0 +1,20 @@
+
+add_executable(test_target_executable_properties "${CMAKE_CURRENT_SOURCE_DIR}/../pic_main.cpp")
+set_target_properties(test_target_executable_properties
+  PROPERTIES
+    POSITION_INDEPENDENT_CODE True
+)
+
+add_library(test_target_shared_library_properties SHARED "${CMAKE_CURRENT_SOURCE_DIR}/../pic_lib.cpp")
+set_target_properties(test_target_shared_library_properties
+  PROPERTIES
+    POSITION_INDEPENDENT_CODE True
+    DEFINE_SYMBOL PIC_TEST_BUILD_DLL
+)
+
+add_library(test_target_static_library_properties STATIC "${CMAKE_CURRENT_SOURCE_DIR}/../pic_lib.cpp")
+set_target_properties(test_target_static_library_properties
+  PROPERTIES
+    POSITION_INDEPENDENT_CODE True
+    COMPILE_DEFINITIONS PIC_TEST_STATIC_BUILD
+)