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. 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
