<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jul 29, 2015 at 6:13 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Wed, Jul 29, 2015 at 6:11 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On Wed, Jul 29, 2015 at 5:26 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br></span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>Author: silvas<br>
Date: Wed Jul 29 19:26:34 2015<br>
New Revision: 243597<br>
<br></span>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D243597-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=DbFl5PXunM8Z5SphChuUH2o-FWbSvNtakfB9EGToj78&s=6GqBWy71rKif1kTXlABIlJ88lWXgh-bv3uajQzEvg20&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=243597&view=rev</a><span><br>
Log:<br>
Avoid failure to canonicalize '..'.<br>
<br>
Also fix completely broken and untested code which was hiding the<br>
primary bug. The !LLVM_ON_UNIX branch of the ifdef was actually a no-op.<br>
<br>
I ran into this in the wild. It was causing failures in our SDK build.<br>
<br>
Ideally we'd have a perfect llvm::sys::fs::canonical, but at least this<br>
is a step in the right direction, and fixes an obviously broken case.<br>
In some sense the test case I've added here is an integration test. We<br>
should have these routines thoroughly unit tested in llvm::sys::fs.<br></span></blockquote><div><br></div><div>This may be correct for the !LLVM_ON_UNIX case (I'm not sure whether the path foo/bar/.. is always the same as foo on Windows), but it's wrong for the LLVM_ON_UNIX case. Please revert (except for the bugfix in getCanonicalName); the right fix is to fix the places that are introducing the unwanted .. path components.</div></div></div></div></blockquote><div><br></div></span><div>I ran into this in the wild where they came from users.</div></div></div></div></blockquote><div><br></div><div>So why do you want to remove them? (Note: I'm OK with removing blah/.. components on Windows, if indeed that is a correct thing to do there. But I'm not OK with it on Unix-like systems, where it's not correct in general.) Which of the three callers of this function did you need this change for?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="HOEnZb"><font color="#888888"><div>-- Sean Silva</div></font></span><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
Added:<br>
cfe/trunk/test/Modules/Inputs/module-map-path-hash/<br>
cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h<br>
cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap<br>
cfe/trunk/test/Modules/module-map-path-hash.cpp<br>
Modified:<br>
cfe/trunk/lib/Basic/FileManager.cpp<br>
<br>
Modified: cfe/trunk/lib/Basic/FileManager.cpp<br></span>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Basic_FileManager.cpp-3Frev-3D243597-26r1-3D243596-26r2-3D243597-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=DbFl5PXunM8Z5SphChuUH2o-FWbSvNtakfB9EGToj78&s=jGaWud4WtthnjZdmRFlA3UUQzq9DvfuWJ9AesxLhpNY&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=243597&r1=243596&r2=243597&view=diff</a><div><div><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/FileManager.cpp (original)<br>
+++ cfe/trunk/lib/Basic/FileManager.cpp Wed Jul 29 19:26:34 2015<br>
@@ -514,7 +514,7 @@ void FileManager::modifyFileEntry(FileEn<br>
File->ModTime = ModificationTime;<br>
}<br>
<br>
-/// Remove '.' path components from the given absolute path.<br>
+/// Remove '.' and '..' path components from the given absolute path.<br>
/// \return \c true if any changes were made.<br>
// FIXME: Move this to llvm::sys::path.<br>
bool FileManager::removeDotPaths(SmallVectorImpl<char> &Path) {<br>
@@ -525,24 +525,24 @@ bool FileManager::removeDotPaths(SmallVe<br>
<br>
// Skip the root path, then look for traversal in the components.<br>
StringRef Rel = path::relative_path(P);<br>
- bool AnyDots = false;<br>
for (StringRef C : llvm::make_range(path::begin(Rel), path::end(Rel))) {<br>
- if (C == ".") {<br>
- AnyDots = true;<br>
+ if (C == ".")<br>
+ continue;<br>
+ if (C == "..") {<br>
+ if (!ComponentStack.empty())<br>
+ ComponentStack.pop_back();<br>
continue;<br>
}<br>
ComponentStack.push_back(C);<br>
}<br>
<br>
- if (!AnyDots)<br>
- return false;<br>
-<br>
SmallString<256> Buffer = path::root_path(P);<br>
for (StringRef C : ComponentStack)<br>
path::append(Buffer, C);<br>
<br>
+ bool Changed = (Path != Buffer);<br>
Path.swap(Buffer);<br>
- return true;<br>
+ return Changed;<br>
}<br>
<br>
StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {<br>
@@ -567,6 +567,9 @@ StringRef FileManager::getCanonicalName(<br>
llvm::sys::fs::make_absolute(CanonicalNameBuf);<br>
llvm::sys::path::native(CanonicalNameBuf);<br>
removeDotPaths(CanonicalNameBuf);<br>
+ char *Mem = CanonicalNameStorage.Allocate<char>(CanonicalNameBuf.size());<br>
+ memcpy(Mem, CanonicalNameBuf.data(), CanonicalNameBuf.size());<br>
+ CanonicalName = StringRef(Mem, CanonicalNameBuf.size());<br>
#endif<br>
<br>
CanonicalDirNames.insert(std::make_pair(Dir, CanonicalName));<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h<br></div></div>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_Modules_Inputs_module-2Dmap-2Dpath-2Dhash_a.h-3Frev-3D243597-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=DbFl5PXunM8Z5SphChuUH2o-FWbSvNtakfB9EGToj78&s=w_ur-XWy68V8MKiTkHvJYHQI0KLR16HcJ9j8Cer_qYk&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h?rev=243597&view=auto</a><span><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h (added)<br>
+++ cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h Wed Jul 29 19:26:34 2015<br>
@@ -0,0 +1,2 @@<br>
+#pragma once<br>
+int a = 42;<br>
<br>
Added: cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap<br></span>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_Modules_Inputs_module-2Dmap-2Dpath-2Dhash_module.modulemap-3Frev-3D243597-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=DbFl5PXunM8Z5SphChuUH2o-FWbSvNtakfB9EGToj78&s=mM-PbEls1mO_PjSKeB7msD59m0-rUrVrc1KUJVKJLEI&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap?rev=243597&view=auto</a><span><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap (added)<br>
+++ cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap Wed Jul 29 19:26:34 2015<br>
@@ -0,0 +1,3 @@<br>
+module a {<br>
+ header "a.h"<br>
+}<br>
<br>
Added: cfe/trunk/test/Modules/module-map-path-hash.cpp<br></span>
URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_Modules_module-2Dmap-2Dpath-2Dhash.cpp-3Frev-3D243597-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=BSqEv9KvKMW_Ob8SyngJ70KdZISM_ASROnREeq0cCxk&m=DbFl5PXunM8Z5SphChuUH2o-FWbSvNtakfB9EGToj78&s=3cu_2cvY7gG8v9K1ZfvkmcdQIZ6DgCKpzBkfjraFD9g&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-map-path-hash.cpp?rev=243597&view=auto</a><span><br>
==============================================================================<br>
--- cfe/trunk/test/Modules/module-map-path-hash.cpp (added)<br>
+++ cfe/trunk/test/Modules/module-map-path-hash.cpp Wed Jul 29 19:26:34 2015<br>
@@ -0,0 +1,9 @@<br>
+// rm -rf %t<br>
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build -I%S/Inputs/module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s<br>
+// xUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build -I%S/Inputs//module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s 2>&1 | FileCheck -allow-empty %s<br>
+// xUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build -I%S/Inputs/./module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s 2>&1 | FileCheck -allow-empty %s<br></span></blockquote><div><br></div><div>Did you mean to commit with disabled RUN lines here?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build -I%S/Inputs/../Inputs/module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s 2>&1 | FileCheck -allow-empty %s<br>
+<br>
+#include "a.h"<br>
+<br>
+// CHECK-NOT: remark: building module<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
</span><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>