[PATCH 0/3] Add an option to give finer-grained control of offset reporting.

[Matthias Maennich]

On Thu, May 14, 2020 at 01:39:48PM +0100, Giuliano Procida wrote:
>I'll just chime in...
>
>I concur that the initial change I proposed is dangerous in the sense that
>it allows abidiff to be used in a fashion that will not find certain ABI
>differences. Even if the mechanism makes sense, perhaps the policy does
>not. However, it's already possible to say --harmless --no-harmful.
>
>The splitting of SIZE/OFFSET categories may have some value for libabigail
>itself. I think SIZE would still be overloaded, including alignment and
>some other miscellaneous things.
>
>As for a concrete proposal, if the two of you feel you have reached a
>consensus, could one of you paste it into the issue tracker?
>
>Now, taking an even bigger step back, the source of this request is to
>further compress reports for human consumption. This means removing
>redundant information but it also means finding smaller diff descriptions
>(while still being legible and having meaningful context). We're
>considering ways of presenting different levels of details to our users,
>you might consider these abidiff options as making a hierarchy:
>
>--redundant
>--no-redundant
>--leaf-changes-only --impacted-interfaces
>--leaf-changes-only --impacted-interface-counts (this doesn't exist)
>--leaf-changes-only
>--leaf-changes-only --summarise-offset-changes (this doesn't exist)
>--leaf-changes-only --offset-changes-are-harmless (was this request)
>
>At the lowest level here, we'd only see root causes of ABI differences -
>useful for people who know there's some difference and just want to track
>it down.
>At the top level we get the full impact, with the change graph expanded
>into a tree as far as possible.
>
>It's quite expensive (in terms of run-time) to generate multiple reports
>(Matthias, we should fork and wait if we need to do this) with different
>variations. It's also (as the present example demonstrates) not ideal to
>add every variation to abidiff itself. So we are doing some things with
>post-processing. It might be straightforward to do this for offsets, for
>example. If there were an XML format for the diffs as well... :-)

I agree that we can solve the problem either in abigail and use
downstream more CPU to generate multiple reports at the same time or do
some postprocessing of the most verbose report. I like the idea of an
intermediate (XML?) format for the diff, actually. That would allow all
sorts of reports and different flags without touching libabigail's core
diffing.

Yet, my main motivation here to chime in was to point out that we should
improve the default for lossless compression and for human consumption.
For more detail, we can add verbosity flags. Maybe even like you
suggested above. But the first target group should be developers that
need to fix an actual breakage.

Cheers,
Matthias

>
>I hope this helps fill in a bit more of the background.
>
>Regards,
>Giuliano.
>
>On Thu, 14 May 2020 at 09:35, Dodji Seketeli <dodji@seketeli.org> wrote:
>
>> Matthias Maennich <maennich@google.com> a écrit:
>>
>> [...]
>>
>> > I think I would like the one line of mention as I suggested. Omitting it
>> > conditionally introduces more logic and likely we confuse users.
>>
>> Either way, we'll have to introdroce more logic anyway, so I am not
>> concerned about that, if, of course that is useful.
>>
>> I guess what I wanted to say is that the one line mention seems like a
>> good idea because that example is simple.  If you have more than one
>> non-contiguous addition/removal/changes of data members in the struct,
>> then I am not sure the one liner mention is still that "relevant".  Or
>> maybe in that case we can avoir the one liner mention and emit the
>> in-extenso message as we have today?  What do you think?
>>
>> > I also do not see them as redundant, just obvious and therefore a
>> > short form is enough to transmit this information.
>>
>> Hmmh, yes, I think you are right.  The offset changes are not redundant
>> per se.  They just happen to be caused by the change, but they don't
>> necessary have to be.  Point taken.
>>
>> [...]
>>
>>
>> >>Also, would it make sense to emit those qualified data member names when
>> >>we are analyzying a C++ program?  Or do you think it's more legible to
>> >>emit always emit non-qualified data member names?
>> >
>> > I think this very much applies to C structs as well. In fact, the above
>> > is a C example.
>>
>> Yeah, I got that.  I really meant my question.  I'll re-phrase it
>> differently.  Do you think we should avoid removing the
>> full-qualification when we are analyzing c++ programs?  Or do do you
>> think what you are proposing for C should apply for C++ programs
>> equally?  I would think yes, but I am not sure.
>>
>> > I think we should use the non-qualified version if we
>> > are in a nested block where the rest of the qualified name is already
>> > emitted. In the above example, we are in the block of 'task_struct'
>> > differences, hence we can strip that part away from
>> > task_struct::in_ubsan'.
>>
>> This does make sense to me.
>>
>> If we agree that we should change this behaviour, then maybe we should
>> create an enhance request for this.  Shall we?
>>
>> Thanks,
>>
>> Cheers,
>>
>> --
>>                 Dodji
>>