[2/2] Do not put {NULL, NULL} entries in a worklist when checking types for equality.

Message ID 154181486826640@webcorp03f.yandex-team.ru
State New, archived
Headers

Commit Message

Maxim Akhmedov Feb. 11, 2017, 3:24 p.m. UTC
  If type1 and type2 are enumeration types, their fields (corresponding to the enum members) have their types set to NULL. I'm not sure whether this is a mistake of reading the DWARF2 information, or if this is an intended behavior, but the shortest way to fix that is to ensure that we do not put pairs consisting of NULLs into a worklist in the check_types_equal procedure when iterating over all fields.

gdb/ChangeLog:
2017-02-11  Maxim Akhmedov  <max42@yandex-team.ru>

	* gdbtypes.c: do not put {NULL, NULL} entries in a worklist in 
	check_types_equal.


---
 gdb/gdbtypes.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

-- 
2.11.0
  

Comments

Luis Machado Feb. 15, 2017, 2:47 p.m. UTC | #1
On 02/11/2017 09:24 AM, Maxim Akhmedov wrote:
> If type1 and type2 are enumeration types, their fields (corresponding to the enum members) have their types set to NULL. I'm not sure whether this is a mistake of reading the DWARF2 information, or if this is an intended behavior, but the shortest way to fix that is to ensure that we do not put pairs consisting of NULLs into a worklist in the check_types_equal procedure when iterating over all fields.
>
> gdb/ChangeLog:
> 2017-02-11  Maxim Akhmedov  <max42@yandex-team.ru>
>
> 	* gdbtypes.c: do not put {NULL, NULL} entries in a worklist in
> 	check_types_equal.
>
>
> ---
>  gdb/gdbtypes.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 5e5db2776b..62bc3d752b 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -3364,6 +3364,8 @@ static int
>  check_types_equal (struct type *type1, struct type *type2,
>                    VEC (type_equality_entry_d) **worklist)
>  {
> +  gdb_assert (type1 != NULL && type2 != NULL);
> +
>    type1 = check_typedef (type1);
>    type2 = check_typedef (type2);
>
> @@ -3448,9 +3450,17 @@ check_types_equal (struct type *type1, struct type *type2,
>                               FIELD_LOC_KIND (*field1));
>             }
>
> -         entry.type1 = FIELD_TYPE (*field1);
> -         entry.type2 = FIELD_TYPE (*field2);
> -         VEC_safe_push (type_equality_entry_d, *worklist, &entry);
> +          /* For some types fields may not have their own type specifications
> +             like enum fields in C++. */

The indentation looks wrong above. Also, two spaces after period.

> +          if (FIELD_TYPE (*field1) != NULL && FIELD_TYPE (*field2) != NULL)
> +            {
> +              entry.type1 = FIELD_TYPE (*field1);
> +              entry.type2 = FIELD_TYPE (*field2);
> +
> +              VEC_safe_push (type_equality_entry_d, *worklist, &entry);
> +            }
> +          else if (FIELD_TYPE (*field1) != NULL || FIELD_TYPE (*field2) != NULL)
> +            return 0;

Is this else leg conditional correct? Do we want to return "not equal" 
when one of the fields is non-NULL and let a case where both are NULL 
through?

What is the outcome with the patch applied? A test to reproduce this 
would indeed be nice.
  

Patch

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 5e5db2776b..62bc3d752b 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -3364,6 +3364,8 @@  static int
 check_types_equal (struct type *type1, struct type *type2,
                   VEC (type_equality_entry_d) **worklist)
 {
+  gdb_assert (type1 != NULL && type2 != NULL);
+
   type1 = check_typedef (type1);
   type2 = check_typedef (type2);
 
@@ -3448,9 +3450,17 @@  check_types_equal (struct type *type1, struct type *type2,
                              FIELD_LOC_KIND (*field1));
            }
 
-         entry.type1 = FIELD_TYPE (*field1);
-         entry.type2 = FIELD_TYPE (*field2);
-         VEC_safe_push (type_equality_entry_d, *worklist, &entry);
+          /* For some types fields may not have their own type specifications
+             like enum fields in C++. */
+          if (FIELD_TYPE (*field1) != NULL && FIELD_TYPE (*field2) != NULL)
+            {
+              entry.type1 = FIELD_TYPE (*field1);
+              entry.type2 = FIELD_TYPE (*field2);
+
+              VEC_safe_push (type_equality_entry_d, *worklist, &entry);
+            }
+          else if (FIELD_TYPE (*field1) != NULL || FIELD_TYPE (*field2) != NULL)
+            return 0;
        }
     }