[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1344
   // This is a signed value, since it's used in arithmetic with signed indices.
-  return svalBuilder.makeIntVal(RegionSize / EleSize, false);
+  return svalBuilder.makeIntVal(RegionSize / EleSize, Ctx.getSignedSizeType());
 }
----------------
a.sidorin wrote:
> ebevhan wrote:
> > a.sidorin wrote:
> > > I think we should initialize SValBuilder::ArrayIndexTy with getSignedSizeType() instead of LongLongTy and use `svalBuilder.getArrayIndexType()` here instead.
> > I made the change, but it caused a spurious out of bounds warning in index-type.c for the 32-bit case. Making the type signed means that anything above MAX/2 will break, and the test uses arrays of that size.
> Hm, yes. ssize_t is 32-bit on 32-bit targets but our indices can exceed it. Even if so, `svalBuilder.getArrayIndexType()` should be fine.
Well, the problem is that it's only enough for objects whose size is less than half of the range. If they're larger, the out of bounds calculation overflows and it thinks that we're trying to index outside the object (at a lower address).

The 64-bit test avoids this since its array is smaller. It's MAX/16 instead.


Repository:
  rC Clang

https://reviews.llvm.org/D46944