Hiralo created this revision. Hiralo added a reviewer: vsk. Hiralo added projects: clang, LLVM. Herald added subscribers: dexonsmith, wenlei, dang, kbarton, nemanjai. Herald added a project: All. Hiralo requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits.
commit 60009ab7a6491095b99acd1445dfe0fc5418cb3f Author: Hiral Oza <hiral....@netapp.com> Date: Fri Mar 11 10:19:51 2022 +0530 Clamp negative coverage counters to zero. Because there is no locking on counter updates, race conditions may cause counters to become negative. Clamp them to zero when exporting LCOV reports. Optionally remove blank lines from coverage Added -omit-blank-lines option to add Skip regions to the coverage map for lines of source code that are blank. Author: thom.h...@netapp.com Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D121457 Files: clang/include/clang/Basic/CodeGenOptions.def clang/include/clang/Driver/Options.td clang/include/clang/Lex/PPCallbacks.h clang/include/clang/Lex/Preprocessor.h clang/lib/CodeGen/CodeGenPGO.cpp clang/lib/CodeGen/CoverageMappingGen.cpp clang/lib/CodeGen/CoverageMappingGen.h clang/lib/Lex/Preprocessor.cpp llvm/tools/llvm-cov/CoverageExporterLcov.cpp
Index: llvm/tools/llvm-cov/CoverageExporterLcov.cpp =================================================================== --- llvm/tools/llvm-cov/CoverageExporterLcov.cpp +++ llvm/tools/llvm-cov/CoverageExporterLcov.cpp @@ -70,7 +70,10 @@ for (; LCI != LCIEnd; ++LCI) { const coverage::LineCoverageStats &LCS = *LCI; if (LCS.isMapped()) { - OS << "DA:" << LCS.getLine() << ',' << LCS.getExecutionCount() << '\n'; + OS << "DA:" << LCS.getLine() << ',' + << ((LCS.getExecutionCount() & (1ULL << 63)) ? 0 + : LCS.getExecutionCount()) + << '\n'; } } } Index: clang/lib/Lex/Preprocessor.cpp =================================================================== --- clang/lib/Lex/Preprocessor.cpp +++ clang/lib/Lex/Preprocessor.cpp @@ -964,6 +964,31 @@ } } + if (Callbacks) { + SourceLocation TokenLoc = Result.getLocation(); + if (TokenLoc.isFileID() && + !SourceMgr.isInSystemHeader(SourceMgr.getSpellingLoc(TokenLoc))) { + bool Invalid = false; + unsigned LineNumber = SourceMgr.getSpellingLineNumber(TokenLoc, &Invalid); + if (!Invalid) { + FileID FID = SourceMgr.getFileID(TokenLoc); + HighestLineNumberMap::iterator fi = HighestLineNumbers.find(FID); + unsigned HighestLineNumber = + fi == HighestLineNumbers.end() ? 0 : fi->second; + if (LineNumber > HighestLineNumber + 1) { + SourceLocation BlankStart = + SourceMgr.translateLineCol(FID, HighestLineNumber + 1, 1); + SourceLocation BlankEnd = + SourceMgr.translateLineCol(FID, LineNumber - 1, 1); + Callbacks->BlankLines(SourceRange(BlankStart, BlankEnd)); + } + if (LineNumber > HighestLineNumber) { + HighestLineNumbers[FID] = LineNumber; + } + } + } + } + LastTokenWasAt = Result.is(tok::at); --LexLevel; Index: clang/lib/CodeGen/CoverageMappingGen.h =================================================================== --- clang/lib/CodeGen/CoverageMappingGen.h +++ clang/lib/CodeGen/CoverageMappingGen.h @@ -23,6 +23,7 @@ namespace clang { +class CodeGenOptions; class LangOptions; class SourceManager; class FileEntry; @@ -50,6 +51,7 @@ public EmptylineHandler { // A vector of skipped source ranges and PrevTokLoc with NextTokLoc. std::vector<SkippedRange> SkippedRanges; + std::vector<SourceRange> BlankLineRanges; SourceManager &SourceMgr; @@ -61,8 +63,10 @@ CoverageSourceInfo(SourceManager &SourceMgr) : SourceMgr(SourceMgr) {} std::vector<SkippedRange> &getSkippedRanges() { return SkippedRanges; } + ArrayRef<SourceRange> getBlankLines() const { return BlankLineRanges; } void AddSkippedRange(SourceRange Range); + void BlankLines(SourceRange Range) override; void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override; @@ -135,18 +139,23 @@ class CoverageMappingGen { CoverageMappingModuleGen &CVM; SourceManager &SM; + const CodeGenOptions &CodeGenOpts; const LangOptions &LangOpts; llvm::DenseMap<const Stmt *, unsigned> *CounterMap; public: CoverageMappingGen(CoverageMappingModuleGen &CVM, SourceManager &SM, + const CodeGenOptions &CodeGenOpts, const LangOptions &LangOpts) - : CVM(CVM), SM(SM), LangOpts(LangOpts), CounterMap(nullptr) {} + : CVM(CVM), SM(SM), CodeGenOpts(CodeGenOpts), LangOpts(LangOpts), + CounterMap(nullptr) {} CoverageMappingGen(CoverageMappingModuleGen &CVM, SourceManager &SM, + const CodeGenOptions &CodeGenOpts, const LangOptions &LangOpts, llvm::DenseMap<const Stmt *, unsigned> *CounterMap) - : CVM(CVM), SM(SM), LangOpts(LangOpts), CounterMap(CounterMap) {} + : CVM(CVM), SM(SM), CodeGenOpts(CodeGenOpts), LangOpts(LangOpts), + CounterMap(CounterMap) {} /// Emit the coverage mapping data which maps the regions of /// code to counters that will be used to find the execution Index: clang/lib/CodeGen/CoverageMappingGen.cpp =================================================================== --- clang/lib/CodeGen/CoverageMappingGen.cpp +++ clang/lib/CodeGen/CoverageMappingGen.cpp @@ -88,6 +88,10 @@ SkippedRanges.back().NextTokLoc = Loc; } +void CoverageSourceInfo::BlankLines(SourceRange Range) { + BlankLineRanges.push_back(Range); +} + namespace { /// A region of source code that can be mapped to a counter. @@ -196,6 +200,7 @@ public: CoverageMappingModuleGen &CVM; SourceManager &SM; + const CodeGenOptions &CodeGenOpts; const LangOptions &LangOpts; private: @@ -218,8 +223,9 @@ SourceRegionFilter; CoverageMappingBuilder(CoverageMappingModuleGen &CVM, SourceManager &SM, + const CodeGenOptions &CodeGenOpts, const LangOptions &LangOpts) - : CVM(CVM), SM(SM), LangOpts(LangOpts) {} + : CVM(CVM), SM(SM), CodeGenOpts(CodeGenOpts), LangOpts(LangOpts) {} /// Return the precise end location for the given token. SourceLocation getPreciseTokenLocEnd(SourceLocation Loc) { @@ -395,6 +401,33 @@ Region.LineEnd <= FileLineRanges[*CovFileID].second) MappingRegions.push_back(Region); } + + if (CodeGenOpts.OmitBlankLines) { + auto BlankLines = CVM.getSourceInfo().getBlankLines(); + for (const auto &I : BlankLines) { + auto LocStart = I.getBegin(); + auto LocEnd = I.getEnd(); + assert(SM.isWrittenInSameFile(LocStart, LocEnd) && + "region spans multiple files"); + assert(!SM.isBeforeInTranslationUnit(LocEnd, LocStart) && + "Blank region end points are out of order"); + + auto CovFileID = getCoverageFileID(LocStart); + if (!CovFileID) + continue; + SpellingRegion SR{SM, LocStart, LocEnd}; + // see comment in readMappingRegionsSubArray in llvm/lib/ProfileData/ + // Coverage/CoverageMappingReader.cpp for an explanation of why we + // use 0 for start and end column on lines we want to omit + auto Region = CounterMappingRegion::makeSkipped( + *CovFileID, SR.LineStart, 0, SR.LineEnd, 0); + // Make sure that we only collect the regions that are inside + // the source code of this function. + if (Region.LineStart >= FileLineRanges[*CovFileID].first && + Region.LineEnd <= FileLineRanges[*CovFileID].second) + MappingRegions.push_back(Region); + } + } } /// Generate the coverage counter mapping regions from collected @@ -480,8 +513,9 @@ /// are not emitted. struct EmptyCoverageMappingBuilder : public CoverageMappingBuilder { EmptyCoverageMappingBuilder(CoverageMappingModuleGen &CVM, SourceManager &SM, + const CodeGenOptions &CodeGenOpts, const LangOptions &LangOpts) - : CoverageMappingBuilder(CVM, SM, LangOpts) {} + : CoverageMappingBuilder(CVM, SM, CodeGenOpts, LangOpts) {} void VisitDecl(const Decl *D) { if (!D->hasBody()) @@ -938,8 +972,9 @@ CounterCoverageMappingBuilder( CoverageMappingModuleGen &CVM, llvm::DenseMap<const Stmt *, unsigned> &CounterMap, SourceManager &SM, - const LangOptions &LangOpts) - : CoverageMappingBuilder(CVM, SM, LangOpts), CounterMap(CounterMap) {} + const CodeGenOptions &CodeGenOpts, const LangOptions &LangOpts) + : CoverageMappingBuilder(CVM, SM, CodeGenOpts, LangOpts), + CounterMap(CounterMap) {} /// Write the mapping data to the output stream void write(llvm::raw_ostream &OS) { @@ -1756,14 +1791,15 @@ void CoverageMappingGen::emitCounterMapping(const Decl *D, llvm::raw_ostream &OS) { assert(CounterMap); - CounterCoverageMappingBuilder Walker(CVM, *CounterMap, SM, LangOpts); + CounterCoverageMappingBuilder Walker(CVM, *CounterMap, SM, CodeGenOpts, + LangOpts); Walker.VisitDecl(D); Walker.write(OS); } void CoverageMappingGen::emitEmptyMapping(const Decl *D, llvm::raw_ostream &OS) { - EmptyCoverageMappingBuilder Walker(CVM, SM, LangOpts); + EmptyCoverageMappingBuilder Walker(CVM, SM, CodeGenOpts, LangOpts); Walker.VisitDecl(D); Walker.write(OS); } Index: clang/lib/CodeGen/CodeGenPGO.cpp =================================================================== --- clang/lib/CodeGen/CodeGenPGO.cpp +++ clang/lib/CodeGen/CodeGenPGO.cpp @@ -889,9 +889,9 @@ std::string CoverageMapping; llvm::raw_string_ostream OS(CoverageMapping); - CoverageMappingGen MappingGen(*CGM.getCoverageMapping(), - CGM.getContext().getSourceManager(), - CGM.getLangOpts(), RegionCounterMap.get()); + CoverageMappingGen MappingGen( + *CGM.getCoverageMapping(), CGM.getContext().getSourceManager(), + CGM.getCodeGenOpts(), CGM.getLangOpts(), RegionCounterMap.get()); MappingGen.emitCounterMapping(D, OS); OS.flush(); @@ -912,7 +912,7 @@ llvm::raw_string_ostream OS(CoverageMapping); CoverageMappingGen MappingGen(*CGM.getCoverageMapping(), CGM.getContext().getSourceManager(), - CGM.getLangOpts()); + CGM.getCodeGenOpts(), CGM.getLangOpts()); MappingGen.emitEmptyMapping(D, OS); OS.flush(); Index: clang/include/clang/Lex/Preprocessor.h =================================================================== --- clang/include/clang/Lex/Preprocessor.h +++ clang/include/clang/Lex/Preprocessor.h @@ -940,6 +940,11 @@ void updateOutOfDateIdentifier(IdentifierInfo &II) const; + typedef std::map<FileID, unsigned> HighestLineNumberMap; + + /// Highest line number where a token was found for each FileID + HighestLineNumberMap HighestLineNumbers; + public: Preprocessor(std::shared_ptr<PreprocessorOptions> PPOpts, DiagnosticsEngine &diags, LangOptions &opts, SourceManager &SM, Index: clang/include/clang/Lex/PPCallbacks.h =================================================================== --- clang/include/clang/Lex/PPCallbacks.h +++ clang/include/clang/Lex/PPCallbacks.h @@ -317,6 +317,12 @@ virtual void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) { } + /// Hook called when blank lines are seen. + /// \param Range A Source Range spanning from column one of the first line to + /// column one of the last line of one or more contiguous blank lines. A + /// blank line is one that has no tokens (comments and whitespace may exist). + virtual void BlankLines(SourceRange Range) {} + enum ConditionValueKind { CVK_NotEvaluated, CVK_False, CVK_True }; @@ -594,6 +600,11 @@ Second->SourceRangeSkipped(Range, EndifLoc); } + void BlankLines(SourceRange Range) override { + First->BlankLines(Range); + Second->BlankLines(Range); + } + /// Hook called whenever an \#if is seen. void If(SourceLocation Loc, SourceRange ConditionRange, ConditionValueKind ConditionValue) override { Index: clang/include/clang/Driver/Options.td =================================================================== --- clang/include/clang/Driver/Options.td +++ clang/include/clang/Driver/Options.td @@ -5158,6 +5158,9 @@ def dump_coverage_mapping : Flag<["-"], "dump-coverage-mapping">, HelpText<"Dump the coverage mapping records, for testing">, MarshallingInfoFlag<CodeGenOpts<"DumpCoverageMapping">>; +def omit_blank_lines : Flag<["-"], "omit-blank-lines">, + HelpText<"Skip blank lines in coverage mapping">, + MarshallingInfoFlag<CodeGenOpts<"OmitBlankLines">>; def fuse_register_sized_bitfield_access: Flag<["-"], "fuse-register-sized-bitfield-access">, HelpText<"Use register sized accesses to bit-fields, when possible.">, MarshallingInfoFlag<CodeGenOpts<"UseRegisterSizedBitfieldAccess">>; Index: clang/include/clang/Basic/CodeGenOptions.def =================================================================== --- clang/include/clang/Basic/CodeGenOptions.def +++ clang/include/clang/Basic/CodeGenOptions.def @@ -211,6 +211,7 @@ ///< enable code coverage analysis. CODEGENOPT(DumpCoverageMapping , 1, 0) ///< Dump the generated coverage mapping ///< regions. +CODEGENOPT(OmitBlankLines , 1, 0) ///< Skip blank lines in coverage mapping. /// If -fpcc-struct-return or -freg-struct-return is specified. ENUM_CODEGENOPT(StructReturnConvention, StructReturnConventionKind, 2, SRCK_Default)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits