[PATCH v3 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

--
  v3: 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 v3 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 | 38 ++++++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/tools/widl/typegen.c b/tools/widl/typegen.c
index b3373ded11d..9aadfd06c2a 100644
--- a/tools/widl/typegen.c
+++ b/tools/widl/typegen.c
@@ -747,8 +747,22 @@ static int type_has_pointers(const type_t *type)
 }

static int type_has_full_pointer(const type_t *type, const attr_list_t *attrs,
-                                 int toplevel_param)
+                                 int toplevel_param, int visited_structs_pos)
 {
+    static const type_t **visited_structs;
+    static int visited_structs_count;
+
+    if (!visited_structs)
+    {
+        visited_structs_count = 100;
+        visited_structs = xmalloc(sizeof (type_t*) * visited_structs_count);
+    }
+
+    if (visited_structs_pos >= visited_structs_count)
+    {
+        visited_structs_count *= 2;
+        visited_structs = xrealloc(visited_structs, sizeof (type_t*) * visited_structs_count);
+    }
     switch (typegen_detect_type(type, NULL, TDT_IGNORE_STRINGS))
     {
     case TGT_USER_TYPE:
@@ -762,14 +776,26 @@ 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(type_array_get_element_type(type), NULL, FALSE, visited_structs_pos);
     case TGT_STRUCT:
     {
+        int i;
         var_list_t *fields = type_struct_get_fields(type);
         const var_t *field;
+
+        visited_structs[visited_structs_pos] = type;
+        for (i = 0; i < visited_structs_pos; i++)
+        {
+            if (visited_structs[i] == type)
+            {
+                /* Found struct we visted already, abort to prevent infinite loop */
+                return FALSE;
+            }
+        }
+
         if (fields) LIST_FOR_EACH_ENTRY( field, fields, const var_t, entry )
         {
-            if (type_has_full_pointer(field->declspec.type, field->attrs, FALSE))
+            if (type_has_full_pointer(field->declspec.type, field->attrs, FALSE, visited_structs_pos + 1))
                 return TRUE;
         }
         break;
@@ -781,7 +807,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(field->declspec.type, field->attrs, FALSE, visited_structs_pos))
                 return TRUE;
         }
         break;
@@ -2082,12 +2108,12 @@ static unsigned int type_buffer_alignment(const type_t *t)
 int is_full_pointer_function(const var_t *func)
 {
     const var_t *var;
-    if (type_has_full_pointer(type_function_get_rettype(func->declspec.type), func->attrs, TRUE))
+    if (type_has_full_pointer(type_function_get_rettype(func->declspec.type), func->attrs, TRUE, 0))
         return TRUE;
     if (!type_function_get_args(func->declspec.type))
         return FALSE;
     LIST_FOR_EACH_ENTRY( var, type_function_get_args(func->declspec.type), const var_t, entry )
-        if (type_has_full_pointer( var->declspec.type, var->attrs, TRUE ))
+        if (type_has_full_pointer( var->declspec.type, var->attrs, TRUE, 0 ))
             return TRUE;
     return FALSE;
 }

[PATCH v3 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 9aadfd06c2a..d2baa3d9c00 100644
--- a/tools/widl/typegen.c
+++ b/tools/widl/typegen.c
@@ -771,7 +771,7 @@ 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 FALSE;
+            return type_has_full_pointer(type_pointer_get_ref_type(type), NULL, FALSE, visited_structs_pos);
     case TGT_ARRAY:
         if (get_pointer_fc(type, attrs, toplevel_param) == FC_FP)
             return TRUE;

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

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=137209

Your paranoid android.

=== debian11b (64 bit WoW report) ===

uiautomationcore:
uiautomation.c:14688: Test failed: Unexpected hr 0x800706b5.
uiautomation.c:14689: Test failed: expected prov_callback_base_hwnd
uiautomation.c:14690: Test failed: expected prov_callback_nonclient
uiautomation.c:14704: Test failed: Wait for event_handle failed.
uiautomation.c:14705: Test failed: expected uia_event_callback
uiautomation.c:15296: Test failed: 5 failures in child process

[Zebediah Figura (＠zfigura)]

On Sat Sep  9 12:55:32 2023 +0000, Fabian Maurer wrote:
> When recursing into array pointer, we get a stackoverflow due to
> infinite recursion as well. Therefore, unless I missed something, we
> need first the infinite-recursion fix, then the array recursion change.
> The file that causes the stackoverflow with array recursion is
> actxprxy_shobjidl.idl. However, even with the current master branch it
> crashes when running widl manually like
> ./tools/widl/widl -o test.c -m32 --nostdinc -Ldlls/\* -Idlls/actxprxy \
>   -I../wine-git/dlls/actxprxy -Iinclude -I../wine-git/include
> -I../wine-git/include/msvcrt -D_UCRT \
>   -D__WINESRC__ ../wine-git/dlls/actxprxy/actxprxy_shobjidl.idl
>
> It works when compiling wine though... Something is very weird here, but
> it means I can't really check which type(s) exactly cause the infinite array-recursion.
> I also updated to use dynamic allocation, but kept the static variable.
> IMHO, since it's single threaded and only short lived, it's easier to
> just leave the memory than constantly allocate and free the memory. Can
> be changed though, if you think that's preferable.

The static variable feels brittle to me; I guess there's no reason it doesn't work but if it were up to me I'd still prefer to avoid it. If it were me I'd just embed all the array parameters in a struct and pass that down.

I think in general what you're calling "pos" and "count", I'm used to calling "count" and "capacity".