Skip to content

[sanitizer] Fix running sanitizer_set_report_path_test on Android #99469

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

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

iii-i
Copy link
Member

@iii-i iii-i commented Jul 18, 2024

sanitizer_set_report_path_test outputs the following on an Android builder [1]:

ERROR: Can't create directory: /data/local/tmp/Output/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/sanitizer_common/asan-arm-Android/Posix/Output/sanitizer_set_report_path_test.cpp.tmp
Path /data/local/tmp/Output/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/sanitizer_common/asan-arm-Android/Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.24954

The order of messages is reversed. The "Path" message is printed with printf(), and the "ERROR:" message is printed with write(STDERR_FILENO). However, there is an fflush() in between, which is supposed to prevent reordering.

Work around the issue by using write(STDERR_FILENO) to write the "Path" message.

[1] https://lab.llvm.org/buildbot/#/builders/186/builds/703/steps/26/logs/stdio

sanitizer_set_report_path_test outputs the following on an Android
builder [1]:

    ERROR: Can't create directory: /data/local/tmp/Output/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/sanitizer_common/asan-arm-Android/Posix/Output/sanitizer_set_report_path_test.cpp.tmp
    Path /data/local/tmp/Output/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/sanitizer_common/asan-arm-Android/Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.24954

The order of messages is reversed. The "Path" message is printed with
printf(), and the "ERROR:" message is printed with
write(STDERR_FILENO). However, there is an fflush() in between, which
is supposed to prevent reordering.

Work around the issue by using write(STDERR_FILENO) to write the
"Path" message.

[1] https://lab.llvm.org/buildbot/#/builders/186/builds/703/steps/26/logs/stdio
@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Ilya Leoshkevich (iii-i)

Changes

sanitizer_set_report_path_test outputs the following on an Android builder [1]:

ERROR: Can't create directory: /data/local/tmp/Output/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/sanitizer_common/asan-arm-Android/Posix/Output/sanitizer_set_report_path_test.cpp.tmp
Path /data/local/tmp/Output/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/sanitizer_common/asan-arm-Android/Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.24954

The order of messages is reversed. The "Path" message is printed with printf(), and the "ERROR:" message is printed with write(STDERR_FILENO). However, there is an fflush() in between, which is supposed to prevent reordering.

Work around the issue by using write(STDERR_FILENO) to write the "Path" message.

[1] https://lab.llvm.org/buildbot/#/builders/186/builds/703/steps/26/logs/stdio


Full diff: https://github.com/llvm/llvm-project/pull/99469.diff

1 Files Affected:

  • (modified) compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_test.cpp (+5-3)
diff --git a/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_test.cpp b/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_test.cpp
index 286eafc315baf..1b5b6c099a383 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_test.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/Posix/sanitizer_set_report_path_test.cpp
@@ -6,6 +6,7 @@
 #include <sanitizer/common_interface_defs.h>
 #include <stdio.h>
 #include <string.h>
+#include <unistd.h>
 
 volatile int *null = 0;
 
@@ -14,13 +15,14 @@ int main(int argc, char **argv) {
   sprintf(buff, "%s.report_path/report", argv[0]);
   __sanitizer_set_report_path(buff);
   assert(strncmp(buff, __sanitizer_get_report_path(), strlen(buff)) == 0);
-  printf("Path %s\n", __sanitizer_get_report_path());
-  fflush(stdout);
+  sprintf(buff, "Path %s\n", __sanitizer_get_report_path());
+  write(STDERR_FILENO, buff, strlen(buff));
 
   // Try setting again with an invalid/inaccessible directory.
   sprintf(buff, "%s/report", argv[0]);
   __sanitizer_set_report_path(buff);
-  printf("Path %s\n", __sanitizer_get_report_path());
+  sprintf(buff, "Path %s\n", __sanitizer_get_report_path());
+  write(STDERR_FILENO, buff, strlen(buff));
 }
 
 // CHECK: Path {{.*}}Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.

Copy link
Collaborator

@vitalybuka vitalybuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android harness collect stdout and stderr separatly on device and then dump sequentially on the host, so stdout and stderr order is wrong.

@vitalybuka vitalybuka merged commit 8be714b into llvm:main Jul 18, 2024
4 of 5 checks passed
@iii-i
Copy link
Member Author

iii-i commented Jul 19, 2024

The build is green again (https://lab.llvm.org/buildbot/#/builders/186/builds/733). Thanks!

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
…9469)

sanitizer_set_report_path_test outputs the following on an Android
builder [1]:

ERROR: Can't create directory:
/data/local/tmp/Output/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/sanitizer_common/asan-arm-Android/Posix/Output/sanitizer_set_report_path_test.cpp.tmp
Path
/data/local/tmp/Output/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/sanitizer_common/asan-arm-Android/Posix/Output/sanitizer_set_report_path_test.cpp.tmp.report_path/report.24954

The order of messages is reversed. 

The test can use strcmp+assert instead of CHECK for `__sanitizer_get_report_path` output.

[1]
https://lab.llvm.org/buildbot/#/builders/186/builds/703/steps/26/logs/stdio

---------

Co-authored-by: Vitaly Buka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants