This is an archive of the discontinued LLVM Phabricator instance.

[asan] For iOS/AArch64, if the dynamic shadow doesn't fit, restrict the VM space
ClosedPublic

Authored by kubamracek on Jul 6 2017, 4:08 PM.

Details

Summary

On iOS/AArch64, the address space is very limited and has a dynamic maximum address based on the configuration of the device. We're already using a dynamic shadow, and we find a large-enough "gap" in the VM where we place the shadow memory. In some cases and some device configuration, we might not be able to find a large-enough gap: E.g. if the main executable is linked against a large number of libraries that are not part of the system, these libraries can fragment the address space, and this happens before ASan starts initializing.

This patch has a solution, where we have a "backup plan" when we cannot find a large-enough gap: We will restrict the address space (via MmapFixedNoAccess) to a limit, for which the shadow limit will fit.

Diff Detail

Repository
rL LLVM

Event Timeline

kubamracek created this revision.Jul 6 2017, 4:08 PM
kubamracek updated this revision to Diff 105650.Jul 7 2017, 9:09 AM
kubamracek updated this revision to Diff 105748.Jul 8 2017, 3:00 PM
dvyukov edited reviewers, added: alekseyshl, vitalybuka; removed: dvyukov.Jul 10 2017, 12:59 AM
vitalybuka added inline comments.Jul 10 2017, 1:53 PM
lib/asan/asan_linux.cc
80 ↗(On Diff #105748)

Maybe it's better to not define function at all and trigger linking error?

vitalybuka edited edge metadata.Jul 10 2017, 2:06 PM

LGTM

lib/asan/asan_linux.cc
80 ↗(On Diff #105748)

Please ignore this, I see this is called from branch which should not be executed on linux.

alekseyshl added inline comments.Jul 10 2017, 2:17 PM
lib/asan/asan_mac.cc
70 ↗(On Diff #105748)

Why to 1 MB? Is it a Mac requirement?

lib/sanitizer_common/sanitizer_common.h
112 ↗(On Diff #105748)

Why does it have to be in sanitizer_common.h? There's sanitizer_mac.h

lib/sanitizer_common/sanitizer_common_nolibc.cc
41 ↗(On Diff #105748)

I wonder what target fails without this definition?

lib/sanitizer_common/sanitizer_mac.cc
843 ↗(On Diff #105748)

Can mmap fail here?

kubamracek added inline comments.Jul 10 2017, 3:30 PM
lib/asan/asan_mac.cc
70 ↗(On Diff #105748)

Not really, this is purely for convenience and debugging - it's nice to see the memory map nicely aligned. I can drop this (but we'll still need to round down to at least 8*PAGE_SIZE, maybe more).

lib/sanitizer_common/sanitizer_common.h
112 ↗(On Diff #105748)

Right.

lib/sanitizer_common/sanitizer_common_nolibc.cc
41 ↗(On Diff #105748)

Actually, it's safestack.a, which uses sanitizer_common, but links in a nolibc mode.

lib/sanitizer_common/sanitizer_mac.cc
843 ↗(On Diff #105748)

I'll add an assert.

alekseyshl added inline comments.Jul 10 2017, 3:51 PM
lib/asan/asan_mac.cc
70 ↗(On Diff #105748)

No, it's fine, just add a comment that it is arbitrary and purely for the debugging convenience.

lib/sanitizer_common/sanitizer_common_nolibc.cc
41 ↗(On Diff #105748)

I presume it's RestrictMemoryToMaxAddress what needs MmapFixedNoAccess. How about moving it into sanitizer_mac_libcdep.cc?

kubamracek added inline comments.Jul 10 2017, 3:55 PM
lib/sanitizer_common/sanitizer_common_nolibc.cc
41 ↗(On Diff #105748)

sanitizer_mac_libcdep.cc doesn't currently exist... Do you want me to create it?

alekseyshl added inline comments.Jul 10 2017, 5:04 PM
lib/sanitizer_common/sanitizer_common_nolibc.cc
41 ↗(On Diff #105748)

Yes, please do create it, it seems like a cleaner solution than an unreachable function.

sanitizer_mac_libcdep.cc doesn't currently exist... Do you want me to create it?

Yes, please do create it, it seems like a cleaner solution than an unreachable function.

Done.

alekseyshl accepted this revision.Jul 12 2017, 4:09 PM
This revision is now accepted and ready to land.Jul 12 2017, 4:09 PM
This revision was automatically updated to reflect the committed changes.
eugenis added inline comments.Oct 6 2017, 2:39 PM
lib/asan/asan_mac.cc
70 ↗(On Diff #105748)

Why does it have to be aligned to 8*PAGE_SIZE?

eugenis added inline comments.Oct 6 2017, 2:54 PM
lib/asan/asan_mac.cc
70 ↗(On Diff #105748)

No matter, I got it. Sorry for the stupid question.