[PATCH] D144565: dwp check overflow

[zhuna via Phabricator via llvm-commits]

zhuna8616 added inline comments.


================
Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:475
+      }
+      ContributionOffsets[Index] = NewOffset.getLimitedValue();
     }
----------------
ayermolo wrote:
> 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.
This patch is meant to backport llvm/11.x version, where no overflow check exists. The latest implementation checks overflow in debug_info section only. Am I supposed to turn the error introduced in this patch to a warning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144565