[PATCH] D17104: [VFS] Drop path traversal assertion

[Bruno Cardoso Lopes via cfe-commits]

bruno created this revision.
bruno added reviewers: bogner, benlangmuir.
bruno added subscribers: cfe-commits, dexonsmith.

This patch removes the path traversals check/assertion related with the presence of ".." in paths.

The rationale is that if source and destination paths in the YAML file contain ".." this is enough
for the file manager to retrieve the right file, meaning that it doesn't matter how we write it
since the FileManager is capable of transforming it in real/feasible paths.

This is really common and has been working unnoticed in non-assert builds for while; example of an
entry like this in the VFS YAML file follow:

{
  'type': 'directory',
  'name': "/llvm-install/bin/../lib/clang/3.8.0/include",
  'contents': [
    {
      'type': 'file',
      'name': "__stddef_max_align_t.h",
      'external-contents': "<whatever_path_to_cache>/vfs/llvm-install/bin/../lib/clang/3.8.0/include/__stddef_max_align_t.h"
    },
  ...

Add a test to cover this up.

http://reviews.llvm.org/D17104

Files:
  lib/Basic/VirtualFileSystem.cpp
  test/Modules/crash-vfs-path-traversal.m

Index: test/Modules/crash-vfs-path-traversal.m
===================================================================
--- /dev/null
+++ test/Modules/crash-vfs-path-traversal.m
@@ -0,0 +1,44 @@
+// REQUIRES: crash-recovery, shell
+
+// FIXME: This XFAIL is cargo-culted from crash-report.c. Do we need it?
+// XFAIL: mingw32
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/i %t/m %t
+
+// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
+// RUN: %clang -fsyntax-only %s -I %S/Inputs/System -isysroot %/t/i/    \
+// RUN: -fmodules -fmodules-cache-path=%t/m/ 2>&1 | FileCheck %s
+
+// RUN: FileCheck --check-prefix=CHECKSRC %s -input-file %t/crash-vfs-*.m
+// RUN: FileCheck --check-prefix=CHECKSH %s -input-file %t/crash-vfs-*.sh
+// RUN: FileCheck --check-prefix=CHECKYAML %s -input-file \
+// RUN: %t/crash-vfs-*.cache/vfs/vfs.yaml
+// RUN: find %t/crash-vfs-*.cache/vfs | \
+// RUN: 	grep "Inputs/System/usr/include/stdio.h" | count 1
+
+#include "usr/include/../include/stdio.h"
+
+// CHECK: Preprocessed source(s) and associated run script(s) are located at:
+// CHECK-NEXT: note: diagnostic msg: {{.*}}.m
+// CHECK-NEXT: note: diagnostic msg: {{.*}}.cache
+
+// CHECKSRC: @import cstd.stdio;
+
+// CHECKSH: # Crash reproducer
+// CHECKSH-NEXT: # Driver args: "-fsyntax-only"
+// CHECKSH-NEXT: # Original command: {{.*$}}
+// CHECKSH-NEXT: "-cc1"
+// CHECKSH: "-isysroot" "{{[^"]*}}/i/"
+// CHECKSH-NOT: "-fmodules-cache-path="
+// CHECKSH: "crash-vfs-{{[^ ]*}}.m"
+// CHECKSH: "-ivfsoverlay" "crash-vfs-{{[^ ]*}}.cache/vfs/vfs.yaml"
+
+// CHECKYAML: 'type': 'directory'
+// CHECKYAML: 'name': "{{[^ ]*}}/Inputs/System/usr/include/../include",
+// CHECKYAML-NEXT: 'contents': [
+// CHECKYAML-NEXT:   {
+// CHECKYAML-NEXT:     'type': 'file',
+// CHECKYAML-NEXT:     'name': "module.map",
+// CHECKYAML-NEXT:     'external-contents': "{{[^ ]*}}/Inputs/System/usr/include/../include/module.map"
+// CHECKYAML-NEXT:   },
Index: lib/Basic/VirtualFileSystem.cpp
===================================================================
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -1376,20 +1376,13 @@
   return UniqueID(std::numeric_limits<uint64_t>::max(), ID);
 }
 
-#ifndef NDEBUG
-static bool pathHasTraversal(StringRef Path) {
-  using namespace llvm::sys;
-  for (StringRef Comp : llvm::make_range(path::begin(Path), path::end(Path)))
-    if (Comp == "." || Comp == "..")
-      return true;
-  return false;
-}
-#endif
-
 void YAMLVFSWriter::addFileMapping(StringRef VirtualPath, StringRef RealPath) {
+  // Note that the path traversals "." or ".." aren't translated (removed)
+  // along the paths, but using the path string without translation is
+  // currently enough for VirtualPath and RealPath to work when reading out the
+  // YAML with the VFS description. Would it be cleaner to handle traversals?
   assert(sys::path::is_absolute(VirtualPath) && "virtual path not absolute");
   assert(sys::path::is_absolute(RealPath) && "real path not absolute");
-  assert(!pathHasTraversal(VirtualPath) && "path traversal is not supported");
   Mappings.emplace_back(VirtualPath, RealPath);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D17104.47532.patch
Type: text/x-patch
Size: 3142 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160210/9d049d2a/attachment.bin>