-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[compiler-rt][asan][tests] Stabilize wchar tests on Darwin/Android #161624
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Android: Force ASan reports to stderr via %env_asan_opts=log_to_stderr=1 to avoid the ERROR header going to logcat. Keep pre-crash marker on stderr and flush to prevent stdout/stderr reordering. Refs: Android buildbot llvm#186/12821: https://lab.llvm.org/buildbot/#/builders/186/builds/12821 Darwin: Relax stack-frame check to only require the function name (wcscpy/wcsncpy/wcscat/wcsncat). Refs: chromium issue 448631142: https://g-issues.chromium.org/issues/448631142 Common: Print "Good so far." to stderr and fflush(stderr); reuse FileCheck var [[ADDR]]; use const wchar_t* to silence -Wwritable-strings. NFC: test-only. Signed-off-by: Yixuan Cao <caoyixuan2019@email.szu.edu.cn>
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Yixuan Cao (Cao-Wuhui) ChangesSummaryStabilize ASan wchar tests across Darwin and Android. NFC: test-only. Follow-up to PR #160493 (adds wchar interceptors/tests). Motivation
Changes
Risk
ReferencesFull diff: https://github.com/llvm/llvm-project/pull/161624.diff 4 Files Affected:
diff --git a/compiler-rt/test/asan/TestCases/wcscat.cpp b/compiler-rt/test/asan/TestCases/wcscat.cpp
index dcdff88c18ef1..f0a8ec12580b3 100644
--- a/compiler-rt/test/asan/TestCases/wcscat.cpp
+++ b/compiler-rt/test/asan/TestCases/wcscat.cpp
@@ -1,26 +1,26 @@
-// RUN: %clangxx_asan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O1 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O0 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O1 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O2 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O3 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
#include <stdio.h>
#include <wchar.h>
int main() {
- wchar_t *start = L"X means ";
- wchar_t *append = L"dog";
+ const wchar_t *start = L"X means ";
+ const wchar_t *append = L"dog";
wchar_t goodDst[12];
wcscpy(goodDst, start);
wcscat(goodDst, append);
wchar_t badDst[9];
wcscpy(badDst, start);
- printf("Good so far.\n");
+ fprintf(stderr, "Good so far.\n");
// CHECK: Good so far.
- fflush(stdout);
+ fflush(stderr);
wcscat(badDst, append); // Boom!
// CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address [[ADDR:0x[0-9a-f]+]] at pc {{0x[0-9a-f]+}} bp {{0x[0-9a-f]+}} sp {{0x[0-9a-f]+}}
- // CHECK: WRITE of size {{[0-9]+}} at [[ADDR:0x[0-9a-f]+]] thread T0
- // CHECK: #0 [[ADDR:0x[0-9a-f]+]] in wcscat{{.*}}sanitizer_common_interceptors.inc:{{[0-9]+}}
+ // CHECK: WRITE of size {{[0-9]+}} at [[ADDR]] thread T0
+ // CHECK: #0 {{0x[0-9a-f]+}} in wcscat
printf("Should have failed with ASAN error.\n");
}
\ No newline at end of file
diff --git a/compiler-rt/test/asan/TestCases/wcscpy.cpp b/compiler-rt/test/asan/TestCases/wcscpy.cpp
index 414d83303a960..a280d29289e37 100644
--- a/compiler-rt/test/asan/TestCases/wcscpy.cpp
+++ b/compiler-rt/test/asan/TestCases/wcscpy.cpp
@@ -1,23 +1,23 @@
-// RUN: %clangxx_asan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O1 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O0 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O1 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O2 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O3 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
#include <stdio.h>
#include <wchar.h>
int main() {
- wchar_t *src = L"X means dog";
+ const wchar_t *src = L"X means dog";
wchar_t goodDst[12];
wcscpy(goodDst, src);
wchar_t badDst[7];
- printf("Good so far.\n");
+ fprintf(stderr, "Good so far.\n");
// CHECK: Good so far.
- fflush(stdout);
+ fflush(stderr);
wcscpy(badDst, src); // Boom!
- // CHECK:ERROR: AddressSanitizer: stack-buffer-overflow on address [[ADDR:0x[0-9a-f]+]] at pc {{0x[0-9a-f]+}} bp {{0x[0-9a-f]+}} sp {{0x[0-9a-f]+}}
- // CHECK: WRITE of size {{[0-9]+}} at [[ADDR:0x[0-9a-f]+]] thread T0
- // CHECK: #0 [[ADDR:0x[0-9a-f]+]] in wcscpy{{.*}}asan_interceptors.cpp:{{[0-9]+}}
+ // CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address [[ADDR:0x[0-9a-f]+]] at pc {{0x[0-9a-f]+}} bp {{0x[0-9a-f]+}} sp {{0x[0-9a-f]+}}
+ // CHECK: WRITE of size {{[0-9]+}} at [[ADDR]] thread T0
+ // CHECK: #0 {{0x[0-9a-f]+}} in wcscpy
printf("Should have failed with ASAN error.\n");
}
\ No newline at end of file
diff --git a/compiler-rt/test/asan/TestCases/wcsncat.cpp b/compiler-rt/test/asan/TestCases/wcsncat.cpp
index 3ab7fc8f55d63..eb7d095e45c7a 100644
--- a/compiler-rt/test/asan/TestCases/wcsncat.cpp
+++ b/compiler-rt/test/asan/TestCases/wcsncat.cpp
@@ -1,14 +1,14 @@
-// RUN: %clangxx_asan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O1 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O0 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O1 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O2 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O3 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
#include <stdio.h>
#include <wchar.h>
int main() {
- wchar_t *start = L"X means ";
- wchar_t *append = L"dog";
+ const wchar_t *start = L"X means ";
+ const wchar_t *append = L"dog";
wchar_t goodDst[15];
wcscpy(goodDst, start);
wcsncat(goodDst, append, 5);
@@ -16,12 +16,12 @@ int main() {
wchar_t badDst[11];
wcscpy(badDst, start);
wcsncat(badDst, append, 1);
- printf("Good so far.\n");
+ fprintf(stderr, "Good so far.\n");
// CHECK: Good so far.
- fflush(stdout);
+ fflush(stderr);
wcsncat(badDst, append, 3); // Boom!
// CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address [[ADDR:0x[0-9a-f]+]] at pc {{0x[0-9a-f]+}} bp {{0x[0-9a-f]+}} sp {{0x[0-9a-f]+}}
- // CHECK: WRITE of size {{[0-9]+}} at [[ADDR:0x[0-9a-f]+]] thread T0
- // CHECK: #0 [[ADDR:0x[0-9a-f]+]] in wcsncat{{.*}}sanitizer_common_interceptors.inc:{{[0-9]+}}
+ // CHECK: WRITE of size {{[0-9]+}} at [[ADDR]] thread T0
+ // CHECK: #0 {{0x[0-9a-f]+}} in wcsncat
printf("Should have failed with ASAN error.\n");
}
\ No newline at end of file
diff --git a/compiler-rt/test/asan/TestCases/wcsncpy.cpp b/compiler-rt/test/asan/TestCases/wcsncpy.cpp
index 6177b72990a0a..1106bf5d264e5 100644
--- a/compiler-rt/test/asan/TestCases/wcsncpy.cpp
+++ b/compiler-rt/test/asan/TestCases/wcsncpy.cpp
@@ -1,25 +1,25 @@
-// RUN: %clangxx_asan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O1 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
-// RUN: %clangxx_asan -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O0 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O1 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O2 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
+// RUN: %clangxx_asan -O3 %s -o %t && not %env_asan_opts=log_to_stderr=1 %run %t 2>&1 | FileCheck %s --check-prefix=CHECK
#include <stdio.h>
#include <wchar.h>
int main() {
- wchar_t *src = L"X means dog";
+ const wchar_t *src = L"X means dog";
wchar_t goodDst[12];
wcsncpy(goodDst, src, 12);
wchar_t badDst[7];
wcsncpy(badDst, src, 7); // This should still work.
- printf("Good so far.\n");
+ fprintf(stderr, "Good so far.\n");
// CHECK: Good so far.
- fflush(stdout);
+ fflush(stderr);
wcsncpy(badDst, src, 15); // Boom!
- // CHECK:ERROR: AddressSanitizer: stack-buffer-overflow on address [[ADDR:0x[0-9a-f]+]] at pc {{0x[0-9a-f]+}} bp {{0x[0-9a-f]+}} sp {{0x[0-9a-f]+}}
- // CHECK: WRITE of size {{[0-9]+}} at [[ADDR:0x[0-9a-f]+]] thread T0
- // CHECK: #0 [[ADDR:0x[0-9a-f]+]] in wcsncpy{{.*}}asan_interceptors.cpp:{{[0-9]+}}
+ // CHECK: ERROR: AddressSanitizer: stack-buffer-overflow on address [[ADDR:0x[0-9a-f]+]] at pc {{0x[0-9a-f]+}} bp {{0x[0-9a-f]+}} sp {{0x[0-9a-f]+}}
+ // CHECK: WRITE of size {{[0-9]+}} at [[ADDR]] thread T0
+ // CHECK: #0 {{0x[0-9a-f]+}} in wcsncpy
printf("Should have failed with ASAN error.\n");
}
\ No newline at end of file
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
For future reference, please note that LLVM encourages reverting first (https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy):
'Any time you learn of a serious problem with a change, you should revert it. We strongly encourage “revert to green” as opposed to “fixing forward”. We encourage reverting first, investigating offline, and then reapplying the fixed patch - possibly after another round of review if warranted.'
…lvm#161624) ### Summary Stabilize ASan wchar tests across Darwin and Android. NFC: test-only. Follow-up to PR llvm#160493 (adds wchar interceptors/tests). ### Motivation - Darwin: The top frame often resolves to `libclang_rt.asan_*` rather than a source file, so strict checks that include file/line can fail. See Chromium issue [448631142](https://g-issues.chromium.org/issues/448631142). - Android: The “ERROR:” header can go to logcat instead of stderr, so FileCheck may not see it; stdout/stderr reordering also makes pre-crash markers racy. See Android Buildbot [186/12821](https://lab.llvm.org/buildbot/#/builders/186/builds/12821). ### Changes - Android: - Force reports to stderr via `%env_asan_opts=log_to_stderr=1`, avoiding the “ERROR:” header going to logcat. - Print the pre-crash “Good so far.” to stderr and `fflush(stderr)` to avoid stdout/stderr reordering. - Darwin: - Relax the stack-frame check to only require the function name (`wcscpy/wcsncpy/wcscat/wcsncat`) to tolerate `libclang_rt.asan_*` frames. - Common: - Reuse FileCheck var `[[ADDR]]` instead of redefining. - Make wide string literals `const wchar_t*` to silence `-Wwritable-strings`. ### Risk - NFC: test-only; no change to runtime behavior. ### References - Follow-up to PR llvm#160493. - Chromium: [448631142](https://g-issues.chromium.org/issues/448631142) (Darwin failures). - Android Buildbot: [186/12821](https://lab.llvm.org/buildbot/#/builders/186/builds/12821). Signed-off-by: Yixuan Cao <caoyixuan2019@email.szu.edu.cn>
I can still see the failure on Darwin with this: `llvm-project/compiler-rt/test/asan/TestCases/wcscat.cpp:22:12: error: CHECK: expected string not found in input Input file: -dump-input=help explains the following input dump. Input was: ` It looks like 'Good so far.' shows up after the ASAN report, while the CHECK expects it before. Maybe 2>&1 merging streams for FileCheck works differently on Darwin compared to other systems? Could you consider 'reverting to green' this and previous PR, as suggested by @thurstond? |
…droid (test-only) Reland of llvm#161624; depends on revert llvm#162001. - Android: keep %env_asan_opts=log_to_stderr=1. - Darwin: use CHECK-DAG for the pre-crash marker and header; frame line matches function only. - Common: reuse [[ADDR]]. NFC: test-only. Signed-off-by: Yixuan Cao <caoyixuan2019@email.szu.edu.cn>
@wrotki Thanks for the detailed report and for pointing out the ordering issue on Darwin. I’ve opened a revert: Revert PR #162001 (for the follow-up #161624) is open. As a follow-up, I’ve prepared a test-only reland as a Draft here: #162002. Root cause on Darwin: the sanitizer report and the pre-crash marker can race even on stderr, so “Good so far.” may appear after the ASan header. The reland makes the two potentially reordered lines a DAG group:
and then keeps strict order for the subsequent lines:
Android tests keep Happy to iterate—thanks again for the quick feedback. |
With the revert of this fix-forward landed, the issues on the original patch (Android, Chromium etc.) are going to break those bots again. Moreover, at this point, the Darwin bots would been broken for 5+ days. #162021 will revert the base patch. |
…n Darwin/Android" (#162001) Reverts llvm/llvm-project#161624
… on Windows" (#162021) Reverts #160493 due to buildbot failures e.g., #160493 (comment) The fix-forward (#161624) still had failures on Darwin, and was reverted in #162001 i.e., this pull request completes the revert to green for this patch stack.
Thanks, understood. I’ll prepare a single reland that brings back the |
…cat/wcsncat on Windows" (#162021) Reverts llvm/llvm-project#160493 due to buildbot failures e.g., llvm/llvm-project#160493 (comment) The fix-forward (llvm/llvm-project#161624) still had failures on Darwin, and was reverted in llvm/llvm-project#162001 i.e., this pull request completes the revert to green for this patch stack.
Summary
Stabilize ASan wchar tests across Darwin and Android. NFC: test-only. Follow-up to PR #160493 (adds wchar interceptors/tests).
Motivation
libclang_rt.asan_*
rather than a source file, so strict checks that include file/line can fail. See Chromium issue 448631142.Changes
%env_asan_opts=log_to_stderr=1
, avoiding the “ERROR:” header going to logcat.fflush(stderr)
to avoid stdout/stderr reordering.wcscpy/wcsncpy/wcscat/wcsncat
) to toleratelibclang_rt.asan_*
frames.[[ADDR]]
instead of redefining.const wchar_t*
to silence-Wwritable-strings
.Risk
References