On Wed, Jul 29, 2015 at 6:11 PM, Richard Smith <[email protected]> wrote:
> On Wed, Jul 29, 2015 at 5:26 PM, Sean Silva <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
