[PATCH v7 0/2] MR3151: widl: Prevent infinite loop when structure contains array of itself
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55115
-- v7: widl: In type_has_full_pointer recurse for pointer types as well widl: Prevent infinite loop when structure contains array of itself
From: Fabian Maurer dark.shadow4@web.de
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 23db6555e9d..2db62266d6b 100644 --- a/tools/widl/typegen.c +++ b/tools/widl/typegen.c @@ -745,8 +745,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)) { @@ -761,17 +768,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: { @@ -780,7 +819,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; @@ -799,6 +838,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;
From: Fabian Maurer dark.shadow4@web.de
--- 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 2db62266d6b..6df0e2b7cb2 100644 --- a/tools/widl/typegen.c +++ b/tools/widl/typegen.c @@ -763,7 +763,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;
On Mon Sep 18 17:36:48 2023 +0000, Fabian Maurer wrote:
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.
@zfigura Would you mind looking at this again?
Zebediah Figura (@zfigura) commented about tools/widl/typegen.c:
{
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;
I'd just do "capacity = max(4, capacity * 2)"; then you don't need the explicit initialization. Or even just copy-paste the array_reserve() helper we use in other places in Wine.
Zebediah Figura (@zfigura) commented about tools/widl/typegen.c:
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;
Setting this before searching the array doesn't look right.
-
Fabian Maurer -
Fabian Maurer (＠DarkShadow44)
-
Zebediah Figura (＠zfigura)