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
