From 3b7deda7137e62810a810ce25b062927a9fc7c71 Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Fri, 24 May 2013 05:44:08 +0000 Subject: [PATCH] [modules] If we hit a failure while loading a PCH/module, abort parsing instead of trying to continue in an invalid state. Also don't let libclang create a PCH with such an error. Fixes rdar://13953768 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182629 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Frontend/ASTUnit.h | 1 + include/clang/Frontend/CompilerInstance.h | 4 ++++ include/clang/Lex/ModuleLoader.h | 4 ++++ include/clang/Lex/Preprocessor.h | 4 ++++ include/clang/Parse/Parser.h | 3 ++- include/clang/Serialization/ASTReader.h | 4 ++++ lib/Frontend/ASTUnit.cpp | 7 +++++- lib/Frontend/CompilerInstance.cpp | 2 ++ lib/Lex/Lexer.cpp | 6 ++++++ lib/Lex/PPDirectives.cpp | 14 ++++++++++++ lib/Parse/Parser.cpp | 8 ++++++- lib/Serialization/ASTReader.cpp | 5 ++++- test/Modules/fatal-module-loader-error.m | 26 +++++++++++++++++++++++ 13 files changed, 84 insertions(+), 4 deletions(-) create mode 100644 test/Modules/fatal-module-loader-error.m diff --git a/include/clang/Frontend/ASTUnit.h b/include/clang/Frontend/ASTUnit.h index 1fc56ce5f3..c915776963 100644 --- a/include/clang/Frontend/ASTUnit.h +++ b/include/clang/Frontend/ASTUnit.h @@ -75,6 +75,7 @@ private: IntrusiveRefCntPtr TargetOpts; IntrusiveRefCntPtr HSOpts; ASTReader *Reader; + bool HadModuleLoaderFatalFailure; struct ASTWriterData; OwningPtr WriterData; diff --git a/include/clang/Frontend/CompilerInstance.h b/include/clang/Frontend/CompilerInstance.h index ee4850580d..3ad05a6510 100644 --- a/include/clang/Frontend/CompilerInstance.h +++ b/include/clang/Frontend/CompilerInstance.h @@ -664,6 +664,10 @@ public: SourceLocation ImportLoc, bool Complain); + bool hadModuleLoaderFatalFailure() const { + return ModuleLoader::HadFatalFailure; + } + }; } // end namespace clang diff --git a/include/clang/Lex/ModuleLoader.h b/include/clang/Lex/ModuleLoader.h index 3acf9151bc..254ab36fe9 100644 --- a/include/clang/Lex/ModuleLoader.h +++ b/include/clang/Lex/ModuleLoader.h @@ -54,6 +54,8 @@ public: /// then loading that module. class ModuleLoader { public: + ModuleLoader() : HadFatalFailure(false) {} + virtual ~ModuleLoader(); /// \brief Attempt to load the given module. @@ -85,6 +87,8 @@ public: Module::NameVisibilityKind Visibility, SourceLocation ImportLoc, bool Complain) = 0; + + bool HadFatalFailure; }; } diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index e70eaa8fc8..082f4e4cde 100644 --- a/include/clang/Lex/Preprocessor.h +++ b/include/clang/Lex/Preprocessor.h @@ -457,6 +457,10 @@ public: /// \brief Retrieve the module loader associated with this preprocessor. ModuleLoader &getModuleLoader() const { return TheModuleLoader; } + bool hadModuleLoaderFatalFailure() const { + return TheModuleLoader.HadFatalFailure; + } + /// \brief True if we are currently preprocessing a #if or #elif directive bool isParsingIfOrElifDirective() const { return ParsingIfOrElifDirective; diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 5a5b0669b2..de45268e04 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -398,7 +398,8 @@ private: /// \brief Abruptly cut off parsing; mainly used when we have reached the /// code-completion point. void cutOffParsing() { - PP.setCodeCompletionReached(); + if (PP.isCodeCompletionEnabled()) + PP.setCodeCompletionReached(); // Cut off parsing by acting as if we reached the end-of-file. Tok.setKind(tok::eof); } diff --git a/include/clang/Serialization/ASTReader.h b/include/clang/Serialization/ASTReader.h index 073baf514c..3861b8f67c 100644 --- a/include/clang/Serialization/ASTReader.h +++ b/include/clang/Serialization/ASTReader.h @@ -309,6 +309,10 @@ private: /// \brief The module manager which manages modules and their dependencies ModuleManager ModuleMgr; + /// \brief The location where the module file will be considered as + /// imported from. For non-module AST types it should be invalid. + SourceLocation CurrentImportLoc; + /// \brief The global module index, if loaded. llvm::OwningPtr GlobalIndex; diff --git a/lib/Frontend/ASTUnit.cpp b/lib/Frontend/ASTUnit.cpp index 7850dc697b..2d916536d3 100644 --- a/lib/Frontend/ASTUnit.cpp +++ b/lib/Frontend/ASTUnit.cpp @@ -216,7 +216,8 @@ const unsigned DefaultPreambleRebuildInterval = 5; static llvm::sys::cas_flag ActiveASTUnitObjects; ASTUnit::ASTUnit(bool _MainFileIsAST) - : Reader(0), OnlyLocalDecls(false), CaptureDiagnostics(false), + : Reader(0), HadModuleLoaderFatalFailure(false), + OnlyLocalDecls(false), CaptureDiagnostics(false), MainFileIsAST(_MainFileIsAST), TUKind(TU_Complete), WantTiming(getenv("LIBCLANG_TIMING")), OwnsRemappedFileBuffers(true), @@ -1705,6 +1706,7 @@ void ASTUnit::transferASTDataFromCompilerInstance(CompilerInstance &CI) { CI.setFileManager(0); Target = &CI.getTarget(); Reader = CI.getModuleManager(); + HadModuleLoaderFatalFailure = CI.hadModuleLoaderFatalFailure(); } StringRef ASTUnit::getMainFileName() const { @@ -2504,6 +2506,9 @@ void ASTUnit::CodeComplete(StringRef File, unsigned Line, unsigned Column, } bool ASTUnit::Save(StringRef File) { + if (HadModuleLoaderFatalFailure) + return true; + // Write to a temporary file and later rename it to the actual file, to avoid // possible race conditions. SmallString<128> TempPath; diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp index cf856fc2ab..b4bb6f3251 100644 --- a/lib/Frontend/CompilerInstance.cpp +++ b/lib/Frontend/CompilerInstance.cpp @@ -1247,12 +1247,14 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, case ASTReader::VersionMismatch: case ASTReader::ConfigurationMismatch: case ASTReader::HadErrors: + ModuleLoader::HadFatalFailure = true; // FIXME: The ASTReader will already have complained, but can we showhorn // that diagnostic information into a more useful form? KnownModules[Path[0].first] = 0; return ModuleLoadResult(); case ASTReader::Failure: + ModuleLoader::HadFatalFailure = true; // Already complained, but note now that we failed. KnownModules[Path[0].first] = 0; ModuleBuildFailed = true; diff --git a/lib/Lex/Lexer.cpp b/lib/Lex/Lexer.cpp index e58581ee06..e3daf34692 100644 --- a/lib/Lex/Lexer.cpp +++ b/lib/Lex/Lexer.cpp @@ -3426,6 +3426,12 @@ HandleDirective: FormTokenWithChars(Result, CurPtr, tok::hash); PP->HandleDirective(Result); + if (PP->hadModuleLoaderFatalFailure()) { + // With a fatal failure in the module loader, we abort parsing. + assert(Result.is(tok::eof) && "Preprocessor did not set tok:eof"); + return; + } + // As an optimization, if the preprocessor didn't switch lexers, tail // recurse. if (PP->isCurrentLexer(this)) { diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp index ba3291aa39..947011d707 100644 --- a/lib/Lex/PPDirectives.cpp +++ b/lib/Lex/PPDirectives.cpp @@ -1512,6 +1512,20 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc, /*IsIncludeDirective=*/true); assert((Imported == 0 || Imported == SuggestedModule) && "the imported module is different than the suggested one"); + + if (!Imported && hadModuleLoaderFatalFailure()) { + // With a fatal failure in the module loader, we abort parsing. + Token &Result = IncludeTok; + if (CurLexer) { + Result.startToken(); + CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof); + CurLexer->cutOffLexing(); + } else { + assert(CurPTHLexer && "#include but no current lexer set!"); + CurPTHLexer->getEOF(Result); + } + return; + } // If this header isn't part of the module we're building, we're done. if (!BuildingImportedModule && Imported) { diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index a5371f9e61..3124b96455 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -1884,7 +1884,13 @@ Parser::DeclGroupPtrTy Parser::ParseModuleImport(SourceLocation AtLoc) { break; } while (true); - + + if (PP.hadModuleLoaderFatalFailure()) { + // With a fatal failure in the module loader, we abort parsing. + cutOffParsing(); + return DeclGroupPtrTy(); + } + DeclResult Import = Actions.ActOnModuleImport(AtLoc, ImportLoc, Path); ExpectAndConsumeSemi(diag::err_module_expected_semi); if (Import.isInvalid()) diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 17f01380ed..a4c8c35307 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -2900,6 +2900,9 @@ ASTReader::ASTReadResult ASTReader::ReadAST(const std::string &FileName, ModuleKind Type, SourceLocation ImportLoc, unsigned ClientLoadCapabilities) { + llvm::SaveAndRestore + SetCurImportLocRAII(CurrentImportLoc, ImportLoc); + // Bump the generation number. unsigned PreviousGeneration = CurrentGeneration++; @@ -7167,7 +7170,7 @@ CXXTemporary *ASTReader::ReadCXXTemporary(ModuleFile &F, } DiagnosticBuilder ASTReader::Diag(unsigned DiagID) { - return Diag(SourceLocation(), DiagID); + return Diag(CurrentImportLoc, DiagID); } DiagnosticBuilder ASTReader::Diag(SourceLocation Loc, unsigned DiagID) { diff --git a/test/Modules/fatal-module-loader-error.m b/test/Modules/fatal-module-loader-error.m new file mode 100644 index 0000000000..acfc539112 --- /dev/null +++ b/test/Modules/fatal-module-loader-error.m @@ -0,0 +1,26 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: touch %t/Module.pcm +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t %s -fdisable-module-hash -F %S/Inputs -verify +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t %s -fdisable-module-hash -F %S/Inputs -DIMPLICIT -verify + +// This tests that after a fatal module loader error, we do not continue parsing. + +#ifdef IMPLICIT + +// expected-error@+1{{does not appear to be}} +#import +#pragma clang __debug crash; + +#else + +// expected-error@+1{{does not appear to be}} +@import Module; +#pragma clang __debug crash; + +#endif + +// Also check that libclang does not create a PCH with such an error. +// RUN: c-index-test -write-pch %t.pch -fmodules -fmodules-cache-path=%t %s \ +// RUN: -Xclang -fdisable-module-hash -F %S/Inputs 2>&1 | Filecheck %s +// CHECK: Unable to write PCH file -- 2.40.0