From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22081 invoked by alias); 24 Mar 2017 16:34:56 -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 21439 invoked by uid 89); 24 Mar 2017 16:34:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.99.2 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_FILL_THIS_FORM_SHORT autolearn=ham version=3.3.2 spammy= X-Spam-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_FILL_THIS_FORM_SHORT autolearn=ham version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-Spam-User: qpsmtpd, 2 recipients X-HELO: ms.seketeli.net From: Dodji Seketeli To: "jwakely.gcc at gmail dot com" Cc: libabigail@sourceware.org Subject: Re: [Bug default/21296] New: abidiff reports possibly bogus differences and crashes Organization: Me, myself and I References: X-Operating-System: Fedora 26 X-URL: http://www.seketeli.net/~dodji Date: Sun, 01 Jan 2017 00:00:00 -0000 In-Reply-To: (jwakely gcc at gmail dot com's message of "Thu, 23 Mar 2017 19:37:58 +0000") Message-ID: <87wpbeprxu.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2017-q1/txt/msg00057.txt.bz2 I have applied the patch https://sourceware.org/git/gitweb.cgi?p=libabigail.git;a=commit;h=0c820488d4a161f1bd0d1fc3b70bb2518130512b to the master branch to fix the abort. Below are my comments for the other issues. > the second changed function seems to only be different with regards to > whitespace. We think that whitespace differences should be > ignored. Can you make the evaluative function ignore the change in the > whitespace. [...] > [C]'function std::tuple my_forward_as_tuple(STR&&)' at > clanggcc.cxx:225:1 has some indirect sub-type changes: > return type changed: > type name changed from 'std::tuple' to 'std::tuple' It's true that here, the name of the type changed from 'tuple' to 'tuple', due to the white space addition. Today, the type comparison engine considers two types with different names as being different. Ideally, clang and gcc should emit type names that are normalized somehow, so that we can compare them. I guess libabigail could indeed normalize the type names, but thas is more involved than just stripping white spaces. For instance: > > 1 base class deletion: > struct std::_Tuple_impl<0ul, STR&&> at clanggcc.cxx:119:1 > 1 base class insertion: > struct std::_Tuple_impl<0, STR &&> at clanggcc.cxx:119:1 Here, in the type name "_Tuple_impl<0ul, STR&&>", the '0ul' constant literal changed to become the constant literal '0'. Both are equivalent. But recognizing this is different from the whitespace case above. I think we can (and should) do better than what we are doing. That is, we should try harder to normalize type names before comparing them. I guess that improvement could even use its own tracking bug. [...] > 3) libabigail seems to miss the true nature of the ABI change which is that one > of the parameters is passed on the stack while the other is passed on a > register. We believe that GCC implements the correct calling convention > according to the Itanium C++ ABI. To date, Libabigail doesn't look at the location of function parameters. So changes in argument passing conventions are not detected at the moment. I think we could add that feature, but that would be a project in its own. Actually, there is an opened enhancement request for this https://sourceware.org/bugzilla/show_bug.cgi?id=19949. > See: https://bugs.llvm.org//show_bug.cgi?id=23034 I haven't looked at the DWARF > yet but jwakely seemed to say that he wasn't surprised that libabigail missed > detecting this ABI change because the DWARF was insufficient. The reason why libabigail missed it is that it's not yet trying to see it :-) But Jonathan is right that just looking at the DWARF as it is today will not necessarily be enough. For instance, let's look at the DWARF of the function "my_forward_as_tuple" by generated libclang.so. I think it's that one you are talking about. It says: [ 498] subprogram low_pc (addr) +0x00000000000010b0 <_Z19my_forward_as_tupleIJ3STREESt5tupleIJDpOT_EES4_> high_pc (data4) 57 (+0x00000000000010e9) frame_base (exprloc) [ 0] reg6 linkage_name (strp) "_Z19my_forward_as_tupleIJ3STREESt5tupleIJDpOT_EES4_" name (strp) "my_forward_as_tuple" [...] [ 4b5] formal_parameter location (exprloc) [ 0] fbreg -16 name (strp) "__args" If I read that correctly, it says that the parameter of the my_forward_as_tuple function is to be found 16 bytes after the "Frame Base" pointer. That's the meaning of the "fbreg -16" value of the DW_AT_location attribute of the formal parameter. And the frame base register is the register number "6", if I believe the value of the DW_AT_frame attribute which is "reg6". The mapping of DWARF register for the x86_64 abi says that register 6 is %rbp. I think that means that clang++ is saying that the argument of the formal_parameter is being passed on the stack. I haven't yet looked at what the assembly code is doing. Looking at the DWARF emitted by GCC, it's saying something similar. The argument is being passed on the stack. I'll look at what we have when libclang.so is compiled with optimizations, and I'll look at the assembly too, before we jump to conclusions. -- Dodji