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