1

I'm trying to find out if this scenario that caused an OverflowException could possibly be something that could be caught by static code analysis.

//IntPtr position = ...
position = (IntPtr) ((int) position + stride);

The above code employs incorrect casting of an IntPtr to a 32-bit number (instead of 64-bit number), which results in an OverflowException when a memory pointer has a 64-bit address. This only happens on a 64-bit version of Windows.

position = (IntPtr) ((long) position + stride);

And here is the fix, casting the IntPtr to a long instead.

It seems to me like something like this could have been caught by static code analysis. However, even running code analysis with Microsoft All Rules returns no findings on this code.

Is there a reason why something like this wasn't raised as a finding? Or is there a ruleset that might have caught it?

Kyle V.
  • 4,752
  • 9
  • 47
  • 81
  • 2
    the static analyzers can't know whether you are going to run the app on a 64 bit or 32 bit machine – Daniel A. White Aug 08 '17 at 13:57
  • 1
    The overflow will also only happen with some values of `position`. For example, if you initialise it like `IntPtr position = new IntPtr(1);` then no overflow will occur. – Matthew Watson Aug 08 '17 at 13:59

1 Answers1

1

IntPtr is designed to work both on 32-bit and 64-bit systems. To that end, it has constructor overloads that take Int32 and Int64 parameters.

You can avoid this problem altogether by using IntPtr.Add method:

position = IntPtr.Add(position, stride);
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • This is interesting, would static code analysis not have been able to detect when I am manipulating `IntPtr` without `Add()` and suggest that I *do* use it? – Kyle V. Aug 08 '17 at 14:06
  • 1
    @KyleV. I think static code analysis could definitely spot this problem, but for some reason it does not. I tried this with ReSharper, which has a pretty good collection of static analysis rules, but it did not spot this problem either. – Sergey Kalinichenko Aug 08 '17 at 14:14