From 3c37d2642d9000a2d01bc46ad0ea74a741bdb658 Mon Sep 17 00:00:00 2001 From: Robert Goulet Date: Fri, 14 Aug 2015 20:35:58 +0000 Subject: [PATCH] cmGeneratorTarget: Avoid recursion in GetOutputName method Since support for generator expressions was added to OUTPUT_NAME it is possible for project code to cause recursion in this method by using a $ genex. Detect and reject such cases. --- Source/cmGeneratorTarget.cxx | 93 ++++++++++++------- Source/cmGeneratorTarget.h | 4 + .../OUTPUT_NAME-recursion-result.txt | 1 + .../OUTPUT_NAME-recursion-stderr.txt | 4 + .../OUTPUT_NAME-recursion.cmake | 3 + .../GeneratorExpression/RunCMakeTest.cmake | 1 + .../TARGET_FILE-recursion.cmake | 1 + 7 files changed, 72 insertions(+), 35 deletions(-) create mode 100644 Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-result.txt create mode 100644 Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-stderr.txt create mode 100644 Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion.cmake diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 299c11264..530acfe83 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -270,48 +270,71 @@ const char *cmGeneratorTarget::GetProperty(const std::string& prop) const std::string cmGeneratorTarget::GetOutputName(const std::string& config, bool implib) const { - std::vector props; - std::string type = this->Target->GetOutputTargetType(implib); - std::string configUpper = cmSystemTools::UpperCase(config); - if(!type.empty() && !configUpper.empty()) + // Lookup/compute/cache the output name for this configuration. + OutputNameKey key(config, implib); + cmGeneratorTarget::OutputNameMapType::iterator i = + this->OutputNameMap.find(key); + if(i == this->OutputNameMap.end()) { - // _OUTPUT_NAME_ - props.push_back(type + "_OUTPUT_NAME_" + configUpper); - } - if(!type.empty()) - { - // _OUTPUT_NAME - props.push_back(type + "_OUTPUT_NAME"); - } - if(!configUpper.empty()) - { - // OUTPUT_NAME_ - props.push_back("OUTPUT_NAME_" + configUpper); - // _OUTPUT_NAME - props.push_back(configUpper + "_OUTPUT_NAME"); - } - // OUTPUT_NAME - props.push_back("OUTPUT_NAME"); + // Add empty name in map to detect potential recursion. + OutputNameMapType::value_type entry(key, ""); + i = this->OutputNameMap.insert(entry).first; - std::string outName; - for(std::vector::const_iterator i = props.begin(); - i != props.end(); ++i) - { - if (const char* outNameProp = this->Target->GetProperty(*i)) + // Compute output name. + std::vector props; + std::string type = this->Target->GetOutputTargetType(implib); + std::string configUpper = cmSystemTools::UpperCase(config); + if(!type.empty() && !configUpper.empty()) { - outName = outNameProp; - break; + // _OUTPUT_NAME_ + props.push_back(type + "_OUTPUT_NAME_" + configUpper); } - } + if(!type.empty()) + { + // _OUTPUT_NAME + props.push_back(type + "_OUTPUT_NAME"); + } + if(!configUpper.empty()) + { + // OUTPUT_NAME_ + props.push_back("OUTPUT_NAME_" + configUpper); + // _OUTPUT_NAME + props.push_back(configUpper + "_OUTPUT_NAME"); + } + // OUTPUT_NAME + props.push_back("OUTPUT_NAME"); - if (outName.empty()) + std::string outName; + for(std::vector::const_iterator it = props.begin(); + it != props.end(); ++it) + { + if (const char* outNameProp = this->Target->GetProperty(*it)) + { + outName = outNameProp; + break; + } + } + + if(outName.empty()) + { + outName = this->GetName(); + } + + // Now evaluate genex and update the previously-prepared map entry. + cmGeneratorExpression ge; + cmsys::auto_ptr cge = ge.Parse(outName); + i->second = cge->Evaluate(this->Makefile, config); + } + else if(i->second.empty()) { - outName = this->GetName(); + // An empty map entry indicates we have been called recursively + // from the above block. + this->Makefile->GetCMakeInstance()->IssueMessage( + cmake::FATAL_ERROR, + "Target '" + this->GetName() + "' OUTPUT_NAME depends on itself.", + this->Target->GetBacktrace()); } - - cmGeneratorExpression ge; - cmsys::auto_ptr cge = ge.Parse(outName); - return cge->Evaluate(this->Makefile, config); + return i->second; } //---------------------------------------------------------------------------- diff --git a/Source/cmGeneratorTarget.h b/Source/cmGeneratorTarget.h index 68e7a8ab8..15b333566 100644 --- a/Source/cmGeneratorTarget.h +++ b/Source/cmGeneratorTarget.h @@ -375,6 +375,10 @@ private: }; mutable std::map LinkImplClosureMap; + typedef std::pair OutputNameKey; + typedef std::map OutputNameMapType; + mutable OutputNameMapType OutputNameMap; + public: std::vector const& GetLinkImplementationClosure(const std::string& config) const; diff --git a/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-result.txt b/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-result.txt new file mode 100644 index 000000000..d00491fd7 --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-stderr.txt b/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-stderr.txt new file mode 100644 index 000000000..bf592e75c --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion-stderr.txt @@ -0,0 +1,4 @@ +CMake Error at OUTPUT_NAME-recursion.cmake:[0-9]+ \(add_executable\): + Target 'empty1' OUTPUT_NAME depends on itself. +Call Stack \(most recent call first\): + CMakeLists.txt:3 \(include\) diff --git a/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion.cmake b/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion.cmake new file mode 100644 index 000000000..5cb80509e --- /dev/null +++ b/Tests/RunCMake/GeneratorExpression/OUTPUT_NAME-recursion.cmake @@ -0,0 +1,3 @@ +enable_language(C) +add_executable(empty1 empty.c) +set_property(TARGET empty1 PROPERTY OUTPUT_NAME $) diff --git a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake index 21fc8518e..06790243b 100644 --- a/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake +++ b/Tests/RunCMake/GeneratorExpression/RunCMakeTest.cmake @@ -26,6 +26,7 @@ run_cmake(COMPILE_LANGUAGE-add_library) run_cmake(COMPILE_LANGUAGE-add_test) run_cmake(COMPILE_LANGUAGE-unknown-lang) run_cmake(TARGET_FILE-recursion) +run_cmake(OUTPUT_NAME-recursion) run_cmake(ImportedTarget-TARGET_PDB_FILE) if(LINKER_SUPPORTS_PDB) diff --git a/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake b/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake index 7633be1b3..e780103d9 100644 --- a/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake +++ b/Tests/RunCMake/GeneratorExpression/TARGET_FILE-recursion.cmake @@ -1,3 +1,4 @@ enable_language(C) add_executable(empty1 empty.c) +set_property(TARGET empty1 PROPERTY OUTPUT_NAME $) set_property(TARGET empty1 PROPERTY RUNTIME_OUTPUT_DIRECTORY $)