[PATCH] D109217: [NOT FOR COMMIT] [llvm-dwarfdump] Fix unsigned overflow when calculating stats

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Seems like a generally reasonabel direction forward.



================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:380-381
     GlobalStats.ScopeBytesCovered += ScopeBytesCovered;
+    assert(GlobalStats.ScopeBytesCovered <= UINT64_MAX &&
+           "ScopeBytesCovered - overflow");
     GlobalStats.ScopeBytes += BytesInScope;
----------------
Orlando wrote:
> I think this assert should come before the assignment and be something like this to catch a value that would "overflow" by wrapping:
> `assert(GlobalStats.ScopeBytesCovered + ScopeBytesCovered >= GlobalStats.ScopeBytesCovered &&  "ScopeBytesCovered - overflow");`
> Otherwise I don't think this assertion will ever catch anything, since all uint64_t values are <= UINT64_MAX.
> 
Yep! I think the more general assert probably looks like this:
```
assert(x <= max - y)
x += y;
```


================
Comment at: llvm/tools/llvm-dwarfdump/Statistics.cpp:386-387
     GlobalStats.ScopeEntryValueBytesCovered += BytesEntryValuesCovered;
+    assert(GlobalStats.ScopeEntryValueBytesCovered <= UINT64_MAX &&
+           "ScopeEntryValueBytesCovered - overflow");
     if (IsParam) {
----------------
When would this assert fire? If `ScopeEntryValueBytesCovered` is a uint64_t, it can't ever be > than the max uint64_t value.

Checking for overflow would usually be done, I tihnk, with a check before the overflow:
```
assert(x <= max - y)
x += y;
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109217/new/

https://reviews.llvm.org/D109217