[PATCH] D144565: dwp check overflow

Wed Feb 22 21:43:28 PST 2023

ayermolo added inline comments.

================
Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:475
+      }
+      ContributionOffsets[Index] = NewOffset.getLimitedValue();
     }
----------------
zhuna8616 wrote:
> zhuna8616 wrote:
> > ayermolo wrote:
> > > Please make sure this doesn't break https://reviews.llvm.org/D137882
> > > It relies on overflow behavior.
> > When nothing overflows, getLimitedValue simply returns the raw data of this APInt. Since there is an overflow check, this is synonyms to something like getValue.
> I tried it on your test case in https://reviews.llvm.org/D137882 and passed.
> {F26615950}
Sorry wasn't clear. That test doesn't use llvm-dwp, I just pointed to the diff to point out that there is a code in llvm that relies on overflow behavior to reconstruct cu-index. 
There is a downstream option for us that changes error for previous check into warning. Forgot I didn't upstream it.
https://github.com/llvm/llvm-project/blob/265ea1c7459da95b7a1d30718ac9f970a3ecc8d1/llvm/lib/DWP/DWP.cpp#L672
(which probably should be removed if check is added here).
Actually now that all the changes have landed in llvm and lldb, maybe there should be an option to turn it into an warning? At least for .debug_info part.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144565