[Bug default/26013] New: abidiff: collapse subsequent data member offset changes

[Bug default/26013] New: abidiff: collapse subsequent data member offset changes

[maennich at android dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26013

            Bug ID: 26013
           Summary: abidiff: collapse subsequent data member offset
                    changes
           Product: libabigail
           Version: unspecified
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P2
         Component: default
          Assignee: dodji at redhat dot com
          Reporter: maennich at android dot com
                CC: libabigail at sourceware dot org
  Target Milestone: ---

The main motivation for this was to reduce the verbosity of subsequent offset
changes in ABI reports.
Often they are obvious and related to a data member insertion or deletion.
Therefore they might be
boring, but not harmless.

I suggest to collapse subsequent offset changes. Instead of dropping the
reports
of those completely, how about we add an option that changes a report from

'struct task_struct at sched.h:635:1' changed:
  type size hasn't changed
  1 data member insertion:
    'unsigned int task_struct::in_ubsan', at offset 16704 (in bits) at
sched.h:1006:1
  there are data member changes:
    'void* task_struct::journal_info' offset changed from 16704 to 16768 (in
bits) (by +64 bits)
    'bio_list* task_struct::bio_list' offset changed from 16768 to 16832 (in
bits) (by +64 bits)
    'blk_plug* task_struct::plug' offset changed from 16832 to 16896 (in bits)
(by +64 bits)
    'reclaim_state* task_struct::reclaim_state' offset changed from 16896 to
16960 (in bits) (by +64 bits)
    'backing_dev_info* task_struct::backing_dev_info' offset changed from 16960
to 17024 (in bits) (by +64 bits)
    'io_context* task_struct::io_context' offset changed from 17024 to 17088
(in bits) (by +64 bits)
    'capture_control* task_struct::capture_control' offset changed from 17088
to 17152 (in bits) (by +64 bits)
    'unsigned long int task_struct::ptrace_message' offset changed from 17152
to 17216 (in bits) (by +64 bits)
    'kernel_siginfo_t* task_struct::last_siginfo' offset changed from 17216 to
17280 (in bits) (by +64 bits)

to

'struct task_struct at sched.h:635:1' changed:
  type size hasn't changed
  1 data member insertion:
    'unsigned int task_struct::in_ubsan', at offset 16704 (in bits) at
sched.h:1006:1

  offset of 9 consecutive data members changed by +64bits (journal_info ..
last_siginfo)


I could even imagine this to be the default.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug default/26013] abidiff: collapse subsequent data member offset changes

[gprocida+abigail at google dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=26013

Giuliano Procida <gprocida+abigail at google dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gprocida+abigail at google dot com

--- Comment #1 from Giuliano Procida <gprocida+abigail at google dot com> ---
Hi Dodji.

This is to give you an update where we are with this. I decided that opening up
and operating on abidiff to achieve the stated aim would be quite (and too)
intrusive, particularly given the relative ease with which this can be done in
post-processing.

We've implemented a post-processor which generates output like the following.

    ...
    8 ('foo' .. 'bar') offsets changed (by +256 bits)
    ...

Only consecutive runs where the members have identical offset changes - and
only offset changes - are summarised like this.

In abidiff, the trickiest bit would be determining that there were no other
changes. This can only be done with a holistic view of either the diff output
(which might mean pushing functionality into the ostream object) or perhaps
member diffs (but that would require adding a lot of functionality and probably
splitting the change categories SIZE and OFFSET as I proposed long ago).

The idea to do something with ostream using for reporting is not terrible. For
example, much of the indentation logic could be centralised there; multiple
post-processing filters could be added.

But... we actually find it useful to have the full unexpurgated report and use
post-processing to yield a shorter report that users look at by default. We
only have to run abidiff once.

That was a bit of a longer update than I intended. The post-processing code is
here, if you're interested. It includes a couple of other filters.

https://android.googlesource.com/kernel/build/+/refs/heads/master/abi/abitool.py

Regards.

-- 
You are receiving this mail because:
You are on the CC list for the bug.