diff --git a/clang/docs/checkedc/3C/CONTRIBUTING.md b/clang/docs/checkedc/3C/CONTRIBUTING.md index a4e330e7a33e..8443c201b5b9 100644 --- a/clang/docs/checkedc/3C/CONTRIBUTING.md +++ b/clang/docs/checkedc/3C/CONTRIBUTING.md @@ -94,6 +94,58 @@ in subdirectories, so `*.c` will not pick up all of them; instead you can use `llvm-lit -vv .` to specify all test files under the current directory. +### Diagnostic verification + +3C supports the standard Clang diagnostic verifier +([`VerifyDiagnosticConsumer`](https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html#details)) +for testing errors and warnings reported by 3C via its main `DiagnosticsEngine`. +(Some 3C errors and warnings are reported via other means and cannot be tested +this way; the best solution we have for them right now is to `grep` the stderr +of 3C.) Diagnostic verification can be enabled via the usual `-Xclang -verify` +compiler option; other diagnostic verification options (`-Xclang +-verify=PREFIX`, etc.) should also work as normal. These must be passed as +_compiler_ options, not `3c` options; for example, if you are using `--` on the +`3c` command line, these options must be passed _after_ the `--`. + +Some notes about diagnostic verification in the context of 3C: + +* Parsing of the source files uses some of the compiler logic and thus may + generate compiler warnings, just as if you ran `clang` on the code. These are + sent to the diagnostic verifier along with diagnostics generated by 3C's + analysis. If you find it distracting to have to include the compiler warnings + in the set of expected diagnostics for a test, you can turn them off via the + `-Wno-everything` compiler option (which does not affect diagnostics generated + by 3C's analysis). + +* The `3c` tool works in several passes, where each pass runs on all translation + units: first `3c` parses the source files, then it runs several passes of + analysis. If a pass encounters at least one error, `3c` exits at the end of + that pass. Diagnostic verification does not change the _point_ at which `3c` + exits, but it changes the exit _code_ to indicate the result of verification + rather than the presence of errors. The verification includes the diagnostics + from all passes up to the point at which `3c` exits (i.e., the same + diagnostics that would be displayed if verification were not used). However, + an error that doesn't go via the main `DiagnosticsEngine` will cause an + unsuccessful exit code regardless of diagnostic verification. (This is + typically the behavior you want for a test.) + +* Diagnostic verification is independent for each translation unit, so in tests + with multiple translation units, you'll have to be careful that preprocessing + of each translation unit sees the correct set of `expected-*` directives for + the diagnostics generated for that translation unit (or an + `expected-no-diagnostics` directive if that translation unit generates no + diagnostics, even if other translation units do generate diagnostics). Be + warned that since which translation unit generated a given diagnostic isn't + visible to a normal user, we don't put much work into coming up with sensible + rules for this, but it should at least be deterministic for testing. + +Note that some 3C tests use diagnostic verification on calls to `clang` rather +than `3c`, so if you see `expected-*` directives in a test, you can look at the +`RUN` commands to see which command has `-Xclang -verify` and is using the +directives. If you want to verify diagnostics of more than one `RUN` command in +the same test, you can use different directive prefixes (`-Xclang +-verify=PREFIX`). + ## Coding guidelines Please follow [LLVM coding diff --git a/clang/include/clang/3C/3C.h b/clang/include/clang/3C/3C.h index a3dfabcf158b..76f9d7679889 100644 --- a/clang/include/clang/3C/3C.h +++ b/clang/include/clang/3C/3C.h @@ -57,7 +57,7 @@ struct _3COptions { bool AddCheckedRegions; - bool DisableCCTypeChecker; + bool EnableCCTypeChecker; bool WarnRootCause; @@ -68,11 +68,6 @@ struct _3COptions { bool ForceItypes; #endif - // Currently applies only to the rewriting phase (because it is the only phase - // that generates diagnostics, except for the declaration merging diagnostics - // that are currently fatal) and uses the default "expected" prefix. - bool VerifyDiagnosticOutput; - bool DumpUnwritableChanges; bool AllowUnwritableChanges; @@ -104,6 +99,8 @@ class _3CInterface { const std::vector &SourceFileList, clang::tooling::CompilationDatabase *CompDB); + ~_3CInterface(); + // Call clang to provide the data bool parseASTs(); @@ -141,10 +138,25 @@ class _3CInterface { // Dump all stats related to performance. bool dumpStats(); + // Determine the exit code that the `3c` tool should exit with after the last + // 3C stage, considering diagnostic verification. Must be called exactly once + // before the _3CInterface is destructed (unless construction failed). + int determineExitCode(); + private: _3CInterface(const struct _3COptions &CCopt, const std::vector &SourceFileList, - clang::tooling::CompilationDatabase *CompDB, bool &Failed); + clang::tooling::CompilationDatabase *CompDB); + + bool ConstructionFailed = false; + bool DeterminedExitCode = false; + + bool HadNonDiagnosticError = false; + + // Determine whether 3C can continue to the next stage of processing. Checks + // HadNonDiagnosticError and error diagnostics but ignores diagnostic + // verification. + bool isSuccessfulSoFar(); // saved ASTs std::vector< std::unique_ptr< ASTUnit >> ASTs; diff --git a/clang/lib/3C/3C.cpp b/clang/lib/3C/3C.cpp index c361983389e4..35a0beb2b86d 100644 --- a/clang/lib/3C/3C.cpp +++ b/clang/lib/3C/3C.cpp @@ -15,6 +15,7 @@ #include "clang/3C/ConstraintBuilder.h" #include "clang/3C/IntermediateToolHook.h" #include "clang/3C/RewriteUtils.h" +#include "clang/Frontend/VerifyDiagnosticConsumer.h" #include "clang/Tooling/ArgumentsAdjusters.h" #include "llvm/Support/TargetSelect.h" @@ -51,11 +52,10 @@ std::string PerWildPtrInfoJson; bool AllTypes; std::string BaseDir; bool AddCheckedRegions; -bool DisableCCTypeChecker; +bool EnableCCTypeChecker; bool WarnRootCause; bool WarnAllRootCause; std::set FilePaths; -bool VerifyDiagnosticOutput; bool DumpUnwritableChanges; bool AllowUnwritableChanges; bool AllowRewriteFailures; @@ -68,59 +68,217 @@ bool ForceItypes; static CompilationDatabase *CurrCompDB = nullptr; static tooling::CommandLineArguments SourceFiles; -ArgumentsAdjuster getIgnoreCheckedPointerAdjuster() { - return [](const CommandLineArguments &Args, StringRef /*unused*/) { - CommandLineArguments AdjustedArgs; - bool HasAdjuster = false; - for (size_t I = 0, E = Args.size(); I < E; ++I) { - StringRef Arg = Args[I]; - AdjustedArgs.push_back(Args[I]); - if (Arg == "-f3c-tool") { - HasAdjuster = true; - break; +// _3CDiagnosticConsumer is a wrapper DiagnosticConsumer that delays the +// EndSourceFile callback until 3C's analysis is complete, making it possible to +// use VerifyDiagnosticConsumer to verify diagnostics generated by 3C. +// +// Normally, the AST parsing process calls BeginSourceFile, passes `expected-*` +// directives and generated diagnostics to VerifyDiagnosticConsumer as they +// occur, and then calls EndSourceFile, which triggers the verification. But 3C +// does not perform its analysis (and generate diagnostics) until after AST +// parsing, so we need to delay the EndSourceFile callback until 3C is done +// generating diagnostics. The alternative of putting another +// BeginSourceFile/EndSourceFile pair around 3C's diagnostics would be difficult +// because VerifyDiagnosticConsumer would expect us to pass the `expected-*` +// directives again. +// +// _3CDiagnosticConsumer serves one additional purpose: its NumErrors field +// counts the errors originally generated by the compiler or 3C so that 3C can +// stop after a stage that generates an error regardless of whether the error +// was "expected", since it might not be safe to continue the analysis after an +// error. (In contrast, NumErrors of a VerifyDiagnosticConsumer gives the status +// of diagnostic verification and will be 0 until verification is completed.) It +// may work to use DiagnosticsEngine::hasErrorOccurred for this instead. +// +// Currently, diagnostic verification is per translation unit. In our few +// multi-translation-unit tests, ensuring that the `expected-*` directives are +// in the right translation units can sometimes be a hassle. But it's unclear +// whether it would be feasible to share all the necessary objects across +// translation units to perform a single verification. +// +// _3CDiagnosticConsumer can also wrap an ordinary DiagnosticConsumer when we +// aren't verifying diagnostics. In that case, diagnostics with source locations +// still need to be generated in a BeginSourceFile/EndSourceFile interval, so +// delaying the EndSourceFile (as we do) is OK, and starting a new +// BeginSourceFile/EndSourceFile interval after AST parsing would also be OK. +// _3CDiagnosticConsumer doesn't actually need to know whether diagnostic +// verification is enabled, because the underlying DiagnosticConsumer's +// getNumErrors() gives us the correct post-verification status either way. +// +// See clang/docs/checkedc/3C/clang-tidy.md#_3c-name-prefix +// NOLINTNEXTLINE(readability-identifier-naming) +class _3CDiagnosticConsumer : public DiagnosticConsumer { + std::unique_ptr UnderlyingConsumer; + + // In general, a DiagnosticConsumer can be used for more than one source file, + // but we use _3CDiagnosticConsumer for only one file, so we can use this + // simple state machine. + enum State { S_Startup, S_ASTParsing, S_3CAnalysis, S_Done }; + State CurrentState = S_Startup; + +public: + _3CDiagnosticConsumer(DiagnosticsEngine &Engine) + : UnderlyingConsumer(Engine.takeClient()) { + // This code currently only supports the default LibTooling setup in which + // the ClangTool has no global DiagnosticConsumer, so + // CompilerInstance::createDiagnostics creates one owned by the + // DiagnosticsEngine. If we needed to support the case in which the + // DiagnosticConsumer isn't owned, we could use an "UnderlyingConsumerOwner" + // pattern like VerifyDiagnosticConsumer does. + assert(UnderlyingConsumer); + } + + void BeginSourceFile(const LangOptions &LangOpts, + const Preprocessor *PP) override { + assert(CurrentState == S_Startup); + CurrentState = S_ASTParsing; + UnderlyingConsumer->BeginSourceFile(LangOpts, PP); + } + + void EndSourceFile() override { + assert(CurrentState == S_ASTParsing); + CurrentState = S_3CAnalysis; + // Delay forwarding the EndSourceFile callback. + } + + void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info) override { + // Count the originally generated diagnostics. + DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info); + UnderlyingConsumer->HandleDiagnostic(DiagLevel, Info); + } + + // Returns true if this translation unit was successful after diagnostic + // verification, i.e., verification is enabled and it succeeded, or + // verification is disabled and there were no errors. The caller must check + // _3CInterface::HadNonDiagnosticError separately. + bool finish3CAnalysis() { + assert(CurrentState == S_3CAnalysis); + CurrentState = S_Done; + UnderlyingConsumer->EndSourceFile(); + return UnderlyingConsumer->getNumErrors() == 0; + } + + ~_3CDiagnosticConsumer() { + // We considered asserting that the state is S_Done here, but if + // ASTUnit::LoadFromCompilerInvocation fails and returns null, the + // _3CDiagnosticConsumer may be destructed without reaching S_Done. However, + // even without such an assertion, we're still well protected against + // forgetting to finish verification and missing an error because if the + // ASTUnit is null, _3CInterface::parseASTs will record a "non-diagnostic + // error", while if the ASTUnit is non-null, it will get added to + // _3CInterface::ASTs and determineExitCode will call finish3CAnalysis. (And + // the _3CInterface destructor enforces that determineExitCode is called.) + } +}; + +// Based on LibTooling's ASTBuilderAction but does several custom things that we +// need. +// +// See clang/docs/checkedc/3C/clang-tidy.md#_3c-name-prefix +// NOLINTNEXTLINE(readability-identifier-naming) +class _3CASTBuilderAction : public ToolAction { + std::vector> &ASTs; + +public: + _3CASTBuilderAction(std::vector> &ASTs) + : ASTs(ASTs) {} + + bool runInvocation(std::shared_ptr Invocation, + FileManager *Files, + std::shared_ptr PCHContainerOps, + DiagnosticConsumer *DiagConsumer) override { + + // Adjust some compiler options. This is similar to what we could do with + // a LibTooling ArgumentsAdjuster, but we access the options in their parsed + // data structure rather than as strings, so it is much more robust. + + if (!EnableCCTypeChecker) + // Corresponds to the -f3c-tool compiler option. + Invocation->LangOpts->_3C = true; + + // Re-canonicalize the path of the main source file, in case it was + // overridden by the compilation database after it was originally + // canonicalized by the _3CInterface constructor. This completely fixes + // https://github.com/correctcomputation/checkedc-clang/issues/515 and + // https://github.com/correctcomputation/checkedc-clang/issues/604 for the + // main source file. + // + // If https://github.com/correctcomputation/checkedc-clang/issues/604 were + // fixed another way, then making the path absolute would be sufficient to + // fix https://github.com/correctcomputation/checkedc-clang/issues/515; the + // path wouldn't need to be canonical. + SmallVectorImpl &Inputs = + Invocation->getFrontendOpts().Inputs; + for (FrontendInputFile *Iter = Inputs.begin(); Iter < Inputs.end(); + Iter++) { + FrontendInputFile &OldInput = *Iter; + // getFile will assert that the input is a file, which should be true for + // 3C. + std::string OldPath = std::string(OldInput.getFile()), NewPath; + std::error_code EC = tryGetCanonicalFilePath(OldPath, NewPath); + if (EC) { + // If the compilation database specifies a bogus, inaccessible file, + // that will normally be caught by Driver::DiagnoseInputExistence before + // we get here. + errs() << "3C error: Failed to re-canonicalize source file path " + << OldPath << " during compiler invocation: " + << EC.message() << "\n"; + return false; } + *Iter = + FrontendInputFile(NewPath, OldInput.getKind(), OldInput.isSystem()); } - if (!DisableCCTypeChecker && !HasAdjuster) - AdjustedArgs.push_back("-f3c-tool"); - return AdjustedArgs; - }; -} -ArgumentsAdjuster addVerifyAdjuster() { - return [](const CommandLineArguments &Args, StringRef /*unused*/) { - CommandLineArguments AdjustedArgs(Args); - if (std::find(AdjustedArgs.begin(),AdjustedArgs.end(),"-verify") - == AdjustedArgs.end()) { - AdjustedArgs.push_back("-Xclang"); - AdjustedArgs.push_back("-verify"); - } - return AdjustedArgs; - }; -} -ArgumentsAdjuster canonicalizeSourceFilePathAdjuster() { - return [](const CommandLineArguments &Args, StringRef FileName) { - std::string AbsoluteFp; - std::error_code EC = tryGetCanonicalFilePath(std::string(FileName), AbsoluteFp); - assert(!EC && "We canonicalized the source file path during 3C " - "initialization; doing it again in " - "canonicalizeSourceFilePathAdjuster shouldn't fail."); - CommandLineArguments AdjustedArgs; - for (auto Arg : Args) { - if (Arg == FileName) { - // XXX We are assuming that any occurrence of the source file path as an - // argument should be replaced with the canonical file path. It might be - // safer to only match the argument that represents the input file. - // Unfortunately, it's hard to do here short of reimplementing compiler - // command line parsing. It would be better to adjust the parsed option - // data structures instead - // (https://github.com/correctcomputation/checkedc-clang/issues/536). - AdjustedArgs.push_back(AbsoluteFp); + + // Canonicalize -I paths to address the same two issues for `#include`d + // files. The situation is analogous except that this is not a complete fix + // for https://github.com/correctcomputation/checkedc-clang/issues/604 + // because the portion of the path in the `#include` directive may be + // non-canonical. + HeaderSearchOptions &HeaderOpts = *Invocation->HeaderSearchOpts; + std::vector NewUserEntries; + for (auto It = HeaderOpts.UserEntries.begin(); + It != HeaderOpts.UserEntries.end();) { + HeaderSearchOptions::Entry &Entry = *It; + std::string OldPath = Entry.Path, NewPath; + std::error_code EC = tryGetCanonicalFilePath(OldPath, NewPath); + if (EC) { + // Normally, if an -I directory isn't accessible, Clang seems to ignore + // it with no diagnostic. Hopefully, removing it from the list here will + // have the same effect and not break anything. If we kept the + // non-canonical entry, we might benefit from any diagnostic that Clang + // might issue in the future, but it's harder to reason about whether + // that might re-expose the original bugs. + It = HeaderOpts.UserEntries.erase(It); } else { - AdjustedArgs.push_back(Arg); + Entry.Path = NewPath; + It++; } } - return AdjustedArgs; - }; -} + + // Set up diagnostics. + IntrusiveRefCntPtr DiagEngine = + CompilerInstance::createDiagnostics(&Invocation->getDiagnosticOpts(), + DiagConsumer, + /*ShouldOwnClient=*/false); + // The _3CDiagnosticConsumer takes ownership of and wraps the engine's + // previous DiagnosticConsumer, i.e., the one created by + // CompilerInstance::createDiagnostics above (which will be a + // VerifyDiagnosticConsumer if requested via the options). + DiagEngine->setClient(new _3CDiagnosticConsumer(*DiagEngine)); + + // Finally, actually build the AST. This part is the same as in + // ASTBuilderAction::runInvocation. + + std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation( + Invocation, std::move(PCHContainerOps), DiagEngine, Files); + if (!AST) + return false; + + ASTs.push_back(std::move(AST)); + return true; + } +}; void dumpConstraintOutputJson(const std::string &PostfixStr, ProgramInfo &Info) { @@ -161,12 +319,11 @@ std::unique_ptr<_3CInterface> _3CInterface::create(const struct _3COptions &CCopt, const std::vector &SourceFileList, CompilationDatabase *CompDB) { - bool Failed = false; // See clang/docs/checkedc/3C/clang-tidy.md#_3c-name-prefix // NOLINTNEXTLINE(readability-identifier-naming) std::unique_ptr<_3CInterface> _3CInter( - new _3CInterface(CCopt, SourceFileList, CompDB, Failed)); - if (Failed) { + new _3CInterface(CCopt, SourceFileList, CompDB)); + if (_3CInter->ConstructionFailed) { return nullptr; } return _3CInter; @@ -174,7 +331,7 @@ _3CInterface::create(const struct _3COptions &CCopt, _3CInterface::_3CInterface(const struct _3COptions &CCopt, const std::vector &SourceFileList, - CompilationDatabase *CompDB, bool &Failed) { + CompilationDatabase *CompDB) { DumpIntermediate = CCopt.DumpIntermediate; Verbose = CCopt.Verbose; @@ -190,11 +347,10 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt, BaseDir = CCopt.BaseDir; AllTypes = CCopt.EnableAllTypes; AddCheckedRegions = CCopt.AddCheckedRegions; - DisableCCTypeChecker = CCopt.DisableCCTypeChecker; + EnableCCTypeChecker = CCopt.EnableCCTypeChecker; AllocatorFunctions = CCopt.AllocatorFunctions; WarnRootCause = CCopt.WarnRootCause || CCopt.WarnAllRootCause; WarnAllRootCause = CCopt.WarnAllRootCause; - VerifyDiagnosticOutput = CCopt.VerifyDiagnosticOutput; DumpUnwritableChanges = CCopt.DumpUnwritableChanges; AllowUnwritableChanges = CCopt.AllowUnwritableChanges; AllowRewriteFailures = CCopt.AllowRewriteFailures; @@ -211,23 +367,16 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt, ConstraintsBuilt = false; - if (VerifyDiagnosticOutput) { - errs() << "3C initialization error: Diagnostic verification is currently " - "unsupported.\n"; - Failed = true; - return; - } - if (OutputPostfix != "-" && !OutputDir.empty()) { errs() << "3C initialization error: Cannot use both -output-postfix and " "-output-dir\n"; - Failed = true; + ConstructionFailed = true; return; } if (OutputPostfix == "-" && OutputDir.empty() && SourceFileList.size() > 1) { errs() << "3C initialization error: Cannot specify more than one input " "file when output is to stdout\n"; - Failed = true; + ConstructionFailed = true; return; } @@ -244,7 +393,7 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt, if (EC) { errs() << "3C initialization error: Failed to canonicalize base directory " << "\"" << BaseDir << "\": " << EC.message() << "\n"; - Failed = true; + ConstructionFailed = true; return; } BaseDir = TmpPath; @@ -256,7 +405,7 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt, if (EC) { errs() << "3C initialization error: Failed to create output directory \"" << OutputDir << "\": " << EC.message() << "\n"; - Failed = true; + ConstructionFailed = true; return; } TmpPath = OutputDir; @@ -264,7 +413,7 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt, if (EC) { errs() << "3C initialization error: Failed to canonicalize output " << "directory \"" << OutputDir << "\": " << EC.message() << "\n"; - Failed = true; + ConstructionFailed = true; return; } OutputDir = TmpPath; @@ -279,7 +428,7 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt, if (EC) { errs() << "3C initialization error: Failed to canonicalize source file " << "path \"" << S << "\": " << EC.message() << "\n"; - Failed = true; + ConstructionFailed = true; continue; } FilePaths.insert(AbsPath); @@ -297,12 +446,12 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt, errs() << "The base directory is currently \"" << BaseDir << "\" and can be changed with the -base-dir option.\n"; if (OutputDir != "") { - Failed = true; + ConstructionFailed = true; errs() << "When using -output-dir, input files outside the base " "directory cannot be handled because there is no way to " "compute their output paths.\n"; } else if (!CCopt.AllowSourcesOutsideBaseDir) { - Failed = true; + ConstructionFailed = true; errs() << "You can use the -allow-sources-outside-base-dir option to " "temporarily downgrade this error to a warning.\n"; } @@ -313,39 +462,54 @@ _3CInterface::_3CInterface(const struct _3COptions &CCopt, GlobalProgramInfo.getPerfStats().startTotalTime(); } +_3CInterface::~_3CInterface() { + assert(ConstructionFailed || DeterminedExitCode); +} + +bool _3CInterface::isSuccessfulSoFar() { + if (HadNonDiagnosticError) + return false; + for (auto &TU : ASTs) + if (TU->getDiagnostics().getClient()->getNumErrors() > 0) + return false; + return true; +} + +int _3CInterface::determineExitCode() { + assert(!DeterminedExitCode); + DeterminedExitCode = true; + + bool Success = isSuccessfulSoFar(); + + bool SuccessAfterDiagnosticVerification = !HadNonDiagnosticError; + for (auto &TU : ASTs) + SuccessAfterDiagnosticVerification &= + ((_3CDiagnosticConsumer *)TU->getDiagnostics().getClient()) + ->finish3CAnalysis(); + + if (!Success && SuccessAfterDiagnosticVerification) + // In this case, the `3c` tool will typically just have printed a "Failure + // occurred while X... Exiting" message. If we let that be the last word, it + // could confuse the user. + llvm::errs() << "(Note: Exiting successfully because the failure was due " + "solely to expected error diagnostics and diagnostic " + "verification succeeded.)\n"; + + return (SuccessAfterDiagnosticVerification ? 0 : 1); +} + bool _3CInterface::parseASTs() { std::lock_guard Lock(InterfaceMutex); auto *Tool = new ClangTool(*CurrCompDB, SourceFiles); - Tool->appendArgumentsAdjuster(getIgnoreCheckedPointerAdjuster()); - // NOTE: This code is currently unreachable because VerifyDiagnosticOutput is - // rejected in the _3CInterface constructor. - // - // TODO: This currently only enables compiler diagnostic verification. - // see https://github.com/correctcomputation/checkedc-clang/issues/425 - // for status. - if (VerifyDiagnosticOutput) - Tool->appendArgumentsAdjuster(addVerifyAdjuster()); - // We canonicalize source file paths in the _3CInterface constructor, but the - // compilation database may override the path with a relative or otherwise - // non-canonical path, which may then affect the presumed filename, which is - // used in the PersistentSourceLoc. Since we need PersistentSourceLocs to be - // unambiguous, we canonicalize the source file path again here. We may still - // get relative presumed filenames from relative -I paths, though - // convert_project tries to change the -I options to avoid this. - Tool->appendArgumentsAdjuster(canonicalizeSourceFilePathAdjuster()); // load the ASTs - if (Tool->buildASTs(ASTs)) - return false; + _3CASTBuilderAction Action(ASTs); + int ToolExitStatus = Tool->run(&Action); + HadNonDiagnosticError |= (ToolExitStatus != 0); - // check for compile errors - unsigned int Errs = 0; - for (auto &TU : ASTs) - Errs += TU->getDiagnostics().getClient()->getNumErrors(); - - return Errs == 0; + return isSuccessfulSoFar(); } bool _3CInterface::addVariables() { @@ -354,15 +518,10 @@ bool _3CInterface::addVariables() { // 1. Add Variables. VariableAdderConsumer VA = VariableAdderConsumer(GlobalProgramInfo, nullptr); - unsigned int Errs = 0; - for (auto &TU : ASTs) { - TU->enableSourceFileDiagnostics(); + for (auto &TU : ASTs) VA.HandleTranslationUnit(TU->getASTContext()); - TU->getDiagnostics().getClient()->EndSourceFile(); - Errs += TU->getDiagnostics().getClient()->getNumErrors(); - } - return Errs == 0; + return isSuccessfulSoFar(); } bool _3CInterface::buildInitialConstraints() { @@ -371,23 +530,20 @@ bool _3CInterface::buildInitialConstraints() { if (!GlobalProgramInfo.link()) { errs() << "Linking failed!\n"; - return false; + HadNonDiagnosticError = true; + return isSuccessfulSoFar(); // False, of course, but follow the pattern. } // 2. Gather constraints. ConstraintBuilderConsumer CB = ConstraintBuilderConsumer(GlobalProgramInfo, nullptr); - unsigned int Errs = 0; - for (auto &TU : ASTs) { - TU->enableSourceFileDiagnostics(); + for (auto &TU : ASTs) CB.HandleTranslationUnit(TU->getASTContext()); - TU->getDiagnostics().getClient()->EndSourceFile(); - Errs += TU->getDiagnostics().getClient()->getNumErrors(); - } - if (Errs > 0) return false; + if (!isSuccessfulSoFar()) + return false; ConstraintsBuilt = true; - return true; + return isSuccessfulSoFar(); } bool _3CInterface::solveConstraints() { @@ -427,14 +583,10 @@ bool _3CInterface::solveConstraints() { // 4. Infer the bounds based on calls to malloc and calloc AllocBasedBoundsInference ABBI = AllocBasedBoundsInference(GlobalProgramInfo, nullptr); - unsigned int Errs = 0; - for (auto &TU : ASTs) { - TU->enableSourceFileDiagnostics(); + for (auto &TU : ASTs) ABBI.HandleTranslationUnit(TU->getASTContext()); - TU->getDiagnostics().getClient()->EndSourceFile(); - Errs += TU->getDiagnostics().getClient()->getNumErrors(); - } - if (Errs > 0) return false; + if (!isSuccessfulSoFar()) + return false; // Propagate the information from allocator bounds. GlobalProgramInfo.getABoundsInfo().performFlowAnalysis(&GlobalProgramInfo); @@ -443,14 +595,10 @@ bool _3CInterface::solveConstraints() { // 5. Run intermediate tool hook to run visitors that need to be executed // after constraint solving but before rewriting. IntermediateToolHook ITH = IntermediateToolHook(GlobalProgramInfo, nullptr); - unsigned int Errs = 0; - for (auto &TU : ASTs) { - TU->enableSourceFileDiagnostics(); + for (auto &TU : ASTs) ITH.HandleTranslationUnit(TU->getASTContext()); - TU->getDiagnostics().getClient()->EndSourceFile(); - Errs += TU->getDiagnostics().getClient()->getNumErrors(); - } - if (Errs > 0) return false; + if (!isSuccessfulSoFar()) + return false; if (AllTypes) { // Propagate data-flow information for Array pointers. @@ -490,7 +638,7 @@ bool _3CInterface::solveConstraints() { } }*/ - return true; + return isSuccessfulSoFar(); } bool _3CInterface::writeAllConvertedFilesToDisk() { @@ -498,18 +646,12 @@ bool _3CInterface::writeAllConvertedFilesToDisk() { // 6. Rewrite the input files. RewriteConsumer RC = RewriteConsumer(GlobalProgramInfo); - unsigned int Errs = 0; - for (auto &TU : ASTs) { - TU->enableSourceFileDiagnostics(); + for (auto &TU : ASTs) RC.HandleTranslationUnit(TU->getASTContext()); - TU->getDiagnostics().getClient()->EndSourceFile(); - Errs += TU->getDiagnostics().getClient()->getNumErrors(); - } - if (Errs > 0) return false; GlobalProgramInfo.getPerfStats().endTotalTime(); GlobalProgramInfo.getPerfStats().startTotalTime(); - return true; + return isSuccessfulSoFar(); } bool _3CInterface::dumpStats() { @@ -546,7 +688,7 @@ bool _3CInterface::dumpStats() { PerWildPtrInfo.close(); } } - return true; + return isSuccessfulSoFar(); } ConstraintsInfo &_3CInterface::getWildPtrsInfo() { diff --git a/clang/test/3C/base_subdir/canwrite_constraints.c b/clang/test/3C/base_subdir/canwrite_constraints.c index 4fcb5538a921..b02afe263c52 100644 --- a/clang/test/3C/base_subdir/canwrite_constraints.c +++ b/clang/test/3C/base_subdir/canwrite_constraints.c @@ -10,9 +10,10 @@ // not allow canwrite_constraints.h to change, and the internal types of q and // the return should remain wild. // -// The root cause warning verification part of this test is currently disabled -// because of https://github.com/correctcomputation/checkedc-clang/issues/503; -// the rest of the test still works. TODO: Re-enable warning verification. +// TODO: Fix the discrepancies in warnings that were introduced while the +// diagnostic verifier was disabled +// (https://github.com/correctcomputation/checkedc-clang/issues/609) +// and then re-enable diagnostic verification here. // // RUN: cd %S && 3c -alltypes -addcr -output-dir=%t.checked/base_subdir -warn-all-root-cause %s -- // RUN: FileCheck -match-full-lines -check-prefixes=CHECK_LOWER --input-file %t.checked/base_subdir/canwrite_constraints.c %s diff --git a/clang/test/3C/base_subdir/canwrite_constraints_symlink.c b/clang/test/3C/base_subdir/canwrite_constraints_symlink.c index f8b1a69eadbd..86f9091b4606 100644 --- a/clang/test/3C/base_subdir/canwrite_constraints_symlink.c +++ b/clang/test/3C/base_subdir/canwrite_constraints_symlink.c @@ -1,7 +1,3 @@ -// TODO: refactor this test -// https://github.com/correctcomputation/checkedc-clang/issues/503 -// XFAIL: * - // Like canwrite_constraints_unimplemented.c but tests the third former base dir // matching bug from // https://github.com/correctcomputation/checkedc-clang/issues/327: symlinks. @@ -24,7 +20,7 @@ // Now 3C should know that it can't write to base_subdir_partial_defn.h because // the symlink goes out of the base dir. // -// RUN: cd %t.base && 3c -addcr -verify canwrite_constraints_symlink.c -- +// RUN: cd %t.base && 3c -addcr canwrite_constraints_symlink.c -- -Xclang -verify // expected-error@base_subdir_partial_defn.h:1 {{3C internal error: 3C generated changes to this file even though it is not allowed to write to the file}} // expected-note@*:* {{-dump-unwritable-changes}} diff --git a/clang/test/3C/base_subdir/canwrite_constraints_unimplemented.c b/clang/test/3C/base_subdir/canwrite_constraints_unimplemented.c index 069ae8931b10..694ecf48e57b 100644 --- a/clang/test/3C/base_subdir/canwrite_constraints_unimplemented.c +++ b/clang/test/3C/base_subdir/canwrite_constraints_unimplemented.c @@ -1,13 +1,9 @@ -// TODO: refactor this test -// https://github.com/correctcomputation/checkedc-clang/issues/503 -// XFAIL: * - // An example of a case that is not handled by the canWrite constraints code and // causes 3C to generate a change to an unwritable file. Test that 3C generates // an error diagnostic. // (https://github.com/correctcomputation/checkedc-clang/issues/387) -// RUN: cd %S && 3c -addcr -verify %s -- +// RUN: cd %S && 3c -addcr %s -- -Xclang -verify // expected-error@../base_subdir_partial_defn.h:1 {{3C internal error: 3C generated changes to this file even though it is not allowed to write to the file}} // expected-note@*:* {{-dump-unwritable-changes}} diff --git a/clang/test/3C/canwrite_constraints.h b/clang/test/3C/canwrite_constraints.h index 11a69b8dcb99..f086d0342ece 100644 --- a/clang/test/3C/canwrite_constraints.h +++ b/clang/test/3C/canwrite_constraints.h @@ -53,7 +53,7 @@ void unwritable_cast(void((*g)(int *q)) : itype(_Ptr)>)) { // // TODO: Add the correct expected root cause warnings once diagnostic // verification is re-enabled -// (https://github.com/correctcomputation/checkedc-clang/issues/503). +// (https://github.com/correctcomputation/checkedc-clang/issues/609). void unwritable_func_with_itype(int *p : itype(_Array_ptr)) {} void unwritable_func_with_itype_and_bounds(int *p diff --git a/clang/test/3C/diag_verifier_fail.c b/clang/test/3C/diag_verifier_fail.c index 3ecdc55d0468..9766cc65b2fc 100644 --- a/clang/test/3C/diag_verifier_fail.c +++ b/clang/test/3C/diag_verifier_fail.c @@ -1,7 +1,3 @@ -// TODO: refactor this test -// https://github.com/correctcomputation/checkedc-clang/issues/503 -// XFAIL: * - // Test that the diagnostic verifier is functioning correctly in 3c, because if // it isn't, all the other regression tests that use the diagnostic verifier may // not be able to catch the problems they are supposed to catch. @@ -11,7 +7,7 @@ // fail. // RUN: rm -rf %t* -// RUN: not 3c -base-dir=%S -extra-arg="-Wno-everything" -verify -warn-root-cause %s -- 2>%t.stderr +// RUN: not 3c -base-dir=%S -warn-root-cause %s -- -Xclang -verify -Wno-everything 2>%t.stderr // RUN: grep -q "error: 'warning' diagnostics expected but not seen:" %t.stderr // RUN: grep -q "error: 'warning' diagnostics seen but not expected:" %t.stderr diff --git a/clang/test/3C/diag_verifier_pass.c b/clang/test/3C/diag_verifier_pass.c index 809240f7d33a..ea68c6379498 100644 --- a/clang/test/3C/diag_verifier_pass.c +++ b/clang/test/3C/diag_verifier_pass.c @@ -1,7 +1,3 @@ -// TODO: refactor this test -// https://github.com/correctcomputation/checkedc-clang/issues/503 -// XFAIL: * - // Test that the diagnostic verifier is functioning correctly in 3c, because if // it isn't, all the other regression tests that use the diagnostic verifier may // not be able to catch the problems they are supposed to catch. This goes with @@ -13,7 +9,7 @@ // those errors and fail, so diag_verifier_{pass,fail}.c only add coverage for // unusual problems with the diagnostic verifier that we haven't seen yet. -// RUN: 3c -base-dir=%S -extra-arg="-Wno-everything" -verify -warn-root-cause %s -- +// RUN: 3c -base-dir=%S -warn-root-cause %s -- -Xclang -verify -Wno-everything // Example warning borrowed from root_cause.c . void *x; // expected-warning {{Default void* type}} diff --git a/clang/test/3C/difftypes1a.c b/clang/test/3C/difftypes1a.c index 2a4f38bfef66..021c27f9360d 100644 --- a/clang/test/3C/difftypes1a.c +++ b/clang/test/3C/difftypes1a.c @@ -1,7 +1,13 @@ -//RUN: rm -rf %t* -//RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1b.c -- 2>%t.stderr -//RUN: grep -q "merging failed" %t.stderr +// Since the RUN commands in difftypes1a.c and difftypes1b.c process the two +// files in different orders and the location where the error is reported +// depends on the order, we need to use a different diagnostic verification +// prefix (and set of corresponding comments) for each RUN command. Verification +// is per translation unit, so the translation unit with no diagnostic needs +// `expected-no-diagnostics`. -// The desired behavior in this case is to fail, so other checks are omitted +//RUN: rm -rf %t* +//RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1b.c -- -Xclang -verify=ab-expected +// ab-expected-no-diagnostics +// ba-expected-error@+1 {{merging failed for 'foo'}} _Ptr foo(int, char); diff --git a/clang/test/3C/difftypes1b.c b/clang/test/3C/difftypes1b.c index 3959be295978..be3a338812bb 100644 --- a/clang/test/3C/difftypes1b.c +++ b/clang/test/3C/difftypes1b.c @@ -1,7 +1,13 @@ -//RUN: rm -rf %t* -//RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1a.c -- 2>%t.stderr -//RUN: grep -q "merging failed" %t.stderr +// Since the RUN commands in difftypes1a.c and difftypes1b.c process the two +// files in different orders and the location where the error is reported +// depends on the order, we need to use a different diagnostic verification +// prefix (and set of corresponding comments) for each RUN command. Verification +// is per translation unit, so the translation unit with no diagnostic needs +// `expected-no-diagnostics`. -// The desired behavior in this case is to fail, so other checks are omitted +//RUN: rm -rf %t* +//RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes1a.c -- -Xclang -verify=ba-expected +// ba-expected-no-diagnostics +// ab-expected-error@+1 {{merging failed for 'foo'}} int *foo(int, char *); diff --git a/clang/test/3C/difftypes2a.c b/clang/test/3C/difftypes2a.c index 3096543990f2..7969c47e7910 100644 --- a/clang/test/3C/difftypes2a.c +++ b/clang/test/3C/difftypes2a.c @@ -1,9 +1,17 @@ +// Since the RUN commands in difftypes2a.c and difftypes2b.c process the two +// files in different orders and the location where the error is reported +// depends on the order, we need to use a different diagnostic verification +// prefix (and set of corresponding comments) for each RUN command. Verification +// is per translation unit, so the translation unit with no diagnostic needs +// `expected-no-diagnostics`. + // RUN: rm -rf %t* -// RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes2b.c -- 2>%t.stderr -// RUN: grep -q "merging failed" %t.stderr +// RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes2b.c -- -Xclang -verify=ab-expected // The desired behavior in this case is to fail, so other checks are omitted // Test no body vs body +// ab-expected-no-diagnostics +// ba-expected-error@+1 {{merging failed for 'foo'}} void foo(char *x); diff --git a/clang/test/3C/difftypes2b.c b/clang/test/3C/difftypes2b.c index c50af4ce24a4..edc480d282a5 100644 --- a/clang/test/3C/difftypes2b.c +++ b/clang/test/3C/difftypes2b.c @@ -1,11 +1,17 @@ -// RUN: rm -rf %t* -// RUN: not 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes2a.c -- 2>%t.stderr -// RUN: grep -q "merging failed" %t.stderr +// Since the RUN commands in difftypes2a.c and difftypes2b.c process the two +// files in different orders and the location where the error is reported +// depends on the order, we need to use a different diagnostic verification +// prefix (and set of corresponding comments) for each RUN command. Verification +// is per translation unit, so the translation unit with no diagnostic needs +// `expected-no-diagnostics`. -// The desired behavior in this case is to fail, so other checks are omitted +// RUN: rm -rf %t* +// RUN: 3c -base-dir=%S -output-dir=%t.checked %s %S/difftypes2a.c -- -Xclang -verify=ba-expected // Test body vs no body +// ba-expected-no-diagnostics +// ab-expected-error@+1 {{merging failed for 'foo'}} void foo(char **y) { // this is the body } diff --git a/clang/test/3C/macro_function_call.c b/clang/test/3C/macro_function_call.c index 1797c7ed9685..f568e80709d7 100644 --- a/clang/test/3C/macro_function_call.c +++ b/clang/test/3C/macro_function_call.c @@ -1,9 +1,9 @@ // RUN: rm -rf %t* -// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s -// RUN: 3c -base-dir=%S -addcr %s -- | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s -// RUN: 3c -base-dir=%S -alltypes -addcr %s -- | %clang -c -fcheckedc-extension -x c -o /dev/null - -// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s -- -// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/macro_function_call.c -- | diff %t.checked/macro_function_call.c - +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- -Xclang -verify | FileCheck -match-full-lines -check-prefixes="CHECK_ALL","CHECK" %s +// RUN: 3c -base-dir=%S -addcr %s -- -Xclang -verify | FileCheck -match-full-lines -check-prefixes="CHECK_NOALL","CHECK" %s +// RUN: 3c -base-dir=%S -alltypes -addcr %s -- -Xclang -verify | %clang -c -fcheckedc-extension -x c -o /dev/null - +// RUN: 3c -base-dir=%S -alltypes -output-dir=%t.checked %s -- -Xclang -verify +// RUN: 3c -base-dir=%t.checked -alltypes %t.checked/macro_function_call.c -- -Xclang -verify | diff %t.checked/macro_function_call.c - // Test fix for https://github.com/correctcomputation/checkedc-clang/issues/439 // We cannot insert casts on function calls inside macros, so constraints must @@ -12,13 +12,6 @@ // 3C emits a warning if it fails inserting a cast. Ensure the test fails if // this happens. -// -// NOTICE: This part of the test is disabled (the -verify option has been -// removed from the 3c RUN commands) until we have a replacement for the -// diagnostic verifier -// (https://github.com/correctcomputation/checkedc-clang/issues/503). -// TODO: Re-enable it when we do. -// // expected-no-diagnostics // Unsafe call in macro. This would require an _Assume_bounds_cast, but we diff --git a/clang/test/3C/macro_rewrite_error.c b/clang/test/3C/macro_rewrite_error.c index 972b184e56a5..cf340f16ab15 100644 --- a/clang/test/3C/macro_rewrite_error.c +++ b/clang/test/3C/macro_rewrite_error.c @@ -1,8 +1,4 @@ -// TODO: refactor this test -// https://github.com/correctcomputation/checkedc-clang/issues/503 -// XFAIL: * - -// RUN: 3c -base-dir=%S -extra-arg="-Wno-everything" -verify -alltypes %s -- +// RUN: 3c -base-dir=%S -alltypes %s -- -Xclang -verify -Wno-everything // clang-format makes a mess of this file. // clang-format off diff --git a/clang/test/3C/root_cause.c b/clang/test/3C/root_cause.c index 2e88b4acbdca..bfbf1e369327 100644 --- a/clang/test/3C/root_cause.c +++ b/clang/test/3C/root_cause.c @@ -1,8 +1,9 @@ -// TODO: Refactor this test -// https://github.com/correctcomputation/checkedc-clang/issues/503 +// TODO: Fix the failures in this test that were introduced while the diagnostic +// verifier was disabled +// (https://github.com/correctcomputation/checkedc-clang/issues/609). // XFAIL: * -// RUN: 3c -base-dir=%S -extra-arg="-Wno-everything" -verify -alltypes -warn-root-cause %s -- +// RUN: 3c -base-dir=%S -alltypes -warn-root-cause %s -- -Xclang -verify -Wno-everything // This test is unusual in that it checks for the errors in the code diff --git a/clang/test/3C/stdout_mode_write_other.c b/clang/test/3C/stdout_mode_write_other.c index aefcdf89c374..731ad45cdbd0 100644 --- a/clang/test/3C/stdout_mode_write_other.c +++ b/clang/test/3C/stdout_mode_write_other.c @@ -1,13 +1,14 @@ -// TODO: refactor this test -// https://github.com/correctcomputation/checkedc-clang/issues/503 -// XFAIL: * - // Like the base_subdir/canwrite_constraints_unimplemented test except that // writing base_subdir_partial_defn.h is not restricted by the base dir, so we // reach the stdout mode restriction. +// +// Since this is currently the simplest 3C regression test that tests an error +// diagnostic, we also use it to test some things about error diagnostics in +// general. // RUN: rm -rf %t* -// RUN: 3c -base-dir=%S -addcr -verify %s -- +// RUN: 3c -base-dir=%S -addcr %s -- -Xclang -verify 2>%t.stderr +// RUN: grep -q 'Exiting successfully because the failure was due solely to expected error diagnostics and diagnostic verification succeeded' %t.stderr // Same as above, except instead of using the diagnostic verifier, manually test // that 3c exits nonzero and prints the error to stderr. This tests that 3c diff --git a/clang/tools/3c/3CStandalone.cpp b/clang/tools/3c/3CStandalone.cpp index d8aa1a6dadac..120bccdf3192 100644 --- a/clang/tools/3c/3CStandalone.cpp +++ b/clang/tools/3c/3CStandalone.cpp @@ -156,10 +156,13 @@ static cl::opt OptAddCheckedRegions("addcr", cl::init(false), cl::cat(_3CCategory)); -static cl::opt - OptDiableCCTypeChecker("disccty", - cl::desc("Do not disable checked c type checker."), - cl::init(false), cl::cat(_3CCategory)); +static cl::opt OptEnableCCTypeChecker( + "enccty", + cl::desc( + "Enable the Checked C type checker. 3c normally disables it (via the " + "equivalent of `clang -f3c-tool`) so that 3c can operate on partially " + "converted programs that may have Checked C type errors."), + cl::init(false), cl::cat(_3CCategory)); static cl::opt OptBaseDir( "base-dir", @@ -191,20 +194,6 @@ static cl::opt "even those unlikely to be interesting."), cl::init(false), cl::cat(_3CCategory)); -// https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html#details -// -// Analogous to the -verify option of `clang -cc1`, but currently applies only -// to the rewriting phase (because it is the only phase that generates -// diagnostics, except for the declaration merging diagnostics that are -// currently fatal). No checking of diagnostics from the other phases is -// performed. We cannot simply have the caller pass `-extra-arg=-Xclang -// -extra-arg=-verify` because that would expect each phase to produce the same -// set of diagnostics. -static cl::opt OptVerifyDiagnosticOutput( - "verify", - cl::desc("Verify diagnostic output (for automated testing of 3c)."), - cl::init(false), cl::cat(_3CCategory), cl::Hidden); - // In the future, we may enhance this to write the output to individual files. // For now, the user has to copy and paste the correct portions of stderr. static cl::opt OptDumpUnwritableChanges( @@ -297,10 +286,9 @@ int main(int argc, const char **argv) { CcOptions.PerPtrInfoJson = OptPerPtrWILDInfoJson.getValue(); CcOptions.AddCheckedRegions = OptAddCheckedRegions; CcOptions.EnableAllTypes = OptAllTypes; - CcOptions.DisableCCTypeChecker = OptDiableCCTypeChecker; + CcOptions.EnableCCTypeChecker = OptEnableCCTypeChecker; CcOptions.WarnRootCause = OptWarnRootCause; CcOptions.WarnAllRootCause = OptWarnAllRootCause; - CcOptions.VerifyDiagnosticOutput = OptVerifyDiagnosticOutput; CcOptions.DumpUnwritableChanges = OptDumpUnwritableChanges; CcOptions.AllowUnwritableChanges = OptAllowUnwritableChanges; CcOptions.AllowRewriteFailures = OptAllowRewriteFailures; @@ -347,7 +335,7 @@ int main(int argc, const char **argv) { // Build AST from source. if (!_3CInterface.parseASTs()) { errs() << "Failure occurred while parsing source files. Exiting.\n"; - return 1; + return _3CInterface.determineExitCode(); } if (OptVerbose) { @@ -358,7 +346,7 @@ int main(int argc, const char **argv) { // Add variables. if (!_3CInterface.addVariables()) { errs() << "Failure occurred while trying to add variables. Exiting.\n"; - return 1; + return _3CInterface.determineExitCode(); } if (OptVerbose) { @@ -369,7 +357,7 @@ int main(int argc, const char **argv) { // Build constraints. if (!_3CInterface.buildInitialConstraints()) { errs() << "Failure occurred while trying to build constraints. Exiting.\n"; - return 1; + return _3CInterface.determineExitCode(); } if (OptVerbose) { @@ -380,7 +368,7 @@ int main(int argc, const char **argv) { // Solve the constraints. if (!_3CInterface.solveConstraints()) { errs() << "Failure occurred while trying to solve constraints. Exiting.\n"; - return 1; + return _3CInterface.determineExitCode(); } if (OptVerbose) { @@ -392,14 +380,17 @@ int main(int argc, const char **argv) { if (!_3CInterface.writeAllConvertedFilesToDisk()) { errs() << "Failure occurred while trying to rewrite converted files back. " "Exiting.\n"; - return 1; + return _3CInterface.determineExitCode(); } // Write all the performance related stats. if (!_3CInterface.dumpStats()) { errs() << "Failure occurred while trying to write performance stats. " "Exiting.\n"; - return 1; + return _3CInterface.determineExitCode(); } - return 0; + + // Even if all passes succeeded, we could still have a diagnostic verification + // failure. + return _3CInterface.determineExitCode(); } diff --git a/clang/tools/3c/utils/port_tools/generate_ccommands.py b/clang/tools/3c/utils/port_tools/generate_ccommands.py index ad05eb77e4e2..4a74d91f93f1 100644 --- a/clang/tools/3c/utils/port_tools/generate_ccommands.py +++ b/clang/tools/3c/utils/port_tools/generate_ccommands.py @@ -133,7 +133,6 @@ def run3C(checkedc_bin, extra_3c_args, translation_units: List[TranslationUnitInfo] = [] all_files = [] - absolute_include_dirs = set() for i in cmds: file_to_add = i['file'] compiler_path = None # XXX Clean this up @@ -165,9 +164,6 @@ def run3C(checkedc_bin, extra_3c_args, target_directory, file_to_add, output_filename) translation_units.append(tu) - for arg in compiler_x_args: - if arg.startswith('-I'): - absolute_include_dirs.add(tu.realpath(arg[len('-I'):])) expandMacros(expand_macros_opts, compilation_base_dir, translation_units) @@ -223,16 +219,6 @@ def run3C(checkedc_bin, extra_3c_args, args.append('-p') args.append(compile_commands_json) args.append('-extra-arg=-w') - # This is a workaround for a bug in Clang LibTooling that affects the use of - # relative paths with -I when different translation units have different - # working directories. For details, see - # https://github.com/correctcomputation/checkedc-clang/issues/515 . - # - # Even if the above issue were fixed, we may still need this in some cases - # to ensure that PersistentSourceLoc filenames are unambiguous: see the - # comment in _3CInterface::parseASTs in clang/lib/3C/3C.cpp . - for dir in absolute_include_dirs: - args.append('-extra-arg-before=-I' + dir) vcodewriter.addClangdArg("-log=verbose") vcodewriter.addClangdArg(args[1:]) args.append('-base-dir="' + compilation_base_dir + '"')