This is an archive of the discontinued LLVM Phabricator instance.

[scudo] Switch to use MemMap in tests
ClosedPublic

Authored by Chia-hungDuan on Mar 21 2023, 2:55 PM.

Diff Detail

Event Timeline

Chia-hungDuan created this revision.Mar 21 2023, 2:55 PM
Chia-hungDuan requested review of this revision.Mar 21 2023, 2:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 21 2023, 2:55 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
cferris accepted this revision.Apr 3 2023, 11:16 PM

I notice a lot of casting in the use of the map. Would it make sense to create some helper functions that return void* and also take void*? I don't think it's worth doing here, but something to think about as a follow-up cl.

For example, you could add getMapBasePtr that returns void*, and add another version of remap/map that takes void* and calls the uptr version. Maybe add some others where you would have had to cast the pointers around.

This revision is now accepted and ready to land.Apr 3 2023, 11:16 PM
Chia-hungDuan retitled this revision from [scudo] Switch to use MemMap for tests to [scudo] Switch to use MemMap in tests.Apr 4 2023, 2:44 PM

Remove some reinterpret_cast<>() by using scudo::uptr if possible. In general,
we only need void* when using memset in these tests.

I notice a lot of casting in the use of the map. Would it make sense to create some helper functions that return void* and also take void*? I don't think it's worth doing here, but something to think about as a follow-up cl.

For example, you could add getMapBasePtr that returns void*, and add another version of remap/map that takes void* and calls the uptr version. Maybe add some others where you would have had to cast the pointers around.

In general, we expect most of operations are operating on uptr. It will use void* only when it binds to other system APIs. For example, we use memset in these tests. I revise a little bit so the we try to use uptr as much as possible and which reduces the use of reinterpret_cast. Let me know if it looks more clean.

cferris accepted this revision.Apr 4 2023, 6:06 PM

LGTM

This revision was automatically updated to reflect the committed changes.
Chia-hungDuan added inline comments.Apr 5 2023, 4:57 PM
compiler-rt/lib/scudo/standalone/tests/memtag_test.cpp
49–52

This is a bug. I'll revert this change