Page MenuHomePhabricator

[HWASAN/rt] Implement support for memory intrinsics
ClosedPublic

Authored by evgeny777 on Dec 11 2018, 4:54 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

evgeny777 created this revision.Dec 11 2018, 4:54 AM
eugenis added inline comments.Dec 11 2018, 1:11 PM
lib/hwasan/hwasan_interceptors_memintrinsics.cc
44 ↗(On Diff #177686)

Internal versions are very slow. Just call "memcpy" directly.

Check how __hwasan_loadN is implemented. Move these to hwasan.cc and do the same.

evgeny777 updated this revision to Diff 177840.Dec 12 2018, 4:28 AM

Addressed some of review comments
Unfortunately I don't know how to use memset from __hwasan_memset, given this comment in hwasan.cc

// ACHTUNG! No system header includes in this file.

Addressed some of review comments
Unfortunately I don't know how to use memset from __hwasan_memset, given this comment in hwasan.cc

// ACHTUNG! No system header includes in this file.

I think it only matters for interceptors, where we need to redefine libc functions without knowing their exact signature. It should be OK to remove this comment an include <string.h>.

lib/hwasan/hwasan.cc
532 ↗(On Diff #177840)

This way -fsanitize-recover does not apply to mem intrinsics. I think, for consistency, there needs to be a __hwasan_memset_noabort variant, used in instrumentation when !Recover.

kcc added a comment.Dec 12 2018, 3:37 PM

please don't add system headers to this file.
instead, create a new file that will only contain __hwasan_mem* functions,
and sure, include what you need there.

eugenis added inline comments.Dec 13 2018, 3:21 PM
lib/hwasan/hwasan.cc
532 ↗(On Diff #177840)

please also address this comment

evgeny777 marked an inline comment as done.Dec 14 2018, 2:10 AM
evgeny777 added inline comments.
lib/hwasan/hwasan.cc
532 ↗(On Diff #177840)

I noticed that ASAN doesn't have noabort versions, instead mem intrinsic behavior is controlled by halt_on_error flag in runtme. How about doing the same here?

eugenis added inline comments.Dec 14 2018, 10:34 AM
lib/hwasan/hwasan.cc
532 ↗(On Diff #177840)

OK, sure.

eugenis added inline comments.Dec 17 2018, 12:43 PM
lib/hwasan/hwasan_memintrinsics.cc
29 ↗(On Diff #178454)

You don't need this - see HwasanOnSIGTRAP, always using ErrorAction::Recover should have the desired behavior.

evgeny777 marked an inline comment as done.

Addressed comments

eugenis accepted this revision.Dec 18 2018, 1:40 PM

LGTM

This revision is now accepted and ready to land.Dec 18 2018, 1:40 PM
This revision was automatically updated to reflect the committed changes.