From 65aa5442b72aa8dda61088bdc0ffa2aa45965646 Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 11 May 2014 14:28:24 +0200 Subject: [PATCH] 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 aa0ed56ad..d27293a4d 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