Page MenuHomePhabricator

[Sanitizers] Modified __aarch64__ to use the 64 bit version of the allocator.
ClosedPublic

Authored by kstoimenov on Oct 31 2022, 4:42 PM.

Details

Summary

This change will switch SizeClassAllocator32 to SizeClassAllocator64 on ARM. This might potentially affect ARM platforms with 39-bit address space. This addresses issues/703, but unlike D60243 it defaults to 64 bit allocator.

Diff Detail

Event Timeline

kstoimenov created this revision.Oct 31 2022, 4:42 PM
kstoimenov requested review of this revision.Oct 31 2022, 4:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 31 2022, 4:42 PM
Herald added subscribers: Restricted Project, pcwang-thead. · View Herald Transcript
kstoimenov edited the summary of this revision. (Show Details)Oct 31 2022, 4:44 PM
kstoimenov added reviewers: vitalybuka, thurston.

does check-all works?

Can you please include https://github.com/google/sanitizers/issues/703 to git commit and review message

kstoimenov edited the summary of this revision. (Show Details)Oct 31 2022, 5:23 PM
vitalybuka accepted this revision.Oct 31 2022, 5:49 PM
This revision is now accepted and ready to land.Oct 31 2022, 5:49 PM
vitalybuka added inline comments.Oct 31 2022, 5:53 PM
compiler-rt/lib/sanitizer_common/sanitizer_platform.h
287–290

Maybe

I included issues/703 and check-all passes on my machine.

compiler-rt/lib/sanitizer_common/sanitizer_platform.h
287–290

But this would affect SANITIZER_ANDROID, right?

MaskRay accepted this revision.Nov 1 2022, 10:30 AM
MaskRay added a subscriber: MaskRay.

I noticed lsan slowness on aarch64 as well. Thanks for the fix!

I noticed lsan slowness on aarch64 as well. Thanks for the fix!

It's one of required patches, @kstoimenov can upload other patches as well, and link using stack?

vitalybuka added inline comments.
compiler-rt/lib/sanitizer_common/sanitizer_platform.h
287–290

Not on ARM64

I believe android and fuchsia IF here is to override aarch64 parrt you already removing.
now they will fallback into "# else" branch and have 1 because of SANITIZER_WORDSIZE == 64

Some concerns if someone actually care about mips64 or RISCV64 on android/fuchsia, and if existing state is working there at all. @phosek
But I think you should try to simplify that.

kstoimenov added inline comments.Nov 1 2022, 1:47 PM
compiler-rt/lib/sanitizer_common/sanitizer_platform.h
287–290

I will submit as is and will send a patch which simplifies it.

fmayer added a subscriber: fmayer.Nov 1 2022, 4:10 PM

Is it possible that this broke this fuzzer?

https://lab.llvm.org/buildbot/#/builders/240/builds/981/steps/8/logs/stdio

AddressSanitizer: CHECK failed: asan_poisoning.cpp:38 "((AddrIsInMem(addr))) != (0)" (0x0, 0x0) (tid=3685666)
    <empty stack>
vitalybuka reopened this revision.Nov 1 2022, 5:06 PM
This revision is now accepted and ready to land.Nov 1 2022, 5:06 PM