[PATCH] D75695: [StackProtector] Catch direct out-of-bounds when checking address-takenness

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/StackProtector.cpp:202
+      APInt MaxOffset(TypeSize, DL.getTypeStoreSize(
+                        AI->getType()->getPointerElementType()));
+      if (!GEP->accumulateConstantOffset(DL, Offset) || Offset.uge(MaxOffset))
----------------
john.brawn wrote:
> efriedma wrote:
> > I think AllocaInst has a getAllocatedType, or something like that?
> > 
> > Should use getTypeAllocSize, since that's what actually determines the number of bytes allocated.
> > I think AllocaInst has a getAllocatedType, or something like that?
> AI isn't necessarily an AllocaInst, e.g. we could be looking at a GEP of a GEP or a GEP of a PHI.
> 
> > Should use getTypeAllocSize, since that's what actually determines the number of bytes allocated.
> Will do.
> 
Oh, I see, the function recurses.

I don't think the recursion works correctly in general.  The values that actually matter are the current offset, determined recursively, the size of the original alloca, and the size of any memory operations using the result of the GEP.  Using the size of the GEP's operand type doesn't work correctly in general; in particular, if there's another GEP or bitcast in the recursive chain of uses, you can come up with the wrong result.


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

https://reviews.llvm.org/D75695