<div dir="ltr">This generates some crashes when running Windows tests. In particular, this asserts is being hit:<div><br></div><div><div> assert(sys::path::is_absolute(VirtualPath) && "virtual path not absolute");<br>
</div></div><div><br></div><div>inside the function void YAMLVFSWriter::addFileMapping(StringRef VirtualPath, StringRef RealPath)</div><div><br></div><div>Inspecting VirtualPath in a debugger shows that the value is:</div>
<div><br></div><div>\\src\\llvm\\tools\\clang\\test\\Modules\\Inputs\\Module.framework\\Frameworks\\SubFramework.framework\\Headers\\SubFramework.h<br></div><div><br></div><div>The path qualified with drive letter on my machine that this resolves to is the aforementioned path prefixed with a D:</div>
<div><br></div><div>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.</div><div><br></div>
<div><br></div><div>Full output of failure command:</div><div><br></div><div><br></div><div><div>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</div>
<div>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"</div>
<div>Assertion failed: sys::path::is_absolute(VirtualPath) && "virtual path not absolute", file ..\..\tools\clang\lib\Basic\VirtualFileSystem.cpp, line 864</div><div>Stack dump:</div><div>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</div>
<div>\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</div>
<div>1. D:\src\llvm\tools\clang\test\Modules\dependency-dump.m:15:15: current parser token ';'</div><div>0x507E14FA (0x0000000A 0x00000000 0x0545D120 0x508B9AC4), memcmp() + 0xABA bytes(s)</div><div>0x508CB26C (0x0545D454 0x0545D134 0x00000097 0x056D0000), abort() + 0x1C bytes(s)</div>
<div>0x508B9AC4 (0x03FAC810 0x03FAC798 0x00000360 0x0545D48C), _wassert() + 0xD4 bytes(s)</div><div>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)</div>
<div>0x011CBE89 (0x0545D27D 0x00000073 0x0545D22C 0x000000C4), clang::ModuleDependencyCollector::addFileMapping() + 0x29 bytes(s), d:\src\llvm\tools\clang\include\clang\frontend\utils.h, line 99</div><div>0x011CB8F1 (0x0545D468 0x05605290 0x00000075 0xCCCCCCCC), `anonymous namespace'::ModuleDependencyListener::copyToRoot() + 0x221 bytes(s), d:\src\llvm\tools\clang\lib\frontend\moduledependencycollector.cpp, line 100</div>
<div>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)</div>
<div>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)</div><div>
0x020D3D93 (0x056CE300 0x0545DDC0 0x00000000 0x00000003), clang::ASTReader::ReadControlBlock() + 0x473 bytes(s), d:\src\llvm\tools\clang\lib\serialization\astreader.cpp, line 2279</div><div>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)</div>
<div>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)</div><div>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)</div>
<div>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)</div><div>0x02D3F2BA (0x0563B908 0x0545E828 0x000001E6 0x0563B900), clang::Preprocessor::Lex() + 0xCA bytes(s), d:\src\llvm\tools\clang\lib\lex\preprocessor.cpp, line 682</div>
<div>0x0205A01A (0x0545E344 0x0545E420 0x0545E828 0xCCCCCCCC), clang::Parser::ConsumeToken() + 0x7A bytes(s), d:\src\llvm\tools\clang\include\clang\parse\parser.h, line 301</div><div>0x020563F8 (0x0545E7CC 0x000001DE 0x0545E7AC 0x00000018), clang::Parser::ParseModuleImport() + 0x168 bytes(s), d:\src\llvm\tools\clang\lib\parse\parser.cpp, line 1914</div>
<div>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)</div><div>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)</div>
<div>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)</div><div>0x0204EB9A (0x056649C8 0x00000000 0x00000000 0x0545E8D0), clang::ParseAST() + 0x11A bytes(s), d:\src\llvm\tools\clang\lib\parse\parseast.cpp, line 135 + 0xC byte(s)</div>
<div>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)</div><div>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)</div>
<div>0x0119EA61 (0x05621BE8 0x0545EEF0 0x0545F9B4 0xCCCCCCCC), clang::CompilerInstance::ExecuteAction() + 0x281 bytes(s), d:\src\llvm\tools\clang\lib\frontend\compilerinstance.cpp, line 745</div><div>0x012E2F4C (0x055E6350 0x0545FDD4 0xCCCCCCCC 0xCCCCCCCC), clang::ExecuteCompilerInvocation() + 0x30C bytes(s), d:\src\llvm\tools\clang\lib\frontendtool\executecompilerinvocation.cpp, line 240 + 0x11 byte(s)</div>
<div>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)</div><div>0x00175F98 (0x0000000E 0x055E5690 0x055E6838 0x5E5CD93B), main() + 0x228 bytes(s), d:\src\llvm\tools\clang\tools\driver\driver.cpp, line 319 + 0x45 byte(s)</div>
<div>0x02E0B3E9 (0x0545FE38 0x752B919F 0x7F6D4000 0x0545FE7C), __tmainCRTStartup() + 0x199 bytes(s), f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c, line 626 + 0x19 byte(s)</div><div>0x02E0B52D (0x7F6D4000 0x0545FE7C 0x776CA8CB 0x7F6D4000), mainCRTStartup() + 0xD bytes(s), f:\dd\vctools\crt\crtw32\dllstuff\crtexe.c, line 466</div>
<div>0x752B919F (0x7F6D4000 0x2DA16D3E 0x00000000 0x00000000), BaseThreadInitThunk() + 0xE bytes(s)</div><div>0x776CA8CB (0xFFFFFFFF 0x776BF679 0x00000000 0x00000000), RtlInitializeExceptionChain() + 0x84 bytes(s)</div><div>
0x776CA8A1 (0x02E0B520 0x7F6D4000 0x00000000 0x00000000), RtlInitializeExceptionChain() + 0x5A bytes(s)</div></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jun 19, 2014 at 1:28 PM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Ben Langmuir <<a href="mailto:blangmuir@apple.com">blangmuir@apple.com</a>> writes:<br>
> LGTM.<br>
<br>
r211302 and r211303.<br>
<div class="HOEnZb"><div class="h5"><br>
>> On Jun 19, 2014, at 11:53 AM, Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> wrote:<br>
>><br>
>> Ben Langmuir <<a href="mailto:blangmuir@apple.com">blangmuir@apple.com</a>> writes:<br>
>>> On Jun 19, 2014, at 10:31 AM, Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>> wrote:<br>
>>><br>
>>> Ben Langmuir <<a href="mailto:blangmuir@apple.com">blangmuir@apple.com</a>> writes:<br>
>>><br>
>>> Hi Justin,<br>
>>><br>
>>> Thanks for working on this! It’s looking pretty close.<br>
>>><br>
>>> +/// Append the absolute path in Nested to the path given by Root.<br>
>>> This will<br>
>>> +/// remove directory traversal from the resulting nested path.<br>
>>> +static void appendNestedPath(SmallVectorImpl<char> &Root,<br>
>>> + SmallVectorImpl<char> &Nested) {<br>
>>> + using namespace llvm::sys;<br>
>>> + SmallVector<StringRef, 16> ComponentStack;<br>
>>> +<br>
>>> + StringRef Rel = path::relative_path(StringRef(Nested.begin(),<br>
>>> Nested.size()));<br>
>>><br>
>>> You seem to have manually inlined Nested.str() ;-) Maybe just make<br>
>>> your Nested parameter a StringRef?<br>
>>><br>
>>> Right, not sure what I was thinking there :). I'll pass in a StringRef<br>
>>> instead.<br>
>>><br>
>>> + // We need an absolute path to append to the root.<br>
>>> + SmallString<256> AbsoluteSrc = Src;<br>
>>> + fs::make_absolute(AbsoluteSrc);<br>
>>> + // Build the destination path.<br>
>>> + SmallString<256> Dest = Collector.getDest();<br>
>>> + size_t RootLen = Dest.size();<br>
>>> + appendNestedPath(Dest, AbsoluteSrc);<br>
>>><br>
>>> Do we need to escape this somehow on Windows, since you might get C:<br>
>>> in the middle of your path?<br>
>>><br>
>>> And in general, will this work if Dest ends with a path separator?<br>
>>> Then you would end up with // in the middle, which could potentially<br>
>>> be eaten at some point (not sure).<br>
>>><br>
>>> The call to path::relative_path in appendNestedPath takes care of both<br>
>>> of these issues. It's strips off root_name (ie, C:) and root_directory<br>
>>> (ie, /).<br>
>>><br>
>>> It sure does, silly me.<br>
>>><br>
>>> +bool ModuleDependencyListener::visitInputFile(StringRef Filename,<br>
>>> bool IsSystem,<br>
>>> + bool IsOverridden)<br>
>>> {<br>
>>> + if (!Collector.insertSeen(Filename))<br>
>>> + return true;<br>
>>> + if (copyToRoot(Filename))<br>
>>> + Collector.setHasErrors();<br>
>>> + return true;<br>
>>> +}<br>
>>><br>
>>> This is clearer to me if you invert the first if, but you decide.<br>
>>> if (Collector.insertSeen(Filename))<br>
>>> if (copyToRoot(Filename))<br>
>>> Collector.setHasErrors();<br>
>>> return true;<br>
>>><br>
>>> Sure, I'm happy with either.<br>
>>><br>
>>> +// RUN: find %t/vfs -type f | FileCheck %s -check-prefix=DUMP<br>
>>> +// DUMP: AlsoDependsOnModule.framework/Headers/<br>
>>> AlsoDependsOnModule.h<br>
>>> +// DUMP:<br>
>>> Module.framework/Frameworks/SubFramework.framework/Headers/<br>
>>> SubFramework.h<br>
>>><br>
>>> REQUIRES: shell, since you need ‘find’. This applies to both tests.<br>
>>> Also you won’t get the path separators you expect on Windows.<br>
>>><br>
>>> Hmm. Is there a good way to check if the files are created without find?<br>
>>> Assuming there is, I'll change it use regex for the path separators, as<br>
>>> I think the extra platform coverage here is worthwhile.<br>
>>><br>
>>> I can’t think of a cleaner way to do this.<br>
>>><br>
>>> This isn’t really the place to discuss llvm patches, but...<br>
>>><br>
>>> + char *Buf = new char[BufSize];<br>
>>><br>
>>> If you don’t want to put 4 KB on the stack, how about std::vector with<br>
>>> its data() method?<br>
>>><br>
>>> Yeah, 4k seemed like a bit much for the stack (though, this is always a<br>
>>> leaf call, so maybe it's fine).<br>
>>><br>
>>> Is a vector really better here? Since I have to manually manage closing<br>
>>> the files anyway, the new/delete doesn't feel out of place, and using a<br>
>>> std::vector or a std::unique_ptr purely as an RAII container muddies up<br>
>>> what this is doing a bit.<br>
>>><br>
>>> I don’t feel strongly about it, so go with what you have.<br>
>>><br>
>>> + for (;;) {<br>
>>> + Bytes = read(ReadFD, Buf, BufSize);<br>
>>> + if (Bytes <= 0)<br>
>>> + break;<br>
>>> + Bytes = write(WriteFD, Buf, Bytes);<br>
>>> + if (Bytes <= 0)<br>
>>> + break;<br>
>>> + }<br>
>>><br>
>>> This doesn’t seem sufficiently paranoid about the number of bytes<br>
>>> actually written by ‘write’.<br>
>>><br>
>>> Right. This should probably loop on the write as well. I'll update that.<br>
>><br>
>> New patches attached.<br>
>><br>
>> <clang-0001-Frontend-Add-a-CC1-flag-to-dump-module-dependencies.4.patch><llvm-0001-Support-Add-llvm-sys-fs-copy_file.3.patch><br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>