Page MenuHomePhabricator

[Asan] Don't use allocator Metadata
Changes PlannedPublic

Authored by vitalybuka on Sep 9 2020, 3:57 AM.

Details

Reviewers
morehouse
Summary

Previously we used Metadata only with the secondary allocator.

Size goes into 48 bits of ChunkHeader.

Pointer to header goes into new field. This let us with confidence identify
initialized headers. This however increase header to 24 bytes.

Now we can find allocation begin from ChunkHeader without call
to allocator.

Remove now unused ChunkHeader fields.

Depends on D87217.

Diff Detail

Event Timeline

vitalybuka created this revision.Sep 9 2020, 3:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2020, 3:57 AM
Herald added subscribers: Restricted Project, jfb. · View Herald Transcript
vitalybuka requested review of this revision.Sep 9 2020, 3:57 AM
vitalybuka planned changes to this revision.Sep 9 2020, 3:58 AM
vitalybuka requested review of this revision.Sep 9 2020, 4:02 AM
vitalybuka updated this revision to Diff 290692.Sep 9 2020, 4:03 AM

remove printf

vitalybuka edited the summary of this revision. (Show Details)
vitalybuka edited the summary of this revision. (Show Details)
vitalybuka edited the summary of this revision. (Show Details)
vitalybuka edited the summary of this revision. (Show Details)Sep 9 2020, 4:52 AM
vitalybuka updated this revision to Diff 290713.Sep 9 2020, 4:53 AM

remove unused fields

vitalybuka edited the summary of this revision. (Show Details)Sep 9 2020, 4:54 AM
vitalybuka edited the summary of this revision. (Show Details)
tejohnson added inline comments.Sep 9 2020, 3:32 PM
compiler-rt/lib/asan/asan_allocator.cpp
565

In trying to figure out how to tweak the heap allocator patch, I'm now confused as to why meta[1] was needed in the first place. We've already saved chunk_beg above in alloc_beg[1] in the case where chunk_beg!=alloc_beg. Why can't we use the same method as for the primary allocator in GetAsanChunk, and avoid the need to save this in the metadata?

compiler-rt/lib/asan/asan_allocator.cpp
565

alloc_beg[0], alloc_beg[1] : we save this for both primary/and secondary, but only allocations from primary will use it

secondary uses only meta.
I don't see good reasons for that as secondary allocation always has enough space left of the used_beg.

In either case these pointers should be set later and as atomics, when AsanChunk is initialized. See SetChunkPtr

BTW. I am going to delay this patch until we have stronger reason to increase header to 24 bytes.
Main problem I tried to solve here is in GetAsanChunk where as-is primary allocation for AsanChunk == alloc_begin (no kAllocBegMagic) can return pointer to uninitialized garbage. And don't see how to reasonable solve this without extending Header.

tejohnson added inline comments.Sep 9 2020, 4:16 PM
compiler-rt/lib/asan/asan_allocator.cpp
565

I don't see good reasons for that as secondary allocation always has enough space left of the used_beg.

Ok thanks for confirming that I'm not completely missing something.

For the heap profiler the possibility of data race that you are trying to solve here is I think less concerning since at the worst the profile is a bit off.

morehouse resigned from this revision.Sep 9 2020, 4:24 PM
vitalybuka planned changes to this revision.Sep 9 2020, 9:48 PM