From: Ulya Trofimovich <skvadrik@gmail.com>
Date: Sat, 28 Jul 2018 09:21:37 +0000 (+0100)
Subject: Fixed error in calculation of maximal skeleton path length.
X-Git-Tag: 1.1~30
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c4ee5ea6226897f3a4aa96ede90d599df11c0733;p=re2c

Fixed error in calculation of maximal skeleton path length.

The error was found by slyfox's fuzzer (a randomly-generated skeleton test).

The bug in the code was, apparently, too early modification of the state's
estimated maximal distance to the end states: the distance was set before
all of the state's children were processed, which resulted in aborting the
accumulation of distance from the remaining children, and, as a consequence,
shorter than necessary max distance for the root itself.
---

diff --git a/re2c/src/skeleton/maxpath.cc b/re2c/src/skeleton/maxpath.cc
index 3daa3a56..0abf6261 100644
--- a/re2c/src/skeleton/maxpath.cc
+++ b/re2c/src/skeleton/maxpath.cc
@@ -1,3 +1,4 @@
+#include <assert.h>
 #include <stddef.h>
 #include "src/util/c99_stdint.h"
 #include <algorithm>
@@ -20,38 +21,46 @@ static const uint32_t DIST_MAX = DIST_ERROR - 1;
 // maximal distance to end node (assuming one iteration per loop)
 // different from YYMAXFILL calculation
 // in the way it handles loops and empty regexp
-static void calc_dist(
-	const Skeleton &skel,
-	std::vector<uint8_t> &loops,
-	std::vector<uint32_t> &dists,
-	size_t i)
+static uint32_t calc_dist(const Skeleton &skel
+	, std::vector<uint8_t> &loops
+	, std::vector<uint32_t> &dists
+	, size_t i)
 {
 	const Node &node = skel.nodes[i];
-	uint8_t &loop = loops[i];
-	uint32_t &dist = dists[i];
+	uint32_t dist = dists[i];
 
 	if (dist != DIST_ERROR) {
-		return;
-	} else if (node.end()) {
-		dist = 0;
-	} else if (loop < 2) {
-		local_inc _(loop);
+		return dist;
+	}
+
+	else if (node.end()) {
+		return dists[i] = 0;
+	}
+
+	// we cut the looping path, so the current node is like
+	// the "end" node; but the actual value for this node
+	// is yet to be calculated on the recursive return
+	else if (loops[i] > 1) {
+		return 0;
+	}
+
+	else {
+		local_inc _(loops[i]);
+
 		Node::arcs_t::const_iterator
 			arc = node.arcs.begin(),
 			end = node.arcs.end();
+
 		for (; arc != end; ++arc) {
-			const size_t j = arc->first;
-			calc_dist(skel, loops, dists, j);
-			uint32_t &d = dists[j];
-			if (d != DIST_ERROR) {
-				if (dist == DIST_ERROR) {
-					dist = d;
-				} else {
-					dist = std::max(dist, d);
-				}
-			}
+			const uint32_t d = calc_dist(skel, loops, dists, arc->first);
+
+			// not necessarily true for dists[arc->first]
+			assert (d != DIST_ERROR);
+
+			dist = (dist == DIST_ERROR) ? d : std::max(dist, d);
 		}
-		dist = std::min(dist + 1, DIST_MAX);
+
+		return dists[i] = std::min(dist + 1, DIST_MAX);
 	}
 }
 
@@ -60,8 +69,7 @@ uint32_t maxpath(const Skeleton &skel)
 {
 	std::vector<uint8_t> loops(skel.nodes_count);
 	std::vector<uint32_t> dists(skel.nodes_count, DIST_ERROR);
-	calc_dist(skel, loops, dists, 0);
-	const uint32_t maxlen = dists[0];
+	const uint32_t maxlen = calc_dist(skel, loops, dists, 0);
 	if (maxlen == DIST_MAX) {
 		fatal("DFA path %sis too long", incond(skel.cond).c_str());
 	}