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);
}
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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
