This is an archive of the discontinued LLVM Phabricator instance.

[HWAsan] Support 4K/8K/16K/64K page size to ring buffer
Needs RevisionPublic

Authored by XiaodongLoong on Apr 17 2022, 5:50 AM.

Details

Summary

The ring buffer size is a multiple of system page size, and now the page size can only be 4K.
It can not work well on other system that has different page size configure.

Depends on D123908

Diff Detail

Event Timeline

XiaodongLoong created this revision.Apr 17 2022, 5:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2022, 5:50 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
XiaodongLoong requested review of this revision.Apr 17 2022, 5:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2022, 5:50 AM

Generally, there is no way to tell the target page size in advance. And there is no need for that, too - just untie this value from the page size instead, and fix the munmap issue in the runtime library by up/down aligning the bounds to the runtime page size. You can also increase the default / minimal ring buffer size at runtime to be a multiple of the page size.

XiaodongLoong edited the summary of this revision. (Show Details)

upate revision for refactor

XiaodongLoong retitled this revision from [HWAsan] Fix the hard code on kernel page size bits to [HWAsan] Support 4K/8K/16K/64K page size to ring buffer.Apr 20 2022, 6:22 AM
XiaodongLoong edited the summary of this revision. (Show Details)

Generally, there is no way to tell the target page size in advance. And there is no need for that, too - just untie this value from the page size instead, and fix the munmap issue in the runtime library by up/down aligning the bounds to the runtime page size. You can also increase the default / minimal ring buffer size at runtime to be a multiple of the page size.

Sorry, my expression may not be accurate at first. I want to support more kind of page size for HWAsan. I add a command option to custom page size in D123908.
I think the runtime can know the real page size by sysconf syscall adaptively.

Thanks!

eugenis requested changes to this revision.Apr 20 2022, 12:52 PM

I still think that my proposal in the comment above is way better than adding an ABI-breaking compiler flag.

This revision now requires changes to proceed.Apr 20 2022, 12:52 PM

I still think that my proposal in the comment above is way better than adding an ABI-breaking compiler flag.

OK, I read the proposal https://github.com/google/sanitizers/issues/1446.

Using pagesize here would be wasteful - ex. on Android we default to a
single 4Kb page for the stack ring buffer. I'd rather replace the mmap in
SavedStackAllocations with InternalAlloc , this should solve the problem.

There is also an efficiency issue in HwasanThreadList::DontNeedThread - if
the buffer is smaller than a page, it will never be able to release
anything. That's a bit harder to fix, and I'm not sure how significant it
actually is.

I'm looking forward to your revision.

Thanks!

Sorry, I did not mean that I was going to implement it.

Sorry, I did not mean that I was going to implement it.

OK, I'll try to implement it as you said.