Well, I updated to cmake 2.8.12.2 but the result of changing that COMPILE_FLAGS to COMPILE_OPTIONS is that quotes are applied incorrectly: quotes are added surrounding the entire set of flags rather than around each individual item in the list. Obviously the build doesn't work (with the compiler looking for files named " -m64 ... ") but checking the relevant build command in build.ninja at least shows me that the relevant path is indeed getting added via set_target_compile_flags: the path is now part of one of these quoted strings. On Feb 12, 2014, at 11:01 AM, Brad King <brad.king at kitware.com> wrote:> On 02/12/2014 02:04 AM, Seth Cantrell wrote: >>> set(DARWIN_iossim_CFLAGS >>> -mios-simulator-version-min=7.0 -isysroot ${IOSSIM_SDK_DIR}) >> >> where IOSSIM_SDK_DIR is the path including spaces and parens. > > It looks like the _CFLAGS values are used in the module > cmake/Modules/CompilerRTUtils.cmake: > > function(set_target_compile_flags target) > foreach(arg ${ARGN}) > set(argstring "${argstring} ${arg}") > endforeach() > set_property(TARGET ${target} PROPERTY COMPILE_FLAGS "${argstring}") > endfunction() > > but that makes no attempt to escape or quote anything. > > The COMPILE_FLAGS target property was a very early CMake feature > and is treated as a raw string to put on the command line. That > is why it needs manual escaping. > > CMake 2.8.12 added the COMPILE_OPTIONS property and supporting command: > > http://www.cmake.org/cmake/help/v2.8.12/cmake.html#prop_tgt:COMPILE_OPTIONS > http://www.cmake.org/cmake/help/v2.8.12/cmake.html#command:target_compile_options > > The property holds a ;-list of options to be passed to the compiler. > CMake handles conversion to a command-line string with all needed > escaping. > > -Brad >
On 2/12/2014 11:15 PM, Seth Cantrell wrote:> Well, I updated to cmake 2.8.12.2 but the result of changing that > COMPILE_FLAGS to COMPILE_OPTIONS is that quotes are applied incorrectlyThat's because the set_target_compile_flags function in LLVM is creating the space-separated command-line string that the old COMPILE_FLAGS option wants:>> function(set_target_compile_flags target) >> foreach(arg ${ARGN}) >> set(argstring "${argstring} ${arg}") >> endforeach() >> set_property(TARGET ${target} PROPERTY COMPILE_FLAGS "${argstring}") >> endfunction()Try changing the function to just: function(set_target_compile_flags target) set_property(TARGET ${target} PROPERTY COMPILE_OPTIONS "${ARGN}") endfunction() Of course an upstream version of this change would be simpler if it just dropped set_target_compile_flags and set the property directly at the call sites. One could even use the APPEND option of set_property to add options incrementally. -Brad
Brad, How does COMPILE_FLAGS and COMPILE_OPTIONS work together? I.e. are their contents joined to produce a single compiler invocation? Also, why there is no COMPILE_OPTIONS property for source files? Looks like we really need to switch to newer CMake and make use of target_compile_options... On Thu, Feb 13, 2014 at 6:08 PM, Brad King <brad.king at kitware.com> wrote:> On 2/12/2014 11:15 PM, Seth Cantrell wrote: > > Well, I updated to cmake 2.8.12.2 but the result of changing that > > COMPILE_FLAGS to COMPILE_OPTIONS is that quotes are applied incorrectly > > That's because the set_target_compile_flags function in LLVM is > creating the space-separated command-line string that the old > COMPILE_FLAGS option wants: > > >> function(set_target_compile_flags target) > >> foreach(arg ${ARGN}) > >> set(argstring "${argstring} ${arg}") > >> endforeach() > >> set_property(TARGET ${target} PROPERTY COMPILE_FLAGS "${argstring}") > >> endfunction() > > Try changing the function to just: > > function(set_target_compile_flags target) > set_property(TARGET ${target} PROPERTY COMPILE_OPTIONS "${ARGN}") > endfunction() > > Of course an upstream version of this change would be simpler > if it just dropped set_target_compile_flags and set the property > directly at the call sites. One could even use the APPEND option > of set_property to add options incrementally. > > -Brad > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- Alexey Samsonov, MSK -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140213/5a8bdbee/attachment.html>
Okay, so after setting COMPILE_OPTIONS properly there was a similar problem with set_target_link_flags. There didn't seem to be a LINK_OPTIONS available so I just manually added quotes when concatenating link flags. Then it turned out that at one point one of the items in the ARGN list of link flags gets added as a single string but is really two items, so I hacked around that by inserting quotes into the string where it's added. With these changes building is successful.> diff --git cmake/Modules/CompilerRTUtils.cmake cmake/Modules/CompilerRTUtils.cmake > index fce37e3..a36096a 100644 > --- cmake/Modules/CompilerRTUtils.cmake > +++ cmake/Modules/CompilerRTUtils.cmake > @@ -2,15 +2,12 @@ > # define a handy helper function for it. The compile flags setting in CMake > # has serious issues that make its syntax challenging at best. > function(set_target_compile_flags target) > - foreach(arg ${ARGN}) > - set(argstring "${argstring} ${arg}") > - endforeach() > - set_property(TARGET ${target} PROPERTY COMPILE_FLAGS "${argstring}") > + set_property(TARGET ${target} PROPERTY COMPILE_OPTIONS "${ARGN}") > endfunction() > > function(set_target_link_flags target) > foreach(arg ${ARGN}) > - set(argstring "${argstring} ${arg}") > + set(argstring "${argstring} \"${arg}\"") > endforeach() > set_property(TARGET ${target} PROPERTY LINK_FLAGS "${argstring}") > endfunction() > diff --git lib/asan/CMakeLists.txt lib/asan/CMakeLists.txt > index 64239fe..2c8d385 100644 > --- lib/asan/CMakeLists.txt > +++ lib/asan/CMakeLists.txt > @@ -75,7 +75,7 @@ if(APPLE) > # Dynamic lookup is needed because shadow scale and offset are > # provided by the instrumented modules. > set(ASAN_RUNTIME_LDFLAGS > - "-undefined dynamic_lookup") > + "-undefined\" \"dynamic_lookup") > add_compiler_rt_darwin_dynamic_runtime(clang_rt.asan_${os}_dynamic ${os} > ARCH ${ASAN_SUPPORTED_ARCH} > SOURCES $<TARGET_OBJECTS:RTAsan.${os}>Thanks, Seth On Feb 13, 2014, at 9:08 AM, Brad King <brad.king at kitware.com> wrote:> On 2/12/2014 11:15 PM, Seth Cantrell wrote: >> Well, I updated to cmake 2.8.12.2 but the result of changing that >> COMPILE_FLAGS to COMPILE_OPTIONS is that quotes are applied incorrectly > > That's because the set_target_compile_flags function in LLVM is > creating the space-separated command-line string that the old > COMPILE_FLAGS option wants: > >>> function(set_target_compile_flags target) >>> foreach(arg ${ARGN}) >>> set(argstring "${argstring} ${arg}") >>> endforeach() >>> set_property(TARGET ${target} PROPERTY COMPILE_FLAGS "${argstring}") >>> endfunction() > > Try changing the function to just: > > function(set_target_compile_flags target) > set_property(TARGET ${target} PROPERTY COMPILE_OPTIONS "${ARGN}") > endfunction() > > Of course an upstream version of this change would be simpler > if it just dropped set_target_compile_flags and set the property > directly at the call sites. One could even use the APPEND option > of set_property to add options incrementally. > > -Brad