Page MenuHomePhabricator

[RISC-V][HWASAN] Add runtime support for HWASAN for RISC-V
ClosedPublic

Authored by smd on Aug 6 2022, 2:46 PM.

Details

Summary

[4/9] patch series to port HWASAN for riscv64

Depends On D131341

Diff Detail

Event Timeline

smd created this revision.Aug 6 2022, 2:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2022, 2:46 PM
smd added a project: Restricted Project.Aug 6 2022, 3:06 PM
smd published this revision for review.Aug 7 2022, 3:29 AM
Herald added subscribers: Restricted Project, pcwang-thead. · View Herald TranscriptAug 7 2022, 3:29 AM
smd updated this revision to Diff 450794.Aug 8 2022, 7:06 AM

Updating the patch

vitalybuka added inline comments.Aug 9 2022, 5:53 PM
compiler-rt/lib/hwasan/hwasan.h
171

Could you please to move HwasanTagMismatch change into a separate patch as it's slightly affects all platforms

smd updated this revision to Diff 451466.Aug 10 2022, 7:59 AM

Addressing comments

smd edited the summary of this revision. (Show Details)Aug 10 2022, 8:00 AM
smd added inline comments.Aug 10 2022, 8:10 AM
compiler-rt/lib/hwasan/hwasan.h
171

Fixed, thanks.

vitalybuka accepted this revision.Aug 10 2022, 10:57 AM

LGTM, from sanitizers, but consider adding as blocking reviewer someone with RISCV expertise

This revision is now accepted and ready to land.Aug 10 2022, 10:57 AM
luismarques added inline comments.Aug 10 2022, 11:04 AM
compiler-rt/lib/hwasan/hwasan_checks.h
44

using a register asm variable like in the aarch64 code would avoid the move instruction.

71–72

Ditto.

compiler-rt/lib/hwasan/hwasan_interceptors.cpp
140–165

Nit: \n instead of ;?

luismarques added inline comments.Aug 10 2022, 11:18 AM
compiler-rt/lib/hwasan/hwasan_checks.h
76

I guess it won't really matter in this case but the n constraint should be an I constraint, to constraint it to a signed 12-bit immediate fit for the addiw.

smd updated this revision to Diff 451868.Aug 11 2022, 8:29 AM

Addressing comments

smd added inline comments.Aug 11 2022, 8:32 AM
compiler-rt/lib/hwasan/hwasan_checks.h
44

Fixed, thanks.

71–72

Fixed, thanks

76

Fixed, thanks

compiler-rt/lib/hwasan/hwasan_interceptors.cpp
140–165

Personally I'd go with ; since it's used in every other inline asm. Do you have objections?
Thanks

I didn't notice any other issues but I have no experience with HWASan. Thanks!

compiler-rt/lib/hwasan/hwasan_interceptors.cpp
140–165

No objection.

This revision was automatically updated to reflect the committed changes.