[asan] Fix bug where suppression of overlapping accesses was ignored.

[delcypher]

Summary

[asan] Fix bug where suppression of overlapping accesses was ignored on
strcpy(), strncpy(), strcat(), and strncat().

rdar://problem/35576899

[kubamracek]

lib/asan/asan_interceptors_memintrinsics.h

144 ↗

(On Diff #135727)

Can we also check for suppressions based on interceptor name? See ACCESS_MEMORY_RANGE above, which is attempting suppressions both by name and by stacktrace.

test/asan/TestCases/strncpy-overlap.cc

29 ↗

(On Diff #135727)

Since the "Linux" line is matching the Darwin output as well, can we just use the Linux version?

[delcypher]

lib/asan/asan_interceptors_memintrinsics.h

144 ↗

(On Diff #135727)

ACCESS_MEMORY_RANGE has access to a ctx object. Where do I get that from?

test/asan/TestCases/strncpy-overlap.cc

29 ↗

(On Diff #135727)

We could do. I think I copied the regex from the existing in strncpy-overflow.cc test. Is that test a bad example of how to do things?

[kubamracek]

lib/asan/asan_interceptors_memintrinsics.h

144 ↗

(On Diff #135727)

I think we can pass ctx from the caller(s), all the callers already seem to have it. Or we can use name directly here, but actually removing it in favor of ctx sounds better to me.

test/asan/TestCases/strncpy-overlap.cc

29 ↗

(On Diff #135727)

It's not a big deal, but it just seems unnecessary to complicate this test with OS specific checks.

[kubamracek]

test/asan/TestCases/strncpy-overlap.cc

29 ↗

(On Diff #135727)

Thinking more about it, I don't see a good reason why the two OS's should differ in behavior here anyway. At some point, we should probably stop printing the "wrap_" prefix on Darwin (it's just a side effect of how interceptors work on Darwin).

[delcypher]

Support interceptor_name suppression.

[delcypher]

lib/asan/asan_interceptors_memintrinsics.h

144 ↗

(On Diff #135727)

I've added support for doing the suppression by interceptor name. However I've not made the ctx change. That looks like an orthogonal refactor which I do in a separate patch.

[kubamracek]

LGTM, thanks!

[delcypher]

Add tests for strcat(), strncat(), and strcpy().

[delcypher]

In D43702#1030327, @kubamracek wrote:

LGTM, thanks!

I decided that I should probably add tests for the other functions where we do this overlap check. Could you check you're still happy with this patch?

Sidenote: It looks like we are missing interceptors for

• stpcpy()
• strlcpy()
• strlcat()

Fixing that is out of scope for this patch though.

[kubamracek]

Looks great. Thanks!

[alekseyshl]

test/asan/TestCases/strncpy-overlap.cc

23 ↗

(On Diff #137584)

--check-prefix=CHECK is the default, let's just drop it

[delcypher]

Remove superflous --check-prefix command line option.

[delcypher]

@alekseyshl Good to go?

test/asan/TestCases/strncpy-overlap.cc

23 ↗

(On Diff #137584)

Oops. Good catch. I forgot to remove those after re-working the tests.

compiler-rt/trunk/lib/asan/asan_interceptors_memintrinsics.h

 
 // Behavior of functions like "memcpy" or "strcpy" is undefined
 // if memory intervals overlap. We report error in this case.
 // Macro is used to avoid creation of new frames.
 static inline bool RangesOverlap(const char *offset1, uptr length1,
                                  const char *offset2, uptr length2) {
   return !((offset1 + length1 <= offset2) || (offset2 + length2 <= offset1));
 }
-#define CHECK_RANGES_OVERLAP(name, _offset1, length1, _offset2, length2) do { \
-  const char *offset1 = (const char*)_offset1; \
-  const char *offset2 = (const char*)_offset2; \
-  if (RangesOverlap(offset1, length1, offset2, length2)) { \
-    GET_STACK_TRACE_FATAL_HERE; \
-    ReportStringFunctionMemoryRangesOverlap(name, offset1, length1, \
+#define CHECK_RANGES_OVERLAP(name, _offset1, length1, _offset2, length2)   \
+  do {                                                                     \
+    const char *offset1 = (const char *)_offset1;                          \
+    const char *offset2 = (const char *)_offset2;                          \
+    if (RangesOverlap(offset1, length1, offset2, length2)) {               \
+      GET_STACK_TRACE_FATAL_HERE;                                          \
+      bool suppressed = IsInterceptorSuppressed(name);                     \
+      if (!suppressed && HaveStackTraceBasedSuppressions()) {              \
+        suppressed = IsStackTraceSuppressed(&stack);                       \
+      }                                                                    \
+      if (!suppressed) {                                                   \
+        ReportStringFunctionMemoryRangesOverlap(name, offset1, length1,    \
                                                 offset2, length2, &stack); \
-  } \
+      }                                                                    \
+    }                                                                      \
   } while (0)
 
 }  // namespace __asan
 
 #endif  // ASAN_MEMINTRIN_H

compiler-rt/trunk/test/asan/TestCases/strcat-overlap.cc

+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strcat" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O1 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strcat" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O2 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strcat" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O3 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strcat" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+
+#include <string.h>
+
+
+// Don't inline function otherwise stacktrace changes.
+__attribute__((noinline)) void bad_function() {
+  char buffer[] = "hello\0XXX";
+  // CHECK: strcat-param-overlap: memory ranges
+  // CHECK: [{{0x.*,[ ]*0x.*}}) and [{{0x.*,[ ]*0x.*}}) overlap
+  // CHECK: {{#0 0x.* in .*strcat}}
+  // CHECK: {{#1 0x.* in bad_function.*strcat-overlap.cc:}}[[@LINE+2]]
+  // CHECK: {{#2 0x.* in main .*strcat-overlap.cc:}}[[@LINE+5]]
+  strcat(buffer, buffer + 1); // BOOM
+}
+
+int main(int argc, char **argv) {
+  bad_function();
+  return 0;
+}

compiler-rt/trunk/test/asan/TestCases/strcpy-overlap.cc

+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strcpy" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O1 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strcpy" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O2 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strcpy" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O3 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strcpy" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+
+#include <string.h>
+
+
+// Don't inline function otherwise stacktrace changes.
+__attribute__((noinline)) void bad_function() {
+  char buffer[] = "hello";
+  // CHECK: strcpy-param-overlap: memory ranges
+  // CHECK: [{{0x.*,[ ]*0x.*}}) and [{{0x.*,[ ]*0x.*}}) overlap
+  // CHECK: {{#0 0x.* in .*strcpy}}
+  // CHECK: {{#1 0x.* in bad_function.*strcpy-overlap.cc:}}[[@LINE+2]]
+  // CHECK: {{#2 0x.* in main .*strcpy-overlap.cc:}}[[@LINE+5]]
+  strcpy(buffer, buffer + 1); // BOOM
+}
+
+int main(int argc, char **argv) {
+  bad_function();
+  return 0;
+}

compiler-rt/trunk/test/asan/TestCases/strncat-overlap.cc

+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strncat" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O1 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strncat" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O2 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strncat" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O3 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strncat" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+
+#include <string.h>
+
+
+// Don't inline function otherwise stacktrace changes.
+__attribute__((noinline)) void bad_function() {
+  char buffer[] = "hello\0XXX";
+  // CHECK: strncat-param-overlap: memory ranges
+  // CHECK: [{{0x.*,[ ]*0x.*}}) and [{{0x.*,[ ]*0x.*}}) overlap
+  // CHECK: {{#0 0x.* in .*strncat}}
+  // CHECK: {{#1 0x.* in bad_function.*strncat-overlap.cc:}}[[@LINE+2]]
+  // CHECK: {{#2 0x.* in main .*strncat-overlap.cc:}}[[@LINE+5]]
+  strncat(buffer, buffer + 1, 3); // BOOM
+}
+
+int main(int argc, char **argv) {
+  bad_function();
+  return 0;
+}

compiler-rt/trunk/test/asan/TestCases/strncpy-overlap.cc

+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strncpy" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O1 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strncpy" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O2 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strncpy" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+//
+// RUN: %clangxx_asan -O3 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+// RUN: echo "interceptor_via_fun:bad_function" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+// RUN: echo "interceptor_name:strncpy" > %t.supp
+// RUN: %env_asan_opts=suppressions='"%t.supp"' %run %t
+
+#include <string.h>
+
+
+// Don't inline function otherwise stacktrace changes.
+__attribute__((noinline)) void bad_function() {
+  char buffer[] = "hello";
+  // CHECK: strncpy-param-overlap: memory ranges
+  // CHECK: [{{0x.*,[ ]*0x.*}}) and [{{0x.*,[ ]*0x.*}}) overlap
+  // CHECK: {{#0 0x.* in .*strncpy}}
+  // CHECK: {{#1 0x.* in bad_function.*strncpy-overlap.cc:}}[[@LINE+2]]
+  // CHECK: {{#2 0x.* in main .*strncpy-overlap.cc:}}[[@LINE+5]]
+  strncpy(buffer, buffer + 1, 5); // BOOM
+}
+
+int main(int argc, char **argv) {
+  bad_function();
+  return 0;
+}