[PATCH] D65417: [SCCP] Update condition to avoid overflow.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: lib/Analysis/ConstantFolding.cpp:551
   // If we're not accessing anything in this constant, the result is undefined.
-  if (Offset >= InitializerSize)
+  if (Offset + BytesLoaded <= 0)
     return UndefValue::get(IntType);
----------------
lebedev.ri wrote:
> asbirlea wrote:
> > lebedev.ri wrote:
> > > This check looks suspicious to me.
> > > I'd expect this to be `if (Offset + BytesLoaded >= InitializerSize)`,
> > > this way we are checking that `BytesLoaded` bytes lies within the global.
> > If `Offset` can be a negative value, it's possible to get an OOB access when `Offset + BytesLoaded <= 0`.  
> > We can also get an OOB access `if (Offset + BytesLoaded >= InitializerSize)`, but the two checks seem orthogonal given this section of code (I'm not familiar with the larger scope of this code).
> Okay, sounds plausible.
I think with the check as it is, there still could be an overflow *in theory*, e.g. for constants that are int64_max big. Given that we only allow global constants with initializers, this would mean a huge bitcode file, but while you're at it, it might be worth changing the comparisons to `if (Offfset <= -BytesLoaded)` as well


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65417