diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def --- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def +++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def @@ -310,6 +310,10 @@ "Emit fix-it hints as remarks for testing purposes", false) +ANALYZER_OPTION(bool, ShouldApplyFixIts, "apply-fixits", + "Apply the fix-it hints to the files", + false) + //===----------------------------------------------------------------------===// // Unsigned analyzer options. //===----------------------------------------------------------------------===// diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp --- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -12,7 +12,6 @@ #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h" #include "ModelInjector.h" -#include "clang/Analysis/PathDiagnostic.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" @@ -21,10 +20,12 @@ #include "clang/Analysis/CFG.h" #include "clang/Analysis/CallGraph.h" #include "clang/Analysis/CodeInjector.h" +#include "clang/Analysis/PathDiagnostic.h" #include "clang/Basic/SourceManager.h" #include "clang/CrossTU/CrossTranslationUnit.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/Preprocessor.h" +#include "clang/Rewrite/Core/Rewriter.h" #include "clang/StaticAnalyzer/Checkers/LocalCheckers.h" #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" @@ -33,6 +34,8 @@ #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" #include "clang/StaticAnalyzer/Frontend/CheckerRegistration.h" +#include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Tooling.h" #include "llvm/ADT/PostOrderIterator.h" #include "llvm/ADT/Statistic.h" #include "llvm/Support/FileSystem.h" @@ -46,6 +49,7 @@ using namespace clang; using namespace ento; +using namespace tooling; #define DEBUG_TYPE "AnalysisConsumer" @@ -84,11 +88,16 @@ namespace { class ClangDiagPathDiagConsumer : public PathDiagnosticConsumer { DiagnosticsEngine &Diag; - bool IncludePath = false, ShouldEmitAsError = false, FixitsAsRemarks = false; + LangOptions LO; + + bool IncludePath = false; + bool ShouldEmitAsError = false; + bool FixitsAsRemarks = false; + bool ApplyFixIts = false; public: - ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag) - : Diag(Diag) {} + ClangDiagPathDiagConsumer(DiagnosticsEngine &Diag, LangOptions LO) + : Diag(Diag), LO(LO) {} ~ClangDiagPathDiagConsumer() override {} StringRef getName() const override { return "ClangDiags"; } @@ -102,6 +111,7 @@ void enablePaths() { IncludePath = true; } void enableWerror() { ShouldEmitAsError = true; } void enableFixitsAsRemarks() { FixitsAsRemarks = true; } + void enableApplyFixIts() { ApplyFixIts = true; } void FlushDiagnosticsImpl(std::vector &Diags, FilesMade *filesMade) override { @@ -111,29 +121,44 @@ : Diag.getCustomDiagID(DiagnosticsEngine::Warning, "%0"); unsigned NoteID = Diag.getCustomDiagID(DiagnosticsEngine::Note, "%0"); unsigned RemarkID = Diag.getCustomDiagID(DiagnosticsEngine::Remark, "%0"); + SourceManager &SM = Diag.getSourceManager(); + + Replacements Repls; + auto reportPiece = [&](unsigned ID, FullSourceLoc Loc, StringRef String, + ArrayRef Ranges, + ArrayRef Fixits) { + if (!FixitsAsRemarks && !ApplyFixIts) { + Diag.Report(Loc, ID) << String << Ranges << Fixits; + return; + } + + Diag.Report(Loc, ID) << String << Ranges; + if (FixitsAsRemarks) { + for (const FixItHint &Hint : Fixits) { + llvm::SmallString<128> Str; + llvm::raw_svector_ostream OS(Str); + // FIXME: Add support for InsertFromRange and + // BeforePreviousInsertion. + assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!"); + assert(!Hint.BeforePreviousInsertions && "Not implemented yet!"); + OS << SM.getSpellingColumnNumber(Hint.RemoveRange.getBegin()) << "-" + << SM.getSpellingColumnNumber(Hint.RemoveRange.getEnd()) << ": '" + << Hint.CodeToInsert << "'"; + Diag.Report(Loc, RemarkID) << OS.str(); + } + } + + if (ApplyFixIts) { + for (const FixItHint &Hint : Fixits) { + Replacement Repl(SM, Hint.RemoveRange, Hint.CodeToInsert); - auto reportPiece = - [&](unsigned ID, SourceLocation Loc, StringRef String, - ArrayRef Ranges, ArrayRef Fixits) { - if (!FixitsAsRemarks) { - Diag.Report(Loc, ID) << String << Ranges << Fixits; - } else { - Diag.Report(Loc, ID) << String << Ranges; - for (const FixItHint &Hint : Fixits) { - SourceManager &SM = Diag.getSourceManager(); - llvm::SmallString<128> Str; - llvm::raw_svector_ostream OS(Str); - // FIXME: Add support for InsertFromRange and - // BeforePreviousInsertion. - assert(!Hint.InsertFromRange.isValid() && "Not implemented yet!"); - assert(!Hint.BeforePreviousInsertions && "Not implemented yet!"); - OS << SM.getSpellingColumnNumber(Hint.RemoveRange.getBegin()) - << "-" << SM.getSpellingColumnNumber(Hint.RemoveRange.getEnd()) - << ": '" << Hint.CodeToInsert << "'"; - Diag.Report(Loc, RemarkID) << OS.str(); - } + if (llvm::Error Err = Repls.add(Repl)) { + llvm::errs() << "Error applying replacement " << Repl.toString() + << ": " << Err << "\n"; } - }; + } + } + }; for (std::vector::iterator I = Diags.begin(), E = Diags.end(); @@ -165,6 +190,16 @@ Piece->getString(), Piece->getRanges(), Piece->getFixits()); } } + + if (!ApplyFixIts || Repls.empty()) + return; + + Rewriter Rewrite(SM, LO); + if (!applyAllReplacements(Repls, Rewrite)) { + llvm::errs() << "An error occured during applying fix-it.\n"; + } + + Rewrite.overwriteChangedFiles(); } }; } // end anonymous namespace @@ -257,7 +292,7 @@ if (Opts->AnalysisDiagOpt != PD_NONE) { // Create the PathDiagnosticConsumer. ClangDiagPathDiagConsumer *clangDiags = - new ClangDiagPathDiagConsumer(PP.getDiagnostics()); + new ClangDiagPathDiagConsumer(PP.getDiagnostics(), PP.getLangOpts()); PathConsumers.push_back(clangDiags); if (Opts->AnalyzerWerror) @@ -266,6 +301,9 @@ if (Opts->ShouldEmitFixItHintsAsRemarks) clangDiags->enableFixitsAsRemarks(); + if (Opts->ShouldApplyFixIts) + clangDiags->enableApplyFixIts(); + if (Opts->AnalysisDiagOpt == PD_TEXT) { clangDiags->enablePaths(); diff --git a/clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt b/clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt --- a/clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt +++ b/clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt @@ -21,4 +21,6 @@ clangLex clangStaticAnalyzerCheckers clangStaticAnalyzerCore + clangRewrite + clangToolingCore ) diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -11,6 +11,7 @@ // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04 // CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01 // CHECK-NEXT: alpha.security.taint.TaintPropagation:Config = "" +// CHECK-NEXT: apply-fixits = false // CHECK-NEXT: avoid-suppressing-null-argument-paths = false // CHECK-NEXT: c++-allocator-inlining = true // CHECK-NEXT: c++-container-inlining = false @@ -100,4 +101,4 @@ // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: widen-loops = false // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 97 +// CHECK-NEXT: num-entries = 98 diff --git a/clang/test/Analysis/check-analyzer-fixit.py b/clang/test/Analysis/check-analyzer-fixit.py new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/check-analyzer-fixit.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python +# +#===- check-analyzer-fixit.py - Static Analyzer test helper ---*- python -*-===# +# +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +# +#===------------------------------------------------------------------------===# +# +# This file copy-pasted mostly from the Clang-Tidy's 'check_clang_tidy.py'. +# +#===------------------------------------------------------------------------===# + +r""" +Clang Static Analyzer test helper +================================= + +This script runs the Analyzer in fix-it mode and verify fixes, warnings, notes. + +Usage: + check-analyzer-fixit.py [analyzer arguments] + +Example: + // RUN: %check-analyzer-fixit %s %t -analyzer-checker=core +""" + +import argparse +import os +import re +import subprocess +import sys + + +def write_file(file_name, text): + with open(file_name, 'w') as f: + f.write(text) + + +def run_test_once(args, extra_args): + input_file_name = args.input_file_name + temp_file_name = args.temp_file_name + clang_analyzer_extra_args = extra_args + + file_name_with_extension = input_file_name + _, extension = os.path.splitext(file_name_with_extension) + if extension not in ['.c', '.hpp', '.m', '.mm']: + extension = '.cpp' + temp_file_name = temp_file_name + extension + + with open(input_file_name, 'r') as input_file: + input_text = input_file.read() + + # Remove the contents of the CHECK lines to avoid CHECKs matching on + # themselves. We need to keep the comments to preserve line numbers while + # avoiding empty lines which could potentially trigger formatting-related + # checks. + cleaned_test = re.sub('// *CHECK-[A-Z0-9\-]*:[^\r\n]*', '//', input_text) + write_file(temp_file_name, cleaned_test) + + original_file_name = temp_file_name + ".orig" + write_file(original_file_name, cleaned_test) + + try: + builtin_include_dir = subprocess.check_output( + ['clang', '-print-file-name=include'], stderr=subprocess.STDOUT) + except subprocess.CalledProcessError as e: + print('Cannot print Clang include directory: ' + e.output.decode()) + + builtin_include_dir = os.path.normpath(builtin_include_dir) + + args = (['clang', '-cc1', '-internal-isystem', builtin_include_dir, + '-nostdsysteminc', '-analyze', '-analyzer-constraints=range', + '-analyzer-config', 'apply-fixits=true'] + + clang_analyzer_extra_args + ['-verify', temp_file_name]) + + print('Running ' + str(args) + '...') + + try: + clang_analyzer_output = \ + subprocess.check_output(args, stderr=subprocess.STDOUT).decode() + except subprocess.CalledProcessError as e: + print('Clang Static Analyzer test failed:\n' + e.output.decode()) + raise + + print('----------------- Clang Static Analyzer output -----------------\n' + + clang_analyzer_output + + '\n--------------------------------------------------------------') + + try: + diff_output = subprocess.check_output( + ['diff', '-u', original_file_name, temp_file_name], + stderr=subprocess.STDOUT) + except subprocess.CalledProcessError as e: + diff_output = e.output + + print('----------------------------- Fixes ----------------------------\n' + + diff_output.decode() + + '\n--------------------------------------------------------------') + + try: + subprocess.check_output( + ['FileCheck', '-input-file=' + temp_file_name, input_file_name, + '-check-prefixes=CHECK-FIXES', '-strict-whitespace'], + stderr=subprocess.STDOUT) + except subprocess.CalledProcessError as e: + print('FileCheck failed:\n' + e.output.decode()) + raise + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument('input_file_name') + parser.add_argument('temp_file_name') + + args, extra_args = parser.parse_known_args() + run_test_once(args, extra_args) + + +if __name__ == '__main__': + main() diff --git a/clang/test/Analysis/virtualcall-fixit.cpp b/clang/test/Analysis/virtualcall-fixit.cpp new file mode 100644 --- /dev/null +++ b/clang/test/Analysis/virtualcall-fixit.cpp @@ -0,0 +1,13 @@ +// RUN: %check_analyzer_fixit %s %t \ +// RUN: -analyzer-checker=core,optin.cplusplus.VirtualCall \ +// RUN: -analyzer-config optin.cplusplus.VirtualCall:ShowFixIts=true + +struct S { + virtual void foo(); + S() { + foo(); + // expected-warning@-1 {{Call to virtual method 'S::foo' during construction bypasses virtual dispatch}} + // CHECK-FIXES: S::foo(); + } + ~S(); +}; diff --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py --- a/clang/test/lit.cfg.py +++ b/clang/test/lit.cfg.py @@ -77,6 +77,11 @@ if config.clang_staticanalyzer_z3 == '1': config.available_features.add('z3') + check_analyzer_fixit_path = os.path.join( + config.test_source_root, "Analysis", "check-analyzer-fixit.py") + config.substitutions.append( + ('%check_analyzer_fixit', + '%s %s' % (config.python_executable, check_analyzer_fixit_path))) llvm_config.add_tool_substitutions(tools, tool_dirs)