From 61ce6547424821710d0fbebf06b0eb411a630253 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 11 May 2014 16:04:53 +0200 Subject: Genex: Fix stack overflow in transitive property evaluation. Commit v2.8.11~156^2~2 (Expand includes and defines transitively in 'external' genexes., 2013-02-13) introduced a recursive loop and a stack overflow during evaluation of a link implementation which depends on a transitive property, such as add_library(empty1 ...) add_library(empty2 ...) target_link_libraries(empty1 PRIVATE $<$,/foo/bar>:empty2> ) There is no use-case for code like that currently, but it should not cause a stack overflow. Avoid the recursion by reporting an error early if a case like this is found. --- Source/cmGeneratorExpressionEvaluator.cxx | 13 +++++++++++++ .../LinkImplementationCycle1-result.txt | 1 + .../LinkImplementationCycle1-stderr.txt | 10 ++++++++++ .../LinkImplementationCycle1.cmake | 8 ++++++++ .../LinkImplementationCycle2-result.txt | 1 + .../LinkImplementationCycle2-stderr.txt | 10 ++++++++++ .../LinkImplementationCycle2.cmake | 8 ++++++++ .../LinkImplementationCycle3-result.txt | 1 + .../LinkImplementationCycle3-stderr.txt | 1 + .../LinkImplementationCycle3.cmake | 10 ++++++++++ .../LinkImplementationCycle4-result.txt | 1 + .../LinkImplementationCycle4-stderr.txt | 8 ++++++++ .../LinkImplementationCycle4.cmake | 14 ++++++++++++++ .../LinkImplementationCycle5-result.txt | 1 + .../LinkImplementationCycle5-stderr.txt | 8 ++++++++ .../LinkImplementationCycle5.cmake | 10 ++++++++++ .../LinkImplementationCycle6-result.txt | 1 + .../LinkImplementationCycle6-stderr.txt | 8 ++++++++ .../LinkImplementationCycle6.cmake | 14 ++++++++++++++ .../TargetPropertyGeneratorExpressions/RunCMakeTest.cmake | 6 ++++++ .../RunCMake/TargetPropertyGeneratorExpressions/empty.cpp | 7 +++++++ 21 files changed, 141 insertions(+) create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1-result.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1-stderr.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1.cmake create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2-result.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2-stderr.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2.cmake create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3-result.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3-stderr.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3.cmake create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4-result.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4-stderr.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4.cmake create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5-result.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5-stderr.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5.cmake create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6-result.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6-stderr.txt create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6.cmake create mode 100644 Tests/RunCMake/TargetPropertyGeneratorExpressions/empty.cpp diff --git a/Source/cmGeneratorExpressionEvaluator.cxx b/Source/cmGeneratorExpressionEvaluator.cxx index c925869..b648eb2 100644 --- a/Source/cmGeneratorExpressionEvaluator.cxx +++ b/Source/cmGeneratorExpressionEvaluator.cxx @@ -1028,6 +1028,19 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode { if (dagCheckerParent->EvaluatingLinkLibraries()) { +#define TRANSITIVE_PROPERTY_COMPARE(PROPERTY) \ + (#PROPERTY == propertyName || "INTERFACE_" #PROPERTY == propertyName) || + if (CM_FOR_EACH_TRANSITIVE_PROPERTY_NAME(TRANSITIVE_PROPERTY_COMPARE) + false) + { + reportError(context, content->GetOriginalExpression(), + "$ expression in link libraries " + "evaluation depends on target property which is transitive " + "over the link libraries, creating a recursion."); + return std::string(); + } +#undef TRANSITIVE_PROPERTY_COMPARE + if(!prop) { return std::string(); diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1-result.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1-stderr.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1-stderr.txt new file mode 100644 index 0000000..7e002f5 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1-stderr.txt @@ -0,0 +1,10 @@ +CMake Error at LinkImplementationCycle1.cmake:5 \(target_link_libraries\): + Error evaluating generator expression: + + \$ + + \$ expression in link libraries evaluation depends on + target property which is transitive over the link libraries, creating a + recursion. +Call Stack \(most recent call first\): + CMakeLists.txt:8 \(include\) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1.cmake b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1.cmake new file mode 100644 index 0000000..4b60214 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle1.cmake @@ -0,0 +1,8 @@ + +add_library(empty1 empty.cpp) +add_library(empty2 empty.cpp) + +target_link_libraries(empty1 + LINK_PUBLIC + $<$,/foo/bar>:empty2> +) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2-result.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2-stderr.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2-stderr.txt new file mode 100644 index 0000000..2f72de6 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2-stderr.txt @@ -0,0 +1,10 @@ +CMake Error at LinkImplementationCycle2.cmake:5 \(target_link_libraries\): + Error evaluating generator expression: + + \$ + + \$ expression in link libraries evaluation depends on + target property which is transitive over the link libraries, creating a + recursion. +Call Stack \(most recent call first\): + CMakeLists.txt:8 \(include\) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2.cmake b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2.cmake new file mode 100644 index 0000000..557eac1 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle2.cmake @@ -0,0 +1,8 @@ + +add_library(empty1 empty.cpp) +add_library(empty2 empty.cpp) + +target_link_libraries(empty1 + LINK_PUBLIC + $<$,/foo/bar>:empty2> +) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3-result.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3-result.txt new file mode 100644 index 0000000..573541a --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3-result.txt @@ -0,0 +1 @@ +0 diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3-stderr.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3-stderr.txt new file mode 100644 index 0000000..10f3293 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3-stderr.txt @@ -0,0 +1 @@ +^$ diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3.cmake b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3.cmake new file mode 100644 index 0000000..0f921d4 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle3.cmake @@ -0,0 +1,10 @@ + +add_library(empty1 empty.cpp) +add_library(empty2 empty.cpp) + +# This is OK, because evaluating the INCLUDE_DIRECTORIES is not affected by +# the content of the INTERFACE_LINK_LIBRARIES. +target_link_libraries(empty1 + INTERFACE + $<$,/foo/bar>:empty2> +) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4-result.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4-stderr.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4-stderr.txt new file mode 100644 index 0000000..5cfeb0a --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4-stderr.txt @@ -0,0 +1,8 @@ +CMake Error: + Error evaluating generator expression: + + \$ + + \$ expression in link libraries evaluation depends on + target property which is transitive over the link libraries, creating a + recursion. diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4.cmake b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4.cmake new file mode 100644 index 0000000..ab6d0b2 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle4.cmake @@ -0,0 +1,14 @@ + +add_library(empty1 empty.cpp) +add_library(empty2 empty.cpp) + +# The INTERFACE_INCLUDE_DIRECTORIES do not depend on the link interface. +# On its own, this is fine. It is only when used by empty3 that an error +# is appropriately issued. +target_link_libraries(empty1 + INTERFACE + $<$,/foo/bar>:empty2> +) + +add_library(empty3 empty.cpp) +target_link_libraries(empty3 empty1) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5-result.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5-stderr.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5-stderr.txt new file mode 100644 index 0000000..5cfeb0a --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5-stderr.txt @@ -0,0 +1,8 @@ +CMake Error: + Error evaluating generator expression: + + \$ + + \$ expression in link libraries evaluation depends on + target property which is transitive over the link libraries, creating a + recursion. diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5.cmake b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5.cmake new file mode 100644 index 0000000..dc180e3 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle5.cmake @@ -0,0 +1,10 @@ + +add_library(empty1 INTERFACE IMPORTED) +add_library(empty2 INTERFACE IMPORTED) + +set_property(TARGET empty1 PROPERTY INTERFACE_LINK_LIBRARIES + $<$,/foo/bar>:empty2> +) + +add_library(empty3 empty.cpp) +target_link_libraries(empty3 empty1) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6-result.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6-result.txt new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6-result.txt @@ -0,0 +1 @@ +1 diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6-stderr.txt b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6-stderr.txt new file mode 100644 index 0000000..5cfeb0a --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6-stderr.txt @@ -0,0 +1,8 @@ +CMake Error: + Error evaluating generator expression: + + \$ + + \$ expression in link libraries evaluation depends on + target property which is transitive over the link libraries, creating a + recursion. diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6.cmake b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6.cmake new file mode 100644 index 0000000..91252d0 --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/LinkImplementationCycle6.cmake @@ -0,0 +1,14 @@ + +add_library(empty1 SHARED empty.cpp) +add_library(empty2 SHARED empty.cpp) + +# The INTERFACE_INCLUDE_DIRECTORIES do not depend on the link interface. +# On its own, this is fine. It is only when used by empty3 that an error +# is appropriately issued. +target_link_libraries(empty1 + INTERFACE + $<$,/foo/bar>:empty2> +) + +add_library(empty3 SHARED empty.cpp) +target_link_libraries(empty3 empty1) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/RunCMakeTest.cmake b/Tests/RunCMake/TargetPropertyGeneratorExpressions/RunCMakeTest.cmake index 0ee3238..645a57d 100644 --- a/Tests/RunCMake/TargetPropertyGeneratorExpressions/RunCMakeTest.cmake +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/RunCMakeTest.cmake @@ -15,3 +15,9 @@ run_cmake(BadInvalidName5) run_cmake(BadInvalidName6) run_cmake(BadInvalidName7) run_cmake(BadInvalidName8) +run_cmake(LinkImplementationCycle1) +run_cmake(LinkImplementationCycle2) +run_cmake(LinkImplementationCycle3) +run_cmake(LinkImplementationCycle4) +run_cmake(LinkImplementationCycle5) +run_cmake(LinkImplementationCycle6) diff --git a/Tests/RunCMake/TargetPropertyGeneratorExpressions/empty.cpp b/Tests/RunCMake/TargetPropertyGeneratorExpressions/empty.cpp new file mode 100644 index 0000000..bfbbdde --- /dev/null +++ b/Tests/RunCMake/TargetPropertyGeneratorExpressions/empty.cpp @@ -0,0 +1,7 @@ +#ifdef _WIN32 +__declspec(dllexport) +#endif +int empty() +{ + return 0; +} -- cgit v0.12 From 65aa5442b72aa8dda61088bdc0ffa2aa45965646 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 11 May 2014 14:28:24 +0200 Subject: Target: Return null when a transitive property is not defined. Commit v2.8.11~310^2~1 (Keep track of INCLUDE_DIRECTORIES as a vector of structs., 2012-11-19) added special case of INCLUDE_DIRECTORIES for the purpose of origin-tracking of individual entries in the property. It introduced a bug in that it returned an empty string instead of '0' in the case that no includes have been set. Commit v2.8.11~289^2~2 (Handle INTERFACE properties transitively for includes and defines., 2012-09-23) introduced transitive handling of the property through the link implementation, together with a whitelist of properties which would be evaluated transitively. Because of the bug introduced previously, the 'prop' in TargetPropertyNode is non-null, meaning that the content (the empty string) would be evaluated as a generator expression. This was harmless as the follow-up code was only for 'INTERFACE_' variants of target properties, so the effect was the same. Commits v2.8.11~280^2~2 (Keep track of properties used to determine linker libraries., 2012-11-05) and v2.8.11~280^2~1 (Add API to calculate link-interface-dependent bool properties or error., 2013-01-06) added a way to track and report errors on properties which both determine and are determined by the link implementation. This was later used in generator expression evaluation by commit v2.8.11~252^2~2 (Make INTERFACE determined properties readable in generator expressions., 2013-01-19). If a property is unset (null), and the link implementation of the target was not being evaluated, this commit made it possible to evaluate the property from the link implementation instead. If the link implementation was being evaluated, an empty string was returned from the generator expression evaluation, which might be later reported as an error. The above logic was written for 'compatible interface' properties, but in fact it should have also included other properties. Because of the empty-string-instead-of-null bug, this code block is not entered for the INCLUDE_DIRECTORIES property. At this point, however, the bug still does not significantly affect behavior, because the follow-up code is still a no-op for the INCLUDE_DIRECTORIES property, and an empty string is returned regardless. Commit v2.8.11~189^2~6 (Use the link information as a source of compile definitions and includes., 2013-02-12) refactored the logic, but also without a change in behavior. Commit v2.8.11~156^2~2 (Expand includes and defines transitively in 'external' genexes., 2013-02-13) refactored the logic again, this time with a change of behavior. The INCLUDE_DIRECTORIES property was then mapped to INTERFACE_INCLUDE_DIRECTORIES during transitive generator expression evaluation. Because the transitive evaluation involved evaluation of the link implementation, this introduced a recursive loop and a segfault with code like: add_library(empty1 ...) add_library(empty2 ...) target_link_libraries(empty1 PRIVATE $<$,/foo/bar>:empty2> ) As there is no real use-case for reading a target property like that while evaluating the link implementation, this went unnoticed. The same pattern was followed for other special-cased reads of transitive target properties such as COMPILE_DEFINITIONS. The segfault was fixed in the parent commit, but change the property to return null when appropriate for other future uses. --- Source/cmTarget.cxx | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index aa0ed56..d27293a 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -3265,6 +3265,11 @@ const char *cmTarget::GetProperty(const std::string& prop, } if(prop == "INCLUDE_DIRECTORIES") { + if (this->Internal->IncludeDirectoriesEntries.empty()) + { + return 0; + } + static std::string output; output = ""; std::string sep; @@ -3283,6 +3288,11 @@ const char *cmTarget::GetProperty(const std::string& prop, } if(prop == "COMPILE_OPTIONS") { + if (this->Internal->CompileOptionsEntries.empty()) + { + return 0; + } + static std::string output; output = ""; std::string sep; @@ -3301,6 +3311,11 @@ const char *cmTarget::GetProperty(const std::string& prop, } if(prop == "COMPILE_FEATURES") { + if (this->Internal->CompileFeaturesEntries.empty()) + { + return 0; + } + static std::string output; output = ""; std::string sep; @@ -3319,6 +3334,11 @@ const char *cmTarget::GetProperty(const std::string& prop, } if(prop == "COMPILE_DEFINITIONS") { + if (this->Internal->CompileDefinitionsEntries.empty()) + { + return 0; + } + static std::string output; output = ""; std::string sep; @@ -3337,6 +3357,11 @@ const char *cmTarget::GetProperty(const std::string& prop, } if(prop == "LINK_LIBRARIES") { + if (this->Internal->LinkImplementationPropertyEntries.empty()) + { + return 0; + } + static std::string output; output = ""; std::string sep; @@ -3359,6 +3384,11 @@ const char *cmTarget::GetProperty(const std::string& prop, if(prop == "SOURCES") { + if (this->Internal->SourceEntries.empty()) + { + return 0; + } + cmOStringStream ss; const char* sep = ""; typedef cmTargetInternals::TargetPropertyEntry -- cgit v0.12