This is an archive of the discontinued LLVM Phabricator instance.

[HWASan][NFC] Introduce constants for tag bits and masks.
ClosedPublic

Authored by morehouse on Mar 5 2021, 12:58 PM.

Details

Summary

x86_64 aliasing mode will use fewer than 8 bits for tags, so refactor
existing code to remove hard-coded 0xff and 8 values.

Also introduces kAddressUntagMask, which in aliasing mode will only
untag the alias bits and not fixed bits in the tag.

Diff Detail

Event Timeline

morehouse created this revision.Mar 5 2021, 12:58 PM
morehouse requested review of this revision.Mar 5 2021, 12:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2021, 12:58 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
vitalybuka added inline comments.Mar 5 2021, 1:04 PM
compiler-rt/lib/hwasan/hwasan.h
43

should this be kTagMask for consistency?

vitalybuka added inline comments.Mar 5 2021, 1:10 PM
compiler-rt/lib/hwasan/hwasan.h
40–47

If we keep this as constexpr, we likely will not be able to control these at runtime.
As-is it does not matter, but we may have to move it into flags later.

morehouse updated this revision to Diff 328633.Mar 5 2021, 1:33 PM
morehouse marked an inline comment as done.
  • Add clarifying comments.
compiler-rt/lib/hwasan/hwasan.h
40–47

Yes. For now I prefer to keep it simple. We can make it a flag in a later change.

43

I think it's best to keep the current naming. Added some comments for clarity.

vitalybuka added inline comments.Mar 5 2021, 2:41 PM
compiler-rt/lib/hwasan/hwasan.h
47

Sorry, I asked about the following

morehouse marked an inline comment as done.Mar 5 2021, 2:49 PM
morehouse added inline comments.
compiler-rt/lib/hwasan/hwasan.h
47

That works for the current implementation, but it won't work when we introduce x86 aliasing mode.

Then we'll still want to extract all 8 bits when checking the tag in memory, but we only want to mask out 3 bits when untagging the pointer. This is why I left the 0xFF here.

vitalybuka accepted this revision.Mar 5 2021, 3:04 PM
This revision is now accepted and ready to land.Mar 5 2021, 3:04 PM
eugenis accepted this revision.Mar 8 2021, 12:00 PM

LGTM

morehouse marked an inline comment as done.
  • Remove kAddressUntagMask. It's unneeded with short tags.
  • Add num_bits param to GenerateRandomTag() for use in aliasing mode free().

@eugenis @vitalybuka I've made some changes to this patch to accommodate changes in the following patches. Please take another look.

eugenis accepted this revision.Mar 17 2021, 2:26 PM

LGTM