[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 llvm-commits at lists.llvm.org
Tue Sep 11 07:40:15 PDT 2018


delcypher added a comment.

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

> In https://reviews.llvm.org/D51648#1224846, @delcypher wrote:
>
> > 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 made me curious: Why don't the scripts delete the file after copying it to the host?


Several reasons

- Debug-ability. It's useful to examine the files left on the device when things go wrong.
- Convention. The scripts have never implicitly deleted files on a device (unless the command being run was a delete operation).

> If the problem here is the extra files leftover, the scripts for iOS devices can handle it, as they already know which files need to be copied. Then we can even avoid adding anything here. It's a much more contained change.

Although that seems like a reasonable approach we are not going to do this. The scripts have never implicitly deleted files on the device and this is not behaviour we want to add. The test itself is the "source of truth" here. The test knows which files need to be deleted but
our device scripts in general do not. It is better to be explicit and have the test explicitly run a command to perform removal of files on the device.

I will change this patch to use the `%device_rm` substitution instead. For now I will make this operation be a no-op for everything except ios devices.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D51648





More information about the llvm-commits mailing list