[PATCH] Teach CC1 to dump module dependencies to a directory

Zachary Turner zturner at google.com
Thu Jun 19 17:39:29 PDT 2014


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/82f81022/attachment.html>


More information about the cfe-commits mailing list