[PATCH v5 0/2] MR3151: widl: Prevent infinite loop when structure contains array of itself

[Fabian Maurer (＠DarkShadow44)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55115

--
  v5: widl: In type_has_full_pointer recurse for pointer types as well
      widl: Prevent infinite loop when structure contains array of itself

https://gitlab.winehq.org/wine/wine/-/merge_requests/3151

[PATCH v5 1/2] widl: Prevent infinite loop when structure contains array of itself

[Fabian Maurer]

From: Fabian Maurer

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55115
---
 tools/widl/typegen.c | 62 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/tools/widl/typegen.c b/tools/widl/typegen.c
index b3373ded11d..96fb2e4d74e 100644
--- a/tools/widl/typegen.c
+++ b/tools/widl/typegen.c
@@ -746,8 +746,15 @@ static int type_has_pointers(const type_t *type)
     return FALSE;
 }

-static int type_has_full_pointer(const type_t *type, const attr_list_t *attrs,
-                                 int toplevel_param)
+struct visited_struct_array
+{
+    const type_t **structs;
+    unsigned int count;
+    unsigned int capacity;
+};
+
+static int type_has_full_pointer_recurse(const type_t *type, const attr_list_t *attrs,
+                                 int toplevel_param, struct visited_struct_array *visited_structs)
 {
     switch (typegen_detect_type(type, NULL, TDT_IGNORE_STRINGS))
     {
@@ -762,17 +769,49 @@ static int type_has_full_pointer(const type_t *type, const attr_list_t *attrs,
         if (get_pointer_fc(type, attrs, toplevel_param) == FC_FP)
             return TRUE;
         else
-            return type_has_full_pointer(type_array_get_element_type(type), NULL, FALSE);
+            return type_has_full_pointer_recurse(type_array_get_element_type(type), NULL, FALSE, visited_structs);
     case TGT_STRUCT:
     {
+        unsigned int i;
+        int ret = FALSE;
         var_list_t *fields = type_struct_get_fields(type);
         const var_t *field;
+
+        if (!visited_structs->structs)
+        {
+            visited_structs->count = 0;
+            visited_structs->capacity = 100;
+            visited_structs->structs = xmalloc(sizeof (type_t*) * visited_structs->capacity);
+        }
+
+        if (visited_structs->count >= visited_structs->capacity)
+        {
+            visited_structs->capacity *= 2;
+            visited_structs->structs = xrealloc(visited_structs->structs, sizeof (type_t*) * visited_structs->capacity);
+        }
+
+        visited_structs->structs[visited_structs->count] = type;
+        for (i = 0; i < visited_structs->count; i++)
+        {
+            if (visited_structs->structs[i] == type)
+            {
+                /* Found struct we visited already, abort to prevent infinite loop.
+                 * Can't be at the first struct we visit, so we can skip cleanup and just return */
+               return FALSE;
+            }
+        }
+
+        visited_structs->count++;
         if (fields) LIST_FOR_EACH_ENTRY( field, fields, const var_t, entry )
         {
-            if (type_has_full_pointer(field->declspec.type, field->attrs, FALSE))
-                return TRUE;
+            if (type_has_full_pointer_recurse(field->declspec.type, field->attrs, FALSE, visited_structs))
+            {
+                ret = TRUE;
+                break;
+            }
         }
-        break;
+        visited_structs->count--;
+        return ret;
     }
     case TGT_UNION:
     {
@@ -781,7 +820,7 @@ static int type_has_full_pointer(const type_t *type, const attr_list_t *attrs,
         fields = type_union_get_cases(type);
         if (fields) LIST_FOR_EACH_ENTRY( field, fields, const var_t, entry )
         {
-            if (field->declspec.type && type_has_full_pointer(field->declspec.type, field->attrs, FALSE))
+            if (field->declspec.type && type_has_full_pointer_recurse(field->declspec.type, field->attrs, FALSE, visited_structs))
                 return TRUE;
         }
         break;
@@ -800,6 +839,15 @@ static int type_has_full_pointer(const type_t *type, const attr_list_t *attrs,
     return FALSE;
 }

+static int type_has_full_pointer(const type_t *type, const attr_list_t *attrs, int toplevel_param)
+{
+    int ret;
+    struct visited_struct_array visited_structs = {0};
+    ret = type_has_full_pointer_recurse(type, attrs, toplevel_param, &visited_structs);
+    free(visited_structs.structs);
+    return ret;
+}
+
 static unsigned short user_type_offset(const char *name)
 {
     user_type_t *ut;

[PATCH v5 2/2] widl: In type_has_full_pointer recurse for pointer types as well

[Fabian Maurer]

From: Fabian Maurer

---
 tools/widl/typegen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/widl/typegen.c b/tools/widl/typegen.c
index 96fb2e4d74e..76d4a611cc6 100644
--- a/tools/widl/typegen.c
+++ b/tools/widl/typegen.c
@@ -764,7 +764,7 @@ static int type_has_full_pointer_recurse(const type_t *type, const attr_list_t *
         if (get_pointer_fc(type, attrs, toplevel_param) == FC_FP)
             return TRUE;
         else
-            return FALSE;
+            return type_has_full_pointer_recurse(type_pointer_get_ref_type(type), NULL, FALSE, visited_structs);
     case TGT_ARRAY:
         if (get_pointer_fc(type, attrs, toplevel_param) == FC_FP)
             return TRUE;

[Fabian Maurer (＠DarkShadow44)]

On Mon Sep 18 17:19:23 2023 +0000, Zebediah Figura wrote:
> Actually since this is a short-lived program you could even skip freeing
> the array.

Alright, pushed an update. Since I don't know how much data is really allocated, I find it safer to free the data, it shouldn't be much overhead anyways.