[llvm-cov] Fix illegal cast from uint64_t to int64_t

[sajjadm]

Summary

Counters are stored as uint64_t in the coverage mapping, but
exporting in JSON requires signed integers. Clamp the values to the
smaller range to make the conversion safe.

[sajjadm]

Discovered this because we were getting negative numbers in some of our coverage data when we had counter overflows.

[vsk]

Thanks, lgtm with a comment.

llvm/tools/llvm-cov/CoverageExporterJson.cpp

65

Please leave a comment explaining why the format makes clamping necessary.

[sajjadm]

• Added comment to clamp function.

[Dor1s]

Ha, nice! LGTM. Is there any test you could easily update or add to reproduce this corner case?

llvm/tools/llvm-cov/CoverageExporterJson.cpp

72

nit: can return std::min(u, std::numeric_limits<int64_t>::max())

[sajjadm]

In D70200#1745743, @Dor1s wrote:

Ha, nice! LGTM. Is there any test you could easily update or add to reproduce this corner case?

I tried to write a loop that would execute INT64_MAX + 10 times, but that takes way too long. Not sure how to test it outside of the case where we have counter overflows.

[sajjadm]

Changed to using std::min

[sajjadm]

llvm/tools/llvm-cov/CoverageExporterJson.cpp

72

FYI we still needed the static_cast to make template deduction work

llvm/tools/llvm-cov/CoverageExporterJson.cpp

 #include "CoverageExporterJson.h"
 #include "CoverageReport.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
 #include <algorithm>
+#include <limits>
 #include <mutex>
 #include <utility>
 
 /// The semantic version combined as a string.
 #define LLVM_COVERAGE_EXPORT_JSON_STR "2.0.0"
 
 /// Unique type identifier for JSON coverage export.
 #define LLVM_COVERAGE_EXPORT_JSON_TYPE_STR "llvm.coverage.json.export"
 
 using namespace llvm;
 
 namespace {
 
+// The JSON library accepts int64_t, but profiling counts are stored as uint64_t.
+// Therefore we need to explicitly convert from unsigned to signed, since a naive
+// cast is implementation-defined behavior when the unsigned value cannot be
+// represented as a signed value. We choose to clamp the values to preserve the
+// invariant that counts are always >= 0.
+int64_t clamp_uint64_to_int64(uint64_t u) {
+  return std::min(u, static_cast<uint64_t>(std::numeric_limits<int64_t>::max()));
+}
+
 json::Array renderSegment(const coverage::CoverageSegment &Segment) {
-  return json::Array({Segment.Line, Segment.Col, int64_t(Segment.Count),
+  return json::Array({Segment.Line, Segment.Col, clamp_uint64_to_int64(Segment.Count),
                       Segment.HasCount, Segment.IsRegionEntry});
 }
 
 json::Array renderRegion(const coverage::CountedRegion &Region) {
   return json::Array({Region.LineStart, Region.ColumnStart, Region.LineEnd,
-                      Region.ColumnEnd, int64_t(Region.ExecutionCount),
+                      Region.ColumnEnd, clamp_uint64_to_int64(Region.ExecutionCount),
                       Region.FileID, Region.ExpandedFileID,
                       int64_t(Region.Kind)});
 }
 
 json::Array renderRegions(ArrayRef<coverage::CountedRegion> Regions) {
   json::Array RegionArray;
   for (const auto &Region : Regions)
     RegionArray.push_back(renderRegion(Region));

 }
 
 json::Array renderFunctions(
     const iterator_range<coverage::FunctionRecordIterator> &Functions) {
   json::Array FunctionArray;
   for (const auto &F : Functions)
     FunctionArray.push_back(
         json::Object({{"name", F.Name},
-                      {"count", int64_t(F.ExecutionCount)},
+                      {"count", clamp_uint64_to_int64(F.ExecutionCount)},
                       {"regions", renderRegions(F.CountedRegions)},
                       {"filenames", json::Array(F.Filenames)}}));
   return FunctionArray;
 }
 
 } // end anonymous namespace
 
 void CoverageExporterJson::renderRoot(const CoverageFilters &IgnoreFilters) {