Support BoxedUint::gcd(_vartime) where self and rhs have a different number of limbs

[fjarri]

The PR currently has just a testcase illustrating the problem.

So the question is, what should the behavior be? We can equalize the sizes of the arguments at the top level, but that would make gcd() not constant time wrt numbers of limbs. Then again, would users assume that it is?

[tarcieri]

It's part of the general implicit widening problem: #312. Implicit widening may make sense here.

In many places we currently just panic rather than performing implicit widening, relying on the caller to widen appropriately.

There are of course a whole heap of potential timing variability concerns to worry about here, as you noted. That's why the panicking approach seemed like the most conservative in order to punt the problem onto the caller.

If we stick with panicking, it should use a proper assert at least.

[fjarri]

I see. Perhaps then I should just add a check for that in gcd() itself, and test that it panics, because otherwise the panic happens deep inside Bernstein-Yang machinery and can be confusing.

[tarcieri]

I can try to add implicit widening in a separate PR, unless you want to add it in this one. I think it's appropriate here.

[fjarri]

That's fine, it can wait until the resolution on #312. It's just that the error caught me off-guard.

[tarcieri]

BoxedUnsatInt needs to allocate anyway, so we can implicitly widen there while also converting to u62-ish limbs

[fjarri]

We need to equalize the number of limbs earlier, when making sure the first argument is odd and conditionally swapping them.

[tarcieri]

@fjarri I was suggesting finding the max of the two limb counts, similar to how it finds the max of two bits, then pass a limb count hint in when constructing BoxedUnsatInt.

That would be as opposed to calling BoxedUint::widen which would add another allocation.

[fjarri]

But the difference in limb numbers will already lead to panic before that, in BoxedUint::gcd()

[tarcieri]

Aah! Yes, that is a problem, though there are several paths through Bernstein-Yang that avoid that.

I'm surprised there isn't a better length assert_eq! for that function.

[fjarri]

Actually, it won't even lead to panic if the first argument is longer, it'll just behave incorrectly.

Edit: that is, in release mode it will, in debug mode there will be a panic from debug_assert().

[tarcieri]

Will merge this to have a set of test cases for widening behavior, and then I'll take a look at actually implementing it. It shouldn't be too hard.