Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -208,6 +208,18 @@ ReportDiag(*Diag, filesMade); } +static llvm::SmallString<32> getIssueHash(const PathDiagnostic &D, + const Preprocessor &PP) { + SourceManager &SMgr = PP.getSourceManager(); + PathDiagnosticLocation UPDLoc = D.getUniqueingLoc(); + FullSourceLoc L(SMgr.getExpansionLoc(UPDLoc.isValid() + ? UPDLoc.asLocation() + : D.getLocation().asLocation()), + SMgr); + return getIssueHash(L, D.getCheckerName(), D.getBugType(), + D.getDeclWithIssue(), PP.getLangOpts()); +} + void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D, FilesMade *filesMade) { // Create the HTML directory if it is missing. @@ -234,11 +246,6 @@ // Create a new rewriter to generate HTML. Rewriter R(const_cast(SMgr), PP.getLangOpts()); - // The file for the first path element is considered the main report file, it - // will usually be equivalent to SMgr.getMainFileID(); however, it might be a - // header when -analyzer-opt-analyze-headers is used. - FileID ReportFile = path.front()->getLocation().asLocation().getExpansionLoc().getFileID(); - // Get the function/method name SmallString<128> declName("unknown"); int offsetDecl = 0; @@ -265,46 +272,52 @@ // Create a path for the target HTML file. int FD; - SmallString<128> Model, ResultPath; - - if (!DiagOpts.ShouldWriteStableReportFilename) { - llvm::sys::path::append(Model, Directory, "report-%%%%%%.html"); - if (std::error_code EC = - llvm::sys::fs::make_absolute(Model)) { - llvm::errs() << "warning: could not make '" << Model - << "' absolute: " << EC.message() << '\n'; - return; - } - if (std::error_code EC = llvm::sys::fs::createUniqueFile( - Model, FD, ResultPath, llvm::sys::fs::OF_Text)) { - llvm::errs() << "warning: could not create file in '" << Directory - << "': " << EC.message() << '\n'; - return; - } - } else { - int i = 1; - std::error_code EC; - do { - // Find a filename which is not already used - const FileEntry* Entry = SMgr.getFileEntryForID(ReportFile); - std::stringstream filename; - Model = ""; - filename << "report-" - << llvm::sys::path::filename(Entry->getName()).str() - << "-" << declName.c_str() - << "-" << offsetDecl - << "-" << i << ".html"; - llvm::sys::path::append(Model, Directory, - filename.str()); - EC = llvm::sys::fs::openFileForReadWrite( - Model, FD, llvm::sys::fs::CD_CreateNew, llvm::sys::fs::OF_None); - if (EC && EC != llvm::errc::file_exists) { - llvm::errs() << "warning: could not create file '" << Model - << "': " << EC.message() << '\n'; - return; - } - i++; - } while (EC); + SmallString<128> ResultPath; + + std::stringstream filename; + filename << "report-"; + + // Historically, neither the stable report filename nor the unstable report + // filename were actually stable. That said, the stable report filename + // was more stable because it was mostly composed of information + // about the bug report instead of being completely random. + // Now both stable and unstable report filenames are in fact stable + // but the stable report filename is still more verbose. + // We should rename the option ("verbose" filename?) but it will break + // people's workflows. + if (DiagOpts.ShouldWriteStableReportFilename) { + // FIXME: This code relies on knowing what constitutes the issue hash. + // Otherwise deduplication won't work correctly. + FileID ReportFile = + path.back()->getLocation().asLocation().getExpansionLoc().getFileID(); + + const FileEntry* Entry = SMgr.getFileEntryForID(ReportFile); + + filename << llvm::sys::path::filename(Entry->getName()).str() << "-" + << declName.c_str() << "-" << offsetDecl << "-"; + } + + filename << StringRef(getIssueHash(D, PP)).substr(0, 6).str() << ".html"; + llvm::sys::path::append(ResultPath, Directory, filename.str()); + if (std::error_code EC = llvm::sys::fs::make_absolute(ResultPath)) { + llvm::errs() << "warning: could not make '" << ResultPath + << "' absolute: " << EC.message() << '\n'; + return; + } + + if (std::error_code EC = llvm::sys::fs::openFileForReadWrite( + ResultPath, FD, llvm::sys::fs::CD_CreateNew, + llvm::sys::fs::OF_None)) { + // Existence of the file corresponds to the situation where a different + // Clang instance has emitted a bug report with the same issue hash. + // This is an entirely normal situation that does not deserve a warning, + // as apart from hash collisions this can happen because the reports + // are in fact similar enough to be considered duplicates of each other. + if (EC != llvm::errc::file_exists) { + llvm::errs() << "warning: could not create file in '" << Directory + << "': " << EC.message() << '\n'; + } + return; } llvm::raw_fd_ostream os(FD, true); @@ -591,7 +604,6 @@ ? UPDLoc.asLocation() : D.getLocation().asLocation()), SMgr); - const Decl *DeclWithIssue = D.getDeclWithIssue(); StringRef BugCategory = D.getCategory(); if (!BugCategory.empty()) @@ -603,9 +615,7 @@ os << "\n\n"; - os << "\n\n"; os << "\n - - - - - - - Index: clang/test/Analysis/scan-build/rebuild_index/subdirectory/report-3.html =================================================================== --- /dev/null +++ clang/test/Analysis/scan-build/rebuild_index/subdirectory/report-3.html @@ -0,0 +1,8 @@ + + + + + + + + Index: clang/test/Analysis/scan-build/rebuild_index/subdirectory/report-4.html =================================================================== --- clang/test/Analysis/scan-build/rebuild_index/subdirectory/report-4.html +++ /dev/null @@ -1,8 +0,0 @@ - - - - - - - - Index: clang/tools/scan-build/bin/scan-build =================================================================== --- clang/tools/scan-build/bin/scan-build +++ clang/tools/scan-build/bin/scan-build @@ -14,7 +14,6 @@ use strict; use warnings; use FindBin qw($RealBin); -use Digest::MD5; use File::Basename; use File::Find; use File::Copy qw(copy); @@ -268,27 +267,6 @@ $ENV{'CCC_ANALYZER_HTML'} = $Dir; } -##----------------------------------------------------------------------------## -# ComputeDigest - Compute a digest of the specified file. -##----------------------------------------------------------------------------## - -sub ComputeDigest { - my $FName = shift; - DieDiag("Cannot read $FName to compute Digest.\n") if (! -r $FName); - - # Use Digest::MD5. We don't have to be cryptographically secure. We're - # just looking for duplicate files that come from a non-malicious source. - # We use Digest::MD5 because it is a standard Perl module that should - # come bundled on most systems. - open(FILE, $FName) or DieDiag("Cannot open $FName when computing Digest.\n"); - binmode FILE; - my $Result = Digest::MD5->new->addfile(*FILE)->hexdigest; - close(FILE); - - # Return the digest. - return $Result; -} - ##----------------------------------------------------------------------------## # UpdatePrefix - Compute the common prefix of files. ##----------------------------------------------------------------------------## @@ -374,8 +352,6 @@ # Sometimes a source file is scanned more than once, and thus produces # multiple error reports. We use a cache to solve this problem. -my %AlreadyScanned; - sub ScanFile { my $Index = shift; @@ -383,19 +359,6 @@ my $FName = shift; my $Stats = shift; - # Compute a digest for the report file. Determine if we have already - # scanned a file that looks just like it. - - my $digest = ComputeDigest("$Dir/$FName"); - - if (defined $AlreadyScanned{$digest}) { - # Redundant file. Remove it. - unlink("$Dir/$FName"); - return; - } - - $AlreadyScanned{$digest} = 1; - # At this point the report file is not world readable. Make it happen. chmod(0644, "$Dir/$FName");