Clamp negative coverage counters to zero.

[Hiralo]

Summary

commit 60009ab7a6491095b99acd1445dfe0fc5418cb3f
Author: Hiral Oza <[e-mail]>
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: [e-mail]

[lebedev.ri]

Wouldn't it be best to instead fix the non-thread-safety of the coverage counters?

[Hiralo]

In D121457#3375039, @lebedev.ri wrote:

Wouldn't it be best to instead fix the non-thread-safety of the coverage counters?

Can you please elaborate or point me to code? Thanks

clang/include/clang/Basic/CodeGenOptions.def

                                    ///< 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)

clang/include/clang/Driver/Options.td

 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">>;

clang/include/clang/Lex/PPCallbacks.h

   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
   };

     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 {

clang/include/clang/Lex/Preprocessor.h

 
   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,

clang/lib/CodeGen/CodeGenPGO.cpp

 
   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();
 

   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();
 

clang/lib/CodeGen/CoverageMappingGen.h

 
 namespace clang {
 
+class CodeGenOptions;
 class LangOptions;
 class SourceManager;
 class FileEntry;

                            public EmptylineHandler {
   // A vector of skipped source ranges and PrevTokLoc with NextTokLoc.
   std::vector<SkippedRange> SkippedRanges;
+  std::vector<SourceRange> BlankLineRanges;
 
   SourceManager &SourceMgr;
 

   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;
 

 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

clang/lib/CodeGen/CoverageMappingGen.cpp

     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.

 public:
   CoverageMappingModuleGen &CVM;
   SourceManager &SM;
+  const CodeGenOptions &CodeGenOpts;
   const LangOptions &LangOpts;
 
 private:

       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) {

           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

 /// 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())

   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) {

 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);
 }

clang/lib/Lex/Preprocessor.cpp

     }
   }
 
+  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;
 

llvm/tools/llvm-cov/CoverageExporterLcov.cpp

   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';
     }
   }
 }