r243597 - Avoid failure to canonicalize '..'.
Sean Silva
chisophugis at gmail.com
Wed Jul 29 18:13:26 PDT 2015
On Wed, Jul 29, 2015 at 6:11 PM, Richard Smith <richard at metafoo.co.uk>
wrote:
> On Wed, Jul 29, 2015 at 5:26 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>> Author: silvas
>> Date: Wed Jul 29 19:26:34 2015
>> New Revision: 243597
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=243597&view=rev
>> Log:
>> Avoid failure to canonicalize '..'.
>>
>> Also fix completely broken and untested code which was hiding the
>> primary bug. The !LLVM_ON_UNIX branch of the ifdef was actually a no-op.
>>
>> I ran into this in the wild. It was causing failures in our SDK build.
>>
>> Ideally we'd have a perfect llvm::sys::fs::canonical, but at least this
>> is a step in the right direction, and fixes an obviously broken case.
>> In some sense the test case I've added here is an integration test. We
>> should have these routines thoroughly unit tested in llvm::sys::fs.
>>
>
> 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.
>
I ran into this in the wild where they came from users.
-- Sean Silva
>
> Added:
>> cfe/trunk/test/Modules/Inputs/module-map-path-hash/
>> cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h
>> cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap
>> cfe/trunk/test/Modules/module-map-path-hash.cpp
>> Modified:
>> cfe/trunk/lib/Basic/FileManager.cpp
>>
>> Modified: cfe/trunk/lib/Basic/FileManager.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=243597&r1=243596&r2=243597&view=diff
>>
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
>> +++ cfe/trunk/lib/Basic/FileManager.cpp Wed Jul 29 19:26:34 2015
>> @@ -514,7 +514,7 @@ void FileManager::modifyFileEntry(FileEn
>> File->ModTime = ModificationTime;
>> }
>>
>> -/// Remove '.' path components from the given absolute path.
>> +/// Remove '.' and '..' path components from the given absolute path.
>> /// \return \c true if any changes were made.
>> // FIXME: Move this to llvm::sys::path.
>> bool FileManager::removeDotPaths(SmallVectorImpl<char> &Path) {
>> @@ -525,24 +525,24 @@ bool FileManager::removeDotPaths(SmallVe
>>
>> // Skip the root path, then look for traversal in the components.
>> StringRef Rel = path::relative_path(P);
>> - bool AnyDots = false;
>> for (StringRef C : llvm::make_range(path::begin(Rel), path::end(Rel)))
>> {
>> - if (C == ".") {
>> - AnyDots = true;
>> + if (C == ".")
>> + continue;
>> + if (C == "..") {
>> + if (!ComponentStack.empty())
>> + ComponentStack.pop_back();
>> continue;
>> }
>> ComponentStack.push_back(C);
>> }
>>
>> - if (!AnyDots)
>> - return false;
>> -
>> SmallString<256> Buffer = path::root_path(P);
>> for (StringRef C : ComponentStack)
>> path::append(Buffer, C);
>>
>> + bool Changed = (Path != Buffer);
>> Path.swap(Buffer);
>> - return true;
>> + return Changed;
>> }
>>
>> StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
>> @@ -567,6 +567,9 @@ StringRef FileManager::getCanonicalName(
>> llvm::sys::fs::make_absolute(CanonicalNameBuf);
>> llvm::sys::path::native(CanonicalNameBuf);
>> removeDotPaths(CanonicalNameBuf);
>> + char *Mem =
>> CanonicalNameStorage.Allocate<char>(CanonicalNameBuf.size());
>> + memcpy(Mem, CanonicalNameBuf.data(), CanonicalNameBuf.size());
>> + CanonicalName = StringRef(Mem, CanonicalNameBuf.size());
>> #endif
>>
>> CanonicalDirNames.insert(std::make_pair(Dir, CanonicalName));
>>
>> Added: cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h?rev=243597&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/module-map-path-hash/a.h Wed Jul 29
>> 19:26:34 2015
>> @@ -0,0 +1,2 @@
>> +#pragma once
>> +int a = 42;
>>
>> Added: cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap?rev=243597&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap
>> (added)
>> +++ cfe/trunk/test/Modules/Inputs/module-map-path-hash/module.modulemap
>> Wed Jul 29 19:26:34 2015
>> @@ -0,0 +1,3 @@
>> +module a {
>> + header "a.h"
>> +}
>>
>> Added: cfe/trunk/test/Modules/module-map-path-hash.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-map-path-hash.cpp?rev=243597&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/module-map-path-hash.cpp (added)
>> +++ cfe/trunk/test/Modules/module-map-path-hash.cpp Wed Jul 29 19:26:34
>> 2015
>> @@ -0,0 +1,9 @@
>> +// rm -rf %t
>> +// 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
>> +// 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
>> +// 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
>>
>
> Did you mean to commit with disabled RUN lines here?
>
>
>> +// 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
>> +
>> +#include "a.h"
>> +
>> +// CHECK-NOT: remark: building module
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150729/e7299b3b/attachment.html>
More information about the cfe-commits
mailing list