From 3a90e31307e7a811eada3d597a7ea45a5472a378 Mon Sep 17 00:00:00 2001 From: nicoo Date: Fri, 19 Jun 2020 13:39:42 +0200 Subject: [PATCH 1/4] factor::numeric::inv_mod_u64: Provide a more-helpful error message --- src/uu/factor/src/numeric.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/factor/src/numeric.rs b/src/uu/factor/src/numeric.rs index e95fd1948..29babb281 100644 --- a/src/uu/factor/src/numeric.rs +++ b/src/uu/factor/src/numeric.rs @@ -175,7 +175,7 @@ impl Arithmetic for Montgomery { // extended Euclid algorithm // precondition: a is odd pub(crate) fn inv_mod_u64(a: u64) -> u64 { - assert!(a % 2 == 1); + assert!(a % 2 == 1, "{} is not odd", a); let mut t = 0u64; let mut newt = 1u64; let mut r = 0u64; From 9eb944b6b931d07eb5d51b582cb887e7c3470b4a Mon Sep 17 00:00:00 2001 From: nicoo Date: Fri, 19 Jun 2020 13:48:00 +0200 Subject: [PATCH 2/4] =?UTF-8?q?factor:=20Add=20test=20exhibiting=20a=20bug?= =?UTF-8?q?=20in=20=CF=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test is repeated 20 times to make it overwhelmingly-likely to fail (the bug itself is only triggered rarely) --- src/uu/factor/src/factor.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index 2445585fb..f10198ca2 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -156,4 +156,12 @@ mod tests { .map(|i| 2 * i + 2u64.pow(32) + 1) .all(|i| factor(i).product() == i)); } + + #[test] + fn factor_recombines_strong_pseudoprime() { + let pseudoprime = 17179869183; + for _ in 0..20 { + assert!(factor(pseudoprime).product() == pseudoprime); + } + } } From 9fe3de72f23093ec61e0420403e85a091ced98fe Mon Sep 17 00:00:00 2001 From: nicoo Date: Fri, 19 Jun 2020 13:51:29 +0200 Subject: [PATCH 3/4] factor::rho: Fix very-unlikely bug (resulting in assertion failure) This bug can only be triggered when: - the Miller-Rabin test produces a divisor `d` (rare) ; - n / d is prime. --- src/uu/factor/src/rho.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/uu/factor/src/rho.rs b/src/uu/factor/src/rho.rs index f15ef5468..e9076144b 100644 --- a/src/uu/factor/src/rho.rs +++ b/src/uu/factor/src/rho.rs @@ -48,7 +48,7 @@ fn find_divisor(n: A) -> u64 { } } -fn _factor(mut num: u64) -> Factors { +fn _factor(num: u64) -> Factors { // Shadow the name, so the recursion automatically goes from “Big” arithmetic to small. let _factor = |n| { // TODO: Optimise with 32 and 64b versions @@ -61,6 +61,7 @@ fn _factor(mut num: u64) -> Factors { } let n = A::new(num); + let divisor; match miller_rabin::test::(n) { Prime => { factors.push(num); @@ -68,14 +69,14 @@ fn _factor(mut num: u64) -> Factors { } Composite(d) => { - num /= d; - factors *= _factor(d) + divisor = d; } - Pseudoprime => {} + Pseudoprime => { + divisor = find_divisor::(n); + } }; - let divisor = find_divisor::(n); factors *= _factor(divisor); factors *= _factor(num / divisor); factors From 45a1408fb066533c30646764eee5efd68fc07382 Mon Sep 17 00:00:00 2001 From: nicoo Date: Fri, 19 Jun 2020 15:28:01 +0200 Subject: [PATCH 4/4] =?UTF-8?q?fixup!=20factor:=20Add=20test=20exhibiting?= =?UTF-8?q?=20a=20bug=20in=20=CF=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/uu/factor/src/factor.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/uu/factor/src/factor.rs b/src/uu/factor/src/factor.rs index f10198ca2..3ba471d73 100644 --- a/src/uu/factor/src/factor.rs +++ b/src/uu/factor/src/factor.rs @@ -159,8 +159,13 @@ mod tests { #[test] fn factor_recombines_strong_pseudoprime() { + // This is a strong pseudoprime (wrt. miller_rabin::BASIS) + // and triggered a bug in rho::factor's codepath handling + // miller_rabbin::Result::Composite let pseudoprime = 17179869183; for _ in 0..20 { + // Repeat the test 20 times, as it only fails some fraction + // of the time. assert!(factor(pseudoprime).product() == pseudoprime); } }