This is an archive of the discontinued LLVM Phabricator instance.

[asan] Implemented flag to emit intrinsics to optimize ASan callbacks.
ClosedPublic

Authored by kstoimenov on Aug 19 2021, 7:57 AM.

Diff Detail

Event Timeline

kstoimenov created this revision.Aug 19 2021, 7:57 AM
kstoimenov requested review of this revision.Aug 19 2021, 7:57 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 19 2021, 7:57 AM

Pulled constants like shadow base, etc out of the intrinsic.

vitalybuka added inline comments.Aug 19 2021, 10:29 AM
llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
151 ↗(On Diff #367500)

Could please please replace Module with targetTriple and pointerSizeInBits.
We don't need entire module for that even if current callers have it.

llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h
90 ↗(On Diff #367500)

I don't see implementation of this one.

And I expected getAddressSanitizerParams here, not in AddressSanitizer.h. Is there a reson to declare it there?

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
566

It would be nice to introduce this function as a separate NFC patch.

vitalybuka added inline comments.Aug 19 2021, 11:14 AM
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
571

Unfortunately we need Kernel as argument, similar to RecoverShift in HWAsan.
Look at CL flags as internal per process constants. So if value is controlled by them then it's OK to assume the same value.
Kernel value however can be set by fronted e.g. from clang and does not match ClEnableKasan.

I propose to have own set of intrinsics for kernel/non-kernel to avoid fancy bit packing or unreadable 1/0 arguments.
Later we can cleunup hwasan as well.

Update before expanding intrinsics.

kstoimenov retitled this revision from [asan] Implemented custom calling convention similar used by HWASan for X86. to [asan] Implemented flag to emit intrinsics to optimize ASan callbacks..Aug 19 2021, 12:03 PM
kstoimenov edited the summary of this revision. (Show Details)

Removed unused encodeMemToShadowInfo.

vitalybuka added inline comments.Aug 19 2021, 1:29 PM
clang/test/CodeGen/asan-use-callbacks.cpp
9

for -mllvm flag we need test under llvm/test not clang.

llvm test needs to be more meaningful, e.g. check precice value passed into intrinsic
and have 3 versions:
... -mllvm -asan-optimize-callbacks=0
... -mllvm -asan-optimize-callbacks=1
... default

vitalybuka added inline comments.Aug 19 2021, 1:34 PM
clang/test/CodeGen/asan-use-callbacks.cpp
9

to clarify
Goal of clang/test/CodeGen/asan-use-callbacks.cpp to test -fsanitize-address-outline-instrumentation which is frontend (clang) flag.
-mllvm are not clang flags, even clang knows how to forward them to llvm.

I believe we will not add corresponding frontend clang flag for asan-optimize-callbacks. We will make it default ON after some testing and benchmarking.

Removed test code which should be added under a different directory.

Flag is not used anywhere?

Moved files from the parent patch which didn't belong there.

This time AddressSanitizer.cpp should be updated for real.

The test is still missing?

Attempt to split AddressSanitizer.cpp.

Tests are still WIP. This is not ready for review yet. I will ping you when it it.

Tests are still WIP. This is not ready for review yet. I will ping you when it it.

There is also "Add Acction..."-> "Plan changes", which will remove it from reviewers dashboards.

kstoimenov planned changes to this revision.Aug 24 2021, 12:24 PM

Thanks!

Updating after pushing D107850.

Upload after submitting the main patch.

Added a test.

Fixed the test.

vitalybuka accepted this revision.Aug 26 2021, 12:39 PM
vitalybuka added inline comments.
llvm/test/Instrumentation/AddressSanitizer/asan-optimize-callbacks.ll
6 ↗(On Diff #368959)

Can you please include negative cases as well?

; RUN: opt < %s -asan -enable-new-pm=0 -asan-instrumentation-with-call-threshold=0 -asan-optimize-callbacks=0 -S | FileCheck %s --implicit-check-not llvm.asan.check.memaccess
; RUN: opt < %s -asan -enable-new-pm=0 -asan-instrumentation-with-call-threshold=0 -asan-optimize-callbacks=0 --asan-kernel -S | FileCheck %s --implicit-check-not llvm.asan.check.memaccess

This revision is now accepted and ready to land.Aug 26 2021, 12:39 PM