[Analyzer] Prevent behavioral change in built-in operators for IntPtr and UIntPtr

[buyaa-n]

Related to #72348 (comment)

With numeric IntPtr feature, System.IntPtr and System.UIntPtr gained some built-in operators (conversions, unary and binary). Those would now throw when overflowing within checked context, which is causing behavioral change in .NET 7

Detect places where the addition, subtraction, and explicit cast operators were used on U?IntPtr, while taking the overflow checking context into account

• For IntPtr
  • operator +(IntPtr, int): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • operator -(IntPtr, int): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • explicit operator IntPtr(long): Checked: same behavior; Unchecked: will not throw an overflow exception when it could have before in 32-bit contexts; Wrap in a checked context to ensure the exception will be thrown as before in 32-bit contexts
  • explicit operator void*(IntPtr): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • explicit operator IntPtr(void*): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • explicit operator int(IntPtr): Checked: same behavior; Unchecked: will not throw an overflow exception when it could have before in 64-bit contexts; Wrap in checked context to ensure the exception will be thrown as before in 64-bit contexts
• For UIntPtr
  • operator +(UIntPtr, int): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • operator -(UIntPtr, int): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • explicit operator UIntPtr(ulong): Checked: same behavior; Unchecked: will not throw an overflow exception when it could have before in 32-bit contexts; Wrap in a checked context to ensure the exception will be thrown as before in 32-bit contexts
  • explicit operator uint(UIntPtr): Checked: same behavior; Unchecked: will not throw an overflow exception when it could have before in 64-bit contexts; Wrap in checked context to ensure the exception will be thrown as before in 64-bit contexts

• These analyzer should be triggered only if the underlying runtime supports numeric IntPtr feature, i.e. it could check if System.Runtime.CompilerServices.RuntimeFeature.NumericIntPtr is available in corelib as mentioned here

Suggested severity : "Warning"
Suggested category : "Usage" or "Reliability"

CC @tannergooding @jeffhandley

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Related to #72348 (comment)

With numeric IntPtr feature, System.IntPtr and System.UIntPtr gained some built-in operators (conversions, unary and binary). Those would now throw when overflowing within checked context, which is causing behavioral change in .NET 7

Detect places where the addition, subtraction, and explicit cast operators were used on U?IntPtr, while taking the overflow checking context into account

• For IntPtr
  • operator +(IntPtr, int): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • operator -(IntPtr, int): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • explicit operator IntPtr(long): Checked: same behavior; Unchecked: will not throw an overflow exception when it could have before in 32-bit contexts; Wrap in a checked context to ensure the exception will be thrown as before in 32-bit contexts
  • explicit operator IntPtr(void*): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • explicit operator int(IntPtr): Checked: same behavior; Unchecked: will not throw an overflow exception when it could have before in 64-bit contexts; Wrap in checked context to ensure the exception will be thrown as before in 64-bit contexts
• For UIntPtr
  • operator +(UIntPtr, int): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • operator -(UIntPtr, int): Unchecked: same behavior; Checked: may overflow now when it didn't before; Wrap in unchecked context to prevent the overflow and preserve behavior
  • explicit operator UIntPtr(ulong): Checked: same behavior; Unchecked: will not throw an overflow exception when it could have before in 32-bit contexts; Wrap in a checked context to ensure the exception will be thrown as before in 32-bit contexts
  • explicit operator uint(UIntPtr): Checked: same behavior; Unchecked: will not throw an overflow exception when it could have before in 64-bit contexts; Wrap in checked context to ensure the exception will be thrown as before in 64-bit contexts

Suggested severity : "Warning"
Suggested category : "Style"

CC @tannergooding @jeffhandley

Author:	buyaa-n
Assignees:	-
Labels:	api-suggestion, area-System.Runtime, code-analyzer
Milestone:	7.0.0

[buyaa-n]

Example:

public unsafe class IntPtrTest
{
    IntPtr intPtr1;
    IntPtr intPtr2;
    UIntPtr uintPtr1;
    UIntPtr uintPtr2;
    long longValue;
    long uLongValue;

    void Test ()
    {
        // IntPtr flag code:
        checked
        {
            intPtr2 = intPtr1 + 2; // operator +(IntPtr, int)

            intPtr2 = intPtr1 - 2; // operator -(IntPtr, int)

            void* ptr = (void*)intPtr1; // explicit operator void*(IntPtr), overflows for values like -1

            intPtr2 = (IntPtr)ptr; // explicit operator IntPtr(void*)
        }

        intPtr1 = (IntPtr)longValue; // explicit operator IntPtr(long) 32 bit unchecked context

        int a = (int)intPtr1; // explicit operator int(IntPtr) 64 bit unchecked context


        // Fixed code
        checked
        {
            unchecked
            {
                intPtr2 = intPtr1 + 2; // maybe suggest the method IntPtr.Add(intPtr1, 2) instead 

                intPtr2 = intPtr1 - 2; // maybe suggest IntPtr.Subtract(IntPtr, Int32) instead

                void* ptr = (void*)intPtr1;

                intPtr2 = (IntPtr)ptr;
            }
        }

        checked
        {
            intPtr1 = (IntPtr)longValue; 

            a = (int)intPtr1; 
        }

// --------------------------------------------------------------

        // UIntPtr flag code:
        checked
        {
            uintPtr2 = uintPtr1 + 2; // operator +(UIntPtr, int)

            uintPtr2 = uintPtr1 - 2; // operator -(UIntPtr, int)
        }

        uintPtr1 = (UIntPtr)uLongValue; // explicit operator IntPtr(long) 32 bit unchecked context

        uint ui = (uint)uintPtr1; // explicit operator uint(UIntPtr) 64 bit unchecked context

        // Fixed code:
        checked
        {
            unchecked
            {
                uintPtr2 = uintPtr1 + 2; // we might want also suggest the Add method instead

                uintPtr2 = uintPtr1 - 2; // Subtract method
            }
        }

        checked
        {
            uintPtr1 = (UIntPtr)uLongValue;

            ui = (uint)uintPtr1;
        }
    }
}

[filipnavara]

behavioral change in .NET 7

Behavioral change relative to what? Xamarin n[u]int types? Previously you could not use subtraction and addition on [U]IntPtr.

[tannergooding]

Previously you could not use subtraction and addition on [U]IntPtr.

IntPtr and UIntPtr have exposed various "user-defined" operators since .NET Framework 4.0. These operators are called out in the top post and include IntPtr + int, IntPtr - int, UIntPtr + int, and UIntPtr - int (as well as the conversions to/from int, long, uint, and ulong).

[terrajobst]

Video

Looks good as proposed.

Severity: Warning
Category: Reliability

[buyaa-n]

After testing the analyzer with runtime and few other repo we decided to degrade the severity to info level. Also the fix for findings not that simple, might better to convert the type nint/nuint or to a real pointer, or wrap with unchecked/checked statements. So suggesting a correct code fix is not feasible, instead we should document the possible fixes well.