[PATCH] D19167: [esan] EfficiencySanitizer instrumentation pass

[Filipe Cabecinhas via llvm-commits]

filcab accepted this revision.
filcab added a reviewer: filcab.
filcab added a comment.

My bigger nits are name related, so I'll give you the LGTM now.


================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:59
@@ +58,3 @@
+STATISTIC(NumFastpaths, "Number of instrumented fastpaths");
+STATISTIC(NumAccessesWithBadSize, "Number of accesses with a bad size");
+
----------------
We probably want to change "Bad" to something else. Non-power-of-two-below-or-equal-to-16 doesn't roll off the tongue, though.
How about "irregular", or something like that? Or even "unusual". My problem here is that it's not explanatory.

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:112
@@ +111,3 @@
+  Function *EsanUnalignedStore[NumberOfAccessSizes];
+  Function *EsanUnusualLoad, *EsanUnusualStore;
+  Function *MemmoveFn, *MemcpyFn, *MemsetFn;
----------------
I'd prefer a better explained name, like EsanStoreN (probably with a comment saying we assume it's unaligned, for now?)

================
Comment at: lib/Transforms/Instrumentation/EfficiencySanitizer.cpp:157
@@ +156,3 @@
+  EsanUnusualLoad = checkSanitizerInterfaceFunction(
+      M.getOrInsertFunction("__esan_unusual_load", IRB.getVoidTy(),
+                            IRB.getInt8PtrTy(), IntptrTy, nullptr));
----------------
Same thing, I'd prefer a `__esan_unaligned_store_n` name (or `__esan_unaligned_store_range`, if you're following tsan's names, instead of asan's), like the other sanitizers.


http://reviews.llvm.org/D19167