[compiler-rt] r342391 - [UBSan] Partially fix `test/ubsan/TestCases/Misc/log-path_test.cc` so that it can run on devices.

[Dan Liew via llvm-commits]

Author: delcypher
Date: Mon Sep 17 06:33:44 2018
New Revision: 342391

URL: http://llvm.org/viewvc/llvm-project?rev=342391&view=rev
Log:
[UBSan] Partially fix `test/ubsan/TestCases/Misc/log-path_test.cc` so that it can run on devices.

Summary:
In order for this test to work the log file needs to be removed from both
from the host and device. To fix this the `rm` `RUN` lines have been
replaced with `RUN: rm` followed by `RUN: %device_rm`.

Initially I tried having it so that `RUN: %run rm` implicitly runs `rm`
on the host as well so that only one `RUN` line is needed. This
simplified writing the test however that had two large drawbacks.

* It's potentially very confusing (e.g. for use of the device scripts outside
  of the lit tests) if asking for `rm` to run on device also causes files
  on the host to be deleted.

* This doesn't work well with the glob patterns used in the test.
  The host shell expands the `%t.log.*` glob pattern and not on the
  device so we could easily miss deleting old log files from previous
  test runs if the corresponding file doesn't exist on the host.

So instead deletion of files on the device and host are explicitly
separate commands.

The command to delete files from a device is provided by a new
substitution `%device_rm` as suggested by Filipe Cabecinhas.

The semantics of `%device_rm` are that:

* It provides a way remove files from a target device when
 the host is not the same as the target. In the case that the
 host and target are the same it is a no-op.

* It interprets shell glob patterns in the context of the device
  file system instead of the host file system.
  This solves the globbing problem provided the argument is quoted so
  that lit's underlying shell doesn't try to expand the glob pattern.

* It supports the `-r` and `-f` flags of the `rm` command,
  with the same semantics.

Right now an implementation of `%device_rm` is provided only for
ios devices. For all other devices a lit warning is emitted and
the `%device_rm` is treated as a no-op. This done to avoid changing
the behaviour for other device types but leaves room for others
to implement `%device_rm`.

The ios device implementation uses the `%run` wrapper to do the work
of removing files on a device.

The `iossim_run.py` script has been fixed so that it just runs `rm`
on the host operating system because the device and host file system
are the same.

rdar://problem/41126835

Reviewers: vsk, kubamracek, george.karpenkov, eugenis

Subscribers: #sanitizers, llvm-commits

Differential Revision: https://reviews.llvm.org/D51648

Modified:
    compiler-rt/trunk/test/lit.common.cfg
    compiler-rt/trunk/test/sanitizer_common/ios_commands/iossim_run.py
    compiler-rt/trunk/test/ubsan/TestCases/Misc/log-path_test.cc

Modified: compiler-rt/trunk/test/lit.common.cfg
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/lit.common.cfg?rev=342391&r1=342390&r2=342391&view=diff
==============================================================================
--- compiler-rt/trunk/test/lit.common.cfg (original)
+++ compiler-rt/trunk/test/lit.common.cfg Mon Sep 17 06:33:44 2018
@@ -106,6 +106,10 @@ config.substitutions.append(
 if config.emulator:
   config.substitutions.append( ('%run', config.emulator) )
   config.substitutions.append( ('%env ', "env ") )
+  # TODO: Implement `%device_rm` to perform removal of files in the emulator.
+  # For now just make it a no-op.
+  lit_config.warning('%device_rm is not implemented')
+  config.substitutions.append( ('%device_rm', 'echo ') )
   config.compile_wrapper = ""
 elif config.host_os == 'Darwin' and config.apple_platform != "osx":
   # Darwin tests can be targetting macOS, a device or a simulator. All devices
@@ -148,6 +152,9 @@ elif config.host_os == 'Darwin' and conf
     config.environment[device_id_env] = os.environ[device_id_env]
   config.substitutions.append(('%run', run_wrapper))
   config.substitutions.append(('%env ', env_wrapper + " "))
+  # Current implementation of %device_rm uses the run_wrapper to do
+  # the work.
+  config.substitutions.append(('%device_rm', '{} rm '.format(run_wrapper)))
   config.compile_wrapper = compile_wrapper
 
   prepare_output = subprocess.check_output([prepare_script, config.apple_platform, config.clang]).strip()
@@ -161,9 +168,15 @@ elif config.android:
   config.compile_wrapper = compile_wrapper
   config.substitutions.append( ('%run', "") )
   config.substitutions.append( ('%env ', "env ") )
+  # TODO: Implement `%device_rm` to perform removal of files on a device.  For
+  # now just make it a no-op.
+  lit_config.warning('%device_rm is not implemented')
+  config.substitutions.append( ('%device_rm', 'echo ') )
 else:
   config.substitutions.append( ('%run', "") )
   config.substitutions.append( ('%env ', "env ") )
+  # When running locally %device_rm is a no-op.
+  config.substitutions.append( ('%device_rm', 'echo ') )
   config.compile_wrapper = ""
 
 # Define CHECK-%os to check for OS-dependent output.

Modified: compiler-rt/trunk/test/sanitizer_common/ios_commands/iossim_run.py
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/sanitizer_common/ios_commands/iossim_run.py?rev=342391&r1=342390&r2=342391&view=diff
==============================================================================
--- compiler-rt/trunk/test/sanitizer_common/ios_commands/iossim_run.py (original)
+++ compiler-rt/trunk/test/sanitizer_common/ios_commands/iossim_run.py Mon Sep 17 06:33:44 2018
@@ -1,6 +1,6 @@
 #!/usr/bin/python
 
-import os, sys, subprocess
+import glob, os, pipes, sys, subprocess
 
 
 if not "SANITIZER_IOSSIM_TEST_DEVICE_IDENTIFIER" in os.environ:
@@ -12,7 +12,25 @@ for e in ["ASAN_OPTIONS", "TSAN_OPTIONS"
   if e in os.environ:
     os.environ["SIMCTL_CHILD_" + e] = os.environ[e]
 
-exitcode = subprocess.call(["xcrun", "simctl", "spawn", device_id] + sys.argv[1:])
+prog = sys.argv[1]
+exit_code = None
+if prog == 'rm':
+  # The simulator and host actually share the same file system so we can just
+  # execute directly on the host.
+  rm_args = []
+  for arg in sys.argv[2:]:
+    if '*' in arg or '?' in arg:
+      # Don't quote glob pattern
+      rm_args.append(arg)
+    else:
+      # FIXME(dliew): pipes.quote() is deprecated
+      rm_args.append(pipes.quote(arg))
+  rm_cmd_line = ["/bin/rm"] + rm_args
+  rm_cmd_line_str = ' '.join(rm_cmd_line)
+  # We use `shell=True` so that any wildcard globs get expanded by the shell.
+  exitcode = subprocess.call(rm_cmd_line_str, shell=True)
+else:
+  exitcode = subprocess.call(["xcrun", "simctl", "spawn", device_id] + sys.argv[1:])
 if exitcode > 125:
   exitcode = 126
 sys.exit(exitcode)

Modified: compiler-rt/trunk/test/ubsan/TestCases/Misc/log-path_test.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/ubsan/TestCases/Misc/log-path_test.cc?rev=342391&r1=342390&r2=342391&view=diff
==============================================================================
--- compiler-rt/trunk/test/ubsan/TestCases/Misc/log-path_test.cc (original)
+++ compiler-rt/trunk/test/ubsan/TestCases/Misc/log-path_test.cc Mon Sep 17 06:33:44 2018
@@ -12,11 +12,13 @@
 
 // Good log_path.
 // RUN: rm -f %t.log.*
+// RUN: %device_rm -f '%t.log.*'
 // RUN: %env_ubsan_opts=log_path='"%t.log"' %run %t -4 2> %t.out
 // RUN: FileCheck %s --check-prefix=CHECK-ERROR < %t.log.*
 
 // Run w/o errors should not produce any log.
 // RUN: rm -f %t.log.*
+// RUN: %device_rm -f '%t.log.*'
 // RUN: %env_ubsan_opts=log_path='"%t.log"'  %run %t 4
 // RUN: not cat %t.log.*