Get the ASAN toolchain working again #7604

steven-johnson · 2023-06-01T22:20:36Z

Various fixes to enable ASAN to finally work (linux x64 only).

Note that this found ~~several~~ one real ASAN failure in the Anderson2021 autoscheduler tests, which is not fixed yet; I'll fix thus in a subsequent PR.

Various fixes to enable ASAN to finally work (linux x64 only). Note that this found several ASAN failures in the Anderson2021 autoscheduler tests, which are *not* fixed yet; I'll fix thus in a subsequent PR.

steven-johnson · 2023-06-02T18:27:07Z

(Note that I'm pushing some temporary debugging code here to try to catch a failure that so far only repros on the buildbots, but not locally -- please ignore when reviewing, for now at least)

(Harvested from #7604 to land separately)

alexreinking · 2023-06-05T17:26:49Z

I'm going to try this locally before doing a full review. There are a few things here that are raising alarm bells for certain toolchain configurations.

steven-johnson · 2023-06-05T17:31:35Z

I'm going to try this locally before doing a full review. There are a few things here that are raising alarm bells for certain toolchain configurations.

Thanks. Everything about ASAN (or really, any of the sanitizers) is pretty ticklish, due to the combination of our unorthodox build setup (eg building to bitcode) and CMake being CMake.

* Fix PCH build failures (Harvested from #7604 to land separately) * Update CMakeLists.txt

alexreinking · 2023-06-20T15:41:24Z

When building with clang 16, I get the following error:

[59/1220] Linking CXX executable test/correctness/correctness_float16_t
FAILED: test/correctness/correctness_float16_t 
: && /local/mnt2/workspace/areinkin/.local/llvm-16/bin/clang++ -fsanitize=address -fuse-ld=/local/mnt2/workspace/areinkin/.local/llvm-16/bin/ld.lld test/CMakeFiles/Halide_terminate_handler.dir/common/terminate_handler.cpp.o test/correctness/CMakeFiles/correctness_float16_t.dir/float16_t.cpp.o -o test/correctness/correctness_float16_t  -Wl,-rpath,/local/mnt2/workspace/areinkin/dev/Halide/build/clang-16/src  src/libHalide.so.16.0.0 && :
ld.lld: error: undefined symbol: __extendhfsf2

Things I did that allowed the build to finish:

Link to the Clang compiler runtime via -rtlib=compiler-rt.
Set -march=haswell in this toolchain (CMAKE_{C,CXX}_FLAGS_INIT) so that native fp16 operations are available and the __extendhfsf2 function isn't emitted in the first place.
Check that a simple test program that uses _Float16 builds; if not, disable the test.

I don't recommend (1)... The default is libgcc, which is sensible for distributing binaries; no guarantee downstream systems have compiler-rt built.

I'm partial to a mix of 2 and 3.

steven-johnson · 2023-06-20T16:02:52Z

I thought that we had to explicitly link to compiler-rt anyway for ASAN to work properly. (Maybe I'm confusing it with something?)

FWIW, I didn't even test with LLVM16 -- my assumption here is that we'll only do ASAN builds with top-of-tree, so as long as it works with LLVM/Clang17+, we're fine.

alexreinking · 2023-06-20T16:44:50Z

Okay, if Clang 17 doesn't need any of this, then I'm cool with the change.

Well, almost -- there's more in here than just the toolchain changes.

alexreinking

I'm proposing a less ad-hoc extension point that feels better to send downstream. I can open a PR-to-PR if you agree with the change, but would prefer not to do this work.

alexreinking · 2023-06-03T17:41:14Z

cmake/HalideGeneratorHelpers.cmake

+ if (Halide_SHARED_ASAN_RUNTIME_LIBRARY)
+ # ASAN's Leak detector will report that we leaked all the PyBind11
+ # Module-support code, so disable it here.
+ set(EXTRA_ENV_VARS ASAN_OPTIONS=detect_leaks=0 ${Halide_SANITIZER_ENV_VARS})
+ endif ()
+ set(GENERATOR_CMD ${CMAKE_COMMAND} -E env PYTHONPATH=${PYTHONPATH} ${EXTRA_ENV_VARS} ${Python3_EXECUTABLE} $<SHELL_PATH:${py_src}>)


As this is code is part of our downstream package interface, the Halide_SHARED_ASAN_RUNTIME_LIBRARY variable would need to be documented. I think a more flexible design would be to add a Halide_PYTHON_LAUNCHER variable that works like the familiar CMAKE_CXX_COMPILER_LAUNCHER variable.

To implement that, this code would be:

Suggested change

if (Halide_SHARED_ASAN_RUNTIME_LIBRARY)

# ASAN's Leak detector will report that we leaked all the PyBind11

# Module-support code, so disable it here.

set(EXTRA_ENV_VARS ASAN_OPTIONS=detect_leaks=0 ${Halide_SANITIZER_ENV_VARS})

endif ()

set(GENERATOR_CMD ${CMAKE_COMMAND} -E env PYTHONPATH=${PYTHONPATH} ${EXTRA_ENV_VARS} ${Python3_EXECUTABLE} $<SHELL_PATH:${py_src}>)

# TODO: when upgrading to CMake 3.24+ add `--` before `${Halide_PYTHON_LAUNCHER}`

set(GENERATOR_CMD ${CMAKE_COMMAND} -E env PYTHONPATH=${PYTHONPATH} ${Halide_PYTHON_LAUNCHER} ${Python3_EXECUTABLE} $<SHELL_PATH:${py_src}>)

Then, in our toolchain file, we could set:

set( Halide_PYTHON_LAUNCHER ${CMAKE_COMMAND} -E env ASAN_OPTIONS=detect_leaks=0 LD_PRELOAD=${Halide_SHARED_ASAN_RUNTIME_LIBRARY} )

test/CMakeLists.txt

alexreinking · 2023-06-21T14:32:30Z

cmake/toolchain.linux-x64-asan.cmake

+set(Halide_SHARED_ASAN_RUNTIME_LIBRARY "${Halide_SHARED_ASAN_RUNTIME_LIBRARY}"
+ CACHE FILEPATH "Library to preload when running Python tests.")
+
+set(Halide_SANITIZER_ENV_VARS "LD_PRELOAD=${Halide_SHARED_ASAN_RUNTIME_LIBRARY}")


Suggested change

set(Halide_SANITIZER_ENV_VARS "LD_PRELOAD=${Halide_SHARED_ASAN_RUNTIME_LIBRARY}")

set(

Halide_PYTHON_LAUNCHER

${CMAKE_COMMAND} -E env ASAN_OPTIONS=detect_leaks=0 LD_PRELOAD=${Halide_SHARED_ASAN_RUNTIME_LIBRARY}

)

As described above.

alexreinking · 2023-06-21T14:35:22Z

python_bindings/CMakeLists.txt

+ if (Halide_SANITIZER_ENV_VARS)
+ list(PREPEND ARG_ENVIRONMENT ${Halide_SANITIZER_ENV_VARS})
 endif ()


Suggested change

if (Halide_SANITIZER_ENV_VARS)

list(PREPEND ARG_ENVIRONMENT ${Halide_SANITIZER_ENV_VARS})

endif ()

Then, below (GitHib won't let me comment there 😠), in the call to add_test replace

COMMAND Python3::Interpreter

with

COMMAND ${Halide_PYTHON_LAUNCHER} "${Python3_EXECUTABLE}"

alexreinking · 2023-06-21T14:37:11Z

test/autoschedulers/li2018/CMakeLists.txt

@@ -42,6 +42,7 @@ if (WITH_PYTHON_BINDINGS)
 set_tests_properties(li2018_gradient_autoscheduler_test_py PROPERTIES
 DEPENDS li2018_test
 LABELS "li2018;autoschedulers;auto_schedule"
+ ENVIRONMENT "${Halide_SANITIZER_ENV_VARS}"


Suggested change

ENVIRONMENT "${Halide_SANITIZER_ENV_VARS}"

Same deal with add_test(... COMMAND Python3::Interpreter ...) above. Use the Halide_PYTHON_LAUNCHER variable.

* Use new Halide_PYTHON_LAUNCHER to set env vars * Update CMake docs for Halide_SANITIZER_ENV_VARS --------- Co-authored-by: Alex Reinking <areinkin@qti.qualcomm.com>

steven-johnson · 2023-06-23T17:51:14Z

With your changes merged, is this good to go now, or are more changes needed?

alexreinking

LGTM now

* Fix PCH build failures (Harvested from halide#7604 to land separately) * Update CMakeLists.txt

* Get the ASAN toolchain working again Various fixes to enable ASAN to finally work (linux x64 only). Note that this found several ASAN failures in the Anderson2021 autoscheduler tests, which are *not* fixed yet; I'll fix thus in a subsequent PR. * Remove stuff that I didn't mean to check in * Configure cuda-specific tests properly too * trigger buildbots * Update CodeGen_LLVM.cpp * Update CodeGen_LLVM.cpp * Fix sloppiness? * Update CMakeLists.txt * trigger buildbots * Use Halide_PYTHON_LAUNCHER to implement ASAN toolchain fixes (halide#7657) * Use new Halide_PYTHON_LAUNCHER to set env vars * Update CMake docs for Halide_SANITIZER_ENV_VARS --------- Co-authored-by: Alex Reinking <areinkin@qti.qualcomm.com> --------- Co-authored-by: Alex Reinking <alex.reinking@gmail.com> Co-authored-by: Alex Reinking <areinkin@qti.qualcomm.com>

Get the ASAN toolchain working again

373f0d1

Various fixes to enable ASAN to finally work (linux x64 only). Note that this found several ASAN failures in the Anderson2021 autoscheduler tests, which are *not* fixed yet; I'll fix thus in a subsequent PR.

steven-johnson requested review from alexreinking and abadams June 1, 2023 22:20

steven-johnson added 4 commits June 1, 2023 15:27

Remove stuff that I didn't mean to check in

16895e5

Configure cuda-specific tests properly too

aacfa7a

trigger buildbots

402c5f5

Update CodeGen_LLVM.cpp

b518dcb

steven-johnson added 4 commits June 2, 2023 11:40

Update CodeGen_LLVM.cpp

82fb264

Fix sloppiness?

5979b7d

Update CMakeLists.txt

7c116e8

trigger buildbots

c196d81

steven-johnson added a commit that referenced this pull request Jun 5, 2023

Fix PCH build failures

ac51807

(Harvested from #7604 to land separately)

steven-johnson mentioned this pull request Jun 5, 2023

Fix PCH build failures #7613

Merged

steven-johnson added a commit that referenced this pull request Jun 6, 2023

Fix PCH build failures (#7613)

123d855

* Fix PCH build failures (Harvested from #7604 to land separately) * Update CMakeLists.txt

Merge branch 'main' into srj/fix-asan

410cd53

alexreinking requested changes Jun 21, 2023

View reviewed changes

alexreinking added the release_notes For changes that may warrant a note in README for official releases. label Jun 21, 2023

alexreinking mentioned this pull request Jun 23, 2023

Use Halide_PYTHON_LAUNCHER to implement ASAN toolchain fixes #7657

Merged

alexreinking and others added 2 commits June 23, 2023 10:47

Use Halide_PYTHON_LAUNCHER to implement ASAN toolchain fixes (#7657)

2bb54ed

* Use new Halide_PYTHON_LAUNCHER to set env vars * Update CMake docs for Halide_SANITIZER_ENV_VARS --------- Co-authored-by: Alex Reinking <areinkin@qti.qualcomm.com>

Merge branch 'main' into srj/fix-asan

9797db2

steven-johnson requested a review from alexreinking June 23, 2023 17:51

alexreinking approved these changes Jun 23, 2023

View reviewed changes

steven-johnson merged commit 2a93cb0 into main Jun 23, 2023

steven-johnson deleted the srj/fix-asan branch June 23, 2023 20:53

BrewTestBot mentioned this pull request Feb 2, 2024

halide 17.0.0 Homebrew/homebrew-core#161602

Closed

ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024

Fix PCH build failures (halide#7613)

e3b303c

* Fix PCH build failures (Harvested from halide#7604 to land separately) * Update CMakeLists.txt

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get the ASAN toolchain working again #7604

Get the ASAN toolchain working again #7604

steven-johnson commented Jun 1, 2023 •

edited

Loading

steven-johnson commented Jun 2, 2023

alexreinking commented Jun 5, 2023

steven-johnson commented Jun 5, 2023

alexreinking commented Jun 20, 2023

steven-johnson commented Jun 20, 2023

alexreinking commented Jun 20, 2023 •

edited

Loading

alexreinking left a comment

alexreinking Jun 3, 2023

alexreinking Jun 21, 2023

alexreinking Jun 21, 2023

alexreinking Jun 21, 2023

steven-johnson commented Jun 23, 2023

alexreinking left a comment

-set(Halide_SANITIZER_ENV_VARS "LD_PRELOAD=${Halide_SHARED_ASAN_RUNTIME_LIBRARY}")
+set(
+ Halide_PYTHON_LAUNCHER
+ ${CMAKE_COMMAND} -E env ASAN_OPTIONS=detect_leaks=0 LD_PRELOAD=${Halide_SHARED_ASAN_RUNTIME_LIBRARY}
+)

	if (Halide_SANITIZER_ENV_VARS)
	list(PREPEND ARG_ENVIRONMENT ${Halide_SANITIZER_ENV_VARS})
	endif ()

Get the ASAN toolchain working again #7604

Get the ASAN toolchain working again #7604

Conversation

steven-johnson commented Jun 1, 2023 • edited Loading

steven-johnson commented Jun 2, 2023

alexreinking commented Jun 5, 2023

steven-johnson commented Jun 5, 2023

alexreinking commented Jun 20, 2023

steven-johnson commented Jun 20, 2023

alexreinking commented Jun 20, 2023 • edited Loading

alexreinking left a comment

Choose a reason for hiding this comment

alexreinking Jun 3, 2023

Choose a reason for hiding this comment

alexreinking Jun 21, 2023

Choose a reason for hiding this comment

alexreinking Jun 21, 2023

Choose a reason for hiding this comment

alexreinking Jun 21, 2023

Choose a reason for hiding this comment

steven-johnson commented Jun 23, 2023

alexreinking left a comment

Choose a reason for hiding this comment

steven-johnson commented Jun 1, 2023 •

edited

Loading

alexreinking commented Jun 20, 2023 •

edited

Loading