From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 54805 invoked by alias); 26 Feb 2016 15:29:31 -0000 Mailing-List: contact libabigail-help@sourceware.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: Sender: libabigail-owner@sourceware.org Received: (qmail 48750 invoked by uid 48); 26 Feb 2016 15:29:26 -0000 From: "dodji at redhat dot com" To: libabigail@sourceware.org Subject: [Bug default/19734] Libabigail skips the 'this' pointer when comparing two member functions Date: Fri, 01 Jan 2016 00:00:00 -0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: libabigail X-Bugzilla-Component: default X-Bugzilla-Version: unspecified X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: dodji at redhat dot com X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: dodji at redhat dot com X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: bug_status short_desc Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://sourceware.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-SW-Source: 2016-q1/txt/msg00154.txt.bz2 https://sourceware.org/bugzilla/show_bug.cgi?id=3D19734 dodji at redhat dot com changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Summary|Shifted exported fields in |Libabigail skips the 'this' |c++ classes are not |pointer when comparing two |detected as abi breakage |member functions --- Comment #1 from dodji at redhat dot com --- Right, this is a real issue. The problem is that today, when libabigail compares two functions (two constructors, for instance) it doesn't compare their implicit 'this' parame= ter. In the case of the example, lego::lego() is the only function that has the "lego" type as a sub-type. But that lego type, is reachable only through t= he implicit 'this' parameter of that function. As libabigail skips that impli= cit this pointer during its comparison, it doesn't see the changes that were ma= de to the lego type. Just to illustrate my point, I added another member function lego. That me= mber explicitly takes a parameter which type "contains" the lego type. Here is the diff of my change to your example: ----------------------------------->8<-------------------- $ diff -u foo.h.orig foo_h --- foo.h.orig 2016-02-26 16:02:43.122804515 +0100 +++ foo_h 2016-02-26 15:10:03.473190139 +0100 @@ -7,6 +7,7 @@ int payload; lego(); + lego(const lego&); int get() { return payload; } int set(int x) { payload =3D x; } dodji@tintin:PR19734$ diff -u foo.cpp.orig foo.cpp --- foo.cpp.orig 2016-02-26 16:02:30.621655698 +0100 +++ foo.cpp 2016-02-26 16:00:01.296878100 +0100 @@ -1,3 +1,10 @@ +#include #include "foo.h" lego::lego() : payload(42) {} + +lego::lego(const lego& l) +{ + memcpy(buggy_symbol, l.buggy_symbol, sizeof(buggy_symbol)); + payload =3D l.payload; +} $ Now abidiff does see the change. This is what running foo.sh now yields: [...] + abidiff libfoo1.so libfoo2.so Functions changes summary: 0 Removed, 1 Changed, 0 Added function Variables changes summary: 0 Removed, 0 Changed, 0 Added variable 1 function with some indirect sub-type change: [C]'method lego::lego(const lego&)' has some indirect sub-type changes: parameter 1 of type 'const lego&' has sub-type changes: in referenced type 'const lego': in unqualified underlying type 'struct lego': type size changed from 96 to 128 bits 2 data member changes: type of 'char lego::buggy_symbol[5]' changed: type name changed from 'char[5]' to 'char[10]' array type size changed from 40 to 80 bits: array type subrange 1 changed length from 5 to 10 'int lego::payload' offset changed from 64 to 96 (in bits) [...] Libabigail skips the this parameter because otherwise, too much noise is emitted in the report. In this small test case, there won't be any noise, = of course. But on big c++ code bases with lots classes that have lots of memb= er functions, there can be lots of duplicated change reports that seriously clutter the output. I made that decision back when the library didn't have= a strong framework for filtering out redundant change reports. It thought th= at in general, there would be other functions that would use the class type explicitly, so skipping the this parameter wouldn't necessarily make user m= iss changes on the class type. And you are with a real life case that rightfully argues otherwise :-) I guess now that the library has stronger filtering framework, the this parameter can be taking in account again when comparing methods. We'll then need to go through the resulting change reports for various interesting binaries, spot the cases where unnecessary duplicated change reports are emitted due to this change, and devise automatic redundancy detection and filtering for those. --=20 You are receiving this mail because: You are on the CC list for the bug.