folly: ASAN-exempt scanHaystackBlock, to avoid an FP buffer overrun

Summary:
scanHaystackBlock may read-overrun the needle.data() buffer by
up to 15 bytes, but that overrun will never cross a page boundary.
The fix is to turn off ASAN-checking for this function, but since
that attribute is accompanied by a "noinline" one (which conflicts
with the function's own "inline"), I have also removed the "inline"
attribute on both decl and defn.  That is a good thing, regardless:
these days, there are very few cases in which we should be trying to
tell the compiler to inline.

Test Plan:
Before, this would elicit an ASAN abort.  Now it passes 100%:

fbconfig --platform-all=gcc-4.8.1-glibc-2.17 --sanitize=address \
folly/test:range_test && fbmake runtests

Reviewed By: philipp@fb.com

FB internal diff: D1162982

diff --git a/folly/Range.cpp b/folly/Range.cpp
index f4055cbba48172cffe21983c4a1fc658325b534a..8a73e805831f6337496de4d308ef628eb1b0d213 100644 (file)
--- a/folly/Range.cpp
+++ b/folly/Range.cpp
@@ -172,20 +172,25 @@ size_t qfind_first_byte_of_byteset(const StringPiece& haystack,
 #if FOLLY_HAVE_EMMINTRIN_H && __GNUC_PREREQ(4, 6)
 
 template <bool HAYSTACK_ALIGNED>
-inline size_t scanHaystackBlock(const StringPiece& haystack,
-                                const StringPiece& needles,
-                                int64_t idx)
+size_t scanHaystackBlock(const StringPiece& haystack,
+                         const StringPiece& needles,
+                         int64_t idx)
 // inline is okay because it's only called from other sse4.2 functions
-  __attribute__ ((__target__("sse4.2")));
+  __attribute__ ((__target__("sse4.2")))
+// Turn off ASAN because the "arr2 = ..." assignment in the loop below reads
+// up to 15 bytes beyond end of the buffer in #needles#.  That is ok because
+// ptr2 is always 16-byte aligned, so the read can never span a page boundary.
+// Also, the extra data that may be read is never actually used.
+  FOLLY_DISABLE_ADDRESS_SANITIZER;
 
 // Scans a 16-byte block of haystack (starting at blockStartIdx) to find first
 // needle. If HAYSTACK_ALIGNED, then haystack must be 16byte aligned.
 // If !HAYSTACK_ALIGNED, then caller must ensure that it is safe to load the
 // block.
 template <bool HAYSTACK_ALIGNED>
-inline size_t scanHaystackBlock(const StringPiece& haystack,
-                                const StringPiece& needles,
-                                int64_t blockStartIdx) {
+size_t scanHaystackBlock(const StringPiece& haystack,
+                         const StringPiece& needles,
+                         int64_t blockStartIdx) {
   DCHECK_GT(needles.size(), 16);  // should handled by *needles16() method
   DCHECK(blockStartIdx + 16 <= haystack.size() ||
          (PAGE_FOR(haystack.data() + blockStartIdx) ==