Page MenuHomePhabricator

[compiler-rt][lsan] Share platform allocator settings between ASan and LSan
Needs ReviewPublic

Authored by leonardchan on Apr 14 2022, 2:25 PM.

Details

Summary

This attempts to reland D87795 which refactors the asan allocator to use lsan settings.

This also updates LSan on aarch64 to use the 64-bit allocator instead of the 32-bit one. This depends on D123814 landing first. Ideally, this does not affect which allocator is used for other platforms.

Diff Detail

Unit TestsFailed

TimeTest
1,390 msx64 debian > Clang.utils/update_cc_test_checks::check-globals.test
Script: -- : 'RUN: at line 1'; rm -rf /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/utils/update_cc_test_checks/Output/check-globals.test.tmp && mkdir /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/utils/update_cc_test_checks/Output/check-globals.test.tmp
590 msx64 debian > Clang.utils/update_cc_test_checks::global-hex-value-regex.test
Script: -- : 'RUN: at line 1'; rm -rf /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/utils/update_cc_test_checks/Output/global-hex-value-regex.test.tmp && mkdir /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/utils/update_cc_test_checks/Output/global-hex-value-regex.test.tmp
580 msx64 debian > Clang.utils/update_cc_test_checks::global-value-regex.test
Script: -- : 'RUN: at line 1'; rm -rf /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/utils/update_cc_test_checks/Output/global-value-regex.test.tmp && mkdir /var/lib/buildkite-agent/builds/llvm-project/build/tools/clang/test/utils/update_cc_test_checks/Output/global-value-regex.test.tmp
600 msx64 debian > SanitizerCommon-asan-x86_64-Linux.Linux::decorate_proc_maps.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=address -m64 -funwind-tables -ldl -g /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/decorate_proc_maps.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/sanitizer_common/asan-x86_64-Linux/Linux/Output/decorate_proc_maps.cpp.tmp
450 msx64 debian > SanitizerCommon-lsan-x86_64-Linux.Linux::decorate_proc_maps.cpp
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=leak -m64 -funwind-tables -ldl -g /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/decorate_proc_maps.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/sanitizer_common/lsan-x86_64-Linux/Linux/Output/decorate_proc_maps.cpp.tmp
View Full Test Results (46 Failed)

Event Timeline

leonardchan created this revision.Apr 14 2022, 2:25 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2022, 2:25 PM
leonardchan requested review of this revision.Apr 14 2022, 2:25 PM
Herald added subscribers: Restricted Project, pcwang-thead. · View Herald TranscriptApr 14 2022, 2:25 PM
vitalybuka added inline comments.Apr 18 2022, 9:25 AM
compiler-rt/lib/asan/asan_allocator.h
129–142

i suspect these smaller values has something to do with ability allocate shadow, which is not an issue for lsan

leonardchan added inline comments.Apr 21 2022, 1:20 PM
compiler-rt/lib/asan/asan_allocator.h
129–142

Do you recommend I should remove them?

*ping* any more comments?

*ping* any more comments?

No objections, but some small concerns that it will be annoying to split them again if we need different setups for these sanitizers.

compiler-rt/lib/asan/asan_allocator.h
129–142

we can't just remove them for asan, because there probably was a reason
but now, with shared config, it's behavior change for lsan

compiler-rt/lib/lsan/lsan_common.h
80

SANITIZER_ARM64 and the rest?

compiler-rt/lib/sanitizer_common/sanitizer_platform.h
252–257

this part looks a little bit unrelated, would it be better to have a separate patch?

Can you swticth to stuff like SANITIZER_ARM?

Also maybe it's better to have condition SANITIZER_WORDSIZE != 64 || the rest?

leonardchan added inline comments.Wed, Jun 1, 2:11 PM
compiler-rt/lib/asan/asan_allocator.h
129–142

Oh I see, yeah perhaps we can instead just "unify" the values that are already matching in the asan and lsan configs rather than moving everything.

compiler-rt/lib/sanitizer_common/sanitizer_platform.h
252–257

Uploaded https://reviews.llvm.org/D126825 which instead just tells lsan to use SANITIZER_CAN_USE_ALLOCATOR64 (which should just be a no-op). Then I'll have a followup patch that sets this macro to 1 for aarch64.

leonardchan added inline comments.Thu, Jun 2, 3:22 PM
compiler-rt/lib/asan/asan_allocator.h
129–142

D126927 should address this.