Raise an error when generating random integers when Height option is nonpositive

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 := (

M2/Macaulay2/e/ZZ.cpp

@@ -67,7 +67,12 @@ std::pair<bool, long> 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,

M2/Macaulay2/e/aring-zz-gmp.hpp

@@ -195,8 +195,7 @@ class ARingZZGMP : public SimpleARing<ARingZZGMP>
   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);
   }
   /** @} */
 

M2/Macaulay2/e/interface/random.cpp

@@ -53,19 +53,29 @@ int32_t rawRandomInt(int32_t max)
   return RandomSeed % max;
 }
 
+void rawSetRandomInteger(mpz_ptr result, gmp_ZZ maxN)
+/* if height is the null pointer, use the default height */
+{
+  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)
 /* 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))
-    {
-      mpz_set_si(result, 0);
-    }
-  else
-    mpz_urandomm(result, state, maxN);
+
+  try {
+    rawSetRandomInteger(result, maxN);
+  } catch (const exc::engine_error& e) {
+    ERROR(e.what());
+    return nullptr;
+  }
+
   mpz_reallocate_limbs(result);
   return result;
 }

M2/Macaulay2/e/interface/random.h

@@ -25,8 +25,13 @@ 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 */
+/* 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 */

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