[PATCH] D108323: [asan] Added -inline-small-callbacks LLVM flag, which would force inline code for 8 and 16 byte data types when otherwise a callback would have been used.

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a comment.

What's the code size implications?



================
Comment at: clang/test/CodeGen/asan-use-callbacks.cpp:15
 
-int deref(int *p) {
+long deref(long *p) {
   return *p;
----------------
As we introduce a difference in behavior for small and large accesses, 
I would extend this test to have 1, 2, 4, and 16-byte accesses. 


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:310
+static cl::opt<bool> ClInlineSmallCallbacks(
+    "asan-inline-small-callbacks",
+    cl::desc("Inline callbacks for 8 and 16 byte types."), cl::Hidden,
----------------
The flag semantics are weird. We first say "use callbacks", then we say "but not for small callbacks". 
Perhaps, instead, introduce a flag asan-short-instrumentation-with-call-threshold
and when present use this flag for 8/16 instead of the asan-instrumentation-with-call-threshold?


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1749
 
+  if (ClInlineSmallCallbacks && AccessSizeIndex > 2) {
+    UseCalls = false;
----------------
perhaps move this logic to where UseCalls is computed initially?
(see my other comment too)


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:1751
+    UseCalls = false;
+  }
+
----------------
for single-statement if bodies this code base does not use {}


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108323/new/

https://reviews.llvm.org/D108323