public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: Ben Woodard <woodard@redhat.com>
To: libabigail@sourceware.org
Subject: Re: inlining change and abidiff noise
Date: Sun, 01 Jan 2017 00:00:00 -0000	[thread overview]
Message-ID: <F4D8321F-4FEB-4A45-9DE8-8619E93A64F4@redhat.com> (raw)
In-Reply-To: <87mv99fnfh.fsf@seketeli.org>


> On Jun 15, 2017, at 5:25 AM, Dodji Seketeli <dodji@seketeli.org> wrote:
> 
> Hello Ben,
> 
> Ben Woodard <woodard@redhat.com> a écrit:
> 
>> One of the challenges doing inter-compiler comparison between object
>> files created by different compilers is that the signal to noise ratio
>> is very high apparently due to different inlining decisions.
>> 
>> Would there be any negative consequences to having abidiff consider
>> changes which appear to be due to just differences in how the
>> compilers choose to inline functions.
>> 
>> As a case in point:
>> 
>> $ ./tools/abidiff ./tools/.libs/abidiff ../build-llvl/tools/.libs/abidiff
>> Functions changes summary: 4 Removed, 0 Changed, 0 Added functions
>> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>> Function symbols changes summary: 7 Removed, 223 Added function
>> symbols not referenced by debug info
>> Variable symbols changes summary: 0 Removed, 3 Added variable symbols
>> not referenced by debug info
>> 
>> when you look 3 out of 4 of the removed functions and all 233 of the
>> added functions you can quickly see that they are all weak symbols.
> 
> A weak symbol is not necessarily a symbol that results from function
> inlining.
> 
> A user can very well write in her code:
> 
>    int __attribute__((weak)) power2(int x);

in addition to a declaration like that, wouldn’t there also need to be a definition somewhere?

> 
> And the symbol of that function 'power2' is going to be weak.
> 
> 
> In this case, however, abidiff is saying that: 
> 
>> Function symbols changes summary: 7 Removed, 223 Added function symbols not referenced by debug info
> 
> In other words, it is saying that the symbols that got added/removed
> match *no* debug info.  That is to say that no function explicitely
> written by the user generated those ELF symbols.  It follows that those
> symbols result from functions that were generated artificially by the
> compiler.
> 
> If this change was happening between two binaries generated with the
> same compiler (and using the same compiler option), it would certainly
> be a hint of a problem.
> 
What if the compiler version changes and the decision about which functions change?
I would also argue that at least for most compiler option changes like changing -O level it shouldn’t create any ABI artifacts and nor should libabigail generate any false positives if you simply change the optimization level. I would argue that the inter-compiler test is just a more rigorous test of the changes that happen when the optimization level changes.
> 
> And the symbol of that function 'power2' is going to be weak.
> 
> 
> In this case, however, abidiff is saying that: 
> 
>> Function symbols changes summary: 7 Removed, 223 Added function symbols not referenced by debug info
> 
> In other words, it is saying that the symbols that got added/removed
> match *no* debug info.  That is to say that no function explicitely
> written by the user generated those ELF symbols.  It follows that those
> symbols result from functions that were generated artificially by the
> compiler.
> 
> If this change was happening between two binaries generated with the
> same compiler (and using the same compiler option), it would certainly
> be a hint of a problem.
> 
> In this case of binaries generated by different compilers, I guess it's
> different, especially after you've analyzed the changes as you did.
> 
> So in this case, you could have used the --no-unreferenced-symbols
> option of abidiff to tell it to avoid showing you changes on symbols for
> which there is no debug info.
> 

I think that is a bit of a big hammer. I think that in the example you give below…

>> The assumption that I'm making is that adding logic like:
>> if a symbol is added or removed and the only reference to it is with a
>> weak symbol then it is a result of a change in inlining and it is
>> therefore harmless.
> 
> I think that assumption is not correct, unfortunately.  A removed weak
> symbol can result from a using removing the declaration below from the
> code:
> 
>    int __attribute__((weak)) power2(int x);
> 
Would a declaration without a definition generate anything at all? 

Isn’t the unique characteristic in this case that we can filter on the fact that we have weak symbols with no DWARF. Is there another example you can contrive where a CU has DWARF (i.e. it isn’t just stripped ELF or the result of a compilation without -g) but the compiler generates weak symbols that do not have associated DWARF where there is in fact a legitimate ABI change?

> So doing this by default might lead us to miss some real problems.


You’re the expert but I’m not completely convinced yet.

> 
> Maybe we could add a --no-weak-symbols option to abidiff that would
> implement the logic you are talking about?  That way, people who know
> what they are doing, like yourself, would use it.
> 
> Then, we'd write a guide for people analysing ABI changes that are due
> to using different compilers.  That guide would introduce the subtleties
> we are talking about here and mention the abidiff options that need to
> be used, when and why.
> 
> What do you think?
> 
> -- 
> 		Dodji

  reply	other threads:[~2017-06-16 22:03 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-01  0:00 Ben Woodard
2017-01-01  0:00 ` Dodji Seketeli
2017-01-01  0:00   ` Ben Woodard [this message]
     [not found]   ` <78E49230-1B78-43CE-A643-63AC1039F694@redhat.com>
2017-01-01  0:00     ` Dodji Seketeli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F4D8321F-4FEB-4A45-9DE8-8619E93A64F4@redhat.com \
    --to=woodard@redhat.com \
    --cc=libabigail@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).