This is an archive of the discontinued LLVM Phabricator instance.

[asan] Fix small X86_64 ShadowOffset for non-default shadow scale
ClosedPublic

Authored by waltl on Oct 31 2017, 1:58 PM.

Details

Summary

The requirement is that shadow memory must be aligned to page
boundaries (4k in this case). Use a closed form equation that always
satisfies this requirement.

Event Timeline

waltl created this revision.Oct 31 2017, 1:58 PM
kosarev added a subscriber: kosarev.Nov 1 2017, 1:38 AM
waltl updated this revision to Diff 122003.Nov 7 2017, 3:43 PM

Miscellaneous cleanups.

waltl updated this revision to Diff 122137.Nov 8 2017, 12:08 PM

Simplify offset computation

vitalybuka added inline comments.Nov 15 2017, 2:27 PM
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
303

@eugenis why do we need 0 here and not just kDefaultShadowScale

537–538

UB of shifting into sign bit?
0xFFFFF000ULL?
Same above.

kcc added inline comments.Nov 15 2017, 2:30 PM
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
537–538

Please don't use constants in the code.

eugenis added inline comments.Nov 15 2017, 2:31 PM
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
303

No idea. Looks strange. Check code history?

vitalybuka added inline comments.Nov 15 2017, 3:50 PM
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
303

it's from the original commit which added entire file. so no explanation

@waltl nothing need to be changed here

537–538

kSmallX86_64ShadowOffset << (Mapping.Scale - kDefaultShadowScale) ?

waltl updated this revision to Diff 123178.Nov 16 2017, 7:30 AM

Address CR comments

waltl marked 3 inline comments as done.Nov 16 2017, 7:34 AM
waltl added inline comments.
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
537–538

This isn't quite right: we'd still need a mask, and the shift value in theory may be negative. Please take a look at the updated code.

vitalybuka accepted this revision.Nov 16 2017, 8:24 AM
This revision is now accepted and ready to land.Nov 16 2017, 8:24 AM
This revision was automatically updated to reflect the committed changes.
waltl marked an inline comment as done.