Fix overflow in GetHexStringCore

[vcsjones]

If destination's length is int.MaxValue, adding 1 will overflow it. To address this, we can use an unsigned shift to the right by one instead of dividing by two.

I don't think it is particularly feasible to unit test this - it would require allocating a 4GB span. Generally those yield unstable tests in CI.

[dotnet-policy-service]

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

[karakasa]

I don't think it is particularly feasible to unit test this - it would require allocating a 4GB span. Generally those yield unstable tests in CI.

Just want to ask: I have a similar hex parsing overflow fix for BigInteger on a rare path (#105570). I wrote a test and put it in OuterLoop (as it requires 1.5~2GB of memory and allocating a big array). Should I remove that test?

[vcsjones]

OuterLoop (as it requires 1.5~2GB of memory).

2.0 GB is iffy. You should probably work with the area owners on figuring out the right thing to do to decide how critical it is to have test coverage for the scenario. I would expect a 2.0 gig allocation to fail on mobile platforms (iOS and tvOS) and stress Linux. Linux's behavior can be unfortunate the oomkiller might just abort the whole test run. (TL;dr on oomkiller: sometimes a large allocation will succeed, but Linux will decide that it is starved for memory, and just do a kill -9 on the whole process)

#100558 is an example of where a 2 GB allocation was aborting test runs.

[karakasa]

ou should probably work with the area owners

Thanks. I thought there was a repo-wide consensus/policy on memory-consuming tests.

[bartonjs]

/ba-g The Windows 8.1 queue seems to be backed up. This change would not uniquely impact that OS, so signing off based on the successful legs.