r206664 - Don't build modules with (submodules with) missing headers
Ben Langmuir
blangmuir at apple.com
Mon Apr 21 08:03:21 PDT 2014
Thanks for the fix!
On Apr 21, 2014, at 12:35 AM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Mon, Apr 21, 2014 at 12:19 AM, Evgeniy Stepanov <eugeni.stepanov at gmail.com> wrote:
> Hi,
>
> this change also broke MSan bot, and r206673 did not help.
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/3084/steps/check-clang%20msan/logs/stdio
>
> ==15691== WARNING: MemorySanitizer: use-of-uninitialized-value
> #0 0x7fab95fb8b9e in clang::ModuleMapParser::parseModuleDecl()
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/ModuleMap.cpp:1482
>
> Thanks, this made finding the bug trivial, should be fixed in r206736.
>
> #1 0x7fab95fb7870 in clang::ModuleMapParser::parseModuleDecl()
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/ModuleMap.cpp:1404
> #2 0x7fab95fc5455 in clang::ModuleMapParser::parseModuleMapFile()
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/ModuleMap.cpp:2204
> #3 0x7fab95fae830 in
> clang::ModuleMap::parseModuleMapFile(clang::FileEntry const*, bool)
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/ModuleMap.cpp:2263
> #4 0x7fab95f4db2b in
> clang::HeaderSearch::loadModuleMapFileImpl(clang::FileEntry const*,
> bool) /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/HeaderSearch.cpp:1182
> #5 0x7fab95f40252 in
> clang::HeaderSearch::loadModuleMapFile(clang::DirectoryEntry const*,
> bool, bool) /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/HeaderSearch.cpp:1260
> #6 0x7fab95f3f5b9 in
> clang::HeaderSearch::lookupModule(llvm::StringRef, bool)
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/HeaderSearch.cpp:179
> #7 0x7fab91f678c8 in
> clang::CompilerInstance::loadModule(clang::SourceLocation,
> llvm::ArrayRef<std::pair<clang::IdentifierInfo*,
> clang::SourceLocation> >, clang::Module::NameVisibilityKind, bool)
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:1134
> #8 0x7fab960612de in
> clang::Preprocessor::LexAfterModuleImport(clang::Token&)
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/Preprocessor.cpp:728
> #9 0x7fab96060c59 in clang::Preprocessor::Lex(clang::Token&)
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Lex/Preprocessor.cpp:683
> #10 0x7fab91fec179 in clang::PreprocessOnlyAction::ExecuteAction()
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Frontend/FrontendActions.cpp:583
> #11 0x7fab91fe044c in clang::FrontendAction::Execute()
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Frontend/FrontendAction.cpp:388
> #12 0x7fab91f65548 in
> clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:727
> #13 0x7fab92167d4f in
> clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:239
> #14 0x7fab8fa4903b in cc1_main(char const**, char const**, char
> const*, void*) /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/tools/driver/cc1_main.cpp:112
> #15 0x7fab8fa43e08 in main
> /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/tools/driver/driver.cpp:318
> #16 0x7fab8dd35ed4 in __libc_start_main
> (/lib/x86_64-linux-gnu/libc.so.6+0x21ed4)
> #17 0x7fab8fa3d77f in _start
> (/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/bin/clang-3.5+0x5fb77f)
>
>
> On Sat, Apr 19, 2014 at 3:58 AM, Ben Langmuir <blangmuir at apple.com> wrote:
> > r206673
> >
> > On Apr 18, 2014, at 4:42 PM, Ben Langmuir <blangmuir at apple.com> wrote:
> >
> >
> > On Apr 18, 2014, at 4:41 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> >
> > Hi Ben,
> >
> > Looks like your commit broke a buildbot:
> > http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/14603
> >
> > Could you fix it or revert?
> >
> >
> > Already on it, thanks! Should have a fix in momentarily.
> >
> >
> > Thanks,
> > -Quentin
> >
> > On Apr 18, 2014, at 3:07 PM, Ben Langmuir <blangmuir at apple.com> wrote:
> >
> > Author: benlangmuir
> > Date: Fri Apr 18 17:07:31 2014
> > New Revision: 206664
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=206664&view=rev
> > Log:
> > Don't build modules with (submodules with) missing headers
> >
> > Unless they are in submodules that aren't available anyway, due to
> > requirements not being met. Also, mark children as unavailable when the
> > parent is.
> >
> > Added:
> > cfe/trunk/test/Modules/missing-header.m
> > Modified:
> > cfe/trunk/include/clang/Basic/Module.h
> > cfe/trunk/lib/Basic/Module.cpp
> > cfe/trunk/lib/Frontend/FrontendActions.cpp
> > cfe/trunk/lib/Lex/ModuleMap.cpp
> > cfe/trunk/test/Modules/Inputs/submodules/module.map
> > cfe/trunk/test/Modules/submodules.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/Module.h
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=206664&r1=206663&r2=206664&view=diff
> > ==============================================================================
> > --- cfe/trunk/include/clang/Basic/Module.h (original)
> > +++ cfe/trunk/include/clang/Basic/Module.h Fri Apr 18 17:07:31 2014
> > @@ -123,8 +123,13 @@ public:
> > /// will be false to indicate that this (sub)module is not available.
> > SmallVector<Requirement, 2> Requirements;
> >
> > - /// \brief Whether this module is available in the current
> > - /// translation unit.
> > + /// \brief Whether this module is missing a feature from \c Requirements.
> > + unsigned IsMissingRequirement : 1;
> > +
> > + /// \brief Whether this module is available in the current translation
> > unit.
> > + ///
> > + /// If the module is missing headers or does not meet all requirements
> > then
> > + /// this bit will be 0.
> > unsigned IsAvailable : 1;
> >
> > /// \brief Whether this module was loaded from a module file.
> > @@ -407,6 +412,9 @@ public:
> > const LangOptions &LangOpts,
> > const TargetInfo &Target);
> >
> > + /// \brief Mark this module and all of its submodules as unavailable.
> > + void markUnavailable();
> > +
> > /// \brief Find the submodule with the given name.
> > ///
> > /// \returns The submodule if found, or NULL otherwise.
> >
> > Modified: cfe/trunk/lib/Basic/Module.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=206664&r1=206663&r2=206664&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Basic/Module.cpp (original)
> > +++ cfe/trunk/lib/Basic/Module.cpp Fri Apr 18 17:07:31 2014
> > @@ -160,6 +160,11 @@ void Module::addRequirement(StringRef Fe
> > if (hasFeature(Feature, LangOpts, Target) == RequiredState)
> > return;
> >
> > + IsMissingRequirement = true;
> > + markUnavailable();
> > +}
> > +
> > +void Module::markUnavailable() {
> > if (!IsAvailable)
> > return;
> >
> >
> > Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=206664&r1=206663&r2=206664&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
> > +++ cfe/trunk/lib/Frontend/FrontendActions.cpp Fri Apr 18 17:07:31 2014
> > @@ -288,7 +288,8 @@ bool GenerateModuleAction::BeginSourceFi
> > if (!Module->isAvailable(CI.getLangOpts(), CI.getTarget(), Requirement,
> > MissingHeader)) {
> > if (MissingHeader.FileNameLoc.isValid()) {
> > - CI.getDiagnostics().Report(diag::err_module_header_missing)
> > + CI.getDiagnostics().Report(MissingHeader.FileNameLoc,
> > + diag::err_module_header_missing)
> > << MissingHeader.IsUmbrella << MissingHeader.FileName;
> > } else {
> > CI.getDiagnostics().Report(diag::err_module_unavailable)
> >
> > Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=206664&r1=206663&r2=206664&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
> > +++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri Apr 18 17:07:31 2014
> > @@ -1477,6 +1477,15 @@ void ModuleMapParser::parseModuleDecl()
> > inferFrameworkLink(ActiveModule, Directory, SourceMgr.getFileManager());
> > }
> >
> > + // If the module meets all requirements but is still unavailable, mark
> > the
> > + // whole tree as unavailable to prevent it from building.
> > + if (!ActiveModule->IsAvailable && !ActiveModule->IsMissingRequirement &&
> > + ActiveModule->Parent) {
> > + ActiveModule->getTopLevelModule()->markUnavailable();
> > + ActiveModule->getTopLevelModule()->MissingHeaders.append(
> > + ActiveModule->MissingHeaders.begin(),
> > ActiveModule->MissingHeaders.end());
> > + }
> > +
> > // We're done parsing this module. Pop back to the previous module.
> > ActiveModule = PreviousActiveModule;
> > }
> > @@ -1705,9 +1714,8 @@ void ModuleMapParser::parseHeaderDecl(MM
> >
> > // If we find a module that has a missing header, we mark this module as
> > // unavailable and store the header directive for displaying
> > diagnostics.
> > - // Other submodules in the same module can still be used.
> > Header.IsUmbrella = LeadingToken == MMToken::UmbrellaKeyword;
> > - ActiveModule->IsAvailable = false;
> > + ActiveModule->markUnavailable();
> > ActiveModule->MissingHeaders.push_back(Header);
> > }
> > }
> >
> > Modified: cfe/trunk/test/Modules/Inputs/submodules/module.map
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/submodules/module.map?rev=206664&r1=206663&r2=206664&view=diff
> > ==============================================================================
> > --- cfe/trunk/test/Modules/Inputs/submodules/module.map (original)
> > +++ cfe/trunk/test/Modules/Inputs/submodules/module.map Fri Apr 18 17:07:31
> > 2014
> > @@ -15,3 +15,11 @@ module missing_headers {
> > module missing { header "missing.h" }
> > module not_missing { header "not_missing.h" }
> > }
> > +
> > +module missing_unavailable_headers {
> > + module missing {
> > + requires !objc
> > + header "missing.h"
> > + }
> > + module not_missing { }
> > +}
> >
> > Added: cfe/trunk/test/Modules/missing-header.m
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/missing-header.m?rev=206664&view=auto
> > ==============================================================================
> > --- cfe/trunk/test/Modules/missing-header.m (added)
> > +++ cfe/trunk/test/Modules/missing-header.m Fri Apr 18 17:07:31 2014
> > @@ -0,0 +1,13 @@
> > +// RUN: rm -rf %t
> > +// RUN: not %clang_cc1 -x objective-c -fmodules-cache-path=%t -fmodules -I
> > %S/Inputs/submodules %s 2>&1 | FileCheck %s
> > +
> > +// FIXME: cannot use -verify, because the error from inside the module
> > build has
> > +// a different source manager than the verifier.
> > +
> > + at import missing_unavailable_headers; // OK
> > + at import missing_unavailable_headers.not_missing; // OK
> > +// CHECK-NOT: missing_unavailable_headers
> > +
> > + at import missing_headers;
> > +// CHECK: module.map:15:27: error: header 'missing.h' not found
> > +// CHECK: could not build module 'missing_headers'
> >
> > Modified: cfe/trunk/test/Modules/submodules.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules.cpp?rev=206664&r1=206663&r2=206664&view=diff
> > ==============================================================================
> > --- cfe/trunk/test/Modules/submodules.cpp (original)
> > +++ cfe/trunk/test/Modules/submodules.cpp Fri Apr 18 17:07:31 2014
> > @@ -32,8 +32,3 @@ extern MyTypeA import_self_test_a; // ex
> > // expected-note at import-self-a.h:1 {{here}}
> > extern MyTypeC import_self_test_c;
> > extern MyTypeD import_self_test_d;
> > -
> > -// expected-error at Inputs/submodules/module.map:15{{header 'missing.h' not
> > found}}
> > - at import missing_headers.missing;
> > - at import missing_headers.not_missing;
> > -void f() { NotMissingFunction(); };
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> _______________________________________________
> 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/20140421/d38325e8/attachment.html>
More information about the cfe-commits
mailing list