rand-fst: Avoid UB in random number generation

std::uniform_int_distribution(a, b) defines a closed interval and requires
a <= b.  Ensure it is never constructed with a > b.

https://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution.html

Change return type of RandFst from void to absl::Status.

This should eventually be moved to absl random functions, but that
will most likely require the same checks.  It's difficult to make sense
of `Uniform(0, -1)`.

Tested:
scripts/test_bazel.sh
scripts/test_cmake.sh
PiperOrigin-RevId: 882610853

Author: jmr

openfst/test/BUILD.bazel

@@ -191,6 +191,7 @@ cc_library(
         "//openfst/lib:weight",
         "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/log",
+        "@com_google_absl//absl/log:check",
         "@com_google_googletest//:gtest",
     ],
 )
@@ -331,6 +332,7 @@ cc_library(
     deps = [
         "//openfst/lib",
         "@com_google_absl//absl/log",
+        "@com_google_absl//absl/status",
     ],
 )
 
@@ -2181,8 +2183,10 @@ cc_test(
             "//openfst/lib:weight",
             "@com_google_absl//absl/flags:flag",
             "@com_google_absl//absl/log",
+            "@com_google_absl//absl/log:check",
             "@com_google_absl//absl/log:flags",
             "@com_google_absl//absl/memory",
+            "@com_google_absl//absl/status",
             "@com_google_googletest//:gtest",
         ],
     )

openfst/test/algo_test.h

@@ -32,6 +32,7 @@
 #include "gtest/gtest.h"
 #include "absl/flags/declare.h"
 #include "absl/flags/flag.h"
+#include "absl/log/check.h"
 #include "absl/log/log.h"
 #include "openfst/lib/arc-map.h"
 #include "openfst/lib/arc.h"
@@ -1383,10 +1384,9 @@ class AlgoTester {
         new UnweightedTester<Arc>(zero_fst_, one_fst_, univ_fst_, seed));
   }
 
-  void MakeRandFst(MutableFst<Arc>* fst) {
-    RandFst<Arc, WeightGenerator>(kNumRandomStates, kNumRandomArcs,
-                                  kNumRandomLabels, kAcyclicProb, generate_,
-                                  rand_(), fst);
+  absl::Status MakeRandFst(MutableFst<Arc>* fst) {
+    return RandFst(kNumRandomStates, kNumRandomArcs, kNumRandomLabels,
+                   kAcyclicProb, generate_, rand_(), fst);
   }
 
   void Test() {
@@ -1397,9 +1397,9 @@ class AlgoTester {
       VectorFst<Arc> T1;
       VectorFst<Arc> T2;
       VectorFst<Arc> T3;
-      MakeRandFst(&T1);
-      MakeRandFst(&T2);
-      MakeRandFst(&T3);
+      CHECK_OK(MakeRandFst(&T1));
+      CHECK_OK(MakeRandFst(&T2));
+      CHECK_OK(MakeRandFst(&T3));
       weighted_tester_->Test(T1, T2, T3);
 
       VectorFst<Arc> A1(T1);

openfst/test/rand-fst.h

@@ -20,6 +20,7 @@
 #include <random>
 
 #include "absl/log/log.h"
+#include "absl/status/status.h"
 #include "openfst/lib/mutable-fst.h"
 #include "openfst/lib/properties.h"
 #include "openfst/lib/verify.h"
@@ -28,9 +29,22 @@ namespace fst {
 
 // Generates a random FST.
 template <class Arc, class Generate>
-void RandFst(const int num_random_states, const int num_random_arcs,
-             const int num_random_labels, const float acyclic_prob,
-             Generate generate, uint64_t seed, MutableFst<Arc>* fst) {
+absl::Status RandFst(const int num_random_states, const int num_random_arcs,
+                     const int num_random_labels, const float acyclic_prob,
+                     Generate generate, uint64_t seed, MutableFst<Arc>* fst) {
+  if (num_random_states < 0) {
+    return absl::InvalidArgumentError("num_random_states must be non-negative");
+  }
+  if (num_random_arcs < 0) {
+    return absl::InvalidArgumentError("num_random_arcs must be non-negative");
+  }
+  if (num_random_arcs > 0 && num_random_labels <= 0) {
+    return absl::InvalidArgumentError("num_random_labels must be positive");
+  }
+  if (acyclic_prob < 0.0 || acyclic_prob > 1.0) {
+    return absl::InvalidArgumentError("acyclic_prob must be in [0, 1]");
+  }
+
   using Label = typename Arc::Label;
   using StateId = typename Arc::StateId;
 
@@ -43,49 +57,51 @@ void RandFst(const int num_random_states, const int num_random_arcs,
     NUM_DIRECTIONS = 3
   };
 
+  fst->DeleteStates();
+  if (num_random_states == 0) return absl::OkStatus();
+
   std::mt19937_64 rand(seed);
   const StateId ns =
       std::uniform_int_distribution<>(0, num_random_states - 1)(rand);
-  std::uniform_int_distribution<size_t> arc_dist(0, num_random_arcs - 1);
-  std::uniform_int_distribution<Label> label_dist(0, num_random_labels - 1);
+  if (ns == 0) return absl::OkStatus();
+  fst->AddStates(ns);
+
   std::uniform_int_distribution<StateId> ns_dist(0, ns - 1);
+  const StateId start = ns_dist(rand);
+  fst->SetStart(start);
 
   ArcDirection arc_direction = ANY_DIRECTION;
   if (!std::bernoulli_distribution(acyclic_prob)(rand)) {
     arc_direction = std::bernoulli_distribution(.5)(rand) ? FORWARD_DIRECTION
                                                           : REVERSE_DIRECTION;
   }
 
-  fst->DeleteStates();
-
-  if (ns == 0) return;
-  fst->AddStates(ns);
-
-  const StateId start = ns_dist(rand);
-  fst->SetStart(start);
-
-  const size_t na = arc_dist(rand);
-  for (size_t n = 0; n < na; ++n) {
-    StateId s = ns_dist(rand);
-    Arc arc;
-    arc.ilabel = label_dist(rand);
-    arc.olabel = label_dist(rand);
-    arc.weight = generate();
-    arc.nextstate = ns_dist(rand);
-    if ((arc_direction == FORWARD_DIRECTION ||
-         arc_direction == REVERSE_DIRECTION) &&
-        s == arc.nextstate) {
-      continue;  // Skips self-loops.
+  if (num_random_arcs > 0) {
+    std::uniform_int_distribution<size_t> arc_dist(0, num_random_arcs - 1);
+    std::uniform_int_distribution<Label> label_dist(0, num_random_labels - 1);
+    const size_t na = arc_dist(rand);
+    for (size_t n = 0; n < na; ++n) {
+      StateId s = ns_dist(rand);
+      Arc arc;
+      arc.ilabel = label_dist(rand);
+      arc.olabel = label_dist(rand);
+      arc.weight = generate();
+      arc.nextstate = ns_dist(rand);
+      if ((arc_direction == FORWARD_DIRECTION ||
+           arc_direction == REVERSE_DIRECTION) &&
+          s == arc.nextstate) {
+        continue;  // Skips self-loops.
+      }
+
+      if ((arc_direction == FORWARD_DIRECTION && s > arc.nextstate) ||
+          (arc_direction == REVERSE_DIRECTION && s < arc.nextstate)) {
+        StateId t = s;  // reverses arcs
+        s = arc.nextstate;
+        arc.nextstate = t;
+      }
+
+      fst->AddArc(s, arc);
     }
-
-    if ((arc_direction == FORWARD_DIRECTION && s > arc.nextstate) ||
-        (arc_direction == REVERSE_DIRECTION && s < arc.nextstate)) {
-      StateId t = s;  // reverses arcs
-      s = arc.nextstate;
-      arc.nextstate = t;
-    }
-
-    fst->AddArc(s, arc);
   }
 
   const StateId nf = std::uniform_int_distribution<>(0, ns)(rand);
@@ -107,6 +123,7 @@ void RandFst(const int num_random_states, const int num_random_arcs,
   }
   mask &= ~kTrinaryProperties;
   fst->SetProperties(props & ~mask, mask);
+  return absl::OkStatus();
 }
 
 }  // namespace fst