[applied] abg-comparison: Do not abort when a static data member is deleted

Message ID 87sejqmtre.fsf@redhat.com
State New
Headers
Series [applied] abg-comparison: Do not abort when a static data member is deleted |

Commit Message

Dodji Seketeli June 23, 2025, 2:12 p.m. UTC
  Hello,

The global variable representing a static data member can appear
several times in a binary as there can be one per translation unit.
In that case, the symbols are put in a COMDAT section and the linker
merge them into once to represent its unique definition.

If that global variable is removed from the new version of the binary
being compared, it can (wrongly) appear to the diff engine as if the
global variable is being removed twice.

This patch handles that case by avoiding being overly eager in the
sanity assert for global variables detection.

	* src/abg-comparison.cc
	(corpus_diff::priv::ensure_lookup_tables_populated): Do not choke
	if a static data member is removed.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to the mainline.
---
 src/abg-comparison.cc | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)
  

Patch

diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 39e2307a..989701a0 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -10021,8 +10021,30 @@  corpus_diff::priv::ensure_lookup_tables_populated()
 	const var_decl_sptr deleted_var = first_->get_variables()[i];
 	string n = deleted_var->get_id();
 	ABG_ASSERT(!n.empty());
-	ABG_ASSERT(deleted_vars_.find(n) == deleted_vars_.end());
-	deleted_vars_[n] = deleted_var;
+	// The below is commented out because there can be several
+	// global variables with the same ID in the corpus.  So
+	// several global variables with the same ID can be deleted.
+	//
+	// In general these are static member variables.  There can be
+	// multiple instances of these (one per translation unit) that
+	// appear to be different, but they are put into a COMDAT
+	// section (with CLOOS ELF binding in modern toolchains) in
+	// the end.
+	//
+	// In that case, let's keep track of the first global variable
+	// removed and let's ignore the subsequent ones as they should
+	// all be "merged" into one by the linker.
+	//
+	// ABG_ASSERT(deleted_vars_.find(n) == deleted_vars_.end());
+	string_var_ptr_map::const_iterator j = deleted_vars_.find(n);
+	if (j != deleted_vars_.end())
+	  {
+	    ABG_ASSERT(is_member_decl(j->second)
+		       && get_member_is_static(j->second));
+	    continue;
+	  }
+	else
+	  deleted_vars_[n] = deleted_var;
       }
 
     for (vector<insertion>::const_iterator it = e.insertions().begin();