[26/27] dwarf-reader, reader.cc: Fix function virtuality setting

Message ID 875xrjjohl.fsf@seketeli.org
State New
Headers
Series Implement type hashing & fix self-comparing gcc-gnat in fc37 |

Commit Message

Dodji Seketeli Aug. 29, 2024, 4:13 p.m. UTC
  Hello,

When setting the virtual-ness of a function (whether it's virtual or
not and its vtable offset), one must set the vtable offset before
setting the is_virtual property because setting the later triggers a
fix-up that uses the former.

It turns out some parts of the code got the order wrong.  To fix that,
this patch introduce a new set_member_function_virtuality that ensures
things are done in the proper order.

SET_member_function_vtable_offset and set_member_function_is_virtual
are now static functions that are used only by the new
set_member_function_virtuality.

	* include/abg-fwd.h (set_member_function_vtable_offset)
	(set_member_function_is_virtual): Remove these function
	declarations.
	(set_member_function_virtuality): Declare new function.
	* include/abg-ir.h (decl_base::get_context_rel): Make this public.
	(set_member_function_is_virtual)
	(set_member_function_vtable_offset): Remove these friend
	declaration for the decl_base class.
	* src/abg-dwarf-reader.cc (finish_member_function_reading): Use
	the new set_member_function_virtuality in lieu of the previous
	set_member_function_is_virtual and
	set_member_function_vtable_offset.
	* src/abg-ir.cc (set_member_function_vtable_offset)
	(set_member_function_is_virtual): Make these functions static.
	(set_member_function_virtuality): Define new functions.
	(class_or_union::add_member_function): Use the new
	set_member_function_virtuality in lieu of
	set_member_function_is_virtual and
	set_member_function_vtable_offset.
	* src/abg-reader.cc (build_class_decl): Likewise.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 include/abg-fwd.h       | 15 +++-------
 include/abg-ir.h        | 27 ++++--------------
 src/abg-dwarf-reader.cc |  4 +--
 src/abg-ir.cc           | 61 +++++++++++++++++++++++++++++++++++++----
 src/abg-reader.cc       |  5 ++--
 5 files changed, 68 insertions(+), 44 deletions(-)
  

Patch

diff --git a/include/abg-fwd.h b/include/abg-fwd.h
index 5f2f92f6..6a7af9ee 100644
--- a/include/abg-fwd.h
+++ b/include/abg-fwd.h
@@ -961,14 +961,6 @@  get_member_function_vtable_offset(const function_decl&);
 ssize_t
 get_member_function_vtable_offset(const function_decl_sptr&);
 
-void
-set_member_function_vtable_offset(const function_decl& f,
-				  ssize_t s);
-
-void
-set_member_function_vtable_offset(const function_decl_sptr &f,
-				  ssize_t s);
-
 bool
 get_member_function_is_virtual(const function_decl&);
 
@@ -979,10 +971,11 @@  bool
 get_member_function_is_virtual(const function_decl*);
 
 void
-set_member_function_is_virtual(function_decl&, bool);
-
+set_member_function_virtuality(function_decl&, bool, ssize_t);
+void
+set_member_function_virtuality(function_decl*, bool, ssize_t);
 void
-set_member_function_is_virtual(const function_decl_sptr&, bool);
+set_member_function_virtuality(const function_decl_sptr&, bool, ssize_t);
 
 type_base_sptr
 strip_typedef(const type_base_sptr);
diff --git a/include/abg-ir.h b/include/abg-ir.h
index 032dc04d..b0d00f6d 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -1615,12 +1615,6 @@  public:
   set_scope(scope_decl*);
 
 protected:
-  const context_rel*
-  get_context_rel() const;
-
-  context_rel*
-  get_context_rel();
-
   void
   set_context_rel(context_rel *c);
   decl_base(const decl_base&);
@@ -1640,6 +1634,12 @@  public:
 
   decl_base(const environment&, const location&);
 
+  const context_rel*
+  get_context_rel() const;
+
+  context_rel*
+  get_context_rel();
+
   const interned_string&
   get_cached_pretty_representation(bool internal = false) const;
 
@@ -1793,9 +1793,6 @@  public:
   friend bool
   get_member_function_is_virtual(const function_decl& f);
 
-  friend void
-  set_member_function_is_virtual(function_decl&, bool);
-
   friend class class_or_union;
   friend class class_decl;
   friend class scope_decl;
@@ -3918,18 +3915,6 @@  public:
   friend ssize_t
   get_member_function_vtable_offset(const function_decl&);
 
-  friend void
-  set_member_function_vtable_offset(function_decl&, ssize_t);
-
-  friend void
-  set_member_function_vtable_offset(const function_decl_sptr&, ssize_t);
-
-  friend bool
-  get_member_function_is_virtual(const function_decl&);
-
-  friend void
-  set_member_function_is_virtual(function_decl&, bool);
-
   virtual ~method_decl();
 };// end class method_decl
 
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 0387aa6c..c0d14581 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -14004,10 +14004,8 @@  finish_member_function_reading(Dwarf_Die*			die,
 
   m->is_declared_inline(is_inline);
   set_member_access_specifier(m, access);
-  if (vindex != -1)
-    set_member_function_vtable_offset(m, vindex);
   if (is_virtual)
-    set_member_function_is_virtual(m, is_virtual);
+    set_member_function_virtuality(m, is_virtual, vindex);
   bool is_static = method_t->get_is_for_static_method();
   set_member_is_static(m, is_static);
   set_member_function_is_ctor(m, is_ctor);
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 713b38d7..ca84a53e 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -6431,7 +6431,7 @@  get_member_function_vtable_offset(const function_decl_sptr& f)
 /// @param s the new vtable offset.  Please note that a vtable offset
 /// of value -1 means that the virtual member function does not (yet)
 /// have any vtable offset associated to it.
-void
+static void
 set_member_function_vtable_offset(function_decl& f, ssize_t s)
 {
   ABG_ASSERT(is_member_function(f));
@@ -6452,7 +6452,7 @@  set_member_function_vtable_offset(function_decl& f, ssize_t s)
 /// @param s the new vtable offset.  Please note that a vtable offset
 /// of value -1 means that the virtual member function does not (yet)
 /// have any vtable offset associated to it.
-void
+static void
 set_member_function_vtable_offset(const function_decl_sptr& f, ssize_t s)
 {return set_member_function_vtable_offset(*f, s);}
 
@@ -6499,7 +6499,7 @@  get_member_function_is_virtual(const function_decl* mem_fn)
 /// @param f the member function to consider.
 ///
 /// @param is_virtual set to true if the function is virtual.
-void
+static void
 set_member_function_is_virtual(function_decl& f, bool is_virtual)
 {
   ABG_ASSERT(is_member_function(f));
@@ -6518,7 +6518,7 @@  set_member_function_is_virtual(function_decl& f, bool is_virtual)
 /// @param f the member function to consider.
 ///
 /// @param is_virtual set to true if the function is virtual.
-void
+static void
 set_member_function_is_virtual(const function_decl_sptr& fn, bool is_virtual)
 {
   if (fn)
@@ -6528,6 +6528,56 @@  set_member_function_is_virtual(const function_decl_sptr& fn, bool is_virtual)
     }
 }
 
+/// Set the virtual-ness of a member fcuntion
+///
+/// @param fn the member function to consider.
+///
+/// @param is_virtual whether the function is virtual.
+///
+/// @param voffset the virtual offset of the virtual function.
+void
+set_member_function_virtuality(function_decl&	fn,
+			       bool		is_virtual,
+			       ssize_t		voffset)
+{
+  // Setting the offset must come first because the second function
+  // does assume the voffset is set, in case of virtuality
+  set_member_function_vtable_offset(fn, voffset);
+  set_member_function_is_virtual(fn, is_virtual);
+}
+
+/// Set the virtual-ness of a member fcuntion
+///
+/// @param fn the member function to consider.
+///
+/// @param is_virtual whether the function is virtual.
+///
+/// @param voffset the virtual offset of the virtual function.
+void
+set_member_function_virtuality(function_decl*	fn,
+			       bool		is_virtual,
+			       ssize_t		voffset)
+{
+  if (fn)
+    set_member_function_virtuality(*fn, is_virtual, voffset);
+}
+
+/// Set the virtual-ness of a member fcuntion
+///
+/// @param fn the member function to consider.
+///
+/// @param is_virtual whether the function is virtual.
+///
+/// @param voffset the virtual offset of the virtual function.
+void
+set_member_function_virtuality(const function_decl_sptr&	fn,
+			       bool				is_virtual,
+			       ssize_t				voffset)
+{
+  set_member_function_vtable_offset(fn, voffset);
+  set_member_function_is_virtual(fn, is_virtual);
+}
+
 /// Recursively returns the the underlying type of a typedef.  The
 /// return type should not be a typedef of anything anymore.
 ///
@@ -25254,10 +25304,9 @@  class_or_union::add_member_function(method_decl_sptr f,
 
   if (class_decl* klass = is_class_type(this))
     {
-      set_member_function_is_virtual(f, is_virtual);
       if (is_virtual)
 	{
-	  set_member_function_vtable_offset(f, vtable_offset);
+	  set_member_function_virtuality(f, is_virtual, vtable_offset);
 	  sort_virtual_member_functions(klass->priv_->virtual_mem_fns_);
 	}
     }
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 9b8b9e41..8c376bba 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -5766,9 +5766,8 @@  build_class_decl(reader&		rdr,
 		  ABG_ASSERT(m);
 		  set_member_access_specifier(m, access);
 		  set_member_is_static(m, is_static);
-		  if (vtable_offset != -1)
-		    set_member_function_vtable_offset(m, vtable_offset);
-		  set_member_function_is_virtual(m, is_virtual);
+		  if (is_virtual)
+		    set_member_function_virtuality(m, is_virtual, vtable_offset);
 		  set_member_function_is_ctor(m, is_ctor);
 		  set_member_function_is_dtor(m, is_dtor);
 		  set_member_function_is_const(m, is_const);