- Intercept the native apple (both iOs and macOS) which is even allowed on App Store (contrary to getentropy for example), thus checking its proper usage in code consumers.
Details
- Reviewers
yln delcypher aralisza eugenis vitalybuka
Diff Detail
Time | Test | |
---|---|---|
510 ms | x64 debian > Clang.Driver::cuda-arch-translation.cu Script:
--
: 'RUN: at line 8'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -### -target x86_64-linux-gnu -c --cuda-gpu-arch=sm_20 /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/cuda-arch-translation.cu 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck -check-prefixes=COMMON,SM20 /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/cuda-arch-translation.cu
| |
250 ms | x64 debian > Clang.Driver::cuda-bad-arch.cu Script:
--
: 'RUN: at line 7'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -### -target x86_64-linux-gnu --cuda-gpu-arch=compute_20 -c /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/cuda-bad-arch.cu 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck -check-prefix BAD /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/cuda-bad-arch.cu
| |
310 ms | x64 debian > Clang.Driver::cuda-flush-denormals-to-zero.cu Script:
--
: 'RUN: at line 5'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=sm_20 -fgpu-flush-denormals-to-zero -nocudainc -nocudalib /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/cuda-flush-denormals-to-zero.cu 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck -check-prefix=FTZ /var/lib/buildkite-agent/builds/llvm-project/clang/test/Driver/cuda-flush-denormals-to-zero.cu
| |
7,780 ms | x64 windows > Clang.Driver::cuda-arch-translation.cu Script:
--
: 'RUN: at line 8'; c:\ws\w5\llvm-project\premerge-checks\build\bin\clang.exe -### -target x86_64-linux-gnu -c --cuda-gpu-arch=sm_20 C:\ws\w5\llvm-project\premerge-checks\clang\test\Driver\cuda-arch-translation.cu 2>&1 | c:\ws\w5\llvm-project\premerge-checks\build\bin\filecheck.exe -check-prefixes=COMMON,SM20 C:\ws\w5\llvm-project\premerge-checks\clang\test\Driver\cuda-arch-translation.cu
| |
2,920 ms | x64 windows > Clang.Driver::cuda-bad-arch.cu Script:
--
: 'RUN: at line 7'; c:\ws\w5\llvm-project\premerge-checks\build\bin\clang.exe -### -target x86_64-linux-gnu --cuda-gpu-arch=compute_20 -c C:\ws\w5\llvm-project\premerge-checks\clang\test\Driver\cuda-bad-arch.cu 2>&1 | c:\ws\w5\llvm-project\premerge-checks\build\bin\filecheck.exe -check-prefix BAD C:\ws\w5\llvm-project\premerge-checks\clang\test\Driver\cuda-bad-arch.cu
| |
View Full Test Results (6 Failed) |
Event Timeline
Hi David, thanks for the patch for this previously unintercepted API. I am happy with the interceptor implementation itself; it follows our "recipe" nicely! :)
I have two small asks:
- In the commit message, can you explain which issue (crash, false positive) for which sanitizer this is addressing?
- Can we write a test that shows this issue? I think the current test only shows that we don't crash, right?
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc | ||
---|---|---|
7246 | Nitpick: can we use the parameter names "bytes" and "count" from the header declaration? CCRNGStatus CCRandomGenerateBytes(void *bytes, size_t count) __OSX_AVAILABLE_STARTING(__MAC_10_10, __IPHONE_8_0); | |
compiler-rt/test/sanitizer_common/TestCases/Darwin/ccrandomgeneratebytes.c | ||
2 | Are we testing here simply that we don't crash in our interceptor? Can we add a comment what we intend to test here. | |
5 | I don't think this will work as intended. We would try to compile and run an empty file. | |
13 | I don't think we will ever return "false", i.e., failing the test here? success -> n = 0 failure -> n = 1 return n <= 0 // n is never <= 0, right?! Maybe just return the return value of CCRandomGenerateBytes() (assuming kCCSuccess is 0)? |
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc | ||
---|---|---|
9836–9845 | Why this renaming? As it it matches https://man7.org/linux/man-pages/man2/getrandom.2.html |
It's titled "Intercept CCRandomGenerateBytes", but I don't see changes in interceptors, only new test.
compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc | ||
---|---|---|
9836–9845 | My bad wrong lines. |
Hi David,
Before I approve this can we do the following?
- In the commit message, can you explain which issue (crash, false positive) for which sanitizer this is addressing?
- Can we write a test that shows this issue, i.e., a test that fails without the added interceptor? (Maybe this test needs to be for a specific sanitizer instead of sanitizer_common?)
Note: my understanding is that the added test is "green" even without the new interceptor. Ignore my second point if that is not the case.