Add end-to-end tests for overflows of byval arguments.

[morehouse]

Summary

Included is one test for passing structs by value and one test for passing C++
objects by value.

[eugenis]

test/asan/TestCases/pass-object-byval.cc

7 ↗

(On Diff #104714)

But the pointer seems unused in the test. Is it possible to replace the IR check with smth like assert(a->me == &a) ?

[vitalybuka]

Probably we need test for UAR as well

A* f(A a) {

return &a;

}

viod b() {

A* a = f(A());
a->  // should likely crash with UAR and pass without it

}

test/asan/TestCases/pass-struct-byval.cc

1 ↗

(On Diff #104714)

what is going to happen with -O1?

[eugenis]

test/asan/TestCases/pass-struct-byval.cc

1 ↗

(On Diff #104714)

The code in bar() will be optimized to undef, I think.
"int * volatile ptr" should protect against that.

10 ↗

(On Diff #104714)

(int *) cast seems unnecessary. (ptr - 1) is already that.

[morehouse]

Add test for UAR. Remove unnecessary casts and make some pointers volatile.

[eugenis]

compiler-rt/test/asan/TestCases/pass-object-byval.cc

4 ↗

(On Diff #105350)

--implicit-check-not

Nice!

compiler-rt/test/asan/TestCases/pass-struct-byval-uar.cc

28 ↗

(On Diff #105350)

Don't need that many -NOT checks. This would be enough:
// CHECK-NO-UAR-NOT: ERROR: AddressSanitizer

Also I'm not sure the NO-UAR case is necessary at all. You are effectively testing undefined behavior.

[morehouse]

Interesting development. It looks like on Android the IR produced doesn't use byval arguments for the pass-struct-byval-uar.cc test. As a result, foo() has no allocas to poison on return.

[morehouse]

The byval attribute is avoided and instead the caller produces a copy of the struct and passes a pointer to it. So foo() can't do any poisoning to its argument for UAR, and instead main() would have to do it. However, ASAN currently does not handle this case. I would suspect that most of the test failures on other architectures are being caused by this same issue.

[vitalybuka]

In D34827#807318, @morehouse wrote:

The byval attribute is avoided and instead the caller produces a copy of the struct and passes a pointer to it. So foo() can't do any poisoning to its argument for UAR, and instead main() would have to do it. However, ASAN currently does not handle this case. I would suspect that most of the test failures on other architectures are being caused by this same issue.

In this case I'd expect compiler creates llvm.lifetime.start/end and this detected as UAS bug

[morehouse]

In this case I'd expect compiler creates llvm.lifetime.start/end and this detected as UAS bug

It looks like llvm.lifetime.start/end are set, but they do not encompass the proper lifetime of the copy. Thus even when compiling with -fsanitize-use-after-scope, UAS is not detected.

[morehouse]

Mark pass-struct-byval-uar.cc as unsupported.

[morehouse]

Change UNSUPPORTED option to REQUIRES. Test now works on x86_64 Linux and shouldn't cause the Android/ARM/Windows buildbots to fail.

[morehouse]

Fix bad diff update.

[morehouse]

@vitalybuka If this looks good, could you land this today since I don't yet have commit access?

[morehouse]

@vitalybuka Does the latest diff look good? Just got commit access so I can land this if you approve.

compiler-rt/trunk/compiler-rt/test/asan/TestCases/pass-object-byval.cc

+// Verify that objects passed by value get red zones and that the copy
+// constructor is called.
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s --implicit-check-not \
+// RUN:     Assertion{{.*}}failed
+#include <cassert>
+
+class A {
+ public:
+  A() : me(this) {}
+  A(const A &other) : me(this) {
+    for (int i = 0; i < 8; ++i) a[i] = other.a[i];
+  }
+
+  int a[8];
+  A *me;
+};
+
+int bar(A *a) {
+  int *volatile ptr = &a->a[0];
+  return *(ptr - 1);
+}
+
+void foo(A a) {
+  assert(a.me == &a);
+  bar(&a);
+}
+
+int main() {
+  A a;
+  foo(a);
+}
+
+// CHECK: ERROR: AddressSanitizer: stack-buffer-overflow
+// CHECK: READ of size 4 at
+// CHECK: is located in stack of thread

compiler-rt/trunk/compiler-rt/test/asan/TestCases/pass-struct-byval-uar.cc

+// Test that use-after-return works with arguments passed by value.
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: %env_asan_opts=detect_stack_use_after_return=0 %run %t 2>&1 | \
+// RUN:     FileCheck --check-prefix=CHECK-NO-UAR %s
+// RUN: not %env_asan_opts=detect_stack_use_after_return=1 %run %t 2>&1 | \
+// RUN:     FileCheck --check-prefix=CHECK-UAR %s
+//
+// On several architectures, the IR does not use byval arguments for foo() and
+// instead creates a copy in main() and gives foo() a pointer to the copy.  In
+// that case, ASAN has nothing to poison on return from foo() and will not
+// detect the UAR.
+// REQUIRES: x86_64-target-arch, linux, not-android
+
+#include <cstdio>
+
+struct A {
+  int a[8];
+};
+
+A *foo(A a) {
+  return &a;
+}
+
+int main() {
+  A *a = foo(A());
+  a->a[0] = 7;
+  std::fprintf(stderr, "\n");  // Ensures some output is generated for FileCheck
+                               // to verify in the case where UAR is not
+                               // detected.
+}
+
+// CHECK-NO-UAR-NOT: ERROR: AddressSanitizer: stack-use-after-return
+// CHECK-NO-UAR-NOT: WRITE of size 4 at
+// CHECK-NO-UAR-NOT: Memory access at offset {{[0-9]+}} is inside this variable
+//
+// CHECK-UAR: ERROR: AddressSanitizer: stack-use-after-return
+// CHECK-UAR: WRITE of size 4 at
+// CHECK-UAR: Memory access at offset {{[0-9]+}} is inside this variable

compiler-rt/trunk/compiler-rt/test/asan/TestCases/pass-struct-byval.cc

+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+struct A {
+  int a[8];
+};
+
+int bar(A *a) {
+  int *volatile ptr = &a->a[0];
+  return *(ptr - 1);
+}
+
+void foo(A a) {
+  bar(&a);
+}
+
+int main() {
+  foo(A());
+}
+
+// CHECK: ERROR: AddressSanitizer: stack-buffer-underflow
+// CHECK: READ of size 4 at
+// CHECK: is located in stack of thread