Actually I've submitted a patch for this. Please take a look: http://reviews.llvm.org/D4224
(Also in llvm-commits) On Thu, Jun 19, 2014 at 5:39 PM, Zachary Turner <[email protected]> wrote: > This generates some crashes when running Windows tests. In particular, > this asserts is being hit: > > assert(sys::path::is_absolute(VirtualPath) && "virtual path not > absolute"); > > inside the function void YAMLVFSWriter::addFileMapping(StringRef > VirtualPath, StringRef RealPath) > > Inspecting VirtualPath in a debugger shows that the value is: > > > \\src\\llvm\\tools\\clang\\test\\Modules\\Inputs\\Module.framework\\Frameworks\\SubFramework.framework\\Headers\\SubFramework.h > > The path qualified with drive letter on my machine that this resolves to > is the aforementioned path prefixed with a D: > > So this should refer to a root path, but either way I'll leave it to you > to decide whether the fix is to append the drive letter, or maybe the bug > is in the is_absolute() function. > > > Full output of failure command: > > > D:\src\llvm\build\ninja>"D:/src/llvm/build/ninja/./bin/clang.EXE" "-cc1" > "-internal-isystem" > "D:\src\llvm\build\ninja\bin\..\lib\clang\3.5.0\include" "-fmodules" > "-fmodules-cache-path=D:\src\llvm\build\ninja\tools\clang\test\Modules\Output\dependency-dump.m.tmp/cache" > "-module-dependency-dir" "D:\sr > c\llvm\build\ninja\tools\clang\test\Modules\Output\dependency-dump.m.tmp/vfs" > "-F" "D:\src\llvm\tools\clang\test\Modules/Inputs" "-I" > "D:\src\llvm\tools\clang\test\Modules/Inputs" "-verify" > "D:\src\llvm\tools\clang\test\Modules\dependency-dump.m" > Assertion failed: sys::path::is_absolute(VirtualPath) && "virtual path not > absolute", file ..\..\tools\clang\lib\Basic\VirtualFileSystem.cpp, line 864 > Stack dump: > 0. Program arguments: D:/src/llvm/build/ninja/./bin/clang.EXE -cc1 > -internal-isystem D:\src\llvm\build\ninja\bin\..\lib\clang\3.5.0\include > -fmodules > -fmodules-cache-path=D:\src\llvm\build\ninja\tools\clang\test\Modules\Output\dependency-dump.m.tmp/cache > -module-dependency-dir D:\src\llvm\build > \ninja\tools\clang\test\Modules\Output\dependency-dump.m.tmp/vfs -F > D:\src\llvm\tools\clang\test\Modules/Inputs -I > D:\src\llvm\tools\clang\test\Modules/Inputs -verify > D:\src\llvm\tools\clang\test\Modules\dependency-dump.m > 1. D:\src\llvm\tools\clang\test\Modules\dependency-dump.m:15:15: > current parser token ';' > 0x507E14FA (0x0000000A 0x00000000 0x0545D120 0x508B9AC4), memcmp() + 0xABA > bytes(s) > 0x508CB26C (0x0545D454 0x0545D134 0x00000097 0x056D0000), abort() + 0x1C > bytes(s) > 0x508B9AC4 (0x03FAC810 0x03FAC798 0x00000360 0x0545D48C), _wassert() + > 0xD4 bytes(s) > 0x01107E73 (0x0545D27D 0x00000073 0x0545D22C 0x000000C4), > clang::vfs::YAMLVFSWriter::addFileMapping() + 0x63 bytes(s), > d:\src\llvm\tools\clang\lib\basic\virtualfilesystem.cpp, line 864 + 0x3F > byte(s) > 0x011CBE89 (0x0545D27D 0x00000073 0x0545D22C 0x000000C4), > clang::ModuleDependencyCollector::addFileMapping() + 0x29 bytes(s), > d:\src\llvm\tools\clang\include\clang\frontend\utils.h, line 99 > 0x011CB8F1 (0x0545D468 0x05605290 0x00000075 0xCCCCCCCC), `anonymous > namespace'::ModuleDependencyListener::copyToRoot() + 0x221 bytes(s), > d:\src\llvm\tools\clang\lib\frontend\moduledependencycollector.cpp, line 100 > 0x011CB9EC (0x05605290 0x00000075 0x00000000 0x00000000), `anonymous > namespace'::ModuleDependencyListener::visitInputFile() + 0x4C bytes(s), > d:\src\llvm\tools\clang\lib\frontend\moduledependencycollector.cpp, line > 106 + 0x14 byte(s) > 0x020D1B18 (0x05605290 0x00000075 0x00000000 0x00000000), > clang::ChainedASTReaderListener::visitInputFile() + 0xB8 bytes(s), > d:\src\llvm\tools\clang\lib\serialization\astreader.cpp, line 141 + 0x2F > byte(s) > 0x020D3D93 (0x056CE300 0x0545DDC0 0x00000000 0x00000003), > clang::ASTReader::ReadControlBlock() + 0x473 bytes(s), > d:\src\llvm\tools\clang\lib\serialization\astreader.cpp, line 2279 > 0x020D369A (0x0569ED08 0x0000006C 0x00000000 0x000001DF), > clang::ASTReader::ReadASTCore() + 0x53A bytes(s), > d:\src\llvm\tools\clang\lib\serialization\astreader.cpp, line 3724 + 0x1B > byte(s) > 0x020E4C08 (0x0545E254 0x00000000 0x000001DF 0x00000003), > clang::ASTReader::ReadAST() + 0xC8 bytes(s), > d:\src\llvm\tools\clang\lib\serialization\astreader.cpp, line 3461 + 0x2E > byte(s) > 0x011A1A91 (0x0545E2D8 0x000001DF 0x05622CB4 0x00000001), > clang::CompilerInstance::loadModule() + 0x361 bytes(s), > d:\src\llvm\tools\clang\lib\frontend\compilerinstance.cpp, line 1239 + 0x20 > byte(s) > 0x02D3F483 (0x0563B908 0x00000004 0xCC00CCCC 0x05622B30), > clang::Preprocessor::LexAfterModuleImport() + 0x153 bytes(s), > d:\src\llvm\tools\clang\lib\lex\preprocessor.cpp, line 729 + 0x3B byte(s) > 0x02D3F2BA (0x0563B908 0x0545E828 0x000001E6 0x0563B900), > clang::Preprocessor::Lex() + 0xCA bytes(s), > d:\src\llvm\tools\clang\lib\lex\preprocessor.cpp, line 682 > 0x0205A01A (0x0545E344 0x0545E420 0x0545E828 0xCCCCCCCC), > clang::Parser::ConsumeToken() + 0x7A bytes(s), > d:\src\llvm\tools\clang\include\clang\parse\parser.h, line 301 > 0x020563F8 (0x0545E7CC 0x000001DE 0x0545E7AC 0x00000018), > clang::Parser::ParseModuleImport() + 0x168 bytes(s), > d:\src\llvm\tools\clang\lib\parse\parser.cpp, line 1914 > 0x0208A049 (0x0545E7CC 0x0545E81C 0xCCCCCCCC 0xCCCCCCCC), > clang::Parser::ParseObjCAtDirectives() + 0x1B9 bytes(s), > d:\src\llvm\tools\clang\lib\parse\parseobjc.cpp, line 83 + 0x10 byte(s) > 0x02053AAB (0x0545E7CC 0x0545E7F4 0x00000000 0x0545E8A4), > clang::Parser::ParseExternalDeclaration() + 0x4FB bytes(s), > d:\src\llvm\tools\clang\lib\parse\parser.cpp, line 674 + 0xC byte(s) > 0x02050A0F (0x0545E85C 0x0545E94C 0x0545E8B4 0x0563B900), > clang::Parser::ParseTopLevelDecl() + 0x1DF bytes(s), > d:\src\llvm\tools\clang\lib\parse\parser.cpp, line 559 + 0x12 byte(s) > 0x0204EB9A (0x056649C8 0x00000000 0x00000000 0x0545E8D0), > clang::ParseAST() + 0x11A bytes(s), > d:\src\llvm\tools\clang\lib\parse\parseast.cpp, line 135 + 0xC byte(s) > 0x011D1271 (0x0545E8F8 0xCCCCCCCC 0xCCCCCCCC 0xCCCCCCCC), > clang::ASTFrontendAction::ExecuteAction() + 0x101 bytes(s), > d:\src\llvm\tools\clang\lib\frontend\frontendaction.cpp, line 514 + 0x30 > byte(s) > 0x011D0E7E (0x0545E9E8 0x0545F9B4 0xCCCCCCCC 0xCCCCCCCC), > clang::FrontendAction::Execute() + 0x7E bytes(s), > d:\src\llvm\tools\clang\lib\frontend\frontendaction.cpp, line 415 + 0xF > byte(s) > 0x0119EA61 (0x05621BE8 0x0545EEF0 0x0545F9B4 0xCCCCCCCC), > clang::CompilerInstance::ExecuteAction() + 0x281 bytes(s), > d:\src\llvm\tools\clang\lib\frontend\compilerinstance.cpp, line 745 > 0x012E2F4C (0x055E6350 0x0545FDD4 0xCCCCCCCC 0xCCCCCCCC), > clang::ExecuteCompilerInvocation() + 0x30C bytes(s), > d:\src\llvm\tools\clang\lib\frontendtool\executecompilerinvocation.cpp, > line 240 + 0x11 byte(s) > 0x001881D2 (0x0545F9BC 0x0545F9EC 0x0561D288 0x000C1366), cc1_main() + > 0x2F2 bytes(s), d:\src\llvm\tools\clang\tools\driver\cc1_main.cpp, line 112 > + 0xE byte(s) > 0x00175F98 (0x0000000E 0x055E5690 0x055E6838 0x5E5CD93B), main() + 0x228 > bytes(s), d:\src\llvm\tools\clang\tools\driver\driver.cpp, line 319 + 0x45 > byte(s) > 0x02E0B3E9 (0x0545FE38 0x752B919F 0x7F6D4000 0x0545FE7C), > __tmainCRTStartup() + 0x199 bytes(s), > f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c, line 626 + 0x19 byte(s) > 0x02E0B52D (0x7F6D4000 0x0545FE7C 0x776CA8CB 0x7F6D4000), mainCRTStartup() > + 0xD bytes(s), f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c, line 466 > 0x752B919F (0x7F6D4000 0x2DA16D3E 0x00000000 0x00000000), > BaseThreadInitThunk() + 0xE bytes(s) > 0x776CA8CB (0xFFFFFFFF 0x776BF679 0x00000000 0x00000000), > RtlInitializeExceptionChain() + 0x84 bytes(s) > 0x776CA8A1 (0x02E0B520 0x7F6D4000 0x00000000 0x00000000), > RtlInitializeExceptionChain() + 0x5A bytes(s) > > > > > On Thu, Jun 19, 2014 at 1:28 PM, Justin Bogner <[email protected]> > wrote: > >> Ben Langmuir <[email protected]> writes: >> > LGTM. >> >> r211302 and r211303. >> >> >> On Jun 19, 2014, at 11:53 AM, Justin Bogner <[email protected]> >> wrote: >> >> >> >> Ben Langmuir <[email protected]> writes: >> >>> On Jun 19, 2014, at 10:31 AM, Justin Bogner <[email protected]> >> wrote: >> >>> >> >>> Ben Langmuir <[email protected]> writes: >> >>> >> >>> Hi Justin, >> >>> >> >>> Thanks for working on this! It’s looking pretty close. >> >>> >> >>> +/// Append the absolute path in Nested to the path given >> by Root. >> >>> This will >> >>> +/// remove directory traversal from the resulting nested >> path. >> >>> +static void appendNestedPath(SmallVectorImpl<char> &Root, >> >>> + SmallVectorImpl<char> >> &Nested) { >> >>> + using namespace llvm::sys; >> >>> + SmallVector<StringRef, 16> ComponentStack; >> >>> + >> >>> + StringRef Rel = >> path::relative_path(StringRef(Nested.begin(), >> >>> Nested.size())); >> >>> >> >>> You seem to have manually inlined Nested.str() ;-) Maybe just >> make >> >>> your Nested parameter a StringRef? >> >>> >> >>> Right, not sure what I was thinking there :). I'll pass in a >> StringRef >> >>> instead. >> >>> >> >>> + // We need an absolute path to append to the root. >> >>> + SmallString<256> AbsoluteSrc = Src; >> >>> + fs::make_absolute(AbsoluteSrc); >> >>> + // Build the destination path. >> >>> + SmallString<256> Dest = Collector.getDest(); >> >>> + size_t RootLen = Dest.size(); >> >>> + appendNestedPath(Dest, AbsoluteSrc); >> >>> >> >>> Do we need to escape this somehow on Windows, since you might >> get C: >> >>> in the middle of your path? >> >>> >> >>> And in general, will this work if Dest ends with a path >> separator? >> >>> Then you would end up with // in the middle, which could >> potentially >> >>> be eaten at some point (not sure). >> >>> >> >>> The call to path::relative_path in appendNestedPath takes care of >> both >> >>> of these issues. It's strips off root_name (ie, C:) and >> root_directory >> >>> (ie, /). >> >>> >> >>> It sure does, silly me. >> >>> >> >>> +bool ModuleDependencyListener::visitInputFile(StringRef >> Filename, >> >>> bool IsSystem, >> >>> + bool >> IsOverridden) >> >>> { >> >>> + if (!Collector.insertSeen(Filename)) >> >>> + return true; >> >>> + if (copyToRoot(Filename)) >> >>> + Collector.setHasErrors(); >> >>> + return true; >> >>> +} >> >>> >> >>> This is clearer to me if you invert the first if, but you >> decide. >> >>> if (Collector.insertSeen(Filename)) >> >>> if (copyToRoot(Filename)) >> >>> Collector.setHasErrors(); >> >>> return true; >> >>> >> >>> Sure, I'm happy with either. >> >>> >> >>> +// RUN: find %t/vfs -type f | FileCheck %s >> -check-prefix=DUMP >> >>> +// DUMP: AlsoDependsOnModule.framework/Headers/ >> >>> AlsoDependsOnModule.h >> >>> +// DUMP: >> >>> Module.framework/Frameworks/SubFramework.framework/Headers/ >> >>> SubFramework.h >> >>> >> >>> REQUIRES: shell, since you need ‘find’. This applies to both >> tests. >> >>> Also you won’t get the path separators you expect on Windows. >> >>> >> >>> Hmm. Is there a good way to check if the files are created without >> find? >> >>> Assuming there is, I'll change it use regex for the path >> separators, as >> >>> I think the extra platform coverage here is worthwhile. >> >>> >> >>> I can’t think of a cleaner way to do this. >> >>> >> >>> This isn’t really the place to discuss llvm patches, but... >> >>> >> >>> + char *Buf = new char[BufSize]; >> >>> >> >>> If you don’t want to put 4 KB on the stack, how about >> std::vector with >> >>> its data() method? >> >>> >> >>> Yeah, 4k seemed like a bit much for the stack (though, this is >> always a >> >>> leaf call, so maybe it's fine). >> >>> >> >>> Is a vector really better here? Since I have to manually manage >> closing >> >>> the files anyway, the new/delete doesn't feel out of place, and >> using a >> >>> std::vector or a std::unique_ptr purely as an RAII container >> muddies up >> >>> what this is doing a bit. >> >>> >> >>> I don’t feel strongly about it, so go with what you have. >> >>> >> >>> + for (;;) { >> >>> + Bytes = read(ReadFD, Buf, BufSize); >> >>> + if (Bytes <= 0) >> >>> + break; >> >>> + Bytes = write(WriteFD, Buf, Bytes); >> >>> + if (Bytes <= 0) >> >>> + break; >> >>> + } >> >>> >> >>> This doesn’t seem sufficiently paranoid about the number of >> bytes >> >>> actually written by ‘write’. >> >>> >> >>> Right. This should probably loop on the write as well. I'll update >> that. >> >> >> >> New patches attached. >> >> >> >> >> <clang-0001-Frontend-Add-a-CC1-flag-to-dump-module-dependencies.4.patch><llvm-0001-Support-Add-llvm-sys-fs-copy_file.3.patch> >> >> _______________________________________________ >> 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
