[PATCH] D64451: [PoisonChecking] Validate inbounds annotation on getelementptr where possible

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: lib/Transforms/Instrumentation/PoisonChecking.cpp:217
+  for (unsigned i = 1; i < GEP.getNumOperands() && IsNonNegativeOffset; i++)
+    IsNonNegativeOffset &= isKnownNonNegative(GEP.getOperand(i), DL);
+
----------------
reames wrote:
> jdoerfert wrote:
> > While I now thing this is correct, we might want to make it less restrictive using existing functionality to accumulate and strip pointer stuff, e.g., along the lines of:
> > 
> > ```
> > const Value *Base = GEP->stripAndAccumulateConstantOffsets(const DataLayout &DL,
> >                                                  APInt &Offset,
> >                                                  bool AllowNonInbounds);
> > IsNonNegativeOffset = (Base == Obj) && Offset >= 0;
> > ```
> > 
> Your proposal is less powerful than the existing code as it's restricted to constants where the current code is didn't.  Did you have a particular missed case in mind?
I think, though I don't quite remember, I wanted to simplify the code. It was only a suggestion anyway.


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

https://reviews.llvm.org/D64451