This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][ASAN] implementation for vfork interceptor for riscv64
ClosedPublic

Authored by EccoTheDolphin on Sep 12 2020, 6:06 PM.

Details

Summary

[5/11] patch series to port ASAN for riscv64

Depends On D87573

Diff Detail

Event Timeline

EccoTheDolphin created this revision.Sep 12 2020, 6:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 12 2020, 6:06 PM
Herald added subscribers: Restricted Project, evandro, luismarques and 26 others. · View Herald Transcript
EccoTheDolphin requested review of this revision.Sep 12 2020, 6:06 PM
smd added a subscriber: smd.Sep 14 2020, 9:38 AM
eugenis accepted this revision.Sep 16 2020, 2:31 PM
eugenis added a subscriber: eugenis.

This looks reasonable, but I'm not very familiar with the ISA.
LGTM

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_vfork_riscv64.inc.S
22

why "??" ?

This revision is now accepted and ready to land.Sep 16 2020, 2:31 PM
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_vfork_riscv64.inc.S
22

This is a remnant from the initial draft. will adjust

comments update

EccoTheDolphin retitled this revision from [RISCV][ASAN] implementation for vfork interceptor to [RISCV][ASAN] implementation for vfork interceptor for riscv64.Sep 20 2020, 11:29 PM
EccoTheDolphin edited the summary of this revision. (Show Details)
EccoTheDolphin marked an inline comment as done.Sep 20 2020, 11:50 PM

addressing comments/rebasing

EccoTheDolphin edited the summary of this revision. (Show Details)Sep 22 2020, 11:39 AM

addressing comments

Can we please just have a riscv version that works for both RV32 and RV64? It should just be a case of changing some ld/sd's to LOAD/STORE macros that are either [ls]w or [ls]d, and the various multiples of 8 to n*__riscv_xlen instead.

compiler-rt/lib/asan/asan_interceptors.h
117

Why is this using SANITIZER_RISCV64 when all the other arches query their compiler-defined macros directly?

Can we please just have a riscv version that works for both RV32 and RV64? It should just be a case of changing some ld/sd's to LOAD/STORE macros that are either [ls]w or [ls]d, and the various multiples of 8 to n*__riscv_xlen instead.

The purpose of this commit series is to make sure sure we have ASAN support functional for RISCV64. I have the setup and a decent passrate. Making sure that some code does work on RV32 requires a dedicated testing environment. I can address RV32 case in a subsequent commit .

EccoTheDolphin added inline comments.Sep 22 2020, 8:23 PM
compiler-rt/lib/asan/asan_interceptors.h
117

This change was requested as part of https://reviews.llvm.org/D87580 review. The motivation is that we don't have a define for RISCV64 rendering the overall condition as:

defined(__riscv) && (__riscv_xlen == 64)

too complex. It's much simpler this way.

This revision was landed with ongoing or failed builds.Sep 22 2020, 10:23 PM
This revision was automatically updated to reflect the committed changes.