This is an archive of the discontinued LLVM Phabricator instance.

compiler-rt: decommit asan memory for unmaps in fuchsia.
ClosedPublic

Authored by charco on May 20 2020, 9:27 PM.

Details

Summary

This CL allows asan allocator in fuchsia to decommit shadow memory
for memory allocated using mmap.

Big allocations in asan end up being allocated via mmap and freed with
munmap. However, when that memory is freed, asan returns the
corresponding shadow memory back to the OS via a call to
ReleaseMemoryPagesToOs.

In fuchsia, ReleaseMemoryPagesToOs is a no-op: to be able to free
memory back to the OS, you have to hold a handle to the vmo you want to
modify, which is tricky at the ReleaseMemoryPagesToOs level as that
function is not exclusively used for shadow memory.

The function __sanitizer_fill_shadow fills a given shadow memory range
with a specific value, and if that value is 0 (unpoison) and the memory
range is bigger than a threshold parameter, it will decommit that memory
if it is all zeroes.

This CL modifies the FlushUnneededASanShadowMemory function in
asan_poisoning.cpp to add a call to __sanitizer_fill_shadow with
value and threshold = 0. This way, all the unneeded shadow memory gets
returned back to the OS.

A test for this behavior can be found in fxrev.dev/391974

Diff Detail

Event Timeline

charco created this revision.May 20 2020, 9:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2020, 9:27 PM
Herald added subscribers: Restricted Project, dberris. · View Herald Transcript
mcgrathr added inline comments.May 20 2020, 10:11 PM
compiler-rt/lib/asan/asan_poisoning.cpp
70

It would more consistent with how most of the OS support is handled to define this function separately in OS-specific files instead. I don't have strong opinions about separate files vs #if in this codebase, but its maintainers might.

If we're going to use #if let's put the other call in the #else branch instead of commenting that it's a no-op.
There's no reason to think it should necessarily be a no-op forever if it has other uses. The reason to call Fuchsia's API here is that Fuchsia offers an optimal and explicit API for doing exactly what needs to be done here, not that the way it's implemented on other systems hasn't been implemented for Fuchsia.

charco updated this revision to Diff 265531.May 21 2020, 9:54 AM

Move FlushUnneededASanShadowMemory into asan_fuchsia.cpp

charco marked an inline comment as done.May 21 2020, 9:58 AM
charco added inline comments.
compiler-rt/lib/asan/asan_poisoning.cpp
70

Moved the code to asan_fuchsia.cpp.

vitalybuka added inline comments.Jun 23 2020, 5:08 PM
compiler-rt/lib/asan/asan_fuchsia.cpp
199 ↗(On Diff #265531)

Can you override empty ReleaseMemoryPagesToOS?

charco marked an inline comment as done.Jun 23 2020, 7:06 PM
charco added inline comments.
compiler-rt/lib/asan/asan_fuchsia.cpp
199 ↗(On Diff #265531)

In Fuchsia, you can't return memory back to the OS if you don't have the corresponding handle of the object that you are releasing. We only hold the handle to the shadow VMO, so "ReleaseMemoryPagesToOS" would only work with the shadow memory.

If we use sanitizer_fill_shadow in ReleaseMemoryPagesToOs, we would need to go back from asan shadow memory to regular memory so we can call sanitizer_fill_shadow.

vitalybuka added inline comments.Jun 23 2020, 10:15 PM
compiler-rt/lib/asan/asan_poisoning.cpp
65

Can you do that without #if
like AsanApplyToGlobals

charco updated this revision to Diff 273590.Jun 25 2020, 9:55 PM

Create platform-specific versions of FlushUnneededASanShadowMemory.

do you need empty version in _fuchsia.cpp ?

charco updated this revision to Diff 273757.Jun 26 2020, 9:18 AM
  • Move FlushUnneededASanShadowMemory into asan_fuchsia.cpp
  • Fork FlushUnneededASanShadowMemory for all platforms.

do you need empty version in _fuchsia.cpp ?

I'm not very good at using arcanist/phabricator. I think I screwed up in the last arc diff, but I think I fixed it now.

I'm not sure what the harbormaster errors are about, is there a way to see the error logs?

vitalybuka accepted this revision.Jul 6 2020, 10:45 PM
This revision is now accepted and ready to land.Jul 6 2020, 10:45 PM

Do you need someone to land it for you?

Do you need someone to land it for you?

I have access, I'm trying to understand why it is failing the build... Harbormaster says that it is failing to apply the patch, even though I rebased it recently.

charco updated this revision to Diff 277589.Jul 13 2020, 3:22 PM

Rebase... Third try.

protoent.cpp?
it's definitely unrelated

charco updated this revision to Diff 279355.Jul 20 2020, 3:25 PM
  • Move FlushUnneededASanShadowMemory into asan_fuchsia.cpp
  • Fork FlushUnneededASanShadowMemory for all platforms.
This revision was automatically updated to reflect the committed changes.