[repacker] add cycle detection to the graph sort.

This allows us to bail early if the graph is not acyclic.
This commit is contained in:
Garret Rieger 2022-12-01 21:51:17 +00:00
parent 9e99d08470
commit 554ed06fac
4 changed files with 31 additions and 6 deletions

View File

@ -50,7 +50,7 @@ struct graph_t
unsigned priority = 0; unsigned priority = 0;
bool link_positions_valid (unsigned parent_index) bool link_positions_valid ()
{ {
hb_set_t assigned_bytes; hb_set_t assigned_bytes;
for (const auto& l : obj.real_links) for (const auto& l : obj.real_links)
@ -321,8 +321,6 @@ struct graph_t
vertices_scratch_.alloc (objects.length); vertices_scratch_.alloc (objects.length);
for (unsigned i = 0; i < objects.length; i++) for (unsigned i = 0; i < objects.length; i++)
{ {
// TODO(grieger): check all links point to valid objects.
// If this graph came from a serialization buffer object 0 is the // If this graph came from a serialization buffer object 0 is the
// nil object. We don't need it for our purposes here so drop it. // nil object. We don't need it for our purposes here so drop it.
if (i == 0 && !objects[i]) if (i == 0 && !objects[i])
@ -335,7 +333,8 @@ struct graph_t
if (check_success (!vertices_.in_error ())) if (check_success (!vertices_.in_error ()))
v->obj = *objects[i]; v->obj = *objects[i];
check_success (v->link_positions_valid (i)); check_success (v->link_positions_valid ());
if (!removed_nil) continue; if (!removed_nil) continue;
// Fix indices to account for removed nil object. // Fix indices to account for removed nil object.
for (auto& l : v->obj.all_links_writer ()) { for (auto& l : v->obj.all_links_writer ()) {
@ -455,6 +454,13 @@ struct graph_t
hb_swap (sorted_graph[new_id], vertices_[next_id]); hb_swap (sorted_graph[new_id], vertices_[next_id]);
const vertex_t& next = sorted_graph[new_id]; const vertex_t& next = sorted_graph[new_id];
if (unlikely (!check_success(new_id >= 0))) {
// We are out of ids. Which means we've visited a node more than once.
// This graph contains a cycle which is not allowed.
DEBUG_MSG (SUBSET_REPACK, nullptr, "Invalid graph. Contains cycle.");
return;
}
id_map[next_id] = new_id--; id_map[next_id] = new_id--;
for (const auto& link : next.obj.all_links ()) { for (const auto& link : next.obj.all_links ()) {

View File

@ -283,6 +283,11 @@ hb_resolve_graph_overflows (hb_tag_t table_tag,
graph_t& sorted_graph /* IN/OUT */) graph_t& sorted_graph /* IN/OUT */)
{ {
sorted_graph.sort_shortest_distance (); sorted_graph.sort_shortest_distance ();
if (sorted_graph.in_error ())
{
DEBUG_MSG (SUBSET_REPACK, nullptr, "Sorted graph in error state after initial sort.");
return false;
}
bool will_overflow = graph::will_overflow (sorted_graph); bool will_overflow = graph::will_overflow (sorted_graph);
if (!will_overflow) if (!will_overflow)

View File

@ -1476,6 +1476,7 @@ static void test_sort_shortest ()
graph_t graph (c.object_graph ()); graph_t graph (c.object_graph ());
graph.sort_shortest_distance (); graph.sort_shortest_distance ();
assert (!graph.in_error ());
assert(strncmp (graph.object (4).head, "abc", 3) == 0); assert(strncmp (graph.object (4).head, "abc", 3) == 0);
assert(graph.object (4).real_links.length == 3); assert(graph.object (4).real_links.length == 3);

View File

@ -15,6 +15,20 @@ typedef struct
uint8_t width; uint8_t width;
} link_t; } link_t;
/* The fuzzer seed contains a serialized representation of a object graph which forms
* the input graph to the repacker call. The binary format is:
*
* table tag: 4 bytes
* number of objects: 2 bytes
* objects[number of objects]:
* blob size: 2 bytes
* blob: blob size bytes
* num of real links: 2 bytes
* links[number of real links]: link_t struct
*
* TODO(garretrieger): add optional virtual links
*/
template <typename T> template <typename T>
bool read(const uint8_t** data, size_t* size, T* out) bool read(const uint8_t** data, size_t* size, T* out)
{ {
@ -108,8 +122,7 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size)
{ {
if (!read<link_t> (&data, &size, &links[i])) goto end; if (!read<link_t> (&data, &size, &links[i])) goto end;
if (links[i].parent >= num_objects if (links[i].parent >= num_objects)
|| links[i].child > links[i].parent) // Enforces DAG graph
goto end; goto end;
} }