diff mbox series

[applied] Bug 31236 - Fix removing a member declaration from its scope

Message ID 87le8pr6uf.fsf@redhat.com
State New
Headers
Series [applied] Bug 31236 - Fix removing a member declaration from its scope |

Commit Message

Dodji Seketeli Jan. 16, 2024, 9 a.m. UTC 
  Hello,

In some C++ binaries, DWARF can represent a member variable using a
global variable /definition/ DIE not having a reference attribute
pointing back to the member variable declaration DIE.  The only way we
know that the global variable is a definition DIE for a member
variable is because its linkage name demangles to
"foo::bar::var_name", with foo::bar being a class name.

So, for each translation unit, when the DWARF reader reads a global
variable DIE, it builds a variable IR node for it and stashes it on
the side.

Then, when the translation unit is built, the DWARF reader looks at
all the stashed global variables, detects those that are actually
member variables and adds them to their class.  But then, before
adding a (former global) variable to its class, the reader has first
to remove it from its global scope.  This removal is done by the
function remove_decl_from_scope, which calls
scope_decl::remove_member_decl.

The issue here is that remove_decl_from_scope forgets to unset the
translation unit property of the global variable.

Then, in the particular case of this problem report, when
scope_decl::add_member_decl is called to add the variable to its
class, it detects that the variable belongs to /another/ translation
unit and (rightfully) aborts.  Ooops.

This patch fixes the issue by making remove_decl_from_scope remove the
variable from its translation unit too, not just from its scope.  The
patch actually delegates the scope & translation unit resetting to
scope_decl::remove_member_decl because it appears to me that this is
where these ought to be handled.

To ensure that the issue is fixed, one needs to unpack the package
webkit2gtk3-2.40.5-1.el9_3.1.x86_64.rpm and run abidw on the binary
$prefix/usr/lib64/libwebkit2gtk-4.0.so.37 like:

   $ abidw --noout $prefix/usr/lib64/libwebkit2gtk-4.0.so.37

Given the size of the library, this takes three hours and a half as
well as ~50GB of ram to complete on my system using a non-optimized
debug build of libabigail.  We definitely need to invest in more speed
optimizations to handle webkit.  That would be for another day, I
guess.

	* src/abg-ir.cc (scope_decl::remove_member_decl): Reset the
	translation unit and the scope of the removed decl.
	(remove_decl_from_scope): Do not reset the scope of the removed
	decl here as it's now done above.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to master.

---
 src/abg-ir.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 56d71465..81549bbf 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -8288,6 +8288,9 @@  scope_decl::remove_member_decl(decl_base_sptr member)
 	    }
 	}
     }
+
+  member->set_scope(nullptr);
+  member->set_translation_unit(nullptr);
 }
 
 /// Return the hash value for the current instance of scope_decl.
@@ -8526,7 +8529,6 @@  remove_decl_from_scope(decl_base_sptr decl)
 
   scope_decl* scope = decl->get_scope();
   scope->remove_member_decl(decl);
-  decl->set_scope(0);
 }
 
 /// Inserts a declaration into a given scope, before a given IR child