On Thu, Apr 3, 2014 at 9:23 PM, Greg Fitzgerald <garious at gmail.com> wrote:> > I don't think it's a good idea to let user hijack > > the driver and stuff in custom version of > > ASan runtime instead the one installed/built > > with compiler :) > > I'm okay with it. This is open source. If someone wants to put the > sanitizers on a shorter release cycle than the full toolchain, who are we > to hold them back? >They can always rebuild the fresh ASan from source and replace the ASan runtime in toolchain with the new one.> > > What do you mean by "invoking the test-suite directly"? > > In Clang's CMakeLists, add_subdirectory() of compiler-rt's 'test' > directory. I suggested we do this earlier, because of clang's hardcoded > dependency on the path to ASan. Instead, I think a better solution is to > break the hardcoded dependency, allowing the test suite to point clang to > compiler-rt's local copy of ASan using a '-L' parameter. >I don't believe it's needed anywhere except in the testing infrastructure you suggest. And, again, we have to ensure that -L/path/to/new/asan provided by user "wins" over -L/path/to/clang/resource/dir, and we have strong reasons to put the latter as early in the link line as possible. That said, ....> > Greg > > > On Thursday, April 3, 2014, Alexey Samsonov <samsonov at google.com> wrote: > >> >> On Thu, Apr 3, 2014 at 5:04 AM, Greg Fitzgerald <garious at gmail.com>wrote: >> >>> > we would still want to use compiler-rt test-suite in a standalone >>> mode, to test fully built/installed toolchains, >>> and even GCC. >>> >>> Sounds good. >>> >>> >>> > Clang driver links the static xsan runtimes from a hardcoded >>> > paths in Clang resource directory, and doesn't add flags like >>> > "-lasan -L/path/to/clang/resource/dir". I find this behavior >>> reasonable. >>> >>> Can we change the flags to "-lasan -L/path/to/clang/resource/dir"? If >>> we make that change, >> >> >> I don't think it's a good idea to let user hijack the driver and stuff in >> custom version of >> ASan runtime instead the one installed/built with compiler :) >> >> >>> I can stop suggesting that the clang build should >>> invoke the compiler-rt test suite directly. :-) >>> >> >> I haven't yet fully understood your complaints. >> >> Note that if you build/configure compiler-rt >> in a standalone mode, then "make check-compiler-rt" in that tree wouldn't >> rebuild runtimes, >> and instead assume that they are provided by toolchain (that is, in >> standalone mode runtimes and >> test suites are effectively independent). >> >^^^^ What is wrong with that approach?> What do you mean by "invoking the test-suite directly"? >> >> >> >> >> > Do you want to give it a try? >> >> I'll take a crack it, sure. >> >> >> By the way, locally, I now have just over half the ASan test suite >> passing ARM-Linux via QEMU. It's been really convenient to build >> compiler-rt in standalone mode. The edit-compile-run cycle is much >> shorter this way. Thanks for your help in making this work! >> >> -Greg >> >> >> >> On Wed, Apr 2, 2014 at 7:56 AM, Alexey Samsonov <samsonov at google.com> >> wrote: >> > Hi Greg, >> > >> > On Wed, Apr 2, 2014 at 2:50 AM, Greg Fitzgerald <garious at gmail.com> >> wrote: >> >> >> >> Alexey, Evgeniy, >> >> >> >> I propose the following steps to unify multi-arch support in >> compiler-rt: >> >> >> >> 1) The compiler-rt test suite adds "-L${CMAKE_CURRENT_BINARY_DIR}/lib" >> >> to its 'clang' variables. This way we can test the sanitizers without >> >> installing any libs to the just-built-clang install directory. >> > >> > >> > This is possible, but please keep in mind that: >> > 1) we would still want to use compiler-rt test-suite in a standalone >> mode, >> > to test fully built/installed toolchains, >> > and even GCC. >> > 2) Adding -L... wouldn't work: Clang driver links the static xsan >> runtimes >> > from a hardcoded paths in Clang resource directory, >> > and doesn't add flags like "-lasan -L/path/to/clang/resource/dir". I >> find >> > this behavior reasonable. >> > >> > I don't see a problem with the current approach - we can make "run >> sanitizer >> > test suite" command in the >> > top-level build tree depend on "build/install compiler-rt >> ExternalProject" >> > steps (see how it's done currently). >> > >> >> >> >> >> >> 2) The "clang/runtime" build calls ExternalProject once for each arch >> >> it needs of compiler-rt. So once to create x86_64 libs and once for >> >> i386 libs. >> >> >> >> 3) The compiler-rt build drops the ${arch} suffix from its libs, and >> >> the "clang/runtime" build uses CMAKE_INSTALL_PREFIX to control where >> >> the compiler-rt libs go. >> >> >> >> >> "CMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR}/lib/clang/${CLANG_VERSION}/x86_64-linux" >> >> >> >> >> >> 4) Remove multi-arch support from the compiler-rt build. Instead, >> >> declare compiler-rt as an "any-arch" build, configured with: >> >> CMAKE_CXX_COMPILER (defaults to just-built-clang) >> >> CMAKE_CXX_FLAGS (defaults to llvm's default target) >> >> COMPILER_RT_TEST_COMPILER (defaults to CMAKE_CXX_COMPILER) >> >> COMPILER_RT_TEST_FLAGS (defaults to CMAKE_CXX_FLAGS) >> > >> > >> > Unfortunately, this could be problematic for Mac - we use "universal >> > binaries" there - a single static/dynamic runtime >> > which contains runtimes for all the possible architectures. We might >> work >> > around this, however, by adding a custom >> > Mac-specific step that would merge all single-arch binaries into a >> > multi-arch one - I think this is fine as long as we >> > greatly simplify the rest of the ugly build system. >> > >> > Do you want to give it a try? >> > >> >> >> >> >> >> Thoughts? >> >> >> >> Thanks, >> >> Greg >> >> >> >> On Tue, Apr 1, 2014 at 12:58 AM, Evgeniy Stepanov >> >> <eugeni.stepanov at gmail.com> wrote: >> >> > It does sound like Android is better suited for "honest" >> >> > cross-compilation, rather than "build compiler-rt for all targets we >> >> > can find" model. >> >> > >> >> > I'm still not convinced that we must require the "ninja install" >> step. >> >> > Could we just "ninja clang" and then build the second stage against >> >> > the first stage build directory? Will this "find_package" thing not >> >> > work that way, or do you mean it as a way to copy target asan runtime >> >> > where the first stage compiler will find it (or probably both)? >> >> > >> >> > On Mon, Mar 31, 2014 at 6:46 PM, Alexey Samsonov < >> samsonov at google.com> >> >> > wrote: >> > >> >> >> >> >> -- >> Alexey Samsonov, MSK >> >-- Alexey Samsonov, MSK -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140404/09bd2f9f/attachment.html>
>>> in a standalone mode, then "make check-compiler-rt" in that tree >>> wouldn't rebuild runtimes, and instead assume that they are provided by toolchain > What is wrong with that approach?Not wrong, just inconsistent. I'm trying to test the just-built-runtime before running the 'install' step. That's the way all the other projects work, "all check-all install" in that order. In other words, "build, verify, commit".> And, again, we have to ensure that -L/path/to/new/asan provided by > user "wins" over -L/path/to/clang/resource/dir, and we have strong > reasons to put the latter as early in the link line as possible.Some good news, the drivers (both gcc and clang) allow us to put the '-L' parameters after the '-l' parameters. So we can put -lasan at the start of the parameter list to get the desired link order and -L at the end so that the user-specified runtime is always chosen. -Greg On Fri, Apr 4, 2014 at 5:57 AM, Alexey Samsonov <samsonov at google.com> wrote:> > On Thu, Apr 3, 2014 at 9:23 PM, Greg Fitzgerald <garious at gmail.com> wrote: >> >> > I don't think it's a good idea to let user hijack >> > the driver and stuff in custom version of >> > ASan runtime instead the one installed/built >> > with compiler :) >> >> I'm okay with it. This is open source. If someone wants to put the >> sanitizers on a shorter release cycle than the full toolchain, who are we to >> hold them back? > > > They can always rebuild the fresh ASan from source and replace the ASan > runtime in toolchain with the new one. > >> >> >> > What do you mean by "invoking the test-suite directly"? >> >> In Clang's CMakeLists, add_subdirectory() of compiler-rt's 'test' >> directory. I suggested we do this earlier, because of clang's hardcoded >> dependency on the path to ASan. Instead, I think a better solution is to >> break the hardcoded dependency, allowing the test suite to point clang to >> compiler-rt's local copy of ASan using a '-L' parameter. > > > I don't believe it's needed anywhere except in the testing infrastructure > you suggest. And, again, we have to ensure that -L/path/to/new/asan provided > by user "wins" over -L/path/to/clang/resource/dir, and we have strong > reasons to put the latter as early in the link line as possible. That said, > .... > >> >> >> Greg >> >> >> On Thursday, April 3, 2014, Alexey Samsonov <samsonov at google.com> wrote: >>> >>> >>> On Thu, Apr 3, 2014 at 5:04 AM, Greg Fitzgerald <garious at gmail.com> >>> wrote: >>>> >>>> > we would still want to use compiler-rt test-suite in a standalone >>>> > mode, to test fully built/installed toolchains, >>>> and even GCC. >>>> >>>> Sounds good. >>>> >>>> >>>> > Clang driver links the static xsan runtimes from a hardcoded >>>> > paths in Clang resource directory, and doesn't add flags like >>>> > "-lasan -L/path/to/clang/resource/dir". I find this behavior >>>> > reasonable. >>>> >>>> Can we change the flags to "-lasan -L/path/to/clang/resource/dir"? If >>>> we make that change, >>> >>> >>> I don't think it's a good idea to let user hijack the driver and stuff in >>> custom version of >>> ASan runtime instead the one installed/built with compiler :) >>> >>>> >>>> I can stop suggesting that the clang build should >>>> invoke the compiler-rt test suite directly. :-) >>> >>> >>> I haven't yet fully understood your complaints. > > > >>> >>> Note that if you build/configure compiler-rt >>> in a standalone mode, then "make check-compiler-rt" in that tree wouldn't >>> rebuild runtimes, >>> and instead assume that they are provided by toolchain (that is, in >>> standalone mode runtimes and >>> test suites are effectively independent). > > > ^^^^ > What is wrong with that approach? > > >>> >>> What do you mean by "invoking the test-suite directly"? >>> >>> >>> >>> >>> > Do you want to give it a try? >>> >>> I'll take a crack it, sure. >>> >>> >>> By the way, locally, I now have just over half the ASan test suite >>> passing ARM-Linux via QEMU. It's been really convenient to build >>> compiler-rt in standalone mode. The edit-compile-run cycle is much >>> shorter this way. Thanks for your help in making this work! >>> >>> -Greg >>> >>> >>> >>> On Wed, Apr 2, 2014 at 7:56 AM, Alexey Samsonov <samsonov at google.com> >>> wrote: >>> > Hi Greg, >>> > >>> > On Wed, Apr 2, 2014 at 2:50 AM, Greg Fitzgerald <garious at gmail.com> >>> > wrote: >>> >> >>> >> Alexey, Evgeniy, >>> >> >>> >> I propose the following steps to unify multi-arch support in >>> >> compiler-rt: >>> >> >>> >> 1) The compiler-rt test suite adds "-L${CMAKE_CURRENT_BINARY_DIR}/lib" >>> >> to its 'clang' variables. This way we can test the sanitizers without >>> >> installing any libs to the just-built-clang install directory. >>> > >>> > >>> > This is possible, but please keep in mind that: >>> > 1) we would still want to use compiler-rt test-suite in a standalone >>> > mode, >>> > to test fully built/installed toolchains, >>> > and even GCC. >>> > 2) Adding -L... wouldn't work: Clang driver links the static xsan >>> > runtimes >>> > from a hardcoded paths in Clang resource directory, >>> > and doesn't add flags like "-lasan -L/path/to/clang/resource/dir". I >>> > find >>> > this behavior reasonable. >>> > >>> > I don't see a problem with the current approach - we can make "run >>> > sanitizer >>> > test suite" command in the >>> > top-level build tree depend on "build/install compiler-rt >>> > ExternalProject" >>> > steps (see how it's done currently). >>> > >>> >> >>> >> >>> >> 2) The "clang/runtime" build calls ExternalProject once for each arch >>> >> it needs of compiler-rt. So once to create x86_64 libs and once for >>> >> i386 libs. >>> >> >>> >> 3) The compiler-rt build drops the ${arch} suffix from its libs, and >>> >> the "clang/runtime" build uses CMAKE_INSTALL_PREFIX to control where >>> >> the compiler-rt libs go. >>> >> >>> >> >>> >> "CMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR}/lib/clang/${CLANG_VERSION}/x86_64-linux" >>> >> >>> >> >>> >> 4) Remove multi-arch support from the compiler-rt build. Instead, >>> >> declare compiler-rt as an "any-arch" build, configured with: >>> >> CMAKE_CXX_COMPILER (defaults to just-built-clang) >>> >> CMAKE_CXX_FLAGS (defaults to llvm's default target) >>> >> COMPILER_RT_TEST_COMPILER (defaults to CMAKE_CXX_COMPILER) >>> >> COMPILER_RT_TEST_FLAGS (defaults to CMAKE_CXX_FLAGS) >>> > >>> > >>> > Unfortunately, this could be problematic for Mac - we use "universal >>> > binaries" there - a single static/dynamic runtime >>> > which contains runtimes for all the possible architectures. We might >>> > work >>> > around this, however, by adding a custom >>> > Mac-specific step that would merge all single-arch binaries into a >>> > multi-arch one - I think this is fine as long as we >>> > greatly simplify the rest of the ugly build system. >>> > >>> > Do you want to give it a try? >>> > >>> >> >>> >> >>> >> Thoughts? >>> >> >>> >> Thanks, >>> >> Greg >>> >> >>> >> On Tue, Apr 1, 2014 at 12:58 AM, Evgeniy Stepanov >>> >> <eugeni.stepanov at gmail.com> wrote: >>> >> > It does sound like Android is better suited for "honest" >>> >> > cross-compilation, rather than "build compiler-rt for all targets we >>> >> > can find" model. >>> >> > >>> >> > I'm still not convinced that we must require the "ninja install" >>> >> > step. >>> >> > Could we just "ninja clang" and then build the second stage against >>> >> > the first stage build directory? Will this "find_package" thing not >>> >> > work that way, or do you mean it as a way to copy target asan >>> >> > runtime >>> >> > where the first stage compiler will find it (or probably both)? >>> >> > >>> >> > On Mon, Mar 31, 2014 at 6:46 PM, Alexey Samsonov >>> >> > <samsonov at google.com> >>> >> > wrote: >>> > >>> >>> >>> >>> >>> -- >>> Alexey Samsonov, MSK > > > > > -- > Alexey Samsonov, MSK
Alexey,>> Some good news, the drivers (both gcc and clang) allow us to put the >> '-L' parameters after the '-l' parameters.I made these changes locally and it went really well. The patch to clang is quite small and only one unit-test needed updating. In compiler-rt, I updated the flags passed to clang to include a '-L${COMPILER_RT_BINARY_DIR}/lib' and '-I${COMPILER_RT_BINARY_DIR}/include' flag. All tests pass except for the default blacklist tests. I had to move those into the clang test suite. As for cross-compilation and test ARM Linux, I needed to update all tests to include '%sim' before invoking cross-compiled executables. For x86, the variable is empty. For cross-compiling, it's a user-configured ${COMPILER_RT_EMULATOR}, which for ARM Linux, is QEMU. There are currently a handful of test failures. If you'd like, I can XFAIL them and commit this stuff sooner than later. Do you want these patches? -Greg On Fri, Apr 4, 2014 at 10:22 AM, Greg Fitzgerald <garious at gmail.com> wrote:>>>> in a standalone mode, then "make check-compiler-rt" in that tree >>>> wouldn't rebuild runtimes, and instead assume that they are provided by toolchain >> What is wrong with that approach? > > Not wrong, just inconsistent. I'm trying to test the > just-built-runtime before running the 'install' step. That's the way > all the other projects work, "all check-all install" in that order. > In other words, "build, verify, commit". > > >> And, again, we have to ensure that -L/path/to/new/asan provided by >> user "wins" over -L/path/to/clang/resource/dir, and we have strong >> reasons to put the latter as early in the link line as possible. > > Some good news, the drivers (both gcc and clang) allow us to put the > '-L' parameters after the '-l' parameters. So we can put -lasan at > the start of the parameter list to get the desired link order and -L > at the end so that the user-specified runtime is always chosen. > > -Greg > > > On Fri, Apr 4, 2014 at 5:57 AM, Alexey Samsonov <samsonov at google.com> wrote: >> >> On Thu, Apr 3, 2014 at 9:23 PM, Greg Fitzgerald <garious at gmail.com> wrote: >>> >>> > I don't think it's a good idea to let user hijack >>> > the driver and stuff in custom version of >>> > ASan runtime instead the one installed/built >>> > with compiler :) >>> >>> I'm okay with it. This is open source. If someone wants to put the >>> sanitizers on a shorter release cycle than the full toolchain, who are we to >>> hold them back? >> >> >> They can always rebuild the fresh ASan from source and replace the ASan >> runtime in toolchain with the new one. >> >>> >>> >>> > What do you mean by "invoking the test-suite directly"? >>> >>> In Clang's CMakeLists, add_subdirectory() of compiler-rt's 'test' >>> directory. I suggested we do this earlier, because of clang's hardcoded >>> dependency on the path to ASan. Instead, I think a better solution is to >>> break the hardcoded dependency, allowing the test suite to point clang to >>> compiler-rt's local copy of ASan using a '-L' parameter. >> >> >> I don't believe it's needed anywhere except in the testing infrastructure >> you suggest. And, again, we have to ensure that -L/path/to/new/asan provided >> by user "wins" over -L/path/to/clang/resource/dir, and we have strong >> reasons to put the latter as early in the link line as possible. That said, >> .... >> >>> >>> >>> Greg >>> >>> >>> On Thursday, April 3, 2014, Alexey Samsonov <samsonov at google.com> wrote: >>>> >>>> >>>> On Thu, Apr 3, 2014 at 5:04 AM, Greg Fitzgerald <garious at gmail.com> >>>> wrote: >>>>> >>>>> > we would still want to use compiler-rt test-suite in a standalone >>>>> > mode, to test fully built/installed toolchains, >>>>> and even GCC. >>>>> >>>>> Sounds good. >>>>> >>>>> >>>>> > Clang driver links the static xsan runtimes from a hardcoded >>>>> > paths in Clang resource directory, and doesn't add flags like >>>>> > "-lasan -L/path/to/clang/resource/dir". I find this behavior >>>>> > reasonable. >>>>> >>>>> Can we change the flags to "-lasan -L/path/to/clang/resource/dir"? If >>>>> we make that change, >>>> >>>> >>>> I don't think it's a good idea to let user hijack the driver and stuff in >>>> custom version of >>>> ASan runtime instead the one installed/built with compiler :) >>>> >>>>> >>>>> I can stop suggesting that the clang build should >>>>> invoke the compiler-rt test suite directly. :-) >>>> >>>> >>>> I haven't yet fully understood your complaints. >> >> >> >>>> >>>> Note that if you build/configure compiler-rt >>>> in a standalone mode, then "make check-compiler-rt" in that tree wouldn't >>>> rebuild runtimes, >>>> and instead assume that they are provided by toolchain (that is, in >>>> standalone mode runtimes and >>>> test suites are effectively independent). >> >> >> ^^^^ >> What is wrong with that approach? >> >> >>>> >>>> What do you mean by "invoking the test-suite directly"? >>>> >>>> >>>> >>>> >>>> > Do you want to give it a try? >>>> >>>> I'll take a crack it, sure. >>>> >>>> >>>> By the way, locally, I now have just over half the ASan test suite >>>> passing ARM-Linux via QEMU. It's been really convenient to build >>>> compiler-rt in standalone mode. The edit-compile-run cycle is much >>>> shorter this way. Thanks for your help in making this work! >>>> >>>> -Greg >>>> >>>> >>>> >>>> On Wed, Apr 2, 2014 at 7:56 AM, Alexey Samsonov <samsonov at google.com> >>>> wrote: >>>> > Hi Greg, >>>> > >>>> > On Wed, Apr 2, 2014 at 2:50 AM, Greg Fitzgerald <garious at gmail.com> >>>> > wrote: >>>> >> >>>> >> Alexey, Evgeniy, >>>> >> >>>> >> I propose the following steps to unify multi-arch support in >>>> >> compiler-rt: >>>> >> >>>> >> 1) The compiler-rt test suite adds "-L${CMAKE_CURRENT_BINARY_DIR}/lib" >>>> >> to its 'clang' variables. This way we can test the sanitizers without >>>> >> installing any libs to the just-built-clang install directory. >>>> > >>>> > >>>> > This is possible, but please keep in mind that: >>>> > 1) we would still want to use compiler-rt test-suite in a standalone >>>> > mode, >>>> > to test fully built/installed toolchains, >>>> > and even GCC. >>>> > 2) Adding -L... wouldn't work: Clang driver links the static xsan >>>> > runtimes >>>> > from a hardcoded paths in Clang resource directory, >>>> > and doesn't add flags like "-lasan -L/path/to/clang/resource/dir". I >>>> > find >>>> > this behavior reasonable. >>>> > >>>> > I don't see a problem with the current approach - we can make "run >>>> > sanitizer >>>> > test suite" command in the >>>> > top-level build tree depend on "build/install compiler-rt >>>> > ExternalProject" >>>> > steps (see how it's done currently). >>>> > >>>> >> >>>> >> >>>> >> 2) The "clang/runtime" build calls ExternalProject once for each arch >>>> >> it needs of compiler-rt. So once to create x86_64 libs and once for >>>> >> i386 libs. >>>> >> >>>> >> 3) The compiler-rt build drops the ${arch} suffix from its libs, and >>>> >> the "clang/runtime" build uses CMAKE_INSTALL_PREFIX to control where >>>> >> the compiler-rt libs go. >>>> >> >>>> >> >>>> >> "CMAKE_INSTALL_PREFIX=${CMAKE_CURRENT_BINARY_DIR}/lib/clang/${CLANG_VERSION}/x86_64-linux" >>>> >> >>>> >> >>>> >> 4) Remove multi-arch support from the compiler-rt build. Instead, >>>> >> declare compiler-rt as an "any-arch" build, configured with: >>>> >> CMAKE_CXX_COMPILER (defaults to just-built-clang) >>>> >> CMAKE_CXX_FLAGS (defaults to llvm's default target) >>>> >> COMPILER_RT_TEST_COMPILER (defaults to CMAKE_CXX_COMPILER) >>>> >> COMPILER_RT_TEST_FLAGS (defaults to CMAKE_CXX_FLAGS) >>>> > >>>> > >>>> > Unfortunately, this could be problematic for Mac - we use "universal >>>> > binaries" there - a single static/dynamic runtime >>>> > which contains runtimes for all the possible architectures. We might >>>> > work >>>> > around this, however, by adding a custom >>>> > Mac-specific step that would merge all single-arch binaries into a >>>> > multi-arch one - I think this is fine as long as we >>>> > greatly simplify the rest of the ugly build system. >>>> > >>>> > Do you want to give it a try? >>>> > >>>> >> >>>> >> >>>> >> Thoughts? >>>> >> >>>> >> Thanks, >>>> >> Greg >>>> >> >>>> >> On Tue, Apr 1, 2014 at 12:58 AM, Evgeniy Stepanov >>>> >> <eugeni.stepanov at gmail.com> wrote: >>>> >> > It does sound like Android is better suited for "honest" >>>> >> > cross-compilation, rather than "build compiler-rt for all targets we >>>> >> > can find" model. >>>> >> > >>>> >> > I'm still not convinced that we must require the "ninja install" >>>> >> > step. >>>> >> > Could we just "ninja clang" and then build the second stage against >>>> >> > the first stage build directory? Will this "find_package" thing not >>>> >> > work that way, or do you mean it as a way to copy target asan >>>> >> > runtime >>>> >> > where the first stage compiler will find it (or probably both)? >>>> >> > >>>> >> > On Mon, Mar 31, 2014 at 6:46 PM, Alexey Samsonov >>>> >> > <samsonov at google.com> >>>> >> > wrote: >>>> > >>>> >>>> >>>> >>>> >>>> -- >>>> Alexey Samsonov, MSK >> >> >> >> >> -- >> Alexey Samsonov, MSK