Ensure that the result for PowMod is fully initialized

[tannergooding]

This resolves dotnet/performance#2575

The fix provided by #74112 was not quite correct. It attempted to resolve the issue by no longer slicing the intermediate result to the computed resultLength.

While this did fix the immediate issue, it exposed a new issue where the intermediate result buffer could then be larger than the destination bits (the final result) buffer.

The root issue for #74112 was that while bits is the final result buffer, it was also being used as a temporary storage location and thus could have values at indices greater than resultLength which were non-zero.

There are multiple ways this could be fixed, but I believe the least risky is to continue slicing result down to resultLength (effectively reverting the fix from #74112) and to instead explicitly clear any indices from bits that are greater than result.Length.

Assuming that the computed resultLength is correct (and it should be or we have more serious issues), then this ensures that bits is exactly result and all remaining indices are zero, which should ensure a correct result.

---

Alternatively we could continue not slicing result where resultLength was being computed and instead do something like:

Span<uint> result = PowCore(valueCopy, value.Length, temp, power, bits);
int resultLength = Math.Min(result.Length, bits.Length);
result.CopyTo(bits);

However, this might imply that the computed resultLength can't be relied upon and could mask an issue in some future refactoring.

-or-

We could add an additional allocation for the temporary data rather than using bits, which is an overall "better" change (less error prone and safer; but also more "expensive"). However, this would also be more involved and therefore riskier to take into RC2.

[tannergooding]

CC. @dakersnar, @sakno

[ghost]

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

Issue Details

---

This resolves dotnet/performance#2575

The fix provided by #74112 was not quite correct. It attempted to resolve the issue by no longer slicing the intermediate result to the computed resultLength.

While this did fix the immediate issue, it exposed a new issue where the intermediate result buffer could then be larger than the destination bits (the final result) buffer.

The root issue for #74112 was that while bits is the final result buffer, it was also being used as a temporary storage location and thus could have values at indices greater than resultLength which were non-zero.

There are multiple ways this could be fixed, but I believe the least risky is to continue slicing result down to resultLength (effectively reverting the fix from #74112) and to instead explicitly clear any indices from bits that are greater than result.Length.

---

Alternatively we could continue not slicing result where resultLength was being computed and instead do something like:

Span<uint> result = PowCore(valueCopy, value.Length, temp, power, bits);
int resultLength = Math.Min(result.Length, bits.Length);
result.CopyTo(bits);

-or-

We could add an additional allocation for the temporary data rather than using bits, which is an overall better change, but is also more involved and therefore riskier.

Author:	tannergooding
Assignees:	-
Labels:	area-System.Numerics
Milestone:	-

[dakersnar]

Looks good to me, thanks for the fix. Glad we caught this.

[tannergooding]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/2935878948