[PATCH] D51648: [UBSan] Partially fix `test/ubsan/TestCases/Misc/log-path_test.cc` so that it can run on devices.

[Dan Liew via Phabricator via llvm-commits]

delcypher added a comment.

In https://reviews.llvm.org/D51648#1224618, @filcab wrote:

> I don't think this is acceptable.
>  We have no guarantees we even have a shell on the devices. The run script might be doing all sort of things for commands to run on the device.


That is true. However for all scripts that are in-tree where this test is meant to pass (currently only the iOS simulator) I've fixed them to work so that `%run rm` has a special meaning.
We have out-of-tree scripts for running on iOS devices and I've modified those to also handle the `%run rm` semantics that is being proposed in this patch.

Out of tree consumers of compiler-rt that have their own device test infrastructure need (if this patch is merged) to specially handle `%run rm`.

> Our approach has usually been to translate paths between host and device on our run script (or, in some tests, to add some code for our platform only). This is ok for us, but not everyone has the ability to have the host filesystem available.
> 
> If we *really* need something like this (what's going on when calling cat later? How do the files show up on the host?),

For the iOS simulator the host and the device file system are the same so no work is required. For iOS devices the scripts handle this by copying back the log file to the host when they detect `log_path=` was in ASAN_OPTIONS/TSAN_OPTIONS/UBSAN_OPTIONS.
This test is not meant to pass for Android so I've put no work in to make it work.

> maybe a %run_rm (or %device_rm, or whatever) would be better. Then platforms can do what they want and, if iossim_run and android_run can handle the rm command, they can add a substitution %device_rm -> %run rm.

I don't object to having a `%run_rm` substitution instead of `%run rm` but it's really not very different from requiring implementers of `%run` to support `rm` specially. Either way implementers have to implement something new so I don't really see the advantage of `%run_rm` really is.



================
Comment at: test/ubsan/TestCases/Misc/log-path_test.cc:15
 // RUN: rm -f %t.log.*
+// RUN: %run rm -f '%t.log.*'
 // RUN: %env_ubsan_opts=log_path='"%t.log"' %run %t -4 2> %t.out
----------------
filcab wrote:
> The device might not have a shell. And might be unable to run an rm command even if it has a shell.
What device are you thinking of? Right now we only support Android and iOS-like devices which all have an accessible shell.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D51648