This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] Implement AddressSanitizer suppressions
ClosedPublic

Authored by kubamracek on Nov 14 2014, 1:15 PM.

Details

Reviewers
samsonov
Summary

This patch adds a issue suppression mechanism into ASan, reusing most of the functionality from TSan and sanitizer_common. Based on recent discussions, being able to suppress reports coming from interceptors sounds like a good idea, especially to be able to keep using libraries that you don't have source code of. Reports coming from memory access instrumentation are not affected. This patch also depends on the refactorings from http://reviews.llvm.org/D6276.

What this patch does is that it introduces 3 new types of suppressions, while keeping the suppression file format the same as TSan (described at https://code.google.com/p/thread-sanitizer/wiki/Suppressions). The new types:

  • asan_interceptor:strcpy … will ignore any reports from the strcpy interceptor, no matter what function calls it
  • asan_function:myfunction … will ignore any report where the function named myfunction is somewhere in the stack trace
  • asan_library:mylibrary … will ignore any report where module named mylibrary is somewhere in the stack trace

There’s no way to specify more than one frame, the suppressions match any frame in the call stack (except asan_interceptor, which only takes a look at the first frame in the call stack, which is the current interceptor). Also note that the asan_library suppression type doesn’t require the symbolizer to work.

The suppression file is set with the suppressions=/path/to/file option in ASAN_OPTIONS, the same way we allow this for TSan.

The patch contains a generic test case and a Darwin-specific testcase.

Related discussions:

Diff Detail

Event Timeline

kubamracek updated this revision to Diff 16249.Nov 14 2014, 1:15 PM
kubamracek retitled this revision from to [compiler-rt] Implement AddressSanitizer suppressions.
kubamracek updated this object.
kubamracek edited the test plan for this revision. (Show Details)
kubamracek added subscribers: Unknown Object (MLST), kcc, samsonov and 2 others.
  1. asan_interceptor: It would be really unfortunate to symbolize a stack trace to learn the name of the interceptor. I mean, ACCESS_MEMORY_RANGE is called inside the interceptor, you should just now the interceptor name at this point. For example, try hacking COMMON_INTERCEPTOR_ENTER to save the name of the interceptor in context. Then you can pass this context down to ASAN_READ_RANGE and eventually to ACCESS_MEMORY_RANGE, and check if the interceptor name is blacklisted there.
  2. asan_library. Have you looked at the neat LibIgnore class that TSan interceptors use to suppress reports from libraries? I don't see why you can't reuse called_from_lib suppression kind for your purposes as well.
  3. asan_function: Hm, this one seems to require symbolization. Is this suppression kind absolutely necessary? Or blacklisting interceptror and/or library would be enough for your immediate purposes?
lib/asan/asan_interceptors.cc
54–64

Please don't. Symbolizing stack trace make take a long time and may not succeed. Also, you don't pass unwinded stack down, which means that you'd have to unwind it once again in __asan_report_error.

I agree with Alexey.
As we discussed previously, suppressions should not be used to suppress errors in instrumented code (just because it's not possible to recover after calling a noreturn version of __asan_report_error).
Therefore we shouldn't allow "asan_function", and "asan_library" should only ignore reports in the interceptors (I suggest to rename it accordingly).

I totally agree that asan_interceptor shouldn't take a stack trace or symbolize anything, and also that we should not use suppressions for instrumented code. We only want suppressions to be used for code that you can't easily modify and recompile, like system or 3rd party libraries, which are always un-instrumented. You're right that the naming of the suppression types should be improved to make it clear that it only affects interceptors.

However, I think having just asan_interceptor and called_from_lib (which only matches the library of the immediate caller) is too coarse-grained. For example, the http://crbug.com/162461 (OpenCL overflow on Mac) was being suppressed by disabling all memcmp, memcpy, memmove, memset interceptors and that disables much more checks than it's necessary and prevents discovery of other bugs. Being able to suppress with just asan_interceptor and called_from_lib sounds like having the same issue: Disabling memcpy interceptor is way too much, just as skipping all checks for the OpenCL library. Also, called_from_lib is only checking the immediate caller, which in a lot of cases could be a general system library, like CoreFoundation, and ignoring this whole library is again an overkill.

I'm not looking for a temporary solution to suppress a specific bug. I'd like to have a generic mechanism that users can use when they hit a bug in code they don't own, while minimizing the impact on detection of other bugs. I know that we want to force users to *fix* bugs instead of suppressing them, but we already have things like replace_intrin=0 or replace_str=0.

I understand the performance concerns and possibly a huge impact of doing symbolication just to be able to match suppressions, but note that I'm not suggesting doing any additional work for the common case, I'm only talking about situations where we were about to crash the program anyway. If I understand correctly, LibIgnore is specially optimized, because TSan uses it for *every* interceptor call, not just when it detects a race. For suppressions that take place just before a report is printed, TSan also symbolizes the stack(s) and then look for suppressions in these.

It also doesn't sound to me that we're *degrading* performance, since again we're only talking about situations where ASan would currently crash the program.

I agree that disabling the interceptor (e.g. memset) for everyone might be too coarse-grained, as you won't sanitize memset in actual code you're testing.

I don't see why disabling all reports from system/third-party library is coarse-grained. Suppose you run your code and see a bug in function foo() from OpenCL, which is a precompiled library you can't fix. You suppress foo(), rerun the code and see a bug in function bar() in OpenCL. Hm. If you can't fix OpenCL anyway, isn't it better to suppress all reports from it completely? However, you're right that called_from_lib may not work for us: some precompiled third_party library might call a code from CoreFoundation, that would call memset() on invalid arguments.

Now, speaking of symbolization. I'm not worried about performance slowdown - sure, the application won't do anything interesting after printing an error report. I'm worried about the wall time needed for printing the report. I *have* seen many cases where ASan wasn't able to finish printing a report because application was killed earlier, and in many cases things got worse because of the expensive symbolization. For your use case we'd have to bite the bullet, but I'd like to reduce the harm:

  1. leave the ability to disable specific interceptor by name, with no symbolization necessary. This would probably be "okay" for many users.
  2. symbolize the stack trace in the interceptor *only* if user has provided a suppression. In this way, we won't have to symbolize the stack trace in the default workflow.

Suppose you run your code and see a bug in function foo() from OpenCL, which is a precompiled library you can't fix. You suppress foo(), rerun the code and see a bug in function bar() in OpenCL. Hm. If you can't fix OpenCL anyway, isn't it better to suppress all reports from it completely?

That absolutely makes sense, but only if we allow suppressions by function name as well, which I was under the impression you were suggesting to remove.

I *have* seen many cases where ASan wasn't able to finish printing a report because application was killed earlier

I see. Do we have an example of that? Or a testcase? Does by any chance the fact that we're doing the suppression only for interceptors make it less likely?

  • leave the ability to disable specific interceptor by name, with no symbolization necessary
  • symbolize the stack trace in the interceptor *only* if user has provided a suppression

That makes sense, I'll post an updated patch.

In D6280#7, @kubabrecka wrote:

Suppose you run your code and see a bug in function foo() from OpenCL, which is a precompiled library you can't fix. You suppress foo(), rerun the code and see a bug in function bar() in OpenCL. Hm. If you can't fix OpenCL anyway, isn't it better to suppress all reports from it completely?

That absolutely makes sense, but only if we allow suppressions by function name as well, which I was under the impression you were suggesting to remove.

I *have* seen many cases where ASan wasn't able to finish printing a report because application was killed earlier

I see. Do we have an example of that? Or a testcase? Does by any chance the fact that we're doing the suppression only for interceptors make it less likely?

Unfortunately no, if we could reproduce this with a simple test case, we would improve the runtime somehow. Also, in our internal ASan setup we use in-process symbolizer (that is, not launch llvm-symbolizer binary and communicate with it via pipe). Symbolizer code (it is essentially a DWARF parser from llvm/lib/DebugInfo) does a lot of work, and a lot of allocations.

  • leave the ability to disable specific interceptor by name, with no symbolization necessary
  • symbolize the stack trace in the interceptor *only* if user has provided a suppression

That makes sense, I'll post an updated patch.

Great, thank you!

kubamracek updated this revision to Diff 16545.Nov 23 2014, 8:55 PM

Second take on AddressSanitizer issue suppressions.

This time:

  • The suppression types are renamed to interceptor_name, interceptor_via_function and interceptor_via_library to make it clear that they only affect interceptors.
  • We provide the interceptor name in the AsanInterceptorContext, similarly to TSan's context.
  • For interceptor_name suppression type we don't take the stacktrace and don't symbolize.
  • For the other two suppression types, we only symbolicate the stack when there is such a suppression present in the suppression file.

There's quite a few changes related to passing the AsanInterceptorContext from all the interceptors, like adding void *ctx to ASan-specific interceptors and passing that to ASAN_READ_RANGE and ASAN_WRITE_RANGE. For __asan_memcpy and related, which are called from instrumented code, I set the context pointer to be NULL, thus ignoring suppressions. I moved the implementation of them to macros, so I didn't need to change the API and so they don't create any additional stack frames.

I didn't reuse LibIgnore and the called_from_lib suppressions, because they only look at the immediate caller, which I don't think is enough for ASan suppressions.

Also, this time I didn't worry about refactoring SymbolizeStack from TSan, but after we agree on how the suppressions should be implemented, I'd be happy to do the proper refactoring so I don't have to repeat the symbolication logic in IsSuppressed.

Added a few more tests, including a test that checks that reports from instrumented code cannot be suppressed.

First part of comments is about SuppressionContext() extension. I think this should go in as a separate change, before we use this functionality in ASan suppressions.

lib/sanitizer_common/sanitizer_suppressions.h
47

Now you can add fast path to SuppressionContext::Match() - if there are no suppressions for a given type, just return false.

49

I'd prefer this to be named HasSuppressonType().

60

I'm slightly worried about default initialization for the array. We try to avoid compiler from inserting memset() into runtime code. Consider zero-initializating the array with the call to internal_memset (it makes sense to move ctor to .cc file in this case).

62

bool has_suppresson_type_[SuppressionTypeCount];

lib/sanitizer_common/tests/sanitizer_suppressions_test.cc
84

Please add unit test for the new public method (SuppressionContext::HasSuppressionType).

samsonov added inline comments.Nov 26 2014, 12:18 PM
lib/asan/asan_interceptors.cc
142

Why do you need to pass VA_ARGS here?

173

s/0/nullptr here and in another similar places.

401

Probably, ASAN_MEMSET_IMPL ?

411

You don't need trailing semicolon here (and in other places), as you use this macro as

ASAN_MEMSET_CTX(...);
426

ditto

lib/asan/asan_suppressions.cc
34

static

63

Please wait till I pass the review and land http://reviews.llvm.org/D6394 - it should make the code easier.

66

GET_STACK_TRACE_FATAL_HERE (we should respect what user told us in common_flags()->fast_unwind_on_fatal.

Also, it seems like the wrong place to fetch the stack trace. The fact that

`IsSuppressed("my_interceptor");`

*unwinds the stack trace* is very surprising and something that is totally unexpected from function declaration. We also have "rule of thumb" in
asan_stack.h: unwind the stack as early as possible. I'd rather split "IsSuppressed" into two methods - suppress by interceptor name, and suppress by stack trace fetched from interceptor. That would compilcate the ACCESS_MEMORY_RANGE macro, but would hopefully make the logic more clear.

There's also an alternative. Instead of calling __asan_report_error directly from that macro, you may introduce

MaybeReportInterceptorError()

or smth. like this. There you can unwind the stack trace (once), then either match suppression and bail out (if there are active suppression), or
proceed to producing an error report and reporting this very same stack trace at hand.

69

You can do the following: first fetch the module name/offset for PC and see if it's suppressed (by SuppressionInterceptorViaLibrary), then symbolize the PC and see if it's suppressed by SuppressionInterceptorViaFunction (you don't need symbolization to fetch the module name). See lsan_common.cc.

84

You don't really want to report this every time. I can imaging library function doing invalid strlen() call (e.g. on the same data) hundreds of times.

lib/asan/asan_suppressions.h
24

See below - you might want to split this to "suppressed by interceptor name" and "suppressed by stack trace".

In D6280#11, @samsonov wrote:

First part of comments is about SuppressionContext() extension. I think this should go in as a separate change, before we use this functionality in ASan suppressions.

Uploaded in a separate patch: http://reviews.llvm.org/D6443

kubamracek updated this revision to Diff 16714.Nov 27 2014, 9:41 PM

Addressing review comments. Moved the sanitizer_suppressions changes into a separate patch (http://reviews.llvm.org/D6443), also used the not-yet-committed Symbolizer::SymbolizePC refactoring (http://reviews.llvm.org/D6394).

The suppression method is now split into two, so we're taking the stackshot in the actual interceptors (and not inside the suppression mechanism) and passing it to the second method. Let me know if you feel the other alternative (something like MaybeReportInterceptorError()) would be a better solution. I also removed the reporting about matched suppressions. Do you think we should keep it and have it print only under verbosity>=1?

Thanks for the review!

samsonov added inline comments.Dec 1 2014, 7:39 PM
lib/asan/asan_interceptors.cc
60

Please fix this so that you wouldn't need to unwind the stack trace if suppressions of relevant kinds are missing.

129

Consider moving this decl above, as it's used above in ACCESS_MEMORY_RANGE macro.

lib/asan/asan_suppressions.cc
19

Do you really need all these headers?

25

Consider checking this flag in Is<..>Suppressed() functions.

kubamracek updated this revision to Diff 16812.Dec 2 2014, 8:53 AM

Addressing review comments.

samsonov edited edge metadata.Dec 4 2014, 1:53 PM

Looks good, I'm mostly happy with the code itself, moving to comments about the test cases.

lib/asan/asan_suppressions.cc
43

Please run the patch through clang-format

56

I think you can define this variable outside of for-loop.

62

Collapse these two if's into a single one with &&

72

Note that function_name can be nullptr here. It's okay, and SuppressionContext::Match() will work correctly and return false, though. Leaving this up to you.

lib/asan/asan_suppressions.h
26

const StackTrace *stack

test/asan/TestCases/Darwin/suppressions-darwin.cc
7

Isn't %tmp expanded to /some/path/to/filemp ? Consider using %t.supp instead, so that file (if we go look for it in the test directory) has a sane name (here and below)

15

Use

ASAN_OPTIONS=suppressions=%t.supp:symbolize=false

instead. Also, you may want to check that suppress-by-interceptor-name also works w/o symbolizer.

22

use 80 cols for this line

23

Do you need to free() the buffer here?

29

This CHECK-IGNORE-NOT piece should go before CHECK-IGNORE above (here and below)

test/asan/TestCases/suppressions-function.cc
3

Can you test it on higher optimization levels? Suppression functionality should work there.

test/asan/TestCases/suppressions-instrumented-code.cc
2

Wait, how do you enforce that? By assuming that memset() in user code will be turned into __asan_memset() call? In fact, I see little harm in suppressing reports from memset() from instrumented code if the user explicitly added interceptor_name:memset to suppression file. After all, this would work for another interceptors (like strcpy), won't it?

4

Also add memcpy/memmove here.

kubamracek updated this revision to Diff 16964.Dec 4 2014, 4:16 PM
kubamracek edited edge metadata.

Addressing review comments.

// Check that reports from instrumented code cannot be suppressed.

Wait, how do you enforce that? By assuming that memset() in user code will be
turned into __asan_memset() call? In fact, I see little harm in suppressing
reports from memset() from instrumented code if the user explicitly added
interceptor_name:memset to suppression file. After all, this would work for
another interceptors (like strcpy), won't it?

I meant memset/memcpy/memmove only. Since these can easily be inlined, the suppression will not work reliably on them, and so it sounds like a good idea to just disallow suppressions for these functions (when used from instrumented code).

earthdok added inline comments.
lib/sanitizer_common/sanitizer_suppressions.cc
28

My $.02: we have "called_from_lib" and "interceptor_via_library". We also have "interceptor_via_function" here and "fun" in the blacklist syntax. Maybe use "lib" and "fun" everywhere for consistency?

In D6280#21, @kubabrecka wrote:

Addressing review comments.

// Check that reports from instrumented code cannot be suppressed.

Wait, how do you enforce that? By assuming that memset() in user code will be
turned into __asan_memset() call? In fact, I see little harm in suppressing
reports from memset() from instrumented code if the user explicitly added
interceptor_name:memset to suppression file. After all, this would work for
another interceptors (like strcpy), won't it?

I meant memset/memcpy/memmove only. Since these can easily be inlined, the suppression will not work reliably on them, and so it sounds like a good idea to just disallow suppressions for these functions (when used from instrumented code).

But wait, how do you *enforce* that suppressions don't kick in if these functions are called from instrumented code? IIRC if the user writes memset() and we build it with Clang, it may or may not be turned into intrinsic and replaced with __asan_memfoo().

earthdok@: I really have no preference here. If you feel strong about consistency and sticking with abbreviated name - feel free to enforce it :)

test/asan/TestCases/suppressions-function.cc
9

just -O0 and -O3

In D6280#25, @samsonov wrote:

But wait, how do you *enforce* that suppressions don't kick in if these functions are called from instrumented code? IIRC if the user writes memset() and we build it with Clang, it may or may not be turned into intrinsic and replaced with __asan_memfoo().

Oh, I see what you mean. I thought it was about intrinsic vs. inlined, but it's about intrinsic vs. regular function call. Ok, I think I'll change the test case to just check for direct memory accesses.

lib/sanitizer_common/sanitizer_suppressions.cc
28

I don't necessarily disagree, but I'd like to point out that they work differently:

  • interceptor_via_library ... suppresses interceptors only and matches library anywhere on the stack
  • interceptor_via_function ... suppresses interceptors only and matches a function name anywhere on the stack
  • called_from_lib ... also interceptors only, but only checks the topmost frame on the stack

Also, we should keep the "interceptor" in the name to indicate that these suppressions work only on interceptors, and not instrumented code (other suppression types work even on instrumented code).

In D6280#26, @kubabrecka wrote:
In D6280#25, @samsonov wrote:

But wait, how do you *enforce* that suppressions don't kick in if these functions are called from instrumented code? IIRC if the user writes memset() and we build it with Clang, it may or may not be turned into intrinsic and replaced with __asan_memfoo().

Oh, I see what you mean. I thought it was about intrinsic vs. inlined, but it's about intrinsic vs. regular function call. Ok, I think I'll change the test case to just check for direct memory accesses.

FWIW, I'd just delete this test case.

earthdok added inline comments.Dec 4 2014, 5:09 PM
lib/sanitizer_common/sanitizer_suppressions.cc
28

Sorry, I could have been clearer. I suggest to name them "interceptor_via_lib" and "interceptor_via_fun" (not simply "lib" and "fun").

kubamracek updated this revision to Diff 16970.Dec 4 2014, 5:09 PM

Addressing comments.

kubamracek updated this revision to Diff 16971.Dec 4 2014, 5:18 PM

Sorry, I could have been clearer. I suggest to name them "interceptor_via_lib" and "interceptor_via_fun" (not simply "lib" and "fun").

I see, that makes sense. Renaming to "interceptor_via_lib" and "interceptor_via_fun".

LGTM. Please watch the bots after the commit. Thank you for working on this and for surviving the nitpicky review!

samsonov accepted this revision.Dec 4 2014, 6:19 PM
samsonov edited edge metadata.
This revision is now accepted and ready to land.Dec 4 2014, 6:19 PM
kubamracek updated this revision to Diff 16989.Dec 5 2014, 10:46 AM
kubamracek edited edge metadata.

I have tried this patch on Linux and I've seen this test failure:

FAIL: AddressSanitizer64 :: TestCases/strncpy-overflow.cc (1 of 1)
...
/tmp/llvm/projects/compiler-rt/test/asan/TestCases/strncpy-overflow.cc:20:12: error: expected string not found in input
// CHECK: {{ #1 0x.* in main .*strncpy-overflow.cc:}}[[@LINE-4]]

The reason for that is that the printed stack trace looks like this:

#0 0x44caf2 in strncpy /tmp/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:637
#1 0x44cd31 in strncpy /tmp/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:637
#2 0x4cdfdb in main (/tmp/llvm-cmake-debug/projects/compiler-rt/test/asan/64bitConfig/TestCases/Output/strncpy-overflow.cc.tmp+0x4cdfdb)
#3 0x7f05b89eee64  (/lib64/libc.so.6+0x24e64)
#4 0x417b08 in _start (/tmp/llvm-cmake-debug/projects/compiler-rt/test/asan/64bitConfig/TestCases/Output/strncpy-overflow.cc.tmp+0x417b08)

The doubled strncpy is the issue. To fix that, I moved the GET_CURRENT_PC_BP_SP line in ACCESS_MEMORY_RANGE right to the call to __asan_report_error.

I'm also seeing another test failure for LSan suppressions when ASan is used together with LSan:

FAIL: LeakSanitizer-AddressSanitizer :: TestCases/suppressions_file.cc (1 of 1)
...
/tmp/llvm/projects/compiler-rt/test/lsan/TestCases/suppressions_file.cc:24:11: error: expected string not found in input
// CHECK: Suppressions used:
          ^
<stdin>:1:1: note: scanning from here
Test alloc: 0x61700000fc80.
^
<stdin>:9:73: note: possible intended match here
 #1 0x4cc625 in main /tmp/llvm/projects/compiler-rt/test/lsan/TestCases/suppressions_file.cc:20:13

It looks like that in ASan+LSan, we only have one "suppressions" flag, and once the SuppressionContext is initialized, it only reads the common_flags()->suppressions once. Should we support having suppressions work for both LSan and ASan at the same time? I.e. running

ASAN_OPTIONS=suppressions=file1 LSAN_OPTIONS=suppressions=file2 ./a.out

In that case we should probably remove the "suppressions" flag from common_flags and have it separately in ASan, LSan, TSan.

Alexey, what do you think?

In D6280#36, @kubabrecka wrote:

I have tried this patch on Linux and I've seen this test failure:

FAIL: AddressSanitizer64 :: TestCases/strncpy-overflow.cc (1 of 1)
...
/tmp/llvm/projects/compiler-rt/test/asan/TestCases/strncpy-overflow.cc:20:12: error: expected string not found in input
// CHECK: {{ #1 0x.* in main .*strncpy-overflow.cc:}}[[@LINE-4]]

The reason for that is that the printed stack trace looks like this:

#0 0x44caf2 in strncpy /tmp/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:637
#1 0x44cd31 in strncpy /tmp/llvm/projects/compiler-rt/lib/asan/asan_interceptors.cc:637
#2 0x4cdfdb in main (/tmp/llvm-cmake-debug/projects/compiler-rt/test/asan/64bitConfig/TestCases/Output/strncpy-overflow.cc.tmp+0x4cdfdb)
#3 0x7f05b89eee64  (/lib64/libc.so.6+0x24e64)
#4 0x417b08 in _start (/tmp/llvm-cmake-debug/projects/compiler-rt/test/asan/64bitConfig/TestCases/Output/strncpy-overflow.cc.tmp+0x417b08)

The doubled strncpy is the issue. To fix that, I moved the GET_CURRENT_PC_BP_SP line in ACCESS_MEMORY_RANGE right to the call to __asan_report_error.

This change is fine. I've applied and tested this patch, and uncovered another nasty problem with this test case:

LeakSanitizer-AddressSanitizer :: TestCases/suppressions_file.cc

Here's the problem: if you use LSan+ASan we still want to match suppressions specified in LSAN_OPTIONS. But after you change we have the following behavior:

  1. __asan_init is called, it parses ASAN_OPTIONS (common_flags()->suppressions is empty)
  2. __asan_init calls SuppressionContext::Init(), and SuppressionContext singleton is initialized to be empty
  3. __lsan::InitCommonLsan() parses LSAN_OPTIONS (common_flags()->suppressions now points to LSan suppression file).
  4. __lsan::InitCommonLsan() calls SuppressionContext::Init(), but it's already initialized, so we do nothing.

Now, this immediate problem can be "fixed" by moving the call to InitializeSuppressions() at the bottom, right before we print "AddressSanitizer Init done" string.
I think that we should probably do that and land your patch. But the situation is terrible - our initialization code is complicated and fragile. Currently ASan can be
(and is) combined with both LSan and UBSan, each of them have their own init logic, and can use XSAN_OPTIONS to setup and override common flags. Sigh.
I think we should put some thought into it and redesign that part soon.

kubamracek updated this revision to Diff 16991.Dec 5 2014, 12:05 PM

Moving the ASan suppression initialization after all other initializations (namely lsan's init to avoid overriding its suppression setting).

OK, let's proceed. Hope I'd be able to fix initialization mayhem soon.

kubamracek closed this revision.Dec 5 2014, 12:27 PM

Landed in r223508. Thanks for review!

The Android buildbot was failing (http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/14431), because the tests can't find the suppression files on the device. Marked them as XFAIL: android in r223540.