[PATCH] Teach CC1 to dump module dependencies to a directory
Zachary Turner
zturner at google.com
Thu Jun 19 17:56:18 PDT 2014
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 <zturner at google.com> 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 <mail at justinbogner.com>
> wrote:
>
>> Ben Langmuir <blangmuir at apple.com> writes:
>> > LGTM.
>>
>> r211302 and r211303.
>>
>> >> On Jun 19, 2014, at 11:53 AM, Justin Bogner <mail at justinbogner.com>
>> wrote:
>> >>
>> >> Ben Langmuir <blangmuir at apple.com> writes:
>> >>> On Jun 19, 2014, at 10:31 AM, Justin Bogner <mail at justinbogner.com>
>> wrote:
>> >>>
>> >>> Ben Langmuir <blangmuir at apple.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140619/f840b370/attachment.html>
More information about the cfe-commits
mailing list