diff mbox series

[2/2] Bug 26646 - unexpected declaration-only types

Message ID 87mtj3lyn8.fsf@seketeli.org
State New
Headers
Series [1/2] symtab-reader: Remove an over-agressive assertion |

Commit Message

Dodji Seketeli Feb. 7, 2022, 11:11 a.m. UTC 
  In a version of the kernel binary referred to in this problem report,
the parameter 'skb' of the udp4_hwcsum function, which is of type
"pointer to struct sk_buff", indirectly refers to a pointer to a
declaration-only struct ip_mc_list.

In another version of that kernel binary, the same parameter skb of
the udp4_hwcsum function is still of type "pointer to struct sk_buff",
but in that case, the sk_buff indirectly refers to a pointer to a
fully defined struct ip_mc_list.

The first kernel only contains a decl-only struct ip_mc_list whereas
the second one contains a fully defined struct ip_mc_list.

This problem comes from the fact that in add_or_update_class_type, we
"reuse" the "struct sk_buff" that we've already seen in the same
binary, if any.  Depending on the order in which types are defined in
the debug information, if the DIE for struct sk_buff that refers to a
decl-only struct ip_mc_list has already been "seen" by the
DWARF-reader, then add_or_update_class_type re-uses the IR of that DIE
that's been constructed already; otherwise, the IR for the struct
sk_buff represented by the current DIE is constructed.

This patch fixes the problem by always constructing an IR for the
DIE that is being seen, in add_or_update_class_type.

	* src/abg-dwarf-reader.cc (add_or_update_class_type): Do not reuse
	the IR for a DIE with the same textual representation as the one
	we are seeing now.
	* tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
	* tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 src/abg-dwarf-reader.cc                          | 13 -------------
 tests/data/test-annotate/test21-pr19092.so.abi   |  2 +-
 tests/data/test-read-dwarf/test21-pr19092.so.abi |  2 +-
 3 files changed, 2 insertions(+), 15 deletions(-)

Comments

Dodji Seketeli Feb. 8, 2022, 10:27 a.m. UTC | #1 
Giuliano Procida <gprocida@google.com> writes:

> This looks good to me.

Thanks!

>
> In the case of the two kernel ABIs, it eliminates one of the two
> differences in declaration-only status. I'll follow up on the other
> difference in the bug when I have a bit more information to share.

Good to know, thank you.

[...]

>>
>>         * src/abg-dwarf-reader.cc (add_or_update_class_type): Do not reuse
>>         the IR for a DIE with the same textual representation as the one
>>         we are seeing now.
>>         * tests/data/test-annotate/test21-pr19092.so.abi: Adjust.
>>         * tests/data/test-read-dwarf/test21-pr19092.so.abi: Likewise.
>>
>> Signed-off-by: Dodji Seketeli <dodji@redhat.com>
>
> Reviewed-by: Giuliano Procida <gprocida@google.com>

Applied to master, thanks!

[...]

>
> Cheers!
>

Cheers!
diff mbox series

Patch

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index d8545b4c..53b5845d 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -12160,19 +12160,6 @@  add_or_update_class_type(read_context&	 ctxt,
       }
   }
 
-  if (!ctxt.die_is_in_cplus_plus(die))
-    // In c++, a given class might be put together "piecewise".  That
-    // is, in a translation unit, some data members of that class
-    // might be defined; then in another later, some member types
-    // might be defined.  So we can't just re-use a class "verbatim"
-    // just because we've seen previously.  So in c++, re-using the
-    // class is a much clever process.  In the other languages however
-    // (like in C) we can re-use a class definition verbatim.
-    if (class_decl_sptr class_type =
-	is_class_type(ctxt.lookup_type_from_die(die)))
-      if (!class_type->get_is_declaration_only())
-	return class_type;
-
   string name, linkage_name;
   location loc;
   die_loc_and_name(ctxt, die, loc, name, linkage_name);
diff --git a/tests/data/test-annotate/test21-pr19092.so.abi b/tests/data/test-annotate/test21-pr19092.so.abi
index e009a191..bb87fc54 100644
--- a/tests/data/test-annotate/test21-pr19092.so.abi
+++ b/tests/data/test-annotate/test21-pr19092.so.abi
@@ -4395,7 +4395,7 @@ 
       </data-member>
     </class-decl>
     <!-- struct htab -->
-    <class-decl name='htab' size-in-bits='896' is-struct='yes' visibility='default' filepath='../.././libcpp/../include/hashtab.h' line='100' column='1' id='type-id-208'>
+    <class-decl name='htab' size-in-bits='896' is-struct='yes' visibility='default' filepath='../.././libiberty/../include/hashtab.h' line='100' column='1' id='type-id-208'>
       <data-member access='public' layout-offset-in-bits='0'>
         <!-- htab_hash htab::hash_f -->
         <var-decl name='hash_f' type-id='type-id-183' visibility='default' filepath='../.././gcc/../include/hashtab.h' line='102' column='1'/>
diff --git a/tests/data/test-read-dwarf/test21-pr19092.so.abi b/tests/data/test-read-dwarf/test21-pr19092.so.abi
index c10916fa..90ecd590 100644
--- a/tests/data/test-read-dwarf/test21-pr19092.so.abi
+++ b/tests/data/test-read-dwarf/test21-pr19092.so.abi
@@ -2915,7 +2915,7 @@ 
         <var-decl name='next' type-id='type-id-207' visibility='default' filepath='../.././gcc/tlink.c' line='199' column='1'/>
       </data-member>
     </class-decl>
-    <class-decl name='htab' size-in-bits='896' is-struct='yes' visibility='default' filepath='../.././libcpp/../include/hashtab.h' line='100' column='1' id='type-id-208'>
+    <class-decl name='htab' size-in-bits='896' is-struct='yes' visibility='default' filepath='../.././libiberty/../include/hashtab.h' line='100' column='1' id='type-id-208'>
       <data-member access='public' layout-offset-in-bits='0'>
         <var-decl name='hash_f' type-id='type-id-183' visibility='default' filepath='../.././gcc/../include/hashtab.h' line='102' column='1'/>
       </data-member>