[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Mitch Phillips via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc414bbefe45a: Make -fsanitize=scudo use scudo_standalone. 
Delete check-scudo. (authored by hctim).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/fuchsia.c
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/CMakeLists.txt
  compiler-rt/test/CMakeLists.txt
  compiler-rt/test/scudo/CMakeLists.txt
  compiler-rt/test/scudo/standalone/CMakeLists.txt
  llvm/docs/GwpAsan.rst

Index: llvm/docs/GwpAsan.rst
===
--- llvm/docs/GwpAsan.rst
+++ llvm/docs/GwpAsan.rst
@@ -143,9 +143,10 @@
   default visibility. This will override the compile time define;
 
 - Depending on allocator support (Scudo has support for this mechanism): Through
-  the environment variable ``GWP_ASAN_OPTIONS``, containing the options string
-  to be parsed. Options defined this way will override any definition made
-  through ``__gwp_asan_default_options``.
+  an environment variable, containing the options string to be parsed. In Scudo,
+  this is through `SCUDO_OPTIONS=GWP_ASAN_${OPTION_NAME}=${VALUE}` (e.g.
+  `SCUDO_OPTIONS=GWP_ASAN_SampleRate=100`). Options defined this way will
+  override any definition made through ``__gwp_asan_default_options``.
 
 The options string follows a syntax similar to ASan, where distinct options
 can be assigned in the same string, separated by colons.
@@ -216,9 +217,9 @@
 
 .. code:: console
 
-  $ clang++ -fsanitize=scudo -std=c++17 -g buggy_code.cpp
-  $ for i in `seq 1 200`; do
-  GWP_ASAN_OPTIONS="SampleRate=100" ./a.out > /dev/null;
+  $ clang++ -fsanitize=scudo -g buggy_code.cpp
+  $ for i in `seq 1 500`; do
+  SCUDO_OPTIONS="GWP_ASAN_SampleRate=100" ./a.out > /dev/null;
 done
   |
   | *** GWP-ASan detected a memory error ***
Index: compiler-rt/test/scudo/standalone/CMakeLists.txt
===
--- compiler-rt/test/scudo/standalone/CMakeLists.txt
+++ compiler-rt/test/scudo/standalone/CMakeLists.txt
@@ -10,12 +10,12 @@
   ${CMAKE_CURRENT_BINARY_DIR}/unit/gwp_asan/lit.site.cfg.py)
 list(APPEND SCUDO_STANDALONE_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/unit/gwp_asan)
   endif()
-endif()
 
-add_lit_testsuite(check-scudo_standalone
-  "Running Scudo Standalone tests"
-  ${SCUDO_STANDALONE_TESTSUITES}
-  DEPENDS ${SCUDO_STANDALONE_TEST_DEPS})
+  add_lit_testsuite(check-scudo_standalone
+"Running Scudo Standalone tests"
+${SCUDO_STANDALONE_TESTSUITES}
+DEPENDS ${SCUDO_STANDALONE_TEST_DEPS})
 
-set_target_properties(check-scudo_standalone
-  PROPERTIES FOLDER "Compiler-RT Tests")
+  set_target_properties(check-scudo_standalone
+PROPERTIES FOLDER "Compiler-RT Tests")
+endif()
Index: compiler-rt/test/scudo/CMakeLists.txt
===
--- compiler-rt/test/scudo/CMakeLists.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-set(SCUDO_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(SCUDO_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-set(SCUDO_TESTSUITES)
-
-set(SCUDO_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND SCUDO_TEST_DEPS scudo)
-endif()
-
-configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
-  )
-
-set(SCUDO_TEST_ARCH ${SCUDO_SUPPORTED_ARCH})
-foreach(arch ${SCUDO_TEST_ARCH})
-  set(SCUDO_TEST_TARGET_ARCH ${arch})
-  string(TOLOWER "-${arch}" SCUDO_TEST_CONFIG_SUFFIX)
-  get_test_cc_for_arch(${arch} SCUDO_TEST_TARGET_CC SCUDO_TEST_TARGET_CFLAGS)
-  string(TOUPPER ${arch} ARCH_UPPER_CASE)
-  set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)
-
-  configure_lit_site_cfg(
-${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
-${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg.py)
-  list(APPEND SCUDO_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
-endforeach()
-
-add_subdirectory(standalone)
-
-add_lit_testsuite(check-scudo "Running the Scudo Hardened Allocator tests"
-  ${SCUDO_TESTSUITES}
-  DEPENDS ${SCUDO_TEST_DEPS})
-set_target_properties(check-scudo PROPERTIES FOLDER "Compiler-RT Misc")
Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -50,7 +50,11 @@
 function(compiler_rt_test_runtime runtime)
   string(TOUPPER ${runtime} runtime_uppercase)
   if(COMPILER_RT_HAS_${runtime_uppercase})
-add_subdirectory(${runtime})
+if (${runtime} STREQUAL scudo_standalone)
+  add_subdirectory(scudo/standalone)
+else()
+  add_subdirectory(${runtime})
+endif()
 foreach(directory ${ARGN})
   

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Mitch Phillips via Phabricator via cfe-commits]

hctim updated this revision to Diff 479693.
hctim added a comment.

Lots of places in scudo_standalone depend on GWP-ASan, and it seems far more 
reasonable to make COMPILER_RT_HAS_GWP_ASAN mean "is GWP-ASan actually going to 
be built", rather than checking three variables in all the places.

Give the same treatment to COMPILER_RT_HAS_SCUDO_STANDALONE.

This should fix the reviewer problems that were identified:

1. scudo_standalone's cmake rules don't get generated when 
COMPILER_RT_BUILD_SANITIZERS=Off or COMPILER_RT_SANITIZERS_TO_BUILD does not 
contain scudo_standalone.
2. check-scudo_standalone's cmake rules don't get generated when 
COMPILER_RT_INCLUDE_TESTS=Off.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/fuchsia.c
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/CMakeLists.txt
  compiler-rt/test/CMakeLists.txt
  compiler-rt/test/scudo/CMakeLists.txt
  compiler-rt/test/scudo/standalone/CMakeLists.txt
  llvm/docs/GwpAsan.rst

Index: llvm/docs/GwpAsan.rst
===
--- llvm/docs/GwpAsan.rst
+++ llvm/docs/GwpAsan.rst
@@ -143,9 +143,10 @@
   default visibility. This will override the compile time define;
 
 - Depending on allocator support (Scudo has support for this mechanism): Through
-  the environment variable ``GWP_ASAN_OPTIONS``, containing the options string
-  to be parsed. Options defined this way will override any definition made
-  through ``__gwp_asan_default_options``.
+  an environment variable, containing the options string to be parsed. In Scudo,
+  this is through `SCUDO_OPTIONS=GWP_ASAN_${OPTION_NAME}=${VALUE}` (e.g.
+  `SCUDO_OPTIONS=GWP_ASAN_SampleRate=100`). Options defined this way will
+  override any definition made through ``__gwp_asan_default_options``.
 
 The options string follows a syntax similar to ASan, where distinct options
 can be assigned in the same string, separated by colons.
@@ -216,9 +217,9 @@
 
 .. code:: console
 
-  $ clang++ -fsanitize=scudo -std=c++17 -g buggy_code.cpp
-  $ for i in `seq 1 200`; do
-  GWP_ASAN_OPTIONS="SampleRate=100" ./a.out > /dev/null;
+  $ clang++ -fsanitize=scudo -g buggy_code.cpp
+  $ for i in `seq 1 500`; do
+  SCUDO_OPTIONS="GWP_ASAN_SampleRate=100" ./a.out > /dev/null;
 done
   |
   | *** GWP-ASan detected a memory error ***
Index: compiler-rt/test/scudo/standalone/CMakeLists.txt
===
--- compiler-rt/test/scudo/standalone/CMakeLists.txt
+++ compiler-rt/test/scudo/standalone/CMakeLists.txt
@@ -10,12 +10,12 @@
   ${CMAKE_CURRENT_BINARY_DIR}/unit/gwp_asan/lit.site.cfg.py)
 list(APPEND SCUDO_STANDALONE_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/unit/gwp_asan)
   endif()
-endif()
 
-add_lit_testsuite(check-scudo_standalone
-  "Running Scudo Standalone tests"
-  ${SCUDO_STANDALONE_TESTSUITES}
-  DEPENDS ${SCUDO_STANDALONE_TEST_DEPS})
+  add_lit_testsuite(check-scudo_standalone
+"Running Scudo Standalone tests"
+${SCUDO_STANDALONE_TESTSUITES}
+DEPENDS ${SCUDO_STANDALONE_TEST_DEPS})
 
-set_target_properties(check-scudo_standalone
-  PROPERTIES FOLDER "Compiler-RT Tests")
+  set_target_properties(check-scudo_standalone
+PROPERTIES FOLDER "Compiler-RT Tests")
+endif()
Index: compiler-rt/test/scudo/CMakeLists.txt
===
--- compiler-rt/test/scudo/CMakeLists.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-set(SCUDO_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(SCUDO_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-set(SCUDO_TESTSUITES)
-
-set(SCUDO_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND SCUDO_TEST_DEPS scudo)
-endif()
-
-configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
-  )
-
-set(SCUDO_TEST_ARCH ${SCUDO_SUPPORTED_ARCH})
-foreach(arch ${SCUDO_TEST_ARCH})
-  set(SCUDO_TEST_TARGET_ARCH ${arch})
-  string(TOLOWER "-${arch}" SCUDO_TEST_CONFIG_SUFFIX)
-  get_test_cc_for_arch(${arch} SCUDO_TEST_TARGET_CC SCUDO_TEST_TARGET_CFLAGS)
-  string(TOUPPER ${arch} ARCH_UPPER_CASE)
-  set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)
-
-  configure_lit_site_cfg(
-${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
-${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg.py)
-  list(APPEND SCUDO_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
-endforeach()
-
-add_subdirectory(standalone)
-
-add_lit_testsuite(check-scudo "Running the Scudo Hardened Allocator tests"
-  ${SCUDO_TESTSUITES}
-  DEPENDS ${SCUDO_TEST_DEPS})
-set_target_properties(check-scudo PROPERTIES FOLDER "Compiler-RT Misc")
Index: compiler-rt/test/CMakeLists.txt
===
--- 

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Mitch Phillips via Phabricator via cfe-commits]

hctim added a comment.

Thanks for the detailed information folks. I'll probably fix it up and 
re-submit next week (after the US holidays) given that the blast radius seems 
to be bigger than I expected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Reverted in 907baeec49bfbe9e76498634a9418e1dc6c973d9 for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Chromium is broken too

  CMake Error at 
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/cmake/modules/AddLLVM.cmake:1915
 (add_dependencies):
The dependency target "ScudoUnitTests" of target "check-runtimes" does not
exist.
  Call Stack (most recent call first):

/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/cmake/modules/AddLLVM.cmake:1955
 (add_lit_target)
CMakeLists.txt:238 (umbrella_lit_testsuite_end)
  
  
  CMake Error at 
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/cmake/modules/AddLLVM.cmake:1915
 (add_dependencies):
The dependency target "ScudoUnitTests" of target "check-scudo_standalone"
does not exist.
  Call Stack (most recent call first):

/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/cmake/modules/AddLLVM.cmake:1981
 (add_lit_target)

/b/s/w/ir/cache/builder/src/third_party/llvm/compiler-rt/test/scudo/standalone/CMakeLists.txt:15
 (add_lit_testsuite)

from 
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8796726895518599345/+/u/package_clang/stdout


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

We (Linaro) also have an issue with a bot that uses 
`-DCOMPILER_RT_BUILD_SANITIZERS=OFF`.

https://lab.llvm.org/buildbot/#/builders/178/builds/3318

  CMake Error at cmake/modules/AddLLVM.cmake:1915 (add_dependencies):
The dependency target "ScudoUnitTests" of target "check-all" does not
exist.
  Call Stack (most recent call first):
cmake/modules/AddLLVM.cmake:1955 (add_lit_target)
CMakeLists.txt:1249 (umbrella_lit_testsuite_end)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

We're seeing a CMake error after this:

  CMake Error at 
/data/users/smeenai/Source/tcdev/external/llvm-project/compiler-rt/cmake/Modules/AddCompilerRT.cmake:368
 (add_library):
Error evaluating generator expression:
  
  $
  
Objects of target "RTGwpAsan.x86_64" referenced but no such target exists.

What appears to be happening is that 
`COMPILER_RT_SCUDO_STANDALONE_BUILD_SHARED` is ON by default, as is 
`COMPILER_RT_BUILD_GWP_ASAN`. However, `gwp_asan` isn't actually built unless 
it's also present in `COMPILER_RT_SANITIZERS_TO_BUILD`, so the combination of 
defaults is broken.

One option would be to add `gwp_asan` to `COMPILER_RT_SANITIZERS_TO_BUILD` if 
`COMPILER_RT_BUILD_GWP_ASAN` is ON, as in P8299 
. Another would be to make Scudo consider 
`COMPILER_RT_SANITIZERS_TO_BUILD` in addition to `COMPILER_RT_HAS_GWP_ASAN` 
before linking the GWP-ASan libraries. What are your thoughts here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Mitch Phillips via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGab1a5991fe76: Make -fsanitize=scudo use scudo_standalone. 
Delete check-scudo. (authored by hctim).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/fuchsia.c
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/CMakeLists.txt
  compiler-rt/test/CMakeLists.txt
  compiler-rt/test/scudo/CMakeLists.txt
  llvm/docs/GwpAsan.rst

Index: llvm/docs/GwpAsan.rst
===
--- llvm/docs/GwpAsan.rst
+++ llvm/docs/GwpAsan.rst
@@ -143,9 +143,10 @@
   default visibility. This will override the compile time define;
 
 - Depending on allocator support (Scudo has support for this mechanism): Through
-  the environment variable ``GWP_ASAN_OPTIONS``, containing the options string
-  to be parsed. Options defined this way will override any definition made
-  through ``__gwp_asan_default_options``.
+  an environment variable, containing the options string to be parsed. In Scudo,
+  this is through `SCUDO_OPTIONS=GWP_ASAN_${OPTION_NAME}=${VALUE}` (e.g.
+  `SCUDO_OPTIONS=GWP_ASAN_SampleRate=100`). Options defined this way will
+  override any definition made through ``__gwp_asan_default_options``.
 
 The options string follows a syntax similar to ASan, where distinct options
 can be assigned in the same string, separated by colons.
@@ -216,9 +217,9 @@
 
 .. code:: console
 
-  $ clang++ -fsanitize=scudo -std=c++17 -g buggy_code.cpp
-  $ for i in `seq 1 200`; do
-  GWP_ASAN_OPTIONS="SampleRate=100" ./a.out > /dev/null;
+  $ clang++ -fsanitize=scudo -g buggy_code.cpp
+  $ for i in `seq 1 500`; do
+  SCUDO_OPTIONS="GWP_ASAN_SampleRate=100" ./a.out > /dev/null;
 done
   |
   | *** GWP-ASan detected a memory error ***
Index: compiler-rt/test/scudo/CMakeLists.txt
===
--- compiler-rt/test/scudo/CMakeLists.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-set(SCUDO_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(SCUDO_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-set(SCUDO_TESTSUITES)
-
-set(SCUDO_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND SCUDO_TEST_DEPS scudo)
-endif()
-
-configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
-  )
-
-set(SCUDO_TEST_ARCH ${SCUDO_SUPPORTED_ARCH})
-foreach(arch ${SCUDO_TEST_ARCH})
-  set(SCUDO_TEST_TARGET_ARCH ${arch})
-  string(TOLOWER "-${arch}" SCUDO_TEST_CONFIG_SUFFIX)
-  get_test_cc_for_arch(${arch} SCUDO_TEST_TARGET_CC SCUDO_TEST_TARGET_CFLAGS)
-  string(TOUPPER ${arch} ARCH_UPPER_CASE)
-  set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)
-
-  configure_lit_site_cfg(
-${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
-${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg.py)
-  list(APPEND SCUDO_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
-endforeach()
-
-add_subdirectory(standalone)
-
-add_lit_testsuite(check-scudo "Running the Scudo Hardened Allocator tests"
-  ${SCUDO_TESTSUITES}
-  DEPENDS ${SCUDO_TEST_DEPS})
-set_target_properties(check-scudo PROPERTIES FOLDER "Compiler-RT Misc")
Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -19,6 +19,8 @@
 # BlocksRuntime (and most of builtins) testsuites are not yet ported to lit.
 # add_subdirectory(BlocksRuntime)
 
+add_subdirectory(scudo/standalone)
+
 set(SANITIZER_COMMON_LIT_TEST_DEPS)
 
 if(COMPILER_RT_BUILD_PROFILE AND COMPILER_RT_HAS_PROFILE)
Index: compiler-rt/lib/CMakeLists.txt
===
--- compiler-rt/lib/CMakeLists.txt
+++ compiler-rt/lib/CMakeLists.txt
@@ -24,12 +24,13 @@
 function(compiler_rt_build_runtime runtime)
   string(TOUPPER ${runtime} runtime_uppercase)
   if(COMPILER_RT_HAS_${runtime_uppercase})
-add_subdirectory(${runtime})
 if(${runtime} STREQUAL tsan)
   add_subdirectory(tsan/dd)
 endif()
-if(${runtime} STREQUAL scudo)
+if(${runtime} STREQUAL scudo_standalone)
   add_subdirectory(scudo/standalone)
+else()
+  add_subdirectory(${runtime})
 endif()
   endif()
 endfunction()
@@ -48,6 +49,8 @@
   foreach(sanitizer ${COMPILER_RT_SANITIZERS_TO_BUILD})
 compiler_rt_build_runtime(${sanitizer})
   endforeach()
+
+  compiler_rt_build_runtime(scudo_standalone)
 endif()
 
 if(COMPILER_RT_BUILD_PROFILE AND COMPILER_RT_HAS_PROFILE)
Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ 

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Mitch Phillips via Phabricator via cfe-commits]

hctim added a comment.

In D138157#3935173 , @phosek wrote:

> Is the plan to eventually rename `scudo_standalone` to `scudo` once the 
> santiizer-based Scudo implementation is deleted?

Ideally, yes (as well as make scudo_standalone not be in a subfolder). 
Practically, no, because of all the folks that depend on it (you@fuchsia, 
android, llvm-libc).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

Is the plan to eventually rename `scudo_standalone` to `scudo` once the 
santiizer-based Scudo implementation is deleted?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Mitch Phillips via Phabricator via cfe-commits]

hctim added inline comments.



Comment at: llvm/docs/GwpAsan.rst:174
 
++-++
-| PerfectlyRightAlign| false   | When allocations are right-aligned, 
should we perfectly align them up to the   |
-|| | page boundary? By default (false), we 
round up allocation size to the nearest  |

vitalybuka wrote:
> how this related to the patch?
unrelated, was a drive-by cleanup. i'll remove it from the diff.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Mitch Phillips via Phabricator via cfe-commits]

hctim updated this revision to Diff 476241.
hctim marked 2 inline comments as done.
hctim added a comment.

Remove unrelated diff from doc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/fuchsia.c
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/CMakeLists.txt
  compiler-rt/test/CMakeLists.txt
  compiler-rt/test/scudo/CMakeLists.txt
  llvm/docs/GwpAsan.rst

Index: llvm/docs/GwpAsan.rst
===
--- llvm/docs/GwpAsan.rst
+++ llvm/docs/GwpAsan.rst
@@ -143,9 +143,10 @@
   default visibility. This will override the compile time define;
 
 - Depending on allocator support (Scudo has support for this mechanism): Through
-  the environment variable ``GWP_ASAN_OPTIONS``, containing the options string
-  to be parsed. Options defined this way will override any definition made
-  through ``__gwp_asan_default_options``.
+  an environment variable, containing the options string to be parsed. In Scudo,
+  this is through `SCUDO_OPTIONS=GWP_ASAN_${OPTION_NAME}=${VALUE}` (e.g.
+  `SCUDO_OPTIONS=GWP_ASAN_SampleRate=100`). Options defined this way will
+  override any definition made through ``__gwp_asan_default_options``.
 
 The options string follows a syntax similar to ASan, where distinct options
 can be assigned in the same string, separated by colons.
@@ -216,9 +217,9 @@
 
 .. code:: console
 
-  $ clang++ -fsanitize=scudo -std=c++17 -g buggy_code.cpp
-  $ for i in `seq 1 200`; do
-  GWP_ASAN_OPTIONS="SampleRate=100" ./a.out > /dev/null;
+  $ clang++ -fsanitize=scudo -g buggy_code.cpp
+  $ for i in `seq 1 500`; do
+  SCUDO_OPTIONS="GWP_ASAN_SampleRate=100" ./a.out > /dev/null;
 done
   |
   | *** GWP-ASan detected a memory error ***
Index: compiler-rt/test/scudo/CMakeLists.txt
===
--- compiler-rt/test/scudo/CMakeLists.txt
+++ /dev/null
@@ -1,35 +0,0 @@
-set(SCUDO_LIT_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(SCUDO_LIT_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-set(SCUDO_TESTSUITES)
-
-set(SCUDO_TEST_DEPS ${SANITIZER_COMMON_LIT_TEST_DEPS})
-if(NOT COMPILER_RT_STANDALONE_BUILD)
-  list(APPEND SCUDO_TEST_DEPS scudo)
-endif()
-
-configure_lit_site_cfg(
-  ${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
-  ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg.py
-  )
-
-set(SCUDO_TEST_ARCH ${SCUDO_SUPPORTED_ARCH})
-foreach(arch ${SCUDO_TEST_ARCH})
-  set(SCUDO_TEST_TARGET_ARCH ${arch})
-  string(TOLOWER "-${arch}" SCUDO_TEST_CONFIG_SUFFIX)
-  get_test_cc_for_arch(${arch} SCUDO_TEST_TARGET_CC SCUDO_TEST_TARGET_CFLAGS)
-  string(TOUPPER ${arch} ARCH_UPPER_CASE)
-  set(CONFIG_NAME ${ARCH_UPPER_CASE}${OS_NAME}Config)
-
-  configure_lit_site_cfg(
-${CMAKE_CURRENT_SOURCE_DIR}/lit.site.cfg.py.in
-${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME}/lit.site.cfg.py)
-  list(APPEND SCUDO_TESTSUITES ${CMAKE_CURRENT_BINARY_DIR}/${CONFIG_NAME})
-endforeach()
-
-add_subdirectory(standalone)
-
-add_lit_testsuite(check-scudo "Running the Scudo Hardened Allocator tests"
-  ${SCUDO_TESTSUITES}
-  DEPENDS ${SCUDO_TEST_DEPS})
-set_target_properties(check-scudo PROPERTIES FOLDER "Compiler-RT Misc")
Index: compiler-rt/test/CMakeLists.txt
===
--- compiler-rt/test/CMakeLists.txt
+++ compiler-rt/test/CMakeLists.txt
@@ -19,6 +19,8 @@
 # BlocksRuntime (and most of builtins) testsuites are not yet ported to lit.
 # add_subdirectory(BlocksRuntime)
 
+add_subdirectory(scudo/standalone)
+
 set(SANITIZER_COMMON_LIT_TEST_DEPS)
 
 if(COMPILER_RT_BUILD_PROFILE AND COMPILER_RT_HAS_PROFILE)
Index: compiler-rt/lib/CMakeLists.txt
===
--- compiler-rt/lib/CMakeLists.txt
+++ compiler-rt/lib/CMakeLists.txt
@@ -24,12 +24,13 @@
 function(compiler_rt_build_runtime runtime)
   string(TOUPPER ${runtime} runtime_uppercase)
   if(COMPILER_RT_HAS_${runtime_uppercase})
-add_subdirectory(${runtime})
 if(${runtime} STREQUAL tsan)
   add_subdirectory(tsan/dd)
 endif()
-if(${runtime} STREQUAL scudo)
+if(${runtime} STREQUAL scudo_standalone)
   add_subdirectory(scudo/standalone)
+else()
+  add_subdirectory(${runtime})
 endif()
   endif()
 endfunction()
@@ -48,6 +49,8 @@
   foreach(sanitizer ${COMPILER_RT_SANITIZERS_TO_BUILD})
 compiler_rt_build_runtime(${sanitizer})
   endforeach()
+
+  compiler_rt_build_runtime(scudo_standalone)
 endif()
 
 if(COMPILER_RT_BUILD_PROFILE AND COMPILER_RT_HAS_PROFILE)
Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -619,9 +619,6 @@
   list_intersect(CFI_SUPPORTED_ARCH
 ALL_CFI_SUPPORTED_ARCH
 

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/docs/GwpAsan.rst:174
 
++-++
-| PerfectlyRightAlign| false   | When allocations are right-aligned, 
should we perfectly align them up to the   |
-|| | page boundary? By default (false), we 
round up allocation size to the nearest  |

how this related to the patch?



Comment at: llvm/docs/GwpAsan.rst:180
 | SampleRate | 5000| The probability (1 / SampleRate) that 
a page is selected for GWP-ASan  |
-|| | sampling. Sample rates up to (2^31 - 
1) are supported. |
+|| | sampling. Sample rates up to (2^30 - 
1) are supported. |
 
++-++

unrelated?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138157/new/

https://reviews.llvm.org/D138157

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138157: Make -fsanitize=scudo use scudo_standalone. Delete check-scudo.

[Mitch Phillips via Phabricator via cfe-commits]

hctim created this revision.
hctim added a reviewer: vitalybuka.
Herald added subscribers: Enna1, abrachet, phosek, cryptoad.
Herald added a project: All.
hctim requested review of this revision.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, MaskRay.
Herald added projects: clang, Sanitizers, LLVM.

Leaves the implementation and tests files in-place for right now, but
deletes the ability to build the old sanitizer-common based scudo. This
has been on life-support for a long time, and the newer scudo_standalone
is much better supported and maintained.

Also patches up some GWP-ASan wording, primarily related to the fact
that -fsanitize=scudo now is scudo_standalone, and therefore the way to
reference the GWP-ASan options through the environment variable has
changed.

Future follow-up patches will delete the original scudo, and migrate all
its tests over to be part of the scudo_standalone test suite.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138157

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/fuchsia.c
  clang/test/Driver/sanitizer-ld.c
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/CMakeLists.txt
  compiler-rt/test/CMakeLists.txt
  compiler-rt/test/scudo/CMakeLists.txt
  llvm/docs/GwpAsan.rst

Index: llvm/docs/GwpAsan.rst
===
--- llvm/docs/GwpAsan.rst
+++ llvm/docs/GwpAsan.rst
@@ -143,9 +143,10 @@
   default visibility. This will override the compile time define;
 
 - Depending on allocator support (Scudo has support for this mechanism): Through
-  the environment variable ``GWP_ASAN_OPTIONS``, containing the options string
-  to be parsed. Options defined this way will override any definition made
-  through ``__gwp_asan_default_options``.
+  an environment variable, containing the options string to be parsed. In Scudo,
+  this is through `SCUDO_OPTIONS=GWP_ASAN_${OPTION_NAME}=${VALUE}` (e.g.
+  `SCUDO_OPTIONS=GWP_ASAN_SampleRate=100`). Options defined this way will
+  override any definition made through ``__gwp_asan_default_options``.
 
 The options string follows a syntax similar to ASan, where distinct options
 can be assigned in the same string, separated by colons.
@@ -171,17 +172,12 @@
 ++-++
 | Enabled| true| Is GWP-ASan enabled?   |
 ++-++
-| PerfectlyRightAlign| false   | When allocations are right-aligned, should we perfectly align them up to the   |
-|| | page boundary? By default (false), we round up allocation size to the nearest  |
-|| | power of two (2, 4, 8, 16) up to a maximum of 16-byte alignment for|
-|| | performance reasons. Setting this to true can find single byte |
-|| | buffer-overflows at the cost of performance, and may be incompatible with  |
-|| | some architectures.|
+| help   | false   | Print a summart of the available options.  |
 ++-++
 | MaxSimultaneousAllocations | 16  | Number of simultaneously-guarded allocations available in the pool.|
 ++-++
 | SampleRate | 5000| The probability (1 / SampleRate) that a page is selected for GWP-ASan  |
-|| | sampling. Sample rates up to (2^31 - 1) are supported. |
+|| | sampling. Sample rates up to (2^30 - 1) are supported. |
 ++-++
 | InstallSignalHandlers  | true| Install GWP-ASan signal handlers for SIGSEGV during dynamic loading. This  |
 || | allows better error reports by providing stack traces for allocation and   |
@@ -216,9 +212,9 @@
 
 .. code:: console
 
-  $ clang++ -fsanitize=scudo -std=c++17 -g buggy_code.cpp
-  $ for i in `seq 1 200`; do
-  GWP_ASAN_OPTIONS="SampleRate=100" ./a.out > /dev/null;
+  $ clang++ -fsanitize=scudo -g buggy_code.cpp
+  $ for i in `seq 1 500`; do
+  SCUDO_OPTIONS="GWP_ASAN_SampleRate=100" ./a.out > /dev/null;
 done
   |
   | *** GWP-ASan detected a