From 8f4c663ff38ef560f85ca1c2e0d9118a9b89348f Mon Sep 17 00:00:00 2001 From: Doug Torrance Date: Fri, 20 Jun 2025 11:17:32 -0400 Subject: [PATCH 1/3] Split rawSetRandomInteger out from rawRandomInteger We already do this for random QQ's. This will prevent us from leaking a ZZ and also allow us to throw errors. --- M2/Macaulay2/e/ZZ.cpp | 7 ++++++- M2/Macaulay2/e/aring-zz-gmp.hpp | 3 +-- M2/Macaulay2/e/interface/random.cpp | 14 +++++++++++--- M2/Macaulay2/e/interface/random.h | 3 +++ 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/M2/Macaulay2/e/ZZ.cpp b/M2/Macaulay2/e/ZZ.cpp index 1de89ebe0b2..011821340aa 100644 --- a/M2/Macaulay2/e/ZZ.cpp +++ b/M2/Macaulay2/e/ZZ.cpp @@ -67,7 +67,12 @@ std::pair RingZZ::coerceToLongInteger(ring_elem a) const mpz_get_si(a.get_mpz())); } -ring_elem RingZZ::random() const { return ring_elem(rawRandomInteger(nullptr)); } +ring_elem RingZZ::random() const { + mpz_ptr result = new_elem(); + rawSetRandomInteger(result, nullptr); + mpz_reallocate_limbs(result); + return ring_elem(result); +} void RingZZ::elem_text_out(buffer &o, const ring_elem ap, diff --git a/M2/Macaulay2/e/aring-zz-gmp.hpp b/M2/Macaulay2/e/aring-zz-gmp.hpp index 587e324a672..70cec3ba488 100644 --- a/M2/Macaulay2/e/aring-zz-gmp.hpp +++ b/M2/Macaulay2/e/aring-zz-gmp.hpp @@ -195,8 +195,7 @@ class ARingZZGMP : public SimpleARing void swap(ElementType& a, ElementType& b) const { mpz_swap(&a, &b); } void random(ElementType& result) const { - // TODO: this leaks a gmp_ZZ - mpz_set(&result, rawRandomInteger(nullptr)); + rawSetRandomInteger(&result, nullptr); } /** @} */ diff --git a/M2/Macaulay2/e/interface/random.cpp b/M2/Macaulay2/e/interface/random.cpp index b37d055b99e..5dfd07baeef 100644 --- a/M2/Macaulay2/e/interface/random.cpp +++ b/M2/Macaulay2/e/interface/random.cpp @@ -53,11 +53,9 @@ int32_t rawRandomInt(int32_t max) return RandomSeed % max; } -gmp_ZZ rawRandomInteger(gmp_ZZ maxN) +void rawSetRandomInteger(mpz_ptr result, gmp_ZZ maxN) /* if height is the null pointer, use the default height */ { - mpz_ptr result = getmemstructtype(mpz_ptr); - mpz_init(result); if (maxN == nullptr) mpz_urandomm(result, state, maxHeight); else if (1 != mpz_sgn(maxN)) @@ -66,6 +64,16 @@ gmp_ZZ rawRandomInteger(gmp_ZZ maxN) } else mpz_urandomm(result, state, maxN); +} + +gmp_ZZ rawRandomInteger(gmp_ZZ maxN) +/* if height is the null pointer, use the default height */ +{ + mpz_ptr result = getmemstructtype(mpz_ptr); + mpz_init(result); + + rawSetRandomInteger(result, maxN); + mpz_reallocate_limbs(result); return result; } diff --git a/M2/Macaulay2/e/interface/random.h b/M2/Macaulay2/e/interface/random.h index 1a05dd760b0..41949a01a2e 100644 --- a/M2/Macaulay2/e/interface/random.h +++ b/M2/Macaulay2/e/interface/random.h @@ -25,6 +25,9 @@ unsigned long rawRandomULong(unsigned long max); int32_t rawRandomInt(int32_t max); /* generate a random number in the range 0..max-1 */ +void rawSetRandomInteger(mpz_ptr result, gmp_ZZ maxN); +/* if height is the null pointer, use the default height */ + gmp_ZZ rawRandomInteger(gmp_ZZ maxN); /* if height is the null pointer, use the default height */ From 6acfc0602233b5be98cc4806ad02aa58835ce9d8 Mon Sep 17 00:00:00 2001 From: Doug Torrance Date: Fri, 20 Jun 2025 13:50:39 -0400 Subject: [PATCH 2/3] Raise error if random(ZZ) called w/ nonpositive height Previously, we silently ignored the negative sign when height was negative, but this was undocumented, and crashed when the height was zero. --- M2/Macaulay2/d/interface.dd | 4 ++-- M2/Macaulay2/e/interface/random.cpp | 20 +++++++++++--------- M2/Macaulay2/tests/normal/randommat.m2 | 3 +++ 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/M2/Macaulay2/d/interface.dd b/M2/Macaulay2/d/interface.dd index 042cbf228b8..7308624b33b 100644 --- a/M2/Macaulay2/d/interface.dd +++ b/M2/Macaulay2/d/interface.dd @@ -47,8 +47,8 @@ setupfun("testCatch",testCatch); export rawRandomZZ(e:Expr):Expr := ( when e - is Nothing do toExpr(Ccode(ZZ, "rawRandomInteger(", "NULL)")) - is maxN:ZZcell do toExpr(Ccode(ZZ, "rawRandomInteger(", maxN.v, ")")) + is Nothing do toExpr(Ccode(ZZorNull, "rawRandomInteger(", "NULL)")) + is maxN:ZZcell do toExpr(Ccode(ZZorNull, "rawRandomInteger(", maxN.v, ")")) else WrongArgZZ()); setupfun("rawRandomZZ",rawRandomZZ); export rawFareyApproximation(e:Expr):Expr := ( diff --git a/M2/Macaulay2/e/interface/random.cpp b/M2/Macaulay2/e/interface/random.cpp index 5dfd07baeef..6a12c64611f 100644 --- a/M2/Macaulay2/e/interface/random.cpp +++ b/M2/Macaulay2/e/interface/random.cpp @@ -56,14 +56,11 @@ int32_t rawRandomInt(int32_t max) void rawSetRandomInteger(mpz_ptr result, gmp_ZZ maxN) /* if height is the null pointer, use the default height */ { - if (maxN == nullptr) - mpz_urandomm(result, state, maxHeight); - else if (1 != mpz_sgn(maxN)) - { - mpz_set_si(result, 0); - } - else - mpz_urandomm(result, state, maxN); + if (maxN == nullptr) maxN = maxHeight; + if (mpz_cmp_si(maxN, 0) <= 0) + throw exc::engine_error("expected a positive height"); + + mpz_urandomm(result, state, maxN); } gmp_ZZ rawRandomInteger(gmp_ZZ maxN) @@ -72,7 +69,12 @@ gmp_ZZ rawRandomInteger(gmp_ZZ maxN) mpz_ptr result = getmemstructtype(mpz_ptr); mpz_init(result); - rawSetRandomInteger(result, maxN); + try { + rawSetRandomInteger(result, maxN); + } catch (const exc::engine_error& e) { + ERROR(e.what()); + return nullptr; + } mpz_reallocate_limbs(result); return result; diff --git a/M2/Macaulay2/tests/normal/randommat.m2 b/M2/Macaulay2/tests/normal/randommat.m2 index 40ac9db11c9..7309c3ae45e 100644 --- a/M2/Macaulay2/tests/normal/randommat.m2 +++ b/M2/Macaulay2/tests/normal/randommat.m2 @@ -34,3 +34,6 @@ assert isSurjective random(R^3,R^6,MaximalRank=>true) assert(random(ZZ^2, ZZ^2, MaximalRank => true) - id_(ZZ^2) != 0) assert(random(QQ^2, QQ^2, MaximalRank => true) - id_(QQ^2) != 0) assert(random(R^2, R^2, MaximalRank => true) - id_(R^2) != 0) + +-- used to crash M2 (#2089) +assert try random(ZZ^2, ZZ^2, Height => 0) then false else true From 13c57583f2c3269fc954eeeb03f6600cc687841a Mon Sep 17 00:00:00 2001 From: Doug Torrance Date: Fri, 7 Nov 2025 14:42:10 -0500 Subject: [PATCH 3/3] Add comments about raw random integer functions & garbage collection [ci skip] --- M2/Macaulay2/e/interface/random.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/M2/Macaulay2/e/interface/random.h b/M2/Macaulay2/e/interface/random.h index 41949a01a2e..11d8fd02ab4 100644 --- a/M2/Macaulay2/e/interface/random.h +++ b/M2/Macaulay2/e/interface/random.h @@ -27,9 +27,11 @@ int32_t rawRandomInt(int32_t max); void rawSetRandomInteger(mpz_ptr result, gmp_ZZ maxN); /* if height is the null pointer, use the default height */ +/* doesn't deal w/ garbage collection */ gmp_ZZ rawRandomInteger(gmp_ZZ maxN); /* if height is the null pointer, use the default height */ +/* returns garbage-collected memory */ void rawSetFareyApproximation(mpq_ptr result, gmp_RR x, gmp_ZZ height); /* sets result = the nearest rational to x w/ denominator <= height */