* [PATCH 1/2] abidiff: More compact references to prior diffs.
@ 2020-04-03 21:53 Giuliano Procida
2020-04-03 21:53 ` [PATCH 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Giuliano Procida @ 2020-04-03 21:53 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida
In both the default and leaf reporting modes, when there is a repeated
or circular reference to a type difference of a member variable, some
placeholder text is emitted instead.
In the leaf reporter:
type 'struct S' of 'foo::bar' changed as reported earlier
In the default reporter, this spans two lines:
type of 'S foo::bar' changed:
details were reported earlier
This patch changes the latter to the more compact:
type of 'S foo::bar' changed, as reported earlier
More generally, this patch makes the punctuation of such placeholder
text more consistent with a comma separating the phrases in all cases.
It doesn't attempt to reconcile the different formatting of member
variable declarations between the two modes.
* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
overload, use consistent punctuation and keep to a single line
of output when referring back to an existing type diff report.
* src/abg-reporter-priv.h: In the macro
RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER, use
consistent punctuation when referring back to an existing type
diff report.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust
formatting of back references to existing type diff reports.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Ditto.
* tests/data/test-diff-filter/test16-report-2.txt: Ditto.
* tests/data/test-diff-filter/test17-1-report.txt: Ditto.
* tests/data/test-diff-filter/test25-cyclic-type-report-1.txt:
Ditto.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Ditto.
* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Ditto.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-reporter-priv.cc | 19 ++++++++-----
src/abg-reporter-priv.h | 8 +++---
.../test-abidiff/test-PR18791-report0.txt | 3 +--
.../PR25058-liblttng-ctl-report-1.txt | 22 +++++++--------
.../data/test-diff-filter/test16-report-2.txt | 2 +-
.../data/test-diff-filter/test17-1-report.txt | 2 +-
.../test25-cyclic-type-report-1.txt | 2 +-
...l7.x86_64-0.12.8-1.el7.x86_64-report-2.txt | 27 +++++++++----------
.../test-diff-suppr/test36-leaf-report-0.txt | 2 +-
9 files changed, 45 insertions(+), 42 deletions(-)
diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index 883c815e..323de503 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -507,11 +507,11 @@ represent(const var_diff_sptr &diff,
if (d->currently_reporting())
{
- out << " as being reported\n";
+ out << ", as being reported\n";
}
else if (d->reported_once())
{
- out << " as reported earlier\n";
+ out << ", as reported earlier\n";
}
else
{
@@ -527,13 +527,20 @@ represent(const var_diff_sptr &diff,
if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
{
out << indent
- << "type of '" << pretty_representation << "' changed:\n";
+ << "type of '" << pretty_representation << "' changed";
if (d->currently_reporting())
- out << indent << " details are being reported\n";
+ {
+ out << ", as being reported\n";
+ }
else if (d->reported_once())
- out << indent << " details were reported earlier\n";
+ {
+ out << ", as reported earlier\n";
+ }
else
- d->report(out, indent + " ");
+ {
+ out << ":\n";
+ d->report(out, indent + " ");
+ }
begin_with_and = true;
emitted = true;
diff --git a/src/abg-reporter-priv.h b/src/abg-reporter-priv.h
index b6c56fdb..b221fdc3 100644
--- a/src/abg-reporter-priv.h
+++ b/src/abg-reporter-priv.h
@@ -68,14 +68,14 @@
{ \
string _name_ = _diff_->first_subject()->get_pretty_representation(); \
if (_diff_->currently_reporting()) \
- out << indent << INTRO_TEXT << " '" << _name_ << "' changed; " \
- "details are being reported\n"; \
+ out << indent << INTRO_TEXT << " '" << _name_ \
+ << "' changed, as being reported\n"; \
else \
{ \
out << indent << INTRO_TEXT << " '" \
- << _name_ << "' changed"; \
+ << _name_ << "' changed"; \
report_loc_info(D->first_subject(), *d.context(), out); \
- out << ", as reported earlier\n"; \
+ out << ", as reported earlier\n"; \
} \
return ; \
} \
diff --git a/tests/data/test-abidiff/test-PR18791-report0.txt b/tests/data/test-abidiff/test-PR18791-report0.txt
index 93faf599..5bc2d08b 100644
--- a/tests/data/test-abidiff/test-PR18791-report0.txt
+++ b/tests/data/test-abidiff/test-PR18791-report0.txt
@@ -123,8 +123,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
type size changed from 128 to 192 (in bits)
1 data member change:
- type of 'std::__detail::_List_node_base std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed:
- details were reported earlier
+ type of 'std::__detail::_List_node_base std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed, as reported earlier
and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
diff --git a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
index dfb63684..44ddc6d6 100644
--- a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
+++ b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
@@ -179,8 +179,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
in pointed to type 'struct lttng_event_field':
type size hasn't changed
1 data member change:
- type of 'lttng_event lttng_event_field::event' changed:
- details were reported earlier
+ type of 'lttng_event lttng_event_field::event' changed, as reported earlier
[C] 'function int lttng_list_tracepoints(lttng_handle*, lttng_event**)' has some indirect sub-type changes:
parameter 2 of type 'lttng_event**' has sub-type changes:
@@ -232,7 +231,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
type size changed from 576 to 640 (in bits)
2 data member changes:
type of 'filter_node* filter_node::parent' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'union {struct {} unknown; struct {filter_node* child;} root; struct {__anonymous_enum__ type; ast_link_type post_op; ast_link_type pre_op; union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;} u; filter_node* prev; filter_node* next;} expression; struct {op_type type; filter_node* lchild; filter_node* rchild;} op; struct {unary_op_type type; filter_node* child;} unary_op;} filter_node::u' changed:
type size changed from 320 to 384 (in bits)
4 data member changes:
@@ -245,21 +244,20 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
type size hasn't changed
1 enumerator insertion:
'ast_link_type::AST_LINK_BRACKET' value '3'
- type of 'ast_link_type pre_op' changed:
- details were reported earlier
+ type of 'ast_link_type pre_op' changed, as reported earlier
type of 'union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;} u' changed:
type size hasn't changed
1 data member change:
type of 'filter_node* child' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type changed from:
union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;}
to:
union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;}
type of 'filter_node* prev' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'filter_node* next' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'struct {op_type type; filter_node* lchild; filter_node* rchild;} op' changed:
type size hasn't changed
3 data member changes:
@@ -278,14 +276,14 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
'op_type::AST_OP_BIT_OR' value '11'
'op_type::AST_OP_BIT_XOR' value '12'
type of 'filter_node* lchild' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'filter_node* rchild' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'struct {filter_node* child;} root' changed:
type size hasn't changed
1 data member change:
type of 'filter_node* child' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'struct {unary_op_type type; filter_node* child;} unary_op' changed:
type size hasn't changed
2 data member changes:
@@ -296,7 +294,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
1 enumerator insertion:
'unary_op_type::AST_UNARY_BIT_NOT' value '4'
type of 'filter_node* child' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
'cds_list_head filter_ast::allocated_nodes' offset changed from 576 to 640 (in bits) (by +64 bits)
[C] 'function YYSTYPE* lttng_yyget_lval(yyscan_t)' has some indirect sub-type changes:
diff --git a/tests/data/test-diff-filter/test16-report-2.txt b/tests/data/test-diff-filter/test16-report-2.txt
index a90e85ad..f69eabdc 100644
--- a/tests/data/test-diff-filter/test16-report-2.txt
+++ b/tests/data/test-diff-filter/test16-report-2.txt
@@ -11,6 +11,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
'int S::m0', at offset 0 (in bits)
1 data member change:
type of 'S* S::m2' changed:
- pointed to type 'struct S' changed; details are being reported
+ pointed to type 'struct S' changed, as being reported
and offset changed from 0 to 64 (in bits) (by +64 bits)
diff --git a/tests/data/test-diff-filter/test17-1-report.txt b/tests/data/test-diff-filter/test17-1-report.txt
index 7c51152a..0b39c5d4 100644
--- a/tests/data/test-diff-filter/test17-1-report.txt
+++ b/tests/data/test-diff-filter/test17-1-report.txt
@@ -11,7 +11,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
'int S::m0', at offset 0 (in bits)
1 data member change:
type of 'S* S::m2' changed:
- pointed to type 'struct S' changed; details are being reported
+ pointed to type 'struct S' changed, as being reported
and offset changed from 0 to 64 (in bits) (by +64 bits)
[C] 'function void foo(S&)' has some indirect sub-type changes:
diff --git a/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt b/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
index 6a6beeef..0215d892 100644
--- a/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
+++ b/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
@@ -11,5 +11,5 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
'char S::m1', at offset 32 (in bits)
1 data member change:
type of 'S* S::m2' changed:
- pointed to type 'struct S' changed; details are being reported
+ pointed to type 'struct S' changed, as being reported
diff --git a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
index 6d9beb85..58c94b7a 100644
--- a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
+++ b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
@@ -37,7 +37,7 @@
13 data member changes:
type of 'QXLInstance* RedDispatcher::qxl' changed:
in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1:
- underlying type 'struct QXLInstance' changed; details are being reported
+ underlying type 'struct QXLInstance' changed, as being reported
type of 'Dispatcher RedDispatcher::dispatcher' changed:
underlying type 'struct Dispatcher' at dispatcher.h:22:1 changed:
type size changed from 960 to 1024 (in bits)
@@ -51,7 +51,7 @@
'int RedDispatcher::use_hardware_cursor' offset changed from 2240 to 2304 (in bits) (by +64 bits)
type of 'RedDispatcher* RedDispatcher::next' changed:
in pointed to type 'typedef RedDispatcher' at red_worker.h:87:1:
- underlying type 'struct RedDispatcher' changed; details are being reported
+ underlying type 'struct RedDispatcher' changed, as being reported
and offset changed from 2304 to 2368 (in bits) (by +64 bits)
'Ring RedDispatcher::async_commands' offset changed from 2368 to 2432 (in bits) (by +64 bits)
'pthread_mutex_t RedDispatcher::async_lock' offset changed from 2496 to 2560 (in bits) (by +64 bits)
@@ -185,7 +185,7 @@
1 data member change:
type of 'SpiceCharDeviceState* SpiceCharDeviceInstance::st' changed:
in pointed to type 'typedef SpiceCharDeviceState' at spice-char.h:34:1:
- underlying type 'struct SpiceCharDeviceState' changed; details are being reported
+ underlying type 'struct SpiceCharDeviceState' changed, as being reported
and offset changed from 896 to 960 (in bits) (by +64 bits)
'int SpiceCharDeviceState::during_read_from_device' offset changed from 960 to 1024 (in bits) (by +64 bits)
'int SpiceCharDeviceState::during_write_to_device' offset changed from 992 to 1056 (in bits) (by +64 bits)
@@ -223,7 +223,7 @@
2 data member changes (2 filtered):
type of 'RedChannel* RedChannelClient::channel' changed:
in pointed to type 'typedef RedChannel' at red_channel.h:130:1:
- underlying type 'struct RedChannel' changed; details are being reported
+ underlying type 'struct RedChannel' changed, as being reported
type of 'RedsStream* RedChannelClient::stream' changed:
in pointed to type 'typedef RedsStream' at reds_stream.h:31:1:
underlying type 'struct RedsStream' at reds.h:68:1 changed:
@@ -391,7 +391,7 @@
in pointed to type 'function type void (RedChannel*, RedClient*, RedsStream*, int, int, uint32_t*, int, uint32_t*)':
parameter 1 of type 'RedChannel*' has sub-type changes:
in pointed to type 'typedef RedChannel' at red_channel.h:130:1:
- underlying type 'struct RedChannel' changed; details are being reported
+ underlying type 'struct RedChannel' changed, as being reported
parameter 3 of type 'RedsStream*' has sub-type changes:
pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
type of 'channel_client_disconnect_proc disconnect' changed:
@@ -524,7 +524,7 @@
pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
type of 'SndWorker* SndChannel::worker' changed:
in pointed to type 'typedef SndWorker' at snd_worker.c:80:1:
- underlying type 'struct SndWorker' changed; details are being reported
+ underlying type 'struct SndWorker' changed, as being reported
type of 'RedChannelClient* SndChannel::channel_client' changed:
pointed to type 'typedef RedChannelClient' changed at red_channel.h:136:1, as reported earlier
type of 'snd_channel_handle_message_proc SndChannel::handle_message' changed:
@@ -532,26 +532,26 @@
in pointed to type 'function type int (SndChannel*, typedef size_t, typedef uint32_t, void*)':
parameter 1 of type 'SndChannel*' has sub-type changes:
in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
- underlying type 'struct SndChannel' changed; details are being reported
+ underlying type 'struct SndChannel' changed, as being reported
type of 'snd_channel_on_message_done_proc SndChannel::on_message_done' changed:
underlying type 'void (SndChannel*)*' changed:
in pointed to type 'function type void (SndChannel*)':
parameter 1 of type 'SndChannel*' has sub-type changes:
in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
- underlying type 'struct SndChannel' changed; details are being reported
+ underlying type 'struct SndChannel' changed, as being reported
type of 'snd_channel_cleanup_channel_proc SndChannel::cleanup' changed:
underlying type 'void (SndChannel*)*' changed:
in pointed to type 'function type void (SndChannel*)':
parameter 1 of type 'SndChannel*' has sub-type changes:
in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
- underlying type 'struct SndChannel' changed; details are being reported
+ underlying type 'struct SndChannel' changed, as being reported
no data member change (1 filtered);
type of 'SndWorker* SndWorker::next' changed:
in pointed to type 'typedef SndWorker' at snd_worker.c:80:1:
- underlying type 'struct SndWorker' changed; details are being reported
+ underlying type 'struct SndWorker' changed, as being reported
type of 'SpicePlaybackInstance* SpicePlaybackState::sin' changed:
in pointed to type 'typedef SpicePlaybackInstance' at spice-audio.h:33:1:
- underlying type 'struct SpicePlaybackInstance' changed; details are being reported
+ underlying type 'struct SpicePlaybackInstance' changed, as being reported
[C] 'function void spice_server_playback_put_samples(SpicePlaybackInstance*, uint32_t*)' at snd_worker.c:1100:1 has some indirect sub-type changes:
parameter 1 of type 'SpicePlaybackInstance*' has sub-type changes:
@@ -590,11 +590,10 @@
1 data member insertion:
'uint32_t SpiceRecordState::frequency', at offset 512 (in bits) at snd_worker.c:166:1
2 data member changes:
- type of 'SndWorker SpiceRecordState::worker' changed:
- details were reported earlier
+ type of 'SndWorker SpiceRecordState::worker' changed, as reported earlier
type of 'SpiceRecordInstance* SpiceRecordState::sin' changed:
in pointed to type 'typedef SpiceRecordInstance' at spice-audio.h:67:1:
- underlying type 'struct SpiceRecordInstance' changed; details are being reported
+ underlying type 'struct SpiceRecordInstance' changed, as being reported
[C] 'function void spice_server_record_set_mute(SpiceRecordInstance*, uint8_t)' at snd_worker.c:1279:1 has some indirect sub-type changes:
parameter 1 of type 'SpiceRecordInstance*' has sub-type changes:
diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
index d270740d..9caa9428 100644
--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
@@ -15,7 +15,7 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
'struct leaf2 at test36-leaf-v0.cc:9:1' changed:
type size changed from 64 to 96 (in bits)
there are data member changes:
- type 'struct leaf1' of 'leaf2::member0' changed as reported earlier
+ type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
and size changed from 32 to 64 (in bits) (by +32 bits)
'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
3 impacted interfaces:
--
2.26.0.292.g33ef6b2f38-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] abidiff: Fix anonymous member size change reports.
2020-04-03 21:53 [PATCH 1/2] abidiff: More compact references to prior diffs Giuliano Procida
@ 2020-04-03 21:53 ` Giuliano Procida
2020-04-06 16:08 ` Matthias Maennich
2020-04-06 15:29 ` [PATCH 1/2] abidiff: More compact references to prior diffs Matthias Maennich
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-04-03 21:53 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida
Data members have names (unless anonymous), types, sizes and
offsets (if a member of a class or struct). It is the responsibility
of the represent function to detail all differences of a changed
member. The size of a member is the same as the size of its type.
A recent change to the function focused on cleaning up the formatting
logic and added a catch-all case to the end of it in case no diff
information had been emitted by then.
This catch-all triggered for anonymous structs and union diffs under
certain circumstances when these anonymous members had changed in
size.
This patch ensures size change information for a member is emitted
exactly once (unless the type change has been or is being reported
already). It also ensures anonymous members are identified
appropriately and includes a test case would otherwise reach the
catch-all case in both default and --leaf-changes-only modes.
* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
overload, factor out some expressions as local variables, rely
on diff_to_be_reported to decide whether to emit a change,
fold together local/non-local change reporting using
local_changes to preserve current name formatting differences,
keep track explicitly of whether size information has been
emitted and ensure it happens if needed, make offset and size
change reporting for anonymous data members more meaningful.
* tests/test-abidiff-exit.cc: Run new test cases.
* tests/data/Makefile.am: Add new test files.
* tests/data/test-abidiff-exit/test-member-size-v0.cc: New
test.
* tests/data/test-abidiff-exit/test-member-size-v0.o: Ditto.
* tests/data/test-abidiff-exit/test-member-size-v1.cc: Ditto.
* tests/data/test-abidiff-exit/test-member-size-v1.o: Ditto.
* tests/data/test-abidiff-exit/test-member-size-report0.txt:
New test, default mode.
* tests/data/test-abidiff-exit/test-member-size-report1.txt:
New test, --leaf-changes-only mode.
* tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
Eliminate duplicate reporting of member sizes.
* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
Ditto.
* tests/data/test-abidiff-exit/test-leaf1-report.txt: Ditto.
* tests/data/test-abidiff-exit/test-no-stray-comma-report.txt:
Ditto.
* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
Add size report for anonymous data member.
* tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
Ditto.
* tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt:
Add missing size change report.
* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Remove
size change report for previously reported type.
* tests/data/test-diff-suppr/test46-PR25128-report-1.txt:
Eliminate duplicate reporting of member size change.
* tests/data/test-diff-suppr/test46-PR25128-report-2.txt:
Ditto.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-reporter-priv.cc | 98 +++++++-----------
tests/data/Makefile.am | 6 ++
.../test-leaf-cxx-members-report.txt | 4 +-
.../test-leaf-peeling-report.txt | 1 -
.../test-abidiff-exit/test-leaf1-report.txt | 1 -
.../test-member-size-report0.txt | 27 +++++
.../test-member-size-report1.txt | 21 ++++
.../test-abidiff-exit/test-member-size-v0.cc | 26 +++++
.../test-abidiff-exit/test-member-size-v0.o | Bin 0 -> 3184 bytes
.../test-abidiff-exit/test-member-size-v1.cc | 27 +++++
.../test-abidiff-exit/test-member-size-v1.o | Bin 0 -> 3192 bytes
.../test-no-stray-comma-report.txt | 1 -
.../test45-anon-dm-change-report-0.txt | 1 +
.../test44-anonymous-data-member-report-0.txt | 1 +
...-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt | 2 +-
.../test-diff-suppr/test36-leaf-report-0.txt | 1 -
.../test46-PR25128-report-1.txt | 1 -
.../test46-PR25128-report-2.txt | 1 -
tests/test-abidiff-exit.cc | 18 ++++
19 files changed, 167 insertions(+), 70 deletions(-)
create mode 100644 tests/data/test-abidiff-exit/test-member-size-report0.txt
create mode 100644 tests/data/test-abidiff-exit/test-member-size-report1.txt
create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.cc
create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.o
create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.cc
create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.o
diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index 323de503..72181367 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -411,16 +411,26 @@ represent(const var_diff_sptr &diff,
var_decl_sptr o = diff->first_var();
var_decl_sptr n = diff->second_var();
+ string name1 = o->get_qualified_name();
+ string name2 = n->get_qualified_name();
+ uint64_t size1 = get_var_size_in_bits(o);
+ uint64_t size2 = get_var_size_in_bits(n);
+ uint64_t offset1 = get_data_member_offset(o);
+ uint64_t offset2 = get_data_member_offset(n);
// Has the main diff text been output?
bool emitted = false;
// Are we continuing on a new line? (implies emitted)
bool begin_with_and = false;
- string name1 = o->get_qualified_name();
- string name2 = n->get_qualified_name();
- string pretty_representation = o->get_pretty_representation();
+ // Have we reported a size change already?
+ bool size_reported = false;
+ // Are we reporting a change to an anonymous member?
bool is_strict_anonymous_data_member_change = false;
+ string pretty_representation = o->get_pretty_representation();
+ bool show_size_offset_changes = ctxt->get_allowed_category()
+ & SIZE_OR_OFFSET_CHANGE_CATEGORY;
+
if (is_anonymous_data_member(o) && is_anonymous_data_member(n))
{
is_strict_anonymous_data_member_change = true;
@@ -430,8 +440,7 @@ represent(const var_diff_sptr &diff,
if (tr1 != tr2)
{
show_offset_or_size(indent + "anonymous data member at offset",
- get_data_member_offset(o),
- *ctxt, out);
+ offset1, *ctxt, out);
out << " changed from:\n"
<< indent << " " << tr1 << "\n"
@@ -460,7 +469,6 @@ represent(const var_diff_sptr &diff,
begin_with_and = true;
emitted = true;
}
- // TODO: else ...?
}
else if (filtering::has_anonymous_data_member_change(diff))
{
@@ -472,9 +480,7 @@ represent(const var_diff_sptr &diff,
+ o->get_pretty_representation()
+ " at offset";
- show_offset_or_size(indent + repr,
- get_data_member_offset(o),
- *ctxt, out);
+ show_offset_or_size(indent + repr, offset1, *ctxt, out);
out << " became data member '"
<< n->get_pretty_representation()
<< "'\n";
@@ -486,9 +492,7 @@ represent(const var_diff_sptr &diff,
+ o->get_pretty_representation()
+ " at offset";
- show_offset_or_size(indent + repr,
- get_data_member_offset(o),
- *ctxt, out);
+ show_offset_or_size(indent + repr, offset1, *ctxt, out);
out << " became anonymous data member '"
<< n->get_pretty_representation()
<< "'\n";
@@ -499,11 +503,15 @@ represent(const var_diff_sptr &diff,
}
else if (diff_sptr d = diff->type_diff())
{
- if (local_only && d->has_local_changes())
+ if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
{
- out << indent << "type '" << get_pretty_representation(o->get_type())
- << "' of '" << o->get_qualified_name()
- << "' changed";
+ if (local_only)
+ out << indent << "type '" << get_pretty_representation(o->get_type())
+ << "' of '" << o->get_qualified_name()
+ << "' changed";
+ else
+ out << indent
+ << "type of '" << pretty_representation << "' changed";
if (d->currently_reporting())
{
@@ -521,30 +529,7 @@ represent(const var_diff_sptr &diff,
begin_with_and = true;
emitted = true;
- }
- else
- {
- if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
- {
- out << indent
- << "type of '" << pretty_representation << "' changed";
- if (d->currently_reporting())
- {
- out << ", as being reported\n";
- }
- else if (d->reported_once())
- {
- out << ", as reported earlier\n";
- }
- else
- {
- out << ":\n";
- d->report(out, indent + " ");
- }
-
- begin_with_and = true;
- emitted = true;
- }
+ size_reported = true;
}
}
@@ -589,9 +574,9 @@ represent(const var_diff_sptr &diff,
out << "now becomes laid out";
emitted = true;
}
- if ((ctxt->get_allowed_category() & SIZE_OR_OFFSET_CHANGE_CATEGORY))
+ if (show_size_offset_changes)
{
- if (get_data_member_offset(o) != get_data_member_offset(n))
+ if (offset1 != offset2)
{
if (begin_with_and)
{
@@ -600,26 +585,20 @@ represent(const var_diff_sptr &diff,
}
else if (!emitted)
{
+ out << indent;
if (is_strict_anonymous_data_member_change)
- out << indent;
- else
- out << indent << "'" << pretty_representation << "' ";
+ out << "anonymous data member ";
+ out << "'" << pretty_representation << "' ";
}
else
out << ", ";
- show_numerical_change("offset",
- get_data_member_offset(o),
- get_data_member_offset(n),
- *ctxt, out);
+ show_numerical_change("offset", offset1, offset2, *ctxt, out);
maybe_show_relative_offset_change(diff, *ctxt, out);
-
emitted = true;
}
- if (// If we are not displaying only local changes, we must
- // have indicated the type size change already.
- local_only
- && (get_var_size_in_bits(o) != get_var_size_in_bits(n)))
+
+ if (!size_reported && size1 != size2)
{
if (begin_with_and)
{
@@ -628,18 +607,15 @@ represent(const var_diff_sptr &diff,
}
else if (!emitted)
{
+ out << indent;
if (is_strict_anonymous_data_member_change)
- out << indent;
- else
- out << indent << "'" << pretty_representation << "' ";
+ out << "anonymous data member ";
+ out << "'" << pretty_representation << "' ";
}
else
out << ", ";
- show_numerical_change("size",
- get_var_size_in_bits(o),
- get_var_size_in_bits(n),
- *ctxt, out);
+ show_numerical_change("size", size1, size2, *ctxt, out);
maybe_show_relative_size_change(diff, *ctxt, out);
emitted = true;
}
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index bb0fd83b..64e36b1a 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -140,6 +140,12 @@ test-abidiff-exit/test-leaf-cxx-members-v0.o \
test-abidiff-exit/test-leaf-cxx-members-v1.cc \
test-abidiff-exit/test-leaf-cxx-members-v1.o \
test-abidiff-exit/test-leaf-cxx-members-report.txt \
+test-abidiff-exit/test-member-size-v0.cc \
+test-abidiff-exit/test-member-size-v0.o \
+test-abidiff-exit/test-member-size-v1.cc \
+test-abidiff-exit/test-member-size-v1.o \
+test-abidiff-exit/test-member-size-report0.txt \
+test-abidiff-exit/test-member-size-report1.txt \
\
test-diff-dwarf/test0-v0.cc \
test-diff-dwarf/test0-v0.o \
diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
index 082db83e..eae74a3b 100644
--- a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
+++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
@@ -35,8 +35,8 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type 'int' of 'ops::deleted_var' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
- and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1, size changed from 32 to 64 (in bits) (by +32 bits)
+ and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1
type 'int' of 'ops::changed_var' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
- and offset changed from 96 to 128 (in bits) (by +32 bits), size changed from 32 to 64 (in bits) (by +32 bits)
+ and offset changed from 96 to 128 (in bits) (by +32 bits)
diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
index bd5f36f8..7ab4dd6e 100644
--- a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
@@ -9,7 +9,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type 'int' of 'foo::z' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
- and size changed from 32 to 64 (in bits) (by +32 bits)
'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
type size hasn't changed
diff --git a/tests/data/test-abidiff-exit/test-leaf1-report.txt b/tests/data/test-abidiff-exit/test-leaf1-report.txt
index 9af6ccaa..aea08b81 100644
--- a/tests/data/test-abidiff-exit/test-leaf1-report.txt
+++ b/tests/data/test-abidiff-exit/test-leaf1-report.txt
@@ -40,4 +40,3 @@ Removed/Changed/Added variables summary: 1 Removed, 1 Changed, 1 Added variable
type 'int' of 'changed::foo' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
- and size changed from 32 to 64 (in bits) (by +32 bits)
diff --git a/tests/data/test-abidiff-exit/test-member-size-report0.txt b/tests/data/test-abidiff-exit/test-member-size-report0.txt
new file mode 100644
index 00000000..ff93dd50
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-member-size-report0.txt
@@ -0,0 +1,27 @@
+Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+2 functions with some indirect sub-type change:
+
+ [C] 'function void reg1(S*, T*, T*)' at test-member-size-v1.cc:26:1 has some indirect sub-type changes:
+ parameter 1 of type 'S*' has sub-type changes:
+ in pointed to type 'struct S' at test-member-size-v1.cc:3:1:
+ type size changed from 128 to 192 (in bits)
+ 1 data member insertion:
+ 'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
+ no data member change (1 filtered);
+ parameter 2 of type 'T*' has sub-type changes:
+ in pointed to type 'struct T' at test-member-size-v1.cc:14:1:
+ type size changed from 192 to 256 (in bits)
+ 2 data member changes:
+ 'S T::s' size changed from 128 to 192 (in bits) (by +64 bits)
+ 'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
+
+ [C] 'function void reg2(U*)' at test-member-size-v1.cc:27:1 has some indirect sub-type changes:
+ parameter 1 of type 'U*' has sub-type changes:
+ in pointed to type 'struct U' at test-member-size-v1.cc:19:1:
+ type size changed from 192 to 256 (in bits)
+ 2 data member changes:
+ anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
+ 'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
+
diff --git a/tests/data/test-abidiff-exit/test-member-size-report1.txt b/tests/data/test-abidiff-exit/test-member-size-report1.txt
new file mode 100644
index 00000000..dbad76be
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-member-size-report1.txt
@@ -0,0 +1,21 @@
+Leaf changes summary: 3 artifacts changed
+Changed leaf types summary: 3 leaf types changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+'struct S at test-member-size-v0.cc:3:1' changed:
+ type size changed from 128 to 192 (in bits)
+ 1 data member insertion:
+ 'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
+
+'struct T at test-member-size-v0.cc:13:1' changed:
+ type size changed from 192 to 256 (in bits)
+ there are data member changes:
+ type 'struct S' of 'T::s' changed, as reported earlier
+ 'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
+
+'struct U at test-member-size-v0.cc:18:1' changed:
+ type size changed from 192 to 256 (in bits)
+ there are data member changes:
+ anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
+ 'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.cc b/tests/data/test-abidiff-exit/test-member-size-v0.cc
new file mode 100644
index 00000000..f974a37f
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-member-size-v0.cc
@@ -0,0 +1,26 @@
+struct M;
+
+struct S {
+ int x;
+ M * m;
+};
+
+struct M {
+ int a;
+ S * s;
+};
+
+struct T {
+ S s;
+ int a;
+};
+
+struct U {
+ struct {
+ S s;
+ };
+ int r;
+};
+
+void reg1(S*, T*, T*) { }
+void reg2(U*) { }
diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.o b/tests/data/test-abidiff-exit/test-member-size-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..e020bef77d57d601420f969061707bbf1486ac99
GIT binary patch
literal 3184
zcmbtWL2MgU5S{P!#_PCtoVcY4LC91fsLC!`E2`o`ifKxds<Z@BqY4SqwX=yW;&rs%
z6t@VFPzA~@m5>058xj(?a^Q}H#JR_E;{Xyz;Dp2hX8!(hHs8(#M)J=5H}hux|2zM$
zKYaO>ODQ3MMZkGD7%>WPKXYK$q*;R!OhJ8R@7I;R_y1bi+x>lIum0QqTR$*_<}#*A
z%V<j`jqQU>@OZ*btwB*3)~g5^eGuskSzvTPn3+CW7NpZ=JBT246$34cFvA0pRe?Pa
z<Cza+plQ8<+O%wZrmn-Z$Xj1iG=>yR0wr-m5{r<kW5ks85aF?+D2@N9LXT%;C}h5s
zj1a^lieV8T9vvgal2eBG(!jN5>5;nE!bwo&m@spQP#O>}5`kupge(nG_iieE0~6e(
zKrtzvDwJ|YVYV<SrZUBo7*)dU%H_`Gp3hZt0@E3JFsyiZxwL>2N<Rs?-1(e=<;@8+
zFJsVHNhCR$JjAh<BVL>_3Nby9MQYQZU6`T7Srda&(>RT1X_hpfcuCTtNAg7uKr~*%
zp8>RwLi}CG<5M;pu2Deb_aeLFcN%_Z_u6-S`_`P(Y)ZCy60&g_;!g{6=1+5C(VYA!
zWnhXUi0g08hyGS&)vW_+mmU>(F6!(+>~UA!wN>}}T4f!Uuhq*-XU@3u<*HM0+;U~k
zt-9`9wQP5y-N0Ym@&i9?H*x5N&DLUn;jDdjzHD!m?M<(DJ7~7TZqVKB*}K8J?O?->
zyvDZQ16W>KI#+(?qTgtHL7drktEaUZa)HO?6L;ax>md3?F?%dy?c=UIhB*C5aN_}9
zDil9XE#=3qVE_)v_X6Q4vg6o)DfY*n;U}iXllaF!!ybA+<Q*16y=mQJAY+sm>a6QN
z0}l25z->U{QW>B$pKM}uU>fm?4*+EukRQSbFraV*eryP*-kwVAgWP+>PAi<pe9Dh%
z-l0xQG$j?~ofuEpg!7CDr`2B|5M?q}z_=R12@R)nNiq{Fxst&FLX6fyLV4t6Bu4;0
zDb4wxR^y*F9IqQGzbH)|L-~1M##IY(o>MArQo~(^pHMjGQShPtF&RW%RJI<^XF<oy
zDxT(;+=rTm^Zdc--R|HqMjeH*Zm|{l{Ro^5FY>@?^m^cg{<i0A_>JAI^>(n?MW3|2
zMkDlZ9ddi27i{6pkfdz41D`2a0-R>I)A8}U<^<iyckpVmdy&_CN0I#2`ex{Ld^Pd^
zsb-N+d&Pao>1xW*?{SJv$`d#sOuqr%P6j1xolpHm{r6w_)IWMoD#!J&XijY?zhCq(
zq(-EK?5JidD+2W!#qn98`+;N;e|k>Bzfs^PG)RaZyNW&?qwqP^q;m-_nR@IM#7513
zU%gP~)BNdn)#tx4A}2y{Q`N82AR&7Fb@Yv@|Glcu`$YBW9@FdZAVxWPo@h$>PSyXM
z0j0S<>-P{J#lNZ=itjH@!#vh^6`%dABfTM&gLxeH1sa5@=`sv`#lJB?VuRPu@TX{u
z+P{Y*-XDFvn9ut<sy5|*pz62Ogz39MUqziy?_|_c7gYbxqdzJ7`YG=yzD3Q3@>Q$`
HPo4iS_W$I+
literal 0
HcmV?d00001
diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.cc b/tests/data/test-abidiff-exit/test-member-size-v1.cc
new file mode 100644
index 00000000..b3bd5988
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-member-size-v1.cc
@@ -0,0 +1,27 @@
+struct M;
+
+struct S {
+ int x;
+ M * m;
+ int y;
+};
+
+struct M {
+ int a;
+ S * s;
+};
+
+struct T {
+ S s;
+ int a;
+};
+
+struct U {
+ struct {
+ S s;
+ };
+ int r;
+};
+
+void reg1(S*, T*, T*) { }
+void reg2(U*) { }
diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.o b/tests/data/test-abidiff-exit/test-member-size-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..eaec9ebead662bc23c00130c537133a5be5c2019
GIT binary patch
literal 3192
zcmbtW&u<%55T57F#_Me3IB}ySq(VjlK~;9iMp4zYq?iWUR6q$$jiO2r*3QPZ8gHug
zx;RCs2oT68S_z3FapNyQ;?M&JBraUJaYO<nBo6#IA#s42w{Psti*tdIyfg2c`DWgG
zJMXRUJpbawj1a&g;5-aQi~>B!4(zft%TR<VsI2vVUhBR6*IKXh`&zH^%iY($VF=Bu
zm}*2uTQX^44>G~SDLb<Pv%;{hBV=?zjAY3IqXojuzK51&?1LDUXaPn>%w`xv<|QbI
zoJDTK2Qj8NZ4h?0M;tIMAGK*Y_{>~`lE_<MP&9^2m;_2<K@w*nQ^AO$^&`Tgv*P&J
zcPex`D?=gsm1IO9W)#CBK+KPlVzHGWKFi~>bM(mEYv3fPazc3M0HN3?yi5d|ITCU-
z$lQBF=}k=V5e14#@mRiS8~OSCq?pPUW-zLVJ7?Qx?Q{0BEij!i4u+KsFBi|?ghn0(
z+dgj_Sl*mK^D+jF7Dbwq9fu^=a>X-qMn0j3vPgOMlM8c{IA>x|W)`RMB+ZiMlPpPE
z^hm$cK8VIE_%nd^QHa0u<M@<)hieoN2kqEt1+8ijIql}{z`5nQwVGs`GmuNl5PwdX
zbAOr>E9T_883R)sL5zaB2V2*dP%o}~6(9{y#jTqP^wzzNb?@57;uc)GQYo#TI^`{u
zmfc0yD=jW~%bvHeTyk1*Ck$5VK^R2M8V>!a)>!GDIqjTYDmnF%v*Wk-!&)QS3->y0
zrxU)}47Z)wukHqIfJ>{ZXG>2!7gU>mm}GXm<&#<sfa7wFxCOVbg6I~+{IRTc7q{dD
z;`AfIT?cqJU-%%iIzD;@18_*brwK=q?Z*C#i9hiieqw4ok$?Pi*h4Reyobe5uUYpP
z$QUMuI`g{EfCEk6cN>tpT>2=@Czl!xOe0zG08o|z<A*Q;49FjWA3KCoV^5{_LGC?b
zCl$_PKH^7pZqj?HHcK@l73G~6OWB0;j0mUIUmy@=GEu;|8p5=O)1f4pOO#wm|MVb6
z>mZ>#@+OiafS;7+{Ew^guNuxh`J2M2V<<oG%d~1C&VzT8loJ~6DZHd`&ZFQ1`(rYQ
zx}a=5p3j1gmsC8>F})9E4d?lT+um>C8AcsPiEgkJ2i+LlZ9n$Gt+w0XM!~M{ZU@y)
zeXAMn?4eKEezh6}w+^`d$PepyEu<;C%`jjJmH@Z5*J=g$QFFt+IB@Y^a@w(9yP-%y
zV{0e!TY;MR|5WqHr@i7n<aA|a=$AOfrsWYF5T@S%ucw1jw$7*iqW=4@eCi)PCza#+
zuWC+hC_i5GZ=_75lyp_IabHruQ5>HYx*teZ@Tcb_{7VIXNQ0E<vFqs5F$zDYnsgz>
zB~y=m0kL88zpq{>^J)HcyXy1b9+DFwxS{G-Xpj=U{ucU%)&Ex2=Y693bdTxvZz4uH
zd7fxW`C8S#&w$cgpY>aa594pDhT{8+(=d<q`-;zg)_3%VR1W5G+^1*|rl!j<yrcM=
zeIzk>{S1GM#<2bSVaWTVuNU)qUx(GE+~2GE4K-o<ZqQdz=hHhGw$!rf|0VRNMPEPV
R9mX$Ev!Q&RsQy#u{|i5f=e7U<
literal 0
HcmV?d00001
diff --git a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
index 1583a40b..52c48b3d 100644
--- a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
+++ b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
@@ -10,4 +10,3 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type name changed from 'int[1]' to 'int[2]'
array type size changed from 32 to 64
array type subrange 1 changed length from 1 to 2
- and size changed from 32 to 64 (in bits) (by +32 bits)
diff --git a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
index 12af287b..2f8f31d0 100644
--- a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
+++ b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
@@ -13,6 +13,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
'char S1::m1' offset changed from 64 to 40 (in bits) (by -24 bits)
1 data member change:
anonymous data member struct {int m0; char m01;} at offset 0 (in bits) became data member 'int S1::m0'
+ and size changed from 64 to 32 (in bits) (by -32 bits)
[C] 'function void foo(S0&)' has some indirect sub-type changes:
parameter 1 of type 'S0&' has sub-type changes:
diff --git a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
index b8bb951c..ed68b550 100644
--- a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
+++ b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
@@ -12,5 +12,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
struct {int b; float c;}
to:
struct {int b; float c; char e;}
+ and size changed from 64 to 96 (in bits) (by +32 bits)
'int S2::d' offset changed from 96 to 128 (in bits) (by +32 bits)
diff --git a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
index b1611303..2a679d60 100644
--- a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
+++ b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
@@ -78,7 +78,7 @@
type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
type size changed from 128 to 192 (in bits)
1 data member change:
- name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
+ name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node', size changed from 128 to 192 (in bits) (by +64 bits)
and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
================ end of changes of 'libsigc-2.0.so.0.0.0'===============
diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
index 9caa9428..fcbb9232 100644
--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
@@ -16,7 +16,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type size changed from 64 to 96 (in bits)
there are data member changes:
type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
- and size changed from 32 to 64 (in bits) (by +32 bits)
'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
3 impacted interfaces:
function void interface1(struct_type*)
diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
index 36439ce7..acff860c 100644
--- a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
+++ b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
@@ -13,5 +13,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
type size changed from 64 to 192 (in bits)
- and size changed from 64 to 192 (in bits) (by +128 bits)
'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
index 67dbf58f..79827103 100644
--- a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
+++ b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
@@ -9,5 +9,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
type size changed from 64 to 192 (in bits)
- and size changed from 64 to 192 (in bits) (by +128 bits)
'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 17523afa..97981a69 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -177,6 +177,24 @@ InOutSpec in_out_specs[] =
"data/test-abidiff-exit/test-leaf-cxx-members-report.txt",
"output/test-abidiff-exit/test-leaf-cxx-members-report.txt"
},
+ {
+ "data/test-abidiff-exit/test-member-size-v0.o",
+ "data/test-abidiff-exit/test-member-size-v1.o",
+ "",
+ "",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+ "data/test-abidiff-exit/test-member-size-report0.txt",
+ "output/test-abidiff-exit/test-member-size-report0.txt"
+ },
+ {
+ "data/test-abidiff-exit/test-member-size-v0.o",
+ "data/test-abidiff-exit/test-member-size-v1.o",
+ "",
+ "--leaf-changes-only",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+ "data/test-abidiff-exit/test-member-size-report1.txt",
+ "output/test-abidiff-exit/test-member-size-report1.txt"
+ },
{0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
};
--
2.26.0.292.g33ef6b2f38-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] abidiff: More compact references to prior diffs.
2020-04-03 21:53 [PATCH 1/2] abidiff: More compact references to prior diffs Giuliano Procida
2020-04-03 21:53 ` [PATCH 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
@ 2020-04-06 15:29 ` Matthias Maennich
2020-04-07 13:47 ` Dodji Seketeli
2020-04-07 21:33 ` [PATCH v2 " Giuliano Procida
3 siblings, 0 replies; 13+ messages in thread
From: Matthias Maennich @ 2020-04-06 15:29 UTC (permalink / raw)
To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team
On Fri, Apr 03, 2020 at 10:53:54PM +0100, Android Kernel Team wrote:
>In both the default and leaf reporting modes, when there is a repeated
>or circular reference to a type difference of a member variable, some
>placeholder text is emitted instead.
>
>In the leaf reporter:
>
> type 'struct S' of 'foo::bar' changed as reported earlier
>
>In the default reporter, this spans two lines:
>
> type of 'S foo::bar' changed:
> details were reported earlier
>
>This patch changes the latter to the more compact:
>
> type of 'S foo::bar' changed, as reported earlier
>
>More generally, this patch makes the punctuation of such placeholder
>text more consistent with a comma separating the phrases in all cases.
>
>It doesn't attempt to reconcile the different formatting of member
>variable declarations between the two modes.
>
> * src/abg-reporter-priv.cc (represent): In the var_diff_sptr
> overload, use consistent punctuation and keep to a single line
> of output when referring back to an existing type diff report.
> * src/abg-reporter-priv.h: In the macro
> RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER, use
> consistent punctuation when referring back to an existing type
> diff report.
> * tests/data/test-abidiff/test-PR18791-report0.txt: Adjust
> formatting of back references to existing type diff reports.
> * tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
> Ditto.
> * tests/data/test-diff-filter/test16-report-2.txt: Ditto.
> * tests/data/test-diff-filter/test17-1-report.txt: Ditto.
> * tests/data/test-diff-filter/test25-cyclic-type-report-1.txt:
> Ditto.
> * tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
> Ditto.
> * tests/data/test-diff-suppr/test36-leaf-report-0.txt: Ditto.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> src/abg-reporter-priv.cc | 19 ++++++++-----
> src/abg-reporter-priv.h | 8 +++---
> .../test-abidiff/test-PR18791-report0.txt | 3 +--
> .../PR25058-liblttng-ctl-report-1.txt | 22 +++++++--------
> .../data/test-diff-filter/test16-report-2.txt | 2 +-
> .../data/test-diff-filter/test17-1-report.txt | 2 +-
> .../test25-cyclic-type-report-1.txt | 2 +-
> ...l7.x86_64-0.12.8-1.el7.x86_64-report-2.txt | 27 +++++++++----------
> .../test-diff-suppr/test36-leaf-report-0.txt | 2 +-
> 9 files changed, 45 insertions(+), 42 deletions(-)
>
>diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
>index 883c815e..323de503 100644
>--- a/src/abg-reporter-priv.cc
>+++ b/src/abg-reporter-priv.cc
>@@ -507,11 +507,11 @@ represent(const var_diff_sptr &diff,
>
> if (d->currently_reporting())
> {
>- out << " as being reported\n";
>+ out << ", as being reported\n";
No need for the braces.
Reviewed-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
> }
> else if (d->reported_once())
> {
>- out << " as reported earlier\n";
>+ out << ", as reported earlier\n";
> }
> else
> {
>@@ -527,13 +527,20 @@ represent(const var_diff_sptr &diff,
> if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
> {
> out << indent
>- << "type of '" << pretty_representation << "' changed:\n";
>+ << "type of '" << pretty_representation << "' changed";
> if (d->currently_reporting())
>- out << indent << " details are being reported\n";
>+ {
>+ out << ", as being reported\n";
>+ }
> else if (d->reported_once())
>- out << indent << " details were reported earlier\n";
>+ {
>+ out << ", as reported earlier\n";
>+ }
> else
>- d->report(out, indent + " ");
>+ {
>+ out << ":\n";
>+ d->report(out, indent + " ");
>+ }
>
> begin_with_and = true;
> emitted = true;
>diff --git a/src/abg-reporter-priv.h b/src/abg-reporter-priv.h
>index b6c56fdb..b221fdc3 100644
>--- a/src/abg-reporter-priv.h
>+++ b/src/abg-reporter-priv.h
>@@ -68,14 +68,14 @@
> { \
> string _name_ = _diff_->first_subject()->get_pretty_representation(); \
> if (_diff_->currently_reporting()) \
>- out << indent << INTRO_TEXT << " '" << _name_ << "' changed; " \
>- "details are being reported\n"; \
>+ out << indent << INTRO_TEXT << " '" << _name_ \
>+ << "' changed, as being reported\n"; \
> else \
> { \
> out << indent << INTRO_TEXT << " '" \
>- << _name_ << "' changed"; \
>+ << _name_ << "' changed"; \
> report_loc_info(D->first_subject(), *d.context(), out); \
>- out << ", as reported earlier\n"; \
>+ out << ", as reported earlier\n"; \
> } \
> return ; \
> } \
>diff --git a/tests/data/test-abidiff/test-PR18791-report0.txt b/tests/data/test-abidiff/test-PR18791-report0.txt
>index 93faf599..5bc2d08b 100644
>--- a/tests/data/test-abidiff/test-PR18791-report0.txt
>+++ b/tests/data/test-abidiff/test-PR18791-report0.txt
>@@ -123,8 +123,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
> type size changed from 128 to 192 (in bits)
> 1 data member change:
>- type of 'std::__detail::_List_node_base std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed:
>- details were reported earlier
>+ type of 'std::__detail::_List_node_base std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed, as reported earlier
> and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
> and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
>
>diff --git a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
>index dfb63684..44ddc6d6 100644
>--- a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
>+++ b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
>@@ -179,8 +179,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
> in pointed to type 'struct lttng_event_field':
> type size hasn't changed
> 1 data member change:
>- type of 'lttng_event lttng_event_field::event' changed:
>- details were reported earlier
>+ type of 'lttng_event lttng_event_field::event' changed, as reported earlier
>
> [C] 'function int lttng_list_tracepoints(lttng_handle*, lttng_event**)' has some indirect sub-type changes:
> parameter 2 of type 'lttng_event**' has sub-type changes:
>@@ -232,7 +231,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
> type size changed from 576 to 640 (in bits)
> 2 data member changes:
> type of 'filter_node* filter_node::parent' changed:
>- pointed to type 'struct filter_node' changed; details are being reported
>+ pointed to type 'struct filter_node' changed, as being reported
> type of 'union {struct {} unknown; struct {filter_node* child;} root; struct {__anonymous_enum__ type; ast_link_type post_op; ast_link_type pre_op; union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;} u; filter_node* prev; filter_node* next;} expression; struct {op_type type; filter_node* lchild; filter_node* rchild;} op; struct {unary_op_type type; filter_node* child;} unary_op;} filter_node::u' changed:
> type size changed from 320 to 384 (in bits)
> 4 data member changes:
>@@ -245,21 +244,20 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
> type size hasn't changed
> 1 enumerator insertion:
> 'ast_link_type::AST_LINK_BRACKET' value '3'
>- type of 'ast_link_type pre_op' changed:
>- details were reported earlier
>+ type of 'ast_link_type pre_op' changed, as reported earlier
> type of 'union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;} u' changed:
> type size hasn't changed
> 1 data member change:
> type of 'filter_node* child' changed:
>- pointed to type 'struct filter_node' changed; details are being reported
>+ pointed to type 'struct filter_node' changed, as being reported
> type changed from:
> union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;}
> to:
> union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;}
> type of 'filter_node* prev' changed:
>- pointed to type 'struct filter_node' changed; details are being reported
>+ pointed to type 'struct filter_node' changed, as being reported
> type of 'filter_node* next' changed:
>- pointed to type 'struct filter_node' changed; details are being reported
>+ pointed to type 'struct filter_node' changed, as being reported
> type of 'struct {op_type type; filter_node* lchild; filter_node* rchild;} op' changed:
> type size hasn't changed
> 3 data member changes:
>@@ -278,14 +276,14 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
> 'op_type::AST_OP_BIT_OR' value '11'
> 'op_type::AST_OP_BIT_XOR' value '12'
> type of 'filter_node* lchild' changed:
>- pointed to type 'struct filter_node' changed; details are being reported
>+ pointed to type 'struct filter_node' changed, as being reported
> type of 'filter_node* rchild' changed:
>- pointed to type 'struct filter_node' changed; details are being reported
>+ pointed to type 'struct filter_node' changed, as being reported
> type of 'struct {filter_node* child;} root' changed:
> type size hasn't changed
> 1 data member change:
> type of 'filter_node* child' changed:
>- pointed to type 'struct filter_node' changed; details are being reported
>+ pointed to type 'struct filter_node' changed, as being reported
> type of 'struct {unary_op_type type; filter_node* child;} unary_op' changed:
> type size hasn't changed
> 2 data member changes:
>@@ -296,7 +294,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
> 1 enumerator insertion:
> 'unary_op_type::AST_UNARY_BIT_NOT' value '4'
> type of 'filter_node* child' changed:
>- pointed to type 'struct filter_node' changed; details are being reported
>+ pointed to type 'struct filter_node' changed, as being reported
> 'cds_list_head filter_ast::allocated_nodes' offset changed from 576 to 640 (in bits) (by +64 bits)
>
> [C] 'function YYSTYPE* lttng_yyget_lval(yyscan_t)' has some indirect sub-type changes:
>diff --git a/tests/data/test-diff-filter/test16-report-2.txt b/tests/data/test-diff-filter/test16-report-2.txt
>index a90e85ad..f69eabdc 100644
>--- a/tests/data/test-diff-filter/test16-report-2.txt
>+++ b/tests/data/test-diff-filter/test16-report-2.txt
>@@ -11,6 +11,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> 'int S::m0', at offset 0 (in bits)
> 1 data member change:
> type of 'S* S::m2' changed:
>- pointed to type 'struct S' changed; details are being reported
>+ pointed to type 'struct S' changed, as being reported
> and offset changed from 0 to 64 (in bits) (by +64 bits)
>
>diff --git a/tests/data/test-diff-filter/test17-1-report.txt b/tests/data/test-diff-filter/test17-1-report.txt
>index 7c51152a..0b39c5d4 100644
>--- a/tests/data/test-diff-filter/test17-1-report.txt
>+++ b/tests/data/test-diff-filter/test17-1-report.txt
>@@ -11,7 +11,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> 'int S::m0', at offset 0 (in bits)
> 1 data member change:
> type of 'S* S::m2' changed:
>- pointed to type 'struct S' changed; details are being reported
>+ pointed to type 'struct S' changed, as being reported
> and offset changed from 0 to 64 (in bits) (by +64 bits)
>
> [C] 'function void foo(S&)' has some indirect sub-type changes:
>diff --git a/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt b/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
>index 6a6beeef..0215d892 100644
>--- a/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
>+++ b/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
>@@ -11,5 +11,5 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> 'char S::m1', at offset 32 (in bits)
> 1 data member change:
> type of 'S* S::m2' changed:
>- pointed to type 'struct S' changed; details are being reported
>+ pointed to type 'struct S' changed, as being reported
>
>diff --git a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
>index 6d9beb85..58c94b7a 100644
>--- a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
>+++ b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
>@@ -37,7 +37,7 @@
> 13 data member changes:
> type of 'QXLInstance* RedDispatcher::qxl' changed:
> in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1:
>- underlying type 'struct QXLInstance' changed; details are being reported
>+ underlying type 'struct QXLInstance' changed, as being reported
> type of 'Dispatcher RedDispatcher::dispatcher' changed:
> underlying type 'struct Dispatcher' at dispatcher.h:22:1 changed:
> type size changed from 960 to 1024 (in bits)
>@@ -51,7 +51,7 @@
> 'int RedDispatcher::use_hardware_cursor' offset changed from 2240 to 2304 (in bits) (by +64 bits)
> type of 'RedDispatcher* RedDispatcher::next' changed:
> in pointed to type 'typedef RedDispatcher' at red_worker.h:87:1:
>- underlying type 'struct RedDispatcher' changed; details are being reported
>+ underlying type 'struct RedDispatcher' changed, as being reported
> and offset changed from 2304 to 2368 (in bits) (by +64 bits)
> 'Ring RedDispatcher::async_commands' offset changed from 2368 to 2432 (in bits) (by +64 bits)
> 'pthread_mutex_t RedDispatcher::async_lock' offset changed from 2496 to 2560 (in bits) (by +64 bits)
>@@ -185,7 +185,7 @@
> 1 data member change:
> type of 'SpiceCharDeviceState* SpiceCharDeviceInstance::st' changed:
> in pointed to type 'typedef SpiceCharDeviceState' at spice-char.h:34:1:
>- underlying type 'struct SpiceCharDeviceState' changed; details are being reported
>+ underlying type 'struct SpiceCharDeviceState' changed, as being reported
> and offset changed from 896 to 960 (in bits) (by +64 bits)
> 'int SpiceCharDeviceState::during_read_from_device' offset changed from 960 to 1024 (in bits) (by +64 bits)
> 'int SpiceCharDeviceState::during_write_to_device' offset changed from 992 to 1056 (in bits) (by +64 bits)
>@@ -223,7 +223,7 @@
> 2 data member changes (2 filtered):
> type of 'RedChannel* RedChannelClient::channel' changed:
> in pointed to type 'typedef RedChannel' at red_channel.h:130:1:
>- underlying type 'struct RedChannel' changed; details are being reported
>+ underlying type 'struct RedChannel' changed, as being reported
> type of 'RedsStream* RedChannelClient::stream' changed:
> in pointed to type 'typedef RedsStream' at reds_stream.h:31:1:
> underlying type 'struct RedsStream' at reds.h:68:1 changed:
>@@ -391,7 +391,7 @@
> in pointed to type 'function type void (RedChannel*, RedClient*, RedsStream*, int, int, uint32_t*, int, uint32_t*)':
> parameter 1 of type 'RedChannel*' has sub-type changes:
> in pointed to type 'typedef RedChannel' at red_channel.h:130:1:
>- underlying type 'struct RedChannel' changed; details are being reported
>+ underlying type 'struct RedChannel' changed, as being reported
> parameter 3 of type 'RedsStream*' has sub-type changes:
> pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
> type of 'channel_client_disconnect_proc disconnect' changed:
>@@ -524,7 +524,7 @@
> pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
> type of 'SndWorker* SndChannel::worker' changed:
> in pointed to type 'typedef SndWorker' at snd_worker.c:80:1:
>- underlying type 'struct SndWorker' changed; details are being reported
>+ underlying type 'struct SndWorker' changed, as being reported
> type of 'RedChannelClient* SndChannel::channel_client' changed:
> pointed to type 'typedef RedChannelClient' changed at red_channel.h:136:1, as reported earlier
> type of 'snd_channel_handle_message_proc SndChannel::handle_message' changed:
>@@ -532,26 +532,26 @@
> in pointed to type 'function type int (SndChannel*, typedef size_t, typedef uint32_t, void*)':
> parameter 1 of type 'SndChannel*' has sub-type changes:
> in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
>- underlying type 'struct SndChannel' changed; details are being reported
>+ underlying type 'struct SndChannel' changed, as being reported
> type of 'snd_channel_on_message_done_proc SndChannel::on_message_done' changed:
> underlying type 'void (SndChannel*)*' changed:
> in pointed to type 'function type void (SndChannel*)':
> parameter 1 of type 'SndChannel*' has sub-type changes:
> in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
>- underlying type 'struct SndChannel' changed; details are being reported
>+ underlying type 'struct SndChannel' changed, as being reported
> type of 'snd_channel_cleanup_channel_proc SndChannel::cleanup' changed:
> underlying type 'void (SndChannel*)*' changed:
> in pointed to type 'function type void (SndChannel*)':
> parameter 1 of type 'SndChannel*' has sub-type changes:
> in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
>- underlying type 'struct SndChannel' changed; details are being reported
>+ underlying type 'struct SndChannel' changed, as being reported
> no data member change (1 filtered);
> type of 'SndWorker* SndWorker::next' changed:
> in pointed to type 'typedef SndWorker' at snd_worker.c:80:1:
>- underlying type 'struct SndWorker' changed; details are being reported
>+ underlying type 'struct SndWorker' changed, as being reported
> type of 'SpicePlaybackInstance* SpicePlaybackState::sin' changed:
> in pointed to type 'typedef SpicePlaybackInstance' at spice-audio.h:33:1:
>- underlying type 'struct SpicePlaybackInstance' changed; details are being reported
>+ underlying type 'struct SpicePlaybackInstance' changed, as being reported
>
> [C] 'function void spice_server_playback_put_samples(SpicePlaybackInstance*, uint32_t*)' at snd_worker.c:1100:1 has some indirect sub-type changes:
> parameter 1 of type 'SpicePlaybackInstance*' has sub-type changes:
>@@ -590,11 +590,10 @@
> 1 data member insertion:
> 'uint32_t SpiceRecordState::frequency', at offset 512 (in bits) at snd_worker.c:166:1
> 2 data member changes:
>- type of 'SndWorker SpiceRecordState::worker' changed:
>- details were reported earlier
>+ type of 'SndWorker SpiceRecordState::worker' changed, as reported earlier
> type of 'SpiceRecordInstance* SpiceRecordState::sin' changed:
> in pointed to type 'typedef SpiceRecordInstance' at spice-audio.h:67:1:
>- underlying type 'struct SpiceRecordInstance' changed; details are being reported
>+ underlying type 'struct SpiceRecordInstance' changed, as being reported
>
> [C] 'function void spice_server_record_set_mute(SpiceRecordInstance*, uint8_t)' at snd_worker.c:1279:1 has some indirect sub-type changes:
> parameter 1 of type 'SpiceRecordInstance*' has sub-type changes:
>diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>index d270740d..9caa9428 100644
>--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>@@ -15,7 +15,7 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> 'struct leaf2 at test36-leaf-v0.cc:9:1' changed:
> type size changed from 64 to 96 (in bits)
> there are data member changes:
>- type 'struct leaf1' of 'leaf2::member0' changed as reported earlier
>+ type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
> and size changed from 32 to 64 (in bits) (by +32 bits)
> 'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
> 3 impacted interfaces:
>--
>2.26.0.292.g33ef6b2f38-goog
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] abidiff: Fix anonymous member size change reports.
2020-04-03 21:53 ` [PATCH 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
@ 2020-04-06 16:08 ` Matthias Maennich
2020-04-07 13:55 ` Dodji Seketeli
2020-04-07 21:45 ` Giuliano Procida
0 siblings, 2 replies; 13+ messages in thread
From: Matthias Maennich @ 2020-04-06 16:08 UTC (permalink / raw)
To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team
On Fri, Apr 03, 2020 at 10:53:55PM +0100, Android Kernel Team wrote:
>Data members have names (unless anonymous), types, sizes and
>offsets (if a member of a class or struct). It is the responsibility
>of the represent function to detail all differences of a changed
>member. The size of a member is the same as the size of its type.
>
>A recent change to the function focused on cleaning up the formatting
>logic and added a catch-all case to the end of it in case no diff
>information had been emitted by then.
>
>This catch-all triggered for anonymous structs and union diffs under
>certain circumstances when these anonymous members had changed in
>size.
>
>This patch ensures size change information for a member is emitted
>exactly once (unless the type change has been or is being reported
>already). It also ensures anonymous members are identified
>appropriately and includes a test case would otherwise reach the
>catch-all case in both default and --leaf-changes-only modes.
>
> * src/abg-reporter-priv.cc (represent): In the var_diff_sptr
> overload, factor out some expressions as local variables, rely
> on diff_to_be_reported to decide whether to emit a change,
> fold together local/non-local change reporting using
> local_changes to preserve current name formatting differences,
> keep track explicitly of whether size information has been
> emitted and ensure it happens if needed, make offset and size
> change reporting for anonymous data members more meaningful.
> * tests/test-abidiff-exit.cc: Run new test cases.
> * tests/data/Makefile.am: Add new test files.
> * tests/data/test-abidiff-exit/test-member-size-v0.cc: New
> test.
> * tests/data/test-abidiff-exit/test-member-size-v0.o: Ditto.
> * tests/data/test-abidiff-exit/test-member-size-v1.cc: Ditto.
> * tests/data/test-abidiff-exit/test-member-size-v1.o: Ditto.
> * tests/data/test-abidiff-exit/test-member-size-report0.txt:
> New test, default mode.
> * tests/data/test-abidiff-exit/test-member-size-report1.txt:
> New test, --leaf-changes-only mode.
> * tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
> Eliminate duplicate reporting of member sizes.
> * tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
> Ditto.
> * tests/data/test-abidiff-exit/test-leaf1-report.txt: Ditto.
> * tests/data/test-abidiff-exit/test-no-stray-comma-report.txt:
> Ditto.
> * tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
> Add size report for anonymous data member.
> * tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
> Ditto.
> * tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt:
> Add missing size change report.
> * tests/data/test-diff-suppr/test36-leaf-report-0.txt: Remove
> size change report for previously reported type.
> * tests/data/test-diff-suppr/test46-PR25128-report-1.txt:
> Eliminate duplicate reporting of member size change.
> * tests/data/test-diff-suppr/test46-PR25128-report-2.txt:
> Ditto.
>
This broke one test case on my machine, that needs trivial adjusting:
test-leaf-peeling-report-indirect.txt:
| @@ -9,7 +9,6 @@
| type 'int' of 'foo::z' changed:
| type name changed from 'int' to 'long int'
| type size changed from 32 to 64 (in bits)
| - and size changed from 32 to 64 (in bits) (by +32 bits)
|
| 'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
| type size hasn't changed
| FAIL runtestabidiffexit (exit status: 1)
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> src/abg-reporter-priv.cc | 98 +++++++-----------
> tests/data/Makefile.am | 6 ++
> .../test-leaf-cxx-members-report.txt | 4 +-
> .../test-leaf-peeling-report.txt | 1 -
> .../test-abidiff-exit/test-leaf1-report.txt | 1 -
> .../test-member-size-report0.txt | 27 +++++
> .../test-member-size-report1.txt | 21 ++++
> .../test-abidiff-exit/test-member-size-v0.cc | 26 +++++
> .../test-abidiff-exit/test-member-size-v0.o | Bin 0 -> 3184 bytes
> .../test-abidiff-exit/test-member-size-v1.cc | 27 +++++
> .../test-abidiff-exit/test-member-size-v1.o | Bin 0 -> 3192 bytes
> .../test-no-stray-comma-report.txt | 1 -
> .../test45-anon-dm-change-report-0.txt | 1 +
> .../test44-anonymous-data-member-report-0.txt | 1 +
> ...-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt | 2 +-
> .../test-diff-suppr/test36-leaf-report-0.txt | 1 -
> .../test46-PR25128-report-1.txt | 1 -
> .../test46-PR25128-report-2.txt | 1 -
> tests/test-abidiff-exit.cc | 18 ++++
> 19 files changed, 167 insertions(+), 70 deletions(-)
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-report0.txt
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-report1.txt
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.cc
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.o
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.cc
> create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.o
>
>diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
>index 323de503..72181367 100644
>--- a/src/abg-reporter-priv.cc
>+++ b/src/abg-reporter-priv.cc
>@@ -411,16 +411,26 @@ represent(const var_diff_sptr &diff,
>
> var_decl_sptr o = diff->first_var();
> var_decl_sptr n = diff->second_var();
>+ string name1 = o->get_qualified_name();
>+ string name2 = n->get_qualified_name();
>+ uint64_t size1 = get_var_size_in_bits(o);
>+ uint64_t size2 = get_var_size_in_bits(n);
>+ uint64_t offset1 = get_data_member_offset(o);
>+ uint64_t offset2 = get_data_member_offset(n);
Those are only used const, hence can be made const.
Also, further down, offset1 feels like to much of an abbreviation.
>
> // Has the main diff text been output?
> bool emitted = false;
> // Are we continuing on a new line? (implies emitted)
> bool begin_with_and = false;
>- string name1 = o->get_qualified_name();
>- string name2 = n->get_qualified_name();
>- string pretty_representation = o->get_pretty_representation();
>+ // Have we reported a size change already?
>+ bool size_reported = false;
>+ // Are we reporting a change to an anonymous member?
> bool is_strict_anonymous_data_member_change = false;
>
>+ string pretty_representation = o->get_pretty_representation();
>+ bool show_size_offset_changes = ctxt->get_allowed_category()
>+ & SIZE_OR_OFFSET_CHANGE_CATEGORY;
>+
> if (is_anonymous_data_member(o) && is_anonymous_data_member(n))
> {
> is_strict_anonymous_data_member_change = true;
>@@ -430,8 +440,7 @@ represent(const var_diff_sptr &diff,
> if (tr1 != tr2)
> {
> show_offset_or_size(indent + "anonymous data member at offset",
>- get_data_member_offset(o),
>- *ctxt, out);
>+ offset1, *ctxt, out);
>
> out << " changed from:\n"
> << indent << " " << tr1 << "\n"
>@@ -460,7 +469,6 @@ represent(const var_diff_sptr &diff,
> begin_with_and = true;
> emitted = true;
> }
>- // TODO: else ...?
> }
> else if (filtering::has_anonymous_data_member_change(diff))
> {
>@@ -472,9 +480,7 @@ represent(const var_diff_sptr &diff,
> + o->get_pretty_representation()
> + " at offset";
>
>- show_offset_or_size(indent + repr,
>- get_data_member_offset(o),
>- *ctxt, out);
>+ show_offset_or_size(indent + repr, offset1, *ctxt, out);
> out << " became data member '"
> << n->get_pretty_representation()
> << "'\n";
>@@ -486,9 +492,7 @@ represent(const var_diff_sptr &diff,
> + o->get_pretty_representation()
> + " at offset";
>
>- show_offset_or_size(indent + repr,
>- get_data_member_offset(o),
>- *ctxt, out);
>+ show_offset_or_size(indent + repr, offset1, *ctxt, out);
> out << " became anonymous data member '"
> << n->get_pretty_representation()
> << "'\n";
>@@ -499,11 +503,15 @@ represent(const var_diff_sptr &diff,
> }
> else if (diff_sptr d = diff->type_diff())
> {
>- if (local_only && d->has_local_changes())
>+ if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
> {
>- out << indent << "type '" << get_pretty_representation(o->get_type())
>- << "' of '" << o->get_qualified_name()
>- << "' changed";
>+ if (local_only)
>+ out << indent << "type '" << get_pretty_representation(o->get_type())
>+ << "' of '" << o->get_qualified_name()
>+ << "' changed";
>+ else
>+ out << indent
>+ << "type of '" << pretty_representation << "' changed";
>
> if (d->currently_reporting())
> {
>@@ -521,30 +529,7 @@ represent(const var_diff_sptr &diff,
>
> begin_with_and = true;
> emitted = true;
>- }
>- else
>- {
>- if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
>- {
>- out << indent
>- << "type of '" << pretty_representation << "' changed";
>- if (d->currently_reporting())
>- {
>- out << ", as being reported\n";
>- }
>- else if (d->reported_once())
>- {
>- out << ", as reported earlier\n";
>- }
>- else
>- {
>- out << ":\n";
>- d->report(out, indent + " ");
>- }
>-
>- begin_with_and = true;
>- emitted = true;
>- }
>+ size_reported = true;
> }
> }
>
>@@ -589,9 +574,9 @@ represent(const var_diff_sptr &diff,
> out << "now becomes laid out";
> emitted = true;
> }
>- if ((ctxt->get_allowed_category() & SIZE_OR_OFFSET_CHANGE_CATEGORY))
>+ if (show_size_offset_changes)
> {
>- if (get_data_member_offset(o) != get_data_member_offset(n))
>+ if (offset1 != offset2)
> {
> if (begin_with_and)
> {
>@@ -600,26 +585,20 @@ represent(const var_diff_sptr &diff,
> }
> else if (!emitted)
> {
>+ out << indent;
> if (is_strict_anonymous_data_member_change)
>- out << indent;
>- else
>- out << indent << "'" << pretty_representation << "' ";
>+ out << "anonymous data member ";
>+ out << "'" << pretty_representation << "' ";
> }
> else
> out << ", ";
>
>- show_numerical_change("offset",
>- get_data_member_offset(o),
>- get_data_member_offset(n),
>- *ctxt, out);
>+ show_numerical_change("offset", offset1, offset2, *ctxt, out);
> maybe_show_relative_offset_change(diff, *ctxt, out);
>-
> emitted = true;
> }
>- if (// If we are not displaying only local changes, we must
>- // have indicated the type size change already.
>- local_only
>- && (get_var_size_in_bits(o) != get_var_size_in_bits(n)))
>+
>+ if (!size_reported && size1 != size2)
> {
> if (begin_with_and)
> {
>@@ -628,18 +607,15 @@ represent(const var_diff_sptr &diff,
> }
> else if (!emitted)
> {
>+ out << indent;
> if (is_strict_anonymous_data_member_change)
>- out << indent;
>- else
>- out << indent << "'" << pretty_representation << "' ";
>+ out << "anonymous data member ";
>+ out << "'" << pretty_representation << "' ";
> }
> else
> out << ", ";
>
>- show_numerical_change("size",
>- get_var_size_in_bits(o),
>- get_var_size_in_bits(n),
>- *ctxt, out);
>+ show_numerical_change("size", size1, size2, *ctxt, out);
> maybe_show_relative_size_change(diff, *ctxt, out);
> emitted = true;
> }
>diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
>index bb0fd83b..64e36b1a 100644
>--- a/tests/data/Makefile.am
>+++ b/tests/data/Makefile.am
>@@ -140,6 +140,12 @@ test-abidiff-exit/test-leaf-cxx-members-v0.o \
> test-abidiff-exit/test-leaf-cxx-members-v1.cc \
> test-abidiff-exit/test-leaf-cxx-members-v1.o \
> test-abidiff-exit/test-leaf-cxx-members-report.txt \
>+test-abidiff-exit/test-member-size-v0.cc \
>+test-abidiff-exit/test-member-size-v0.o \
>+test-abidiff-exit/test-member-size-v1.cc \
>+test-abidiff-exit/test-member-size-v1.o \
>+test-abidiff-exit/test-member-size-report0.txt \
>+test-abidiff-exit/test-member-size-report1.txt \
> \
> test-diff-dwarf/test0-v0.cc \
> test-diff-dwarf/test0-v0.o \
>diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
>index 082db83e..eae74a3b 100644
>--- a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
>+++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
>@@ -35,8 +35,8 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> type 'int' of 'ops::deleted_var' changed:
> type name changed from 'int' to 'long int'
> type size changed from 32 to 64 (in bits)
>- and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1, size changed from 32 to 64 (in bits) (by +32 bits)
>+ and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1
> type 'int' of 'ops::changed_var' changed:
> type name changed from 'int' to 'long int'
> type size changed from 32 to 64 (in bits)
>- and offset changed from 96 to 128 (in bits) (by +32 bits), size changed from 32 to 64 (in bits) (by +32 bits)
>+ and offset changed from 96 to 128 (in bits) (by +32 bits)
>diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>index bd5f36f8..7ab4dd6e 100644
>--- a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
>@@ -9,7 +9,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> type 'int' of 'foo::z' changed:
> type name changed from 'int' to 'long int'
> type size changed from 32 to 64 (in bits)
>- and size changed from 32 to 64 (in bits) (by +32 bits)
As above, the same patch needs to be applied to
test-leaf-peeling-report-indirect.txt.
>
> 'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
> type size hasn't changed
>diff --git a/tests/data/test-abidiff-exit/test-leaf1-report.txt b/tests/data/test-abidiff-exit/test-leaf1-report.txt
>index 9af6ccaa..aea08b81 100644
>--- a/tests/data/test-abidiff-exit/test-leaf1-report.txt
>+++ b/tests/data/test-abidiff-exit/test-leaf1-report.txt
>@@ -40,4 +40,3 @@ Removed/Changed/Added variables summary: 1 Removed, 1 Changed, 1 Added variable
> type 'int' of 'changed::foo' changed:
> type name changed from 'int' to 'long int'
> type size changed from 32 to 64 (in bits)
>- and size changed from 32 to 64 (in bits) (by +32 bits)
>diff --git a/tests/data/test-abidiff-exit/test-member-size-report0.txt b/tests/data/test-abidiff-exit/test-member-size-report0.txt
>new file mode 100644
>index 00000000..ff93dd50
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-member-size-report0.txt
>@@ -0,0 +1,27 @@
>+Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
>+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>+
>+2 functions with some indirect sub-type change:
>+
>+ [C] 'function void reg1(S*, T*, T*)' at test-member-size-v1.cc:26:1 has some indirect sub-type changes:
>+ parameter 1 of type 'S*' has sub-type changes:
>+ in pointed to type 'struct S' at test-member-size-v1.cc:3:1:
>+ type size changed from 128 to 192 (in bits)
>+ 1 data member insertion:
>+ 'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
>+ no data member change (1 filtered);
>+ parameter 2 of type 'T*' has sub-type changes:
>+ in pointed to type 'struct T' at test-member-size-v1.cc:14:1:
>+ type size changed from 192 to 256 (in bits)
>+ 2 data member changes:
>+ 'S T::s' size changed from 128 to 192 (in bits) (by +64 bits)
>+ 'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
>+
>+ [C] 'function void reg2(U*)' at test-member-size-v1.cc:27:1 has some indirect sub-type changes:
>+ parameter 1 of type 'U*' has sub-type changes:
>+ in pointed to type 'struct U' at test-member-size-v1.cc:19:1:
>+ type size changed from 192 to 256 (in bits)
>+ 2 data member changes:
>+ anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
>+ 'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
>+
>diff --git a/tests/data/test-abidiff-exit/test-member-size-report1.txt b/tests/data/test-abidiff-exit/test-member-size-report1.txt
>new file mode 100644
>index 00000000..dbad76be
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-member-size-report1.txt
>@@ -0,0 +1,21 @@
>+Leaf changes summary: 3 artifacts changed
>+Changed leaf types summary: 3 leaf types changed
>+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
>+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
>+
>+'struct S at test-member-size-v0.cc:3:1' changed:
>+ type size changed from 128 to 192 (in bits)
>+ 1 data member insertion:
>+ 'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
>+
>+'struct T at test-member-size-v0.cc:13:1' changed:
>+ type size changed from 192 to 256 (in bits)
>+ there are data member changes:
>+ type 'struct S' of 'T::s' changed, as reported earlier
>+ 'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
>+
>+'struct U at test-member-size-v0.cc:18:1' changed:
>+ type size changed from 192 to 256 (in bits)
>+ there are data member changes:
>+ anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
>+ 'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
>diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.cc b/tests/data/test-abidiff-exit/test-member-size-v0.cc
>new file mode 100644
>index 00000000..f974a37f
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-member-size-v0.cc
>@@ -0,0 +1,26 @@
>+struct M;
>+
>+struct S {
>+ int x;
>+ M * m;
>+};
>+
>+struct M {
>+ int a;
>+ S * s;
>+};
>+
>+struct T {
>+ S s;
>+ int a;
>+};
>+
>+struct U {
>+ struct {
>+ S s;
>+ };
>+ int r;
>+};
>+
>+void reg1(S*, T*, T*) { }
>+void reg2(U*) { }
>diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.o b/tests/data/test-abidiff-exit/test-member-size-v0.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..e020bef77d57d601420f969061707bbf1486ac99
>GIT binary patch
>literal 3184
>zcmbtWL2MgU5S{P!#_PCtoVcY4LC91fsLC!`E2`o`ifKxds<Z@BqY4SqwX=yW;&rs%
>z6t@VFPzA~@m5>058xj(?a^Q}H#JR_E;{Xyz;Dp2hX8!(hHs8(#M)J=5H}hux|2zM$
>zKYaO>ODQ3MMZkGD7%>WPKXYK$q*;R!OhJ8R@7I;R_y1bi+x>lIum0QqTR$*_<}#*A
>z%V<j`jqQU>@OZ*btwB*3)~g5^eGuskSzvTPn3+CW7NpZ=JBT246$34cFvA0pRe?Pa
>z<Cza+plQ8<+O%wZrmn-Z$Xj1iG=>yR0wr-m5{r<kW5ks85aF?+D2@N9LXT%;C}h5s
>zj1a^lieV8T9vvgal2eBG(!jN5>5;nE!bwo&m@spQP#O>}5`kupge(nG_iieE0~6e(
>zKrtzvDwJ|YVYV<SrZUBo7*)dU%H_`Gp3hZt0@E3JFsyiZxwL>2N<Rs?-1(e=<;@8+
>zFJsVHNhCR$JjAh<BVL>_3Nby9MQYQZU6`T7Srda&(>RT1X_hpfcuCTtNAg7uKr~*%
>zp8>RwLi}CG<5M;pu2Deb_aeLFcN%_Z_u6-S`_`P(Y)ZCy60&g_;!g{6=1+5C(VYA!
>zWnhXUi0g08hyGS&)vW_+mmU>(F6!(+>~UA!wN>}}T4f!Uuhq*-XU@3u<*HM0+;U~k
>zt-9`9wQP5y-N0Ym@&i9?H*x5N&DLUn;jDdjzHD!m?M<(DJ7~7TZqVKB*}K8J?O?->
>zyvDZQ16W>KI#+(?qTgtHL7drktEaUZa)HO?6L;ax>md3?F?%dy?c=UIhB*C5aN_}9
>zDil9XE#=3qVE_)v_X6Q4vg6o)DfY*n;U}iXllaF!!ybA+<Q*16y=mQJAY+sm>a6QN
>z0}l25z->U{QW>B$pKM}uU>fm?4*+EukRQSbFraV*eryP*-kwVAgWP+>PAi<pe9Dh%
>z-l0xQG$j?~ofuEpg!7CDr`2B|5M?q}z_=R12@R)nNiq{Fxst&FLX6fyLV4t6Bu4;0
>zDb4wxR^y*F9IqQGzbH)|L-~1M##IY(o>MArQo~(^pHMjGQShPtF&RW%RJI<^XF<oy
>zDxT(;+=rTm^Zdc--R|HqMjeH*Zm|{l{Ro^5FY>@?^m^cg{<i0A_>JAI^>(n?MW3|2
>zMkDlZ9ddi27i{6pkfdz41D`2a0-R>I)A8}U<^<iyckpVmdy&_CN0I#2`ex{Ld^Pd^
>zsb-N+d&Pao>1xW*?{SJv$`d#sOuqr%P6j1xolpHm{r6w_)IWMoD#!J&XijY?zhCq(
>zq(-EK?5JidD+2W!#qn98`+;N;e|k>Bzfs^PG)RaZyNW&?qwqP^q;m-_nR@IM#7513
>zU%gP~)BNdn)#tx4A}2y{Q`N82AR&7Fb@Yv@|Glcu`$YBW9@FdZAVxWPo@h$>PSyXM
>z0j0S<>-P{J#lNZ=itjH@!#vh^6`%dABfTM&gLxeH1sa5@=`sv`#lJB?VuRPu@TX{u
>z+P{Y*-XDFvn9ut<sy5|*pz62Ogz39MUqziy?_|_c7gYbxqdzJ7`YG=yzD3Q3@>Q$`
>HPo4iS_W$I+
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.cc b/tests/data/test-abidiff-exit/test-member-size-v1.cc
>new file mode 100644
>index 00000000..b3bd5988
>--- /dev/null
>+++ b/tests/data/test-abidiff-exit/test-member-size-v1.cc
>@@ -0,0 +1,27 @@
>+struct M;
>+
>+struct S {
>+ int x;
>+ M * m;
>+ int y;
>+};
>+
>+struct M {
>+ int a;
>+ S * s;
>+};
>+
>+struct T {
>+ S s;
>+ int a;
>+};
>+
>+struct U {
>+ struct {
>+ S s;
>+ };
>+ int r;
>+};
>+
>+void reg1(S*, T*, T*) { }
>+void reg2(U*) { }
While these are only test cases, I would like for them to have a bit
more descriptive names or a comment for each of them to make it easier
to read the test case.
Other than those little nits, this is a very nice patch:
Reviewed-by: Matthias Maennich <maennich@google.com>
Tested-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
>diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.o b/tests/data/test-abidiff-exit/test-member-size-v1.o
>new file mode 100644
>index 0000000000000000000000000000000000000000..eaec9ebead662bc23c00130c537133a5be5c2019
>GIT binary patch
>literal 3192
>zcmbtW&u<%55T57F#_Me3IB}ySq(VjlK~;9iMp4zYq?iWUR6q$$jiO2r*3QPZ8gHug
>zx;RCs2oT68S_z3FapNyQ;?M&JBraUJaYO<nBo6#IA#s42w{Psti*tdIyfg2c`DWgG
>zJMXRUJpbawj1a&g;5-aQi~>B!4(zft%TR<VsI2vVUhBR6*IKXh`&zH^%iY($VF=Bu
>zm}*2uTQX^44>G~SDLb<Pv%;{hBV=?zjAY3IqXojuzK51&?1LDUXaPn>%w`xv<|QbI
>zoJDTK2Qj8NZ4h?0M;tIMAGK*Y_{>~`lE_<MP&9^2m;_2<K@w*nQ^AO$^&`Tgv*P&J
>zcPex`D?=gsm1IO9W)#CBK+KPlVzHGWKFi~>bM(mEYv3fPazc3M0HN3?yi5d|ITCU-
>z$lQBF=}k=V5e14#@mRiS8~OSCq?pPUW-zLVJ7?Qx?Q{0BEij!i4u+KsFBi|?ghn0(
>z+dgj_Sl*mK^D+jF7Dbwq9fu^=a>X-qMn0j3vPgOMlM8c{IA>x|W)`RMB+ZiMlPpPE
>z^hm$cK8VIE_%nd^QHa0u<M@<)hieoN2kqEt1+8ijIql}{z`5nQwVGs`GmuNl5PwdX
>zbAOr>E9T_883R)sL5zaB2V2*dP%o}~6(9{y#jTqP^wzzNb?@57;uc)GQYo#TI^`{u
>zmfc0yD=jW~%bvHeTyk1*Ck$5VK^R2M8V>!a)>!GDIqjTYDmnF%v*Wk-!&)QS3->y0
>zrxU)}47Z)wukHqIfJ>{ZXG>2!7gU>mm}GXm<&#<sfa7wFxCOVbg6I~+{IRTc7q{dD
>z;`AfIT?cqJU-%%iIzD;@18_*brwK=q?Z*C#i9hiieqw4ok$?Pi*h4Reyobe5uUYpP
>z$QUMuI`g{EfCEk6cN>tpT>2=@Czl!xOe0zG08o|z<A*Q;49FjWA3KCoV^5{_LGC?b
>zCl$_PKH^7pZqj?HHcK@l73G~6OWB0;j0mUIUmy@=GEu;|8p5=O)1f4pOO#wm|MVb6
>z>mZ>#@+OiafS;7+{Ew^guNuxh`J2M2V<<oG%d~1C&VzT8loJ~6DZHd`&ZFQ1`(rYQ
>zx}a=5p3j1gmsC8>F})9E4d?lT+um>C8AcsPiEgkJ2i+LlZ9n$Gt+w0XM!~M{ZU@y)
>zeXAMn?4eKEezh6}w+^`d$PepyEu<;C%`jjJmH@Z5*J=g$QFFt+IB@Y^a@w(9yP-%y
>zV{0e!TY;MR|5WqHr@i7n<aA|a=$AOfrsWYF5T@S%ucw1jw$7*iqW=4@eCi)PCza#+
>zuWC+hC_i5GZ=_75lyp_IabHruQ5>HYx*teZ@Tcb_{7VIXNQ0E<vFqs5F$zDYnsgz>
>zB~y=m0kL88zpq{>^J)HcyXy1b9+DFwxS{G-Xpj=U{ucU%)&Ex2=Y693bdTxvZz4uH
>zd7fxW`C8S#&w$cgpY>aa594pDhT{8+(=d<q`-;zg)_3%VR1W5G+^1*|rl!j<yrcM=
>zeIzk>{S1GM#<2bSVaWTVuNU)qUx(GE+~2GE4K-o<ZqQdz=hHhGw$!rf|0VRNMPEPV
>R9mX$Ev!Q&RsQy#u{|i5f=e7U<
>
>literal 0
>HcmV?d00001
>
>diff --git a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
>index 1583a40b..52c48b3d 100644
>--- a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
>+++ b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
>@@ -10,4 +10,3 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> type name changed from 'int[1]' to 'int[2]'
> array type size changed from 32 to 64
> array type subrange 1 changed length from 1 to 2
>- and size changed from 32 to 64 (in bits) (by +32 bits)
>diff --git a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
>index 12af287b..2f8f31d0 100644
>--- a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
>+++ b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
>@@ -13,6 +13,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> 'char S1::m1' offset changed from 64 to 40 (in bits) (by -24 bits)
> 1 data member change:
> anonymous data member struct {int m0; char m01;} at offset 0 (in bits) became data member 'int S1::m0'
>+ and size changed from 64 to 32 (in bits) (by -32 bits)
>
> [C] 'function void foo(S0&)' has some indirect sub-type changes:
> parameter 1 of type 'S0&' has sub-type changes:
>diff --git a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
>index b8bb951c..ed68b550 100644
>--- a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
>+++ b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
>@@ -12,5 +12,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> struct {int b; float c;}
> to:
> struct {int b; float c; char e;}
>+ and size changed from 64 to 96 (in bits) (by +32 bits)
> 'int S2::d' offset changed from 96 to 128 (in bits) (by +32 bits)
>
>diff --git a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
>index b1611303..2a679d60 100644
>--- a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
>+++ b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
>@@ -78,7 +78,7 @@
> type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
> type size changed from 128 to 192 (in bits)
> 1 data member change:
>- name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
>+ name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node', size changed from 128 to 192 (in bits) (by +64 bits)
> and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
>
> ================ end of changes of 'libsigc-2.0.so.0.0.0'===============
>diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>index 9caa9428..fcbb9232 100644
>--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
>@@ -16,7 +16,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> type size changed from 64 to 96 (in bits)
> there are data member changes:
> type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
>- and size changed from 32 to 64 (in bits) (by +32 bits)
> 'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
> 3 impacted interfaces:
> function void interface1(struct_type*)
>diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
>index 36439ce7..acff860c 100644
>--- a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
>+++ b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
>@@ -13,5 +13,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
> entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
> type size changed from 64 to 192 (in bits)
>- and size changed from 64 to 192 (in bits) (by +128 bits)
> 'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
>diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
>index 67dbf58f..79827103 100644
>--- a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
>+++ b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
>@@ -9,5 +9,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
> entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
> type size changed from 64 to 192 (in bits)
>- and size changed from 64 to 192 (in bits) (by +128 bits)
> 'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
>diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
>index 17523afa..97981a69 100644
>--- a/tests/test-abidiff-exit.cc
>+++ b/tests/test-abidiff-exit.cc
>@@ -177,6 +177,24 @@ InOutSpec in_out_specs[] =
> "data/test-abidiff-exit/test-leaf-cxx-members-report.txt",
> "output/test-abidiff-exit/test-leaf-cxx-members-report.txt"
> },
>+ {
>+ "data/test-abidiff-exit/test-member-size-v0.o",
>+ "data/test-abidiff-exit/test-member-size-v1.o",
>+ "",
>+ "",
>+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>+ "data/test-abidiff-exit/test-member-size-report0.txt",
>+ "output/test-abidiff-exit/test-member-size-report0.txt"
>+ },
>+ {
>+ "data/test-abidiff-exit/test-member-size-v0.o",
>+ "data/test-abidiff-exit/test-member-size-v1.o",
>+ "",
>+ "--leaf-changes-only",
>+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
>+ "data/test-abidiff-exit/test-member-size-report1.txt",
>+ "output/test-abidiff-exit/test-member-size-report1.txt"
>+ },
> {0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
> };
>
>--
>2.26.0.292.g33ef6b2f38-goog
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] abidiff: More compact references to prior diffs.
2020-04-03 21:53 [PATCH 1/2] abidiff: More compact references to prior diffs Giuliano Procida
2020-04-03 21:53 ` [PATCH 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
2020-04-06 15:29 ` [PATCH 1/2] abidiff: More compact references to prior diffs Matthias Maennich
@ 2020-04-07 13:47 ` Dodji Seketeli
2020-04-07 21:32 ` Giuliano Procida
2020-04-07 21:33 ` [PATCH v2 " Giuliano Procida
3 siblings, 1 reply; 13+ messages in thread
From: Dodji Seketeli @ 2020-04-07 13:47 UTC (permalink / raw)
To: Giuliano Procida; +Cc: libabigail, kernel-team
Hello,
I like the patch overall, I just have some nits to pick I guess :-)
Giuliano Procida <gprocida@google.com> a écrit:
[...]
> diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
> index 883c815e..323de503 100644
> --- a/src/abg-reporter-priv.cc
> +++ b/src/abg-reporter-priv.cc
> @@ -507,11 +507,11 @@ represent(const var_diff_sptr &diff,
>
> if (d->currently_reporting())
> {
> - out << " as being reported\n";
> + out << ", as being reported\n";
> }
In general, throughout the code we try to only use braces when
necessary. I know this change didn't create this situation here (I must
have missed it from a previous patch or something) but could you please
remove the brace here, for the sake of consistency with the rest of the
code.
> else if (d->reported_once())
> {
> - out << " as reported earlier\n";
> + out << ", as reported earlier\n";
> }
Likewise.
[...]
> @@ -527,13 +527,20 @@ represent(const var_diff_sptr &diff,
> if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
> {
> out << indent
> - << "type of '" << pretty_representation << "' changed:\n";
> + << "type of '" << pretty_representation << "' changed";
> if (d->currently_reporting())
> - out << indent << " details are being reported\n";
> + {
> + out << ", as being reported\n";
> + }
Likewise.
> else if (d->reported_once())
> - out << indent << " details were reported earlier\n";
> + {
> + out << ", as reported earlier\n";
> + }
Likewise.
> else
> - d->report(out, indent + " ");
> + {
> + out << ":\n";
> + d->report(out, indent + " ");
> + }
Likewise.
[...]
Matthias Maennich <maennich@google.com> a écrit:
[...]
>>+++ b/src/abg-reporter-priv.cc
>>@@ -507,11 +507,11 @@ represent(const var_diff_sptr &diff,
>>
>> if (d->currently_reporting())
>> {
>>- out << " as being reported\n";
>>+ out << ", as being reported\n";
>
> No need for the braces.
Agreed.
Thank you Matthias for the review.
Cheers,
--
Dodji
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] abidiff: Fix anonymous member size change reports.
2020-04-06 16:08 ` Matthias Maennich
@ 2020-04-07 13:55 ` Dodji Seketeli
2020-04-07 21:31 ` Giuliano Procida
2020-04-07 21:45 ` Giuliano Procida
1 sibling, 1 reply; 13+ messages in thread
From: Dodji Seketeli @ 2020-04-07 13:55 UTC (permalink / raw)
To: Matthias Maennich; +Cc: Giuliano Procida, libabigail, kernel-team
Matthias Maennich <maennich@google.com> a écrit:
[...]
>
> This broke one test case on my machine, that needs trivial adjusting:
>
> test-leaf-peeling-report-indirect.txt:
>
> | @@ -9,7 +9,6 @@
> | type 'int' of 'foo::z' changed:
> | type name changed from 'int' to 'long int'
> | type size changed from 32 to 64 (in bits)
> | - and size changed from 32 to 64 (in bits) (by +32 bits)
> |
> | 'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
> | type size hasn't changed
> | FAIL runtestabidiffexit (exit status: 1)
Right. Also, I think you'll need to rebase this patch on top of current
master as I have committed a patch from you that renames
test-abidiff-exit/test-leaf1-report.txt.
Thanks.
Cheers,
--
Dodji
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] abidiff: Fix anonymous member size change reports.
2020-04-07 13:55 ` Dodji Seketeli
@ 2020-04-07 21:31 ` Giuliano Procida
0 siblings, 0 replies; 13+ messages in thread
From: Giuliano Procida @ 2020-04-07 21:31 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: Matthias Maennich, libabigail, kernel-team
Agreed.
I spotted this on rebase.
On Tue, 7 Apr 2020 at 14:56, Dodji Seketeli <dodji@seketeli.org> wrote:
>
> Matthias Maennich <maennich@google.com> a écrit:
>
> [...]
>
> >
> > This broke one test case on my machine, that needs trivial adjusting:
> >
> > test-leaf-peeling-report-indirect.txt:
> >
> > | @@ -9,7 +9,6 @@
> > | type 'int' of 'foo::z' changed:
> > | type name changed from 'int' to 'long int'
> > | type size changed from 32 to 64 (in bits)
> > | - and size changed from 32 to 64 (in bits) (by +32 bits)
> > |
> > | 'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
> > | type size hasn't changed
> > | FAIL runtestabidiffexit (exit status: 1)
>
> Right. Also, I think you'll need to rebase this patch on top of current
> master as I have committed a patch from you that renames
> test-abidiff-exit/test-leaf1-report.txt.
>
> Thanks.
>
> Cheers,
>
> --
> Dodji
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] abidiff: More compact references to prior diffs.
2020-04-07 13:47 ` Dodji Seketeli
@ 2020-04-07 21:32 ` Giuliano Procida
0 siblings, 0 replies; 13+ messages in thread
From: Giuliano Procida @ 2020-04-07 21:32 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: libabigail, kernel-team
All done.
v2 patches on the way.
Giuliano.
On Tue, 7 Apr 2020 at 14:47, Dodji Seketeli <dodji@seketeli.org> wrote:
>
> Hello,
>
> I like the patch overall, I just have some nits to pick I guess :-)
>
> Giuliano Procida <gprocida@google.com> a écrit:
>
> [...]
>
> > diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
> > index 883c815e..323de503 100644
> > --- a/src/abg-reporter-priv.cc
> > +++ b/src/abg-reporter-priv.cc
> > @@ -507,11 +507,11 @@ represent(const var_diff_sptr &diff,
> >
> > if (d->currently_reporting())
> > {
> > - out << " as being reported\n";
> > + out << ", as being reported\n";
> > }
>
> In general, throughout the code we try to only use braces when
> necessary. I know this change didn't create this situation here (I must
> have missed it from a previous patch or something) but could you please
> remove the brace here, for the sake of consistency with the rest of the
> code.
>
> > else if (d->reported_once())
> > {
> > - out << " as reported earlier\n";
> > + out << ", as reported earlier\n";
> > }
>
> Likewise.
>
> [...]
>
> > @@ -527,13 +527,20 @@ represent(const var_diff_sptr &diff,
> > if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
> > {
> > out << indent
> > - << "type of '" << pretty_representation << "' changed:\n";
> > + << "type of '" << pretty_representation << "' changed";
> > if (d->currently_reporting())
> > - out << indent << " details are being reported\n";
> > + {
> > + out << ", as being reported\n";
> > + }
>
> Likewise.
> > else if (d->reported_once())
> > - out << indent << " details were reported earlier\n";
> > + {
> > + out << ", as reported earlier\n";
> > + }
>
> Likewise.
> > else
> > - d->report(out, indent + " ");
> > + {
> > + out << ":\n";
> > + d->report(out, indent + " ");
> > + }
>
> Likewise.
>
> [...]
>
>
> Matthias Maennich <maennich@google.com> a écrit:
>
> [...]
>
> >>+++ b/src/abg-reporter-priv.cc
> >>@@ -507,11 +507,11 @@ represent(const var_diff_sptr &diff,
> >>
> >> if (d->currently_reporting())
> >> {
> >>- out << " as being reported\n";
> >>+ out << ", as being reported\n";
> >
> > No need for the braces.
>
> Agreed.
>
> Thank you Matthias for the review.
>
> Cheers,
>
> --
> Dodji
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/2] abidiff: More compact references to prior diffs.
2020-04-03 21:53 [PATCH 1/2] abidiff: More compact references to prior diffs Giuliano Procida
` (2 preceding siblings ...)
2020-04-07 13:47 ` Dodji Seketeli
@ 2020-04-07 21:33 ` Giuliano Procida
2020-04-07 21:33 ` [PATCH v2 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
2020-04-14 10:42 ` [PATCH v2 1/2] abidiff: More compact references to prior diffs Dodji Seketeli
3 siblings, 2 replies; 13+ messages in thread
From: Giuliano Procida @ 2020-04-07 21:33 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida
In both the default and leaf reporting modes, when there is a repeated
or circular reference to a type difference of a member variable, some
placeholder text is emitted instead.
In the leaf reporter:
type 'struct S' of 'foo::bar' changed as reported earlier
In the default reporter, this spans two lines:
type of 'S foo::bar' changed:
details were reported earlier
This patch changes the latter to the more compact:
type of 'S foo::bar' changed, as reported earlier
More generally, this patch makes the punctuation of such placeholder
text more consistent with a comma separating the phrases in all cases.
It doesn't attempt to reconcile the different formatting of member
variable declarations between the two modes.
* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
overload, use consistent punctuation and keep to a single line
of output when referring back to an existing type diff report.
Remove unnecessary braces around single line conditional
blocks.
* src/abg-reporter-priv.h: In the macro
RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER, use
consistent punctuation when referring back to an existing type
diff report.
* tests/data/test-abidiff/test-PR18791-report0.txt: Adjust
formatting of back references to existing type diff reports.
* tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
Ditto.
* tests/data/test-diff-filter/test16-report-2.txt: Ditto.
* tests/data/test-diff-filter/test17-1-report.txt: Ditto.
* tests/data/test-diff-filter/test25-cyclic-type-report-1.txt:
Ditto.
* tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
Ditto.
* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Ditto.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-reporter-priv.cc | 25 ++++++++---------
src/abg-reporter-priv.h | 8 +++---
.../test-abidiff/test-PR18791-report0.txt | 3 +--
.../PR25058-liblttng-ctl-report-1.txt | 22 +++++++--------
.../data/test-diff-filter/test16-report-2.txt | 2 +-
.../data/test-diff-filter/test17-1-report.txt | 2 +-
.../test25-cyclic-type-report-1.txt | 2 +-
...l7.x86_64-0.12.8-1.el7.x86_64-report-2.txt | 27 +++++++++----------
.../test-diff-suppr/test36-leaf-report-0.txt | 2 +-
9 files changed, 43 insertions(+), 50 deletions(-)
diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index 883c815e..eb7df341 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -506,13 +506,9 @@ represent(const var_diff_sptr &diff,
<< "' changed";
if (d->currently_reporting())
- {
- out << " as being reported\n";
- }
+ out << ", as being reported\n";
else if (d->reported_once())
- {
- out << " as reported earlier\n";
- }
+ out << ", as reported earlier\n";
else
{
out << ":\n";
@@ -527,13 +523,16 @@ represent(const var_diff_sptr &diff,
if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
{
out << indent
- << "type of '" << pretty_representation << "' changed:\n";
+ << "type of '" << pretty_representation << "' changed";
if (d->currently_reporting())
- out << indent << " details are being reported\n";
+ out << ", as being reported\n";
else if (d->reported_once())
- out << indent << " details were reported earlier\n";
+ out << ", as reported earlier\n";
else
- d->report(out, indent + " ");
+ {
+ out << ":\n";
+ d->report(out, indent + " ");
+ }
begin_with_and = true;
emitted = true;
@@ -715,10 +714,8 @@ represent(const var_diff_sptr &diff,
out << indent << "'" << pretty_representation
<< "' has *some* difference - please report as a bug";
else
- {
- // do nothing
- ;
- }
+ // do nothing
+ ;
emitted = true;
if (!begin_with_and)
diff --git a/src/abg-reporter-priv.h b/src/abg-reporter-priv.h
index b6c56fdb..b221fdc3 100644
--- a/src/abg-reporter-priv.h
+++ b/src/abg-reporter-priv.h
@@ -68,14 +68,14 @@
{ \
string _name_ = _diff_->first_subject()->get_pretty_representation(); \
if (_diff_->currently_reporting()) \
- out << indent << INTRO_TEXT << " '" << _name_ << "' changed; " \
- "details are being reported\n"; \
+ out << indent << INTRO_TEXT << " '" << _name_ \
+ << "' changed, as being reported\n"; \
else \
{ \
out << indent << INTRO_TEXT << " '" \
- << _name_ << "' changed"; \
+ << _name_ << "' changed"; \
report_loc_info(D->first_subject(), *d.context(), out); \
- out << ", as reported earlier\n"; \
+ out << ", as reported earlier\n"; \
} \
return ; \
} \
diff --git a/tests/data/test-abidiff/test-PR18791-report0.txt b/tests/data/test-abidiff/test-PR18791-report0.txt
index 93faf599..5bc2d08b 100644
--- a/tests/data/test-abidiff/test-PR18791-report0.txt
+++ b/tests/data/test-abidiff/test-PR18791-report0.txt
@@ -123,8 +123,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
type size changed from 128 to 192 (in bits)
1 data member change:
- type of 'std::__detail::_List_node_base std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed:
- details were reported earlier
+ type of 'std::__detail::_List_node_base std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed, as reported earlier
and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
diff --git a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
index dfb63684..44ddc6d6 100644
--- a/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
+++ b/tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt
@@ -179,8 +179,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
in pointed to type 'struct lttng_event_field':
type size hasn't changed
1 data member change:
- type of 'lttng_event lttng_event_field::event' changed:
- details were reported earlier
+ type of 'lttng_event lttng_event_field::event' changed, as reported earlier
[C] 'function int lttng_list_tracepoints(lttng_handle*, lttng_event**)' has some indirect sub-type changes:
parameter 2 of type 'lttng_event**' has sub-type changes:
@@ -232,7 +231,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
type size changed from 576 to 640 (in bits)
2 data member changes:
type of 'filter_node* filter_node::parent' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'union {struct {} unknown; struct {filter_node* child;} root; struct {__anonymous_enum__ type; ast_link_type post_op; ast_link_type pre_op; union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;} u; filter_node* prev; filter_node* next;} expression; struct {op_type type; filter_node* lchild; filter_node* rchild;} op; struct {unary_op_type type; filter_node* child;} unary_op;} filter_node::u' changed:
type size changed from 320 to 384 (in bits)
4 data member changes:
@@ -245,21 +244,20 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
type size hasn't changed
1 enumerator insertion:
'ast_link_type::AST_LINK_BRACKET' value '3'
- type of 'ast_link_type pre_op' changed:
- details were reported earlier
+ type of 'ast_link_type pre_op' changed, as reported earlier
type of 'union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;} u' changed:
type size hasn't changed
1 data member change:
type of 'filter_node* child' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type changed from:
union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;}
to:
union {char* string; uint64_t constant; double float_constant; char* identifier; filter_node* child;}
type of 'filter_node* prev' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'filter_node* next' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'struct {op_type type; filter_node* lchild; filter_node* rchild;} op' changed:
type size hasn't changed
3 data member changes:
@@ -278,14 +276,14 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
'op_type::AST_OP_BIT_OR' value '11'
'op_type::AST_OP_BIT_XOR' value '12'
type of 'filter_node* lchild' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'filter_node* rchild' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'struct {filter_node* child;} root' changed:
type size hasn't changed
1 data member change:
type of 'filter_node* child' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
type of 'struct {unary_op_type type; filter_node* child;} unary_op' changed:
type size hasn't changed
2 data member changes:
@@ -296,7 +294,7 @@ Variables changes summary: 0 Removed, 0 Changed, 3 Added variables
1 enumerator insertion:
'unary_op_type::AST_UNARY_BIT_NOT' value '4'
type of 'filter_node* child' changed:
- pointed to type 'struct filter_node' changed; details are being reported
+ pointed to type 'struct filter_node' changed, as being reported
'cds_list_head filter_ast::allocated_nodes' offset changed from 576 to 640 (in bits) (by +64 bits)
[C] 'function YYSTYPE* lttng_yyget_lval(yyscan_t)' has some indirect sub-type changes:
diff --git a/tests/data/test-diff-filter/test16-report-2.txt b/tests/data/test-diff-filter/test16-report-2.txt
index a90e85ad..f69eabdc 100644
--- a/tests/data/test-diff-filter/test16-report-2.txt
+++ b/tests/data/test-diff-filter/test16-report-2.txt
@@ -11,6 +11,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
'int S::m0', at offset 0 (in bits)
1 data member change:
type of 'S* S::m2' changed:
- pointed to type 'struct S' changed; details are being reported
+ pointed to type 'struct S' changed, as being reported
and offset changed from 0 to 64 (in bits) (by +64 bits)
diff --git a/tests/data/test-diff-filter/test17-1-report.txt b/tests/data/test-diff-filter/test17-1-report.txt
index 7c51152a..0b39c5d4 100644
--- a/tests/data/test-diff-filter/test17-1-report.txt
+++ b/tests/data/test-diff-filter/test17-1-report.txt
@@ -11,7 +11,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
'int S::m0', at offset 0 (in bits)
1 data member change:
type of 'S* S::m2' changed:
- pointed to type 'struct S' changed; details are being reported
+ pointed to type 'struct S' changed, as being reported
and offset changed from 0 to 64 (in bits) (by +64 bits)
[C] 'function void foo(S&)' has some indirect sub-type changes:
diff --git a/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt b/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
index 6a6beeef..0215d892 100644
--- a/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
+++ b/tests/data/test-diff-filter/test25-cyclic-type-report-1.txt
@@ -11,5 +11,5 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
'char S::m1', at offset 32 (in bits)
1 data member change:
type of 'S* S::m2' changed:
- pointed to type 'struct S' changed; details are being reported
+ pointed to type 'struct S' changed, as being reported
diff --git a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
index 6d9beb85..58c94b7a 100644
--- a/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
+++ b/tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt
@@ -37,7 +37,7 @@
13 data member changes:
type of 'QXLInstance* RedDispatcher::qxl' changed:
in pointed to type 'typedef QXLInstance' at spice-qxl.h:34:1:
- underlying type 'struct QXLInstance' changed; details are being reported
+ underlying type 'struct QXLInstance' changed, as being reported
type of 'Dispatcher RedDispatcher::dispatcher' changed:
underlying type 'struct Dispatcher' at dispatcher.h:22:1 changed:
type size changed from 960 to 1024 (in bits)
@@ -51,7 +51,7 @@
'int RedDispatcher::use_hardware_cursor' offset changed from 2240 to 2304 (in bits) (by +64 bits)
type of 'RedDispatcher* RedDispatcher::next' changed:
in pointed to type 'typedef RedDispatcher' at red_worker.h:87:1:
- underlying type 'struct RedDispatcher' changed; details are being reported
+ underlying type 'struct RedDispatcher' changed, as being reported
and offset changed from 2304 to 2368 (in bits) (by +64 bits)
'Ring RedDispatcher::async_commands' offset changed from 2368 to 2432 (in bits) (by +64 bits)
'pthread_mutex_t RedDispatcher::async_lock' offset changed from 2496 to 2560 (in bits) (by +64 bits)
@@ -185,7 +185,7 @@
1 data member change:
type of 'SpiceCharDeviceState* SpiceCharDeviceInstance::st' changed:
in pointed to type 'typedef SpiceCharDeviceState' at spice-char.h:34:1:
- underlying type 'struct SpiceCharDeviceState' changed; details are being reported
+ underlying type 'struct SpiceCharDeviceState' changed, as being reported
and offset changed from 896 to 960 (in bits) (by +64 bits)
'int SpiceCharDeviceState::during_read_from_device' offset changed from 960 to 1024 (in bits) (by +64 bits)
'int SpiceCharDeviceState::during_write_to_device' offset changed from 992 to 1056 (in bits) (by +64 bits)
@@ -223,7 +223,7 @@
2 data member changes (2 filtered):
type of 'RedChannel* RedChannelClient::channel' changed:
in pointed to type 'typedef RedChannel' at red_channel.h:130:1:
- underlying type 'struct RedChannel' changed; details are being reported
+ underlying type 'struct RedChannel' changed, as being reported
type of 'RedsStream* RedChannelClient::stream' changed:
in pointed to type 'typedef RedsStream' at reds_stream.h:31:1:
underlying type 'struct RedsStream' at reds.h:68:1 changed:
@@ -391,7 +391,7 @@
in pointed to type 'function type void (RedChannel*, RedClient*, RedsStream*, int, int, uint32_t*, int, uint32_t*)':
parameter 1 of type 'RedChannel*' has sub-type changes:
in pointed to type 'typedef RedChannel' at red_channel.h:130:1:
- underlying type 'struct RedChannel' changed; details are being reported
+ underlying type 'struct RedChannel' changed, as being reported
parameter 3 of type 'RedsStream*' has sub-type changes:
pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
type of 'channel_client_disconnect_proc disconnect' changed:
@@ -524,7 +524,7 @@
pointed to type 'typedef RedsStream' changed at red_channel.h:134:1, as reported earlier
type of 'SndWorker* SndChannel::worker' changed:
in pointed to type 'typedef SndWorker' at snd_worker.c:80:1:
- underlying type 'struct SndWorker' changed; details are being reported
+ underlying type 'struct SndWorker' changed, as being reported
type of 'RedChannelClient* SndChannel::channel_client' changed:
pointed to type 'typedef RedChannelClient' changed at red_channel.h:136:1, as reported earlier
type of 'snd_channel_handle_message_proc SndChannel::handle_message' changed:
@@ -532,26 +532,26 @@
in pointed to type 'function type int (SndChannel*, typedef size_t, typedef uint32_t, void*)':
parameter 1 of type 'SndChannel*' has sub-type changes:
in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
- underlying type 'struct SndChannel' changed; details are being reported
+ underlying type 'struct SndChannel' changed, as being reported
type of 'snd_channel_on_message_done_proc SndChannel::on_message_done' changed:
underlying type 'void (SndChannel*)*' changed:
in pointed to type 'function type void (SndChannel*)':
parameter 1 of type 'SndChannel*' has sub-type changes:
in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
- underlying type 'struct SndChannel' changed; details are being reported
+ underlying type 'struct SndChannel' changed, as being reported
type of 'snd_channel_cleanup_channel_proc SndChannel::cleanup' changed:
underlying type 'void (SndChannel*)*' changed:
in pointed to type 'function type void (SndChannel*)':
parameter 1 of type 'SndChannel*' has sub-type changes:
in pointed to type 'typedef SndChannel' at snd_worker.c:74:1:
- underlying type 'struct SndChannel' changed; details are being reported
+ underlying type 'struct SndChannel' changed, as being reported
no data member change (1 filtered);
type of 'SndWorker* SndWorker::next' changed:
in pointed to type 'typedef SndWorker' at snd_worker.c:80:1:
- underlying type 'struct SndWorker' changed; details are being reported
+ underlying type 'struct SndWorker' changed, as being reported
type of 'SpicePlaybackInstance* SpicePlaybackState::sin' changed:
in pointed to type 'typedef SpicePlaybackInstance' at spice-audio.h:33:1:
- underlying type 'struct SpicePlaybackInstance' changed; details are being reported
+ underlying type 'struct SpicePlaybackInstance' changed, as being reported
[C] 'function void spice_server_playback_put_samples(SpicePlaybackInstance*, uint32_t*)' at snd_worker.c:1100:1 has some indirect sub-type changes:
parameter 1 of type 'SpicePlaybackInstance*' has sub-type changes:
@@ -590,11 +590,10 @@
1 data member insertion:
'uint32_t SpiceRecordState::frequency', at offset 512 (in bits) at snd_worker.c:166:1
2 data member changes:
- type of 'SndWorker SpiceRecordState::worker' changed:
- details were reported earlier
+ type of 'SndWorker SpiceRecordState::worker' changed, as reported earlier
type of 'SpiceRecordInstance* SpiceRecordState::sin' changed:
in pointed to type 'typedef SpiceRecordInstance' at spice-audio.h:67:1:
- underlying type 'struct SpiceRecordInstance' changed; details are being reported
+ underlying type 'struct SpiceRecordInstance' changed, as being reported
[C] 'function void spice_server_record_set_mute(SpiceRecordInstance*, uint8_t)' at snd_worker.c:1279:1 has some indirect sub-type changes:
parameter 1 of type 'SpiceRecordInstance*' has sub-type changes:
diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
index d270740d..9caa9428 100644
--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
@@ -15,7 +15,7 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
'struct leaf2 at test36-leaf-v0.cc:9:1' changed:
type size changed from 64 to 96 (in bits)
there are data member changes:
- type 'struct leaf1' of 'leaf2::member0' changed as reported earlier
+ type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
and size changed from 32 to 64 (in bits) (by +32 bits)
'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
3 impacted interfaces:
--
2.26.0.110.g2183baf09c-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/2] abidiff: Fix anonymous member size change reports.
2020-04-07 21:33 ` [PATCH v2 " Giuliano Procida
@ 2020-04-07 21:33 ` Giuliano Procida
2020-04-14 10:42 ` Dodji Seketeli
2020-04-14 10:42 ` [PATCH v2 1/2] abidiff: More compact references to prior diffs Dodji Seketeli
1 sibling, 1 reply; 13+ messages in thread
From: Giuliano Procida @ 2020-04-07 21:33 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida
Data members have names (unless anonymous), types, sizes and
offsets (if a member of a class or struct). It is the responsibility
of the represent function to detail all differences of a changed
member. The size of a member is the same as the size of its type.
A recent change to the function focused on cleaning up the formatting
logic and added a catch-all case to the end of it in case no diff
information had been emitted by then.
This catch-all triggered for anonymous structs and union diffs under
certain circumstances when these anonymous members had changed in
size.
This patch ensures size change information for a member is emitted
exactly once (unless the type change has been or is being reported
already). It also ensures anonymous members are identified
appropriately and includes a test case would otherwise reach the
catch-all case in both default and --leaf-changes-only modes.
* src/abg-reporter-priv.cc (represent): In the var_diff_sptr
overload, factor out some expressions as local variables, rely
on diff_to_be_reported to decide whether to emit a change,
fold together local/non-local change reporting using
local_changes to preserve current name formatting differences,
keep track explicitly of whether size information has been
emitted and ensure it happens if needed, make offset and size
change reporting for anonymous data members more meaningful.
* tests/test-abidiff-exit.cc: Run new test cases.
* tests/data/Makefile.am: Add new test files.
* tests/data/test-abidiff-exit/test-member-size-v0.cc: New
test.
* tests/data/test-abidiff-exit/test-member-size-v0.o: Ditto.
* tests/data/test-abidiff-exit/test-member-size-v1.cc: Ditto.
* tests/data/test-abidiff-exit/test-member-size-v1.o: Ditto.
* tests/data/test-abidiff-exit/test-member-size-report0.txt:
New test, default mode.
* tests/data/test-abidiff-exit/test-member-size-report1.txt:
New test, --leaf-changes-only mode.
* tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
Eliminate duplicate reporting of member sizes.
* tests/data/test-abidiff-exit/test-leaf-more-report.txt: Ditto.
* tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
Ditto.
* tests/data/test-abidiff-exit/test-no-stray-comma-report.txt:
Ditto.
* tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
Add size report for anonymous data member.
* tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
Ditto.
* tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt:
Add missing size change report.
* tests/data/test-diff-suppr/test36-leaf-report-0.txt: Remove
size change report for previously reported type.
* tests/data/test-diff-suppr/test46-PR25128-report-1.txt:
Eliminate duplicate reporting of member size change.
* tests/data/test-diff-suppr/test46-PR25128-report-2.txt:
Ditto.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-reporter-priv.cc | 94 +++++++-----------
tests/data/Makefile.am | 6 ++
.../test-leaf-cxx-members-report.txt | 4 +-
.../test-leaf-more-report.txt | 1 -
.../test-leaf-peeling-report.txt | 1 -
.../test-member-size-report0.txt | 27 +++++
.../test-member-size-report1.txt | 21 ++++
.../test-abidiff-exit/test-member-size-v0.cc | 26 +++++
.../test-abidiff-exit/test-member-size-v0.o | Bin 0 -> 3184 bytes
.../test-abidiff-exit/test-member-size-v1.cc | 27 +++++
.../test-abidiff-exit/test-member-size-v1.o | Bin 0 -> 3192 bytes
.../test-no-stray-comma-report.txt | 1 -
.../test45-anon-dm-change-report-0.txt | 1 +
.../test44-anonymous-data-member-report-0.txt | 1 +
...-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt | 2 +-
.../test-diff-suppr/test36-leaf-report-0.txt | 1 -
.../test46-PR25128-report-1.txt | 1 -
.../test46-PR25128-report-2.txt | 1 -
tests/test-abidiff-exit.cc | 18 ++++
19 files changed, 167 insertions(+), 66 deletions(-)
create mode 100644 tests/data/test-abidiff-exit/test-member-size-report0.txt
create mode 100644 tests/data/test-abidiff-exit/test-member-size-report1.txt
create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.cc
create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.o
create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.cc
create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.o
diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index eb7df341..5fb5fbc5 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -411,16 +411,26 @@ represent(const var_diff_sptr &diff,
var_decl_sptr o = diff->first_var();
var_decl_sptr n = diff->second_var();
+ string name1 = o->get_qualified_name();
+ string name2 = n->get_qualified_name();
+ uint64_t size1 = get_var_size_in_bits(o);
+ uint64_t size2 = get_var_size_in_bits(n);
+ uint64_t offset1 = get_data_member_offset(o);
+ uint64_t offset2 = get_data_member_offset(n);
// Has the main diff text been output?
bool emitted = false;
// Are we continuing on a new line? (implies emitted)
bool begin_with_and = false;
- string name1 = o->get_qualified_name();
- string name2 = n->get_qualified_name();
- string pretty_representation = o->get_pretty_representation();
+ // Have we reported a size change already?
+ bool size_reported = false;
+ // Are we reporting a change to an anonymous member?
bool is_strict_anonymous_data_member_change = false;
+ string pretty_representation = o->get_pretty_representation();
+ bool show_size_offset_changes = ctxt->get_allowed_category()
+ & SIZE_OR_OFFSET_CHANGE_CATEGORY;
+
if (is_anonymous_data_member(o) && is_anonymous_data_member(n))
{
is_strict_anonymous_data_member_change = true;
@@ -430,8 +440,7 @@ represent(const var_diff_sptr &diff,
if (tr1 != tr2)
{
show_offset_or_size(indent + "anonymous data member at offset",
- get_data_member_offset(o),
- *ctxt, out);
+ offset1, *ctxt, out);
out << " changed from:\n"
<< indent << " " << tr1 << "\n"
@@ -460,7 +469,6 @@ represent(const var_diff_sptr &diff,
begin_with_and = true;
emitted = true;
}
- // TODO: else ...?
}
else if (filtering::has_anonymous_data_member_change(diff))
{
@@ -472,9 +480,7 @@ represent(const var_diff_sptr &diff,
+ o->get_pretty_representation()
+ " at offset";
- show_offset_or_size(indent + repr,
- get_data_member_offset(o),
- *ctxt, out);
+ show_offset_or_size(indent + repr, offset1, *ctxt, out);
out << " became data member '"
<< n->get_pretty_representation()
<< "'\n";
@@ -486,9 +492,7 @@ represent(const var_diff_sptr &diff,
+ o->get_pretty_representation()
+ " at offset";
- show_offset_or_size(indent + repr,
- get_data_member_offset(o),
- *ctxt, out);
+ show_offset_or_size(indent + repr, offset1, *ctxt, out);
out << " became anonymous data member '"
<< n->get_pretty_representation()
<< "'\n";
@@ -499,11 +503,15 @@ represent(const var_diff_sptr &diff,
}
else if (diff_sptr d = diff->type_diff())
{
- if (local_only && d->has_local_changes())
+ if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
{
- out << indent << "type '" << get_pretty_representation(o->get_type())
- << "' of '" << o->get_qualified_name()
- << "' changed";
+ if (local_only)
+ out << indent << "type '" << get_pretty_representation(o->get_type())
+ << "' of '" << o->get_qualified_name()
+ << "' changed";
+ else
+ out << indent
+ << "type of '" << pretty_representation << "' changed";
if (d->currently_reporting())
out << ", as being reported\n";
@@ -517,26 +525,7 @@ represent(const var_diff_sptr &diff,
begin_with_and = true;
emitted = true;
- }
- else
- {
- if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
- {
- out << indent
- << "type of '" << pretty_representation << "' changed";
- if (d->currently_reporting())
- out << ", as being reported\n";
- else if (d->reported_once())
- out << ", as reported earlier\n";
- else
- {
- out << ":\n";
- d->report(out, indent + " ");
- }
-
- begin_with_and = true;
- emitted = true;
- }
+ size_reported = true;
}
}
@@ -581,9 +570,9 @@ represent(const var_diff_sptr &diff,
out << "now becomes laid out";
emitted = true;
}
- if ((ctxt->get_allowed_category() & SIZE_OR_OFFSET_CHANGE_CATEGORY))
+ if (show_size_offset_changes)
{
- if (get_data_member_offset(o) != get_data_member_offset(n))
+ if (offset1 != offset2)
{
if (begin_with_and)
{
@@ -592,26 +581,20 @@ represent(const var_diff_sptr &diff,
}
else if (!emitted)
{
+ out << indent;
if (is_strict_anonymous_data_member_change)
- out << indent;
- else
- out << indent << "'" << pretty_representation << "' ";
+ out << "anonymous data member ";
+ out << "'" << pretty_representation << "' ";
}
else
out << ", ";
- show_numerical_change("offset",
- get_data_member_offset(o),
- get_data_member_offset(n),
- *ctxt, out);
+ show_numerical_change("offset", offset1, offset2, *ctxt, out);
maybe_show_relative_offset_change(diff, *ctxt, out);
-
emitted = true;
}
- if (// If we are not displaying only local changes, we must
- // have indicated the type size change already.
- local_only
- && (get_var_size_in_bits(o) != get_var_size_in_bits(n)))
+
+ if (!size_reported && size1 != size2)
{
if (begin_with_and)
{
@@ -620,18 +603,15 @@ represent(const var_diff_sptr &diff,
}
else if (!emitted)
{
+ out << indent;
if (is_strict_anonymous_data_member_change)
- out << indent;
- else
- out << indent << "'" << pretty_representation << "' ";
+ out << "anonymous data member ";
+ out << "'" << pretty_representation << "' ";
}
else
out << ", ";
- show_numerical_change("size",
- get_var_size_in_bits(o),
- get_var_size_in_bits(n),
- *ctxt, out);
+ show_numerical_change("size", size1, size2, *ctxt, out);
maybe_show_relative_size_change(diff, *ctxt, out);
emitted = true;
}
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 565c328f..6e926ebd 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -140,6 +140,12 @@ test-abidiff-exit/test-leaf-cxx-members-v0.o \
test-abidiff-exit/test-leaf-cxx-members-v1.cc \
test-abidiff-exit/test-leaf-cxx-members-v1.o \
test-abidiff-exit/test-leaf-cxx-members-report.txt \
+test-abidiff-exit/test-member-size-v0.cc \
+test-abidiff-exit/test-member-size-v0.o \
+test-abidiff-exit/test-member-size-v1.cc \
+test-abidiff-exit/test-member-size-v1.o \
+test-abidiff-exit/test-member-size-report0.txt \
+test-abidiff-exit/test-member-size-report1.txt \
\
test-diff-dwarf/test0-v0.cc \
test-diff-dwarf/test0-v0.o \
diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
index 082db83e..eae74a3b 100644
--- a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
+++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
@@ -35,8 +35,8 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type 'int' of 'ops::deleted_var' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
- and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1, size changed from 32 to 64 (in bits) (by +32 bits)
+ and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1
type 'int' of 'ops::changed_var' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
- and offset changed from 96 to 128 (in bits) (by +32 bits), size changed from 32 to 64 (in bits) (by +32 bits)
+ and offset changed from 96 to 128 (in bits) (by +32 bits)
diff --git a/tests/data/test-abidiff-exit/test-leaf-more-report.txt b/tests/data/test-abidiff-exit/test-leaf-more-report.txt
index 9af6ccaa..aea08b81 100644
--- a/tests/data/test-abidiff-exit/test-leaf-more-report.txt
+++ b/tests/data/test-abidiff-exit/test-leaf-more-report.txt
@@ -40,4 +40,3 @@ Removed/Changed/Added variables summary: 1 Removed, 1 Changed, 1 Added variable
type 'int' of 'changed::foo' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
- and size changed from 32 to 64 (in bits) (by +32 bits)
diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
index bd5f36f8..7ab4dd6e 100644
--- a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
@@ -9,7 +9,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type 'int' of 'foo::z' changed:
type name changed from 'int' to 'long int'
type size changed from 32 to 64 (in bits)
- and size changed from 32 to 64 (in bits) (by +32 bits)
'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
type size hasn't changed
diff --git a/tests/data/test-abidiff-exit/test-member-size-report0.txt b/tests/data/test-abidiff-exit/test-member-size-report0.txt
new file mode 100644
index 00000000..ff93dd50
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-member-size-report0.txt
@@ -0,0 +1,27 @@
+Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
+2 functions with some indirect sub-type change:
+
+ [C] 'function void reg1(S*, T*, T*)' at test-member-size-v1.cc:26:1 has some indirect sub-type changes:
+ parameter 1 of type 'S*' has sub-type changes:
+ in pointed to type 'struct S' at test-member-size-v1.cc:3:1:
+ type size changed from 128 to 192 (in bits)
+ 1 data member insertion:
+ 'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
+ no data member change (1 filtered);
+ parameter 2 of type 'T*' has sub-type changes:
+ in pointed to type 'struct T' at test-member-size-v1.cc:14:1:
+ type size changed from 192 to 256 (in bits)
+ 2 data member changes:
+ 'S T::s' size changed from 128 to 192 (in bits) (by +64 bits)
+ 'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
+
+ [C] 'function void reg2(U*)' at test-member-size-v1.cc:27:1 has some indirect sub-type changes:
+ parameter 1 of type 'U*' has sub-type changes:
+ in pointed to type 'struct U' at test-member-size-v1.cc:19:1:
+ type size changed from 192 to 256 (in bits)
+ 2 data member changes:
+ anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
+ 'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
+
diff --git a/tests/data/test-abidiff-exit/test-member-size-report1.txt b/tests/data/test-abidiff-exit/test-member-size-report1.txt
new file mode 100644
index 00000000..dbad76be
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-member-size-report1.txt
@@ -0,0 +1,21 @@
+Leaf changes summary: 3 artifacts changed
+Changed leaf types summary: 3 leaf types changed
+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
+
+'struct S at test-member-size-v0.cc:3:1' changed:
+ type size changed from 128 to 192 (in bits)
+ 1 data member insertion:
+ 'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
+
+'struct T at test-member-size-v0.cc:13:1' changed:
+ type size changed from 192 to 256 (in bits)
+ there are data member changes:
+ type 'struct S' of 'T::s' changed, as reported earlier
+ 'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
+
+'struct U at test-member-size-v0.cc:18:1' changed:
+ type size changed from 192 to 256 (in bits)
+ there are data member changes:
+ anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
+ 'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.cc b/tests/data/test-abidiff-exit/test-member-size-v0.cc
new file mode 100644
index 00000000..f974a37f
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-member-size-v0.cc
@@ -0,0 +1,26 @@
+struct M;
+
+struct S {
+ int x;
+ M * m;
+};
+
+struct M {
+ int a;
+ S * s;
+};
+
+struct T {
+ S s;
+ int a;
+};
+
+struct U {
+ struct {
+ S s;
+ };
+ int r;
+};
+
+void reg1(S*, T*, T*) { }
+void reg2(U*) { }
diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.o b/tests/data/test-abidiff-exit/test-member-size-v0.o
new file mode 100644
index 0000000000000000000000000000000000000000..e020bef77d57d601420f969061707bbf1486ac99
GIT binary patch
literal 3184
zcmbtWL2MgU5S{P!#_PCtoVcY4LC91fsLC!`E2`o`ifKxds<Z@BqY4SqwX=yW;&rs%
z6t@VFPzA~@m5>058xj(?a^Q}H#JR_E;{Xyz;Dp2hX8!(hHs8(#M)J=5H}hux|2zM$
zKYaO>ODQ3MMZkGD7%>WPKXYK$q*;R!OhJ8R@7I;R_y1bi+x>lIum0QqTR$*_<}#*A
z%V<j`jqQU>@OZ*btwB*3)~g5^eGuskSzvTPn3+CW7NpZ=JBT246$34cFvA0pRe?Pa
z<Cza+plQ8<+O%wZrmn-Z$Xj1iG=>yR0wr-m5{r<kW5ks85aF?+D2@N9LXT%;C}h5s
zj1a^lieV8T9vvgal2eBG(!jN5>5;nE!bwo&m@spQP#O>}5`kupge(nG_iieE0~6e(
zKrtzvDwJ|YVYV<SrZUBo7*)dU%H_`Gp3hZt0@E3JFsyiZxwL>2N<Rs?-1(e=<;@8+
zFJsVHNhCR$JjAh<BVL>_3Nby9MQYQZU6`T7Srda&(>RT1X_hpfcuCTtNAg7uKr~*%
zp8>RwLi}CG<5M;pu2Deb_aeLFcN%_Z_u6-S`_`P(Y)ZCy60&g_;!g{6=1+5C(VYA!
zWnhXUi0g08hyGS&)vW_+mmU>(F6!(+>~UA!wN>}}T4f!Uuhq*-XU@3u<*HM0+;U~k
zt-9`9wQP5y-N0Ym@&i9?H*x5N&DLUn;jDdjzHD!m?M<(DJ7~7TZqVKB*}K8J?O?->
zyvDZQ16W>KI#+(?qTgtHL7drktEaUZa)HO?6L;ax>md3?F?%dy?c=UIhB*C5aN_}9
zDil9XE#=3qVE_)v_X6Q4vg6o)DfY*n;U}iXllaF!!ybA+<Q*16y=mQJAY+sm>a6QN
z0}l25z->U{QW>B$pKM}uU>fm?4*+EukRQSbFraV*eryP*-kwVAgWP+>PAi<pe9Dh%
z-l0xQG$j?~ofuEpg!7CDr`2B|5M?q}z_=R12@R)nNiq{Fxst&FLX6fyLV4t6Bu4;0
zDb4wxR^y*F9IqQGzbH)|L-~1M##IY(o>MArQo~(^pHMjGQShPtF&RW%RJI<^XF<oy
zDxT(;+=rTm^Zdc--R|HqMjeH*Zm|{l{Ro^5FY>@?^m^cg{<i0A_>JAI^>(n?MW3|2
zMkDlZ9ddi27i{6pkfdz41D`2a0-R>I)A8}U<^<iyckpVmdy&_CN0I#2`ex{Ld^Pd^
zsb-N+d&Pao>1xW*?{SJv$`d#sOuqr%P6j1xolpHm{r6w_)IWMoD#!J&XijY?zhCq(
zq(-EK?5JidD+2W!#qn98`+;N;e|k>Bzfs^PG)RaZyNW&?qwqP^q;m-_nR@IM#7513
zU%gP~)BNdn)#tx4A}2y{Q`N82AR&7Fb@Yv@|Glcu`$YBW9@FdZAVxWPo@h$>PSyXM
z0j0S<>-P{J#lNZ=itjH@!#vh^6`%dABfTM&gLxeH1sa5@=`sv`#lJB?VuRPu@TX{u
z+P{Y*-XDFvn9ut<sy5|*pz62Ogz39MUqziy?_|_c7gYbxqdzJ7`YG=yzD3Q3@>Q$`
HPo4iS_W$I+
literal 0
HcmV?d00001
diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.cc b/tests/data/test-abidiff-exit/test-member-size-v1.cc
new file mode 100644
index 00000000..b3bd5988
--- /dev/null
+++ b/tests/data/test-abidiff-exit/test-member-size-v1.cc
@@ -0,0 +1,27 @@
+struct M;
+
+struct S {
+ int x;
+ M * m;
+ int y;
+};
+
+struct M {
+ int a;
+ S * s;
+};
+
+struct T {
+ S s;
+ int a;
+};
+
+struct U {
+ struct {
+ S s;
+ };
+ int r;
+};
+
+void reg1(S*, T*, T*) { }
+void reg2(U*) { }
diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.o b/tests/data/test-abidiff-exit/test-member-size-v1.o
new file mode 100644
index 0000000000000000000000000000000000000000..eaec9ebead662bc23c00130c537133a5be5c2019
GIT binary patch
literal 3192
zcmbtW&u<%55T57F#_Me3IB}ySq(VjlK~;9iMp4zYq?iWUR6q$$jiO2r*3QPZ8gHug
zx;RCs2oT68S_z3FapNyQ;?M&JBraUJaYO<nBo6#IA#s42w{Psti*tdIyfg2c`DWgG
zJMXRUJpbawj1a&g;5-aQi~>B!4(zft%TR<VsI2vVUhBR6*IKXh`&zH^%iY($VF=Bu
zm}*2uTQX^44>G~SDLb<Pv%;{hBV=?zjAY3IqXojuzK51&?1LDUXaPn>%w`xv<|QbI
zoJDTK2Qj8NZ4h?0M;tIMAGK*Y_{>~`lE_<MP&9^2m;_2<K@w*nQ^AO$^&`Tgv*P&J
zcPex`D?=gsm1IO9W)#CBK+KPlVzHGWKFi~>bM(mEYv3fPazc3M0HN3?yi5d|ITCU-
z$lQBF=}k=V5e14#@mRiS8~OSCq?pPUW-zLVJ7?Qx?Q{0BEij!i4u+KsFBi|?ghn0(
z+dgj_Sl*mK^D+jF7Dbwq9fu^=a>X-qMn0j3vPgOMlM8c{IA>x|W)`RMB+ZiMlPpPE
z^hm$cK8VIE_%nd^QHa0u<M@<)hieoN2kqEt1+8ijIql}{z`5nQwVGs`GmuNl5PwdX
zbAOr>E9T_883R)sL5zaB2V2*dP%o}~6(9{y#jTqP^wzzNb?@57;uc)GQYo#TI^`{u
zmfc0yD=jW~%bvHeTyk1*Ck$5VK^R2M8V>!a)>!GDIqjTYDmnF%v*Wk-!&)QS3->y0
zrxU)}47Z)wukHqIfJ>{ZXG>2!7gU>mm}GXm<&#<sfa7wFxCOVbg6I~+{IRTc7q{dD
z;`AfIT?cqJU-%%iIzD;@18_*brwK=q?Z*C#i9hiieqw4ok$?Pi*h4Reyobe5uUYpP
z$QUMuI`g{EfCEk6cN>tpT>2=@Czl!xOe0zG08o|z<A*Q;49FjWA3KCoV^5{_LGC?b
zCl$_PKH^7pZqj?HHcK@l73G~6OWB0;j0mUIUmy@=GEu;|8p5=O)1f4pOO#wm|MVb6
z>mZ>#@+OiafS;7+{Ew^guNuxh`J2M2V<<oG%d~1C&VzT8loJ~6DZHd`&ZFQ1`(rYQ
zx}a=5p3j1gmsC8>F})9E4d?lT+um>C8AcsPiEgkJ2i+LlZ9n$Gt+w0XM!~M{ZU@y)
zeXAMn?4eKEezh6}w+^`d$PepyEu<;C%`jjJmH@Z5*J=g$QFFt+IB@Y^a@w(9yP-%y
zV{0e!TY;MR|5WqHr@i7n<aA|a=$AOfrsWYF5T@S%ucw1jw$7*iqW=4@eCi)PCza#+
zuWC+hC_i5GZ=_75lyp_IabHruQ5>HYx*teZ@Tcb_{7VIXNQ0E<vFqs5F$zDYnsgz>
zB~y=m0kL88zpq{>^J)HcyXy1b9+DFwxS{G-Xpj=U{ucU%)&Ex2=Y693bdTxvZz4uH
zd7fxW`C8S#&w$cgpY>aa594pDhT{8+(=d<q`-;zg)_3%VR1W5G+^1*|rl!j<yrcM=
zeIzk>{S1GM#<2bSVaWTVuNU)qUx(GE+~2GE4K-o<ZqQdz=hHhGw$!rf|0VRNMPEPV
R9mX$Ev!Q&RsQy#u{|i5f=e7U<
literal 0
HcmV?d00001
diff --git a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
index 1583a40b..52c48b3d 100644
--- a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
+++ b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
@@ -10,4 +10,3 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type name changed from 'int[1]' to 'int[2]'
array type size changed from 32 to 64
array type subrange 1 changed length from 1 to 2
- and size changed from 32 to 64 (in bits) (by +32 bits)
diff --git a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
index 12af287b..2f8f31d0 100644
--- a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
+++ b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
@@ -13,6 +13,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
'char S1::m1' offset changed from 64 to 40 (in bits) (by -24 bits)
1 data member change:
anonymous data member struct {int m0; char m01;} at offset 0 (in bits) became data member 'int S1::m0'
+ and size changed from 64 to 32 (in bits) (by -32 bits)
[C] 'function void foo(S0&)' has some indirect sub-type changes:
parameter 1 of type 'S0&' has sub-type changes:
diff --git a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
index b8bb951c..ed68b550 100644
--- a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
+++ b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
@@ -12,5 +12,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
struct {int b; float c;}
to:
struct {int b; float c; char e;}
+ and size changed from 64 to 96 (in bits) (by +32 bits)
'int S2::d' offset changed from 96 to 128 (in bits) (by +32 bits)
diff --git a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
index b1611303..2a679d60 100644
--- a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
+++ b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
@@ -78,7 +78,7 @@
type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
type size changed from 128 to 192 (in bits)
1 data member change:
- name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
+ name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node', size changed from 128 to 192 (in bits) (by +64 bits)
and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
================ end of changes of 'libsigc-2.0.so.0.0.0'===============
diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
index 9caa9428..fcbb9232 100644
--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
@@ -16,7 +16,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type size changed from 64 to 96 (in bits)
there are data member changes:
type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
- and size changed from 32 to 64 (in bits) (by +32 bits)
'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
3 impacted interfaces:
function void interface1(struct_type*)
diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
index 36439ce7..acff860c 100644
--- a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
+++ b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
@@ -13,5 +13,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
type size changed from 64 to 192 (in bits)
- and size changed from 64 to 192 (in bits) (by +128 bits)
'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
index 67dbf58f..79827103 100644
--- a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
+++ b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
@@ -9,5 +9,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
type size changed from 64 to 192 (in bits)
- and size changed from 64 to 192 (in bits) (by +128 bits)
'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
index 68950275..25f58b34 100644
--- a/tests/test-abidiff-exit.cc
+++ b/tests/test-abidiff-exit.cc
@@ -176,6 +176,24 @@ InOutSpec in_out_specs[] =
"data/test-abidiff-exit/test-leaf-cxx-members-report.txt",
"output/test-abidiff-exit/test-leaf-cxx-members-report.txt"
},
+ {
+ "data/test-abidiff-exit/test-member-size-v0.o",
+ "data/test-abidiff-exit/test-member-size-v1.o",
+ "",
+ "",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+ "data/test-abidiff-exit/test-member-size-report0.txt",
+ "output/test-abidiff-exit/test-member-size-report0.txt"
+ },
+ {
+ "data/test-abidiff-exit/test-member-size-v0.o",
+ "data/test-abidiff-exit/test-member-size-v1.o",
+ "",
+ "--leaf-changes-only",
+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
+ "data/test-abidiff-exit/test-member-size-report1.txt",
+ "output/test-abidiff-exit/test-member-size-report1.txt"
+ },
{0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
};
--
2.26.0.110.g2183baf09c-goog
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] abidiff: Fix anonymous member size change reports.
2020-04-06 16:08 ` Matthias Maennich
2020-04-07 13:55 ` Dodji Seketeli
@ 2020-04-07 21:45 ` Giuliano Procida
1 sibling, 0 replies; 13+ messages in thread
From: Giuliano Procida @ 2020-04-07 21:45 UTC (permalink / raw)
To: Matthias Maennich; +Cc: libabigail, Dodji Seketeli, kernel-team
Hi.
On Mon, 6 Apr 2020 at 17:08, Matthias Maennich <maennich@google.com> wrote:
>
> On Fri, Apr 03, 2020 at 10:53:55PM +0100, Android Kernel Team wrote:
> >Data members have names (unless anonymous), types, sizes and
> >offsets (if a member of a class or struct). It is the responsibility
> >of the represent function to detail all differences of a changed
> >member. The size of a member is the same as the size of its type.
> >
> >A recent change to the function focused on cleaning up the formatting
> >logic and added a catch-all case to the end of it in case no diff
> >information had been emitted by then.
> >
> >This catch-all triggered for anonymous structs and union diffs under
> >certain circumstances when these anonymous members had changed in
> >size.
> >
> >This patch ensures size change information for a member is emitted
> >exactly once (unless the type change has been or is being reported
> >already). It also ensures anonymous members are identified
> >appropriately and includes a test case would otherwise reach the
> >catch-all case in both default and --leaf-changes-only modes.
> >
> > * src/abg-reporter-priv.cc (represent): In the var_diff_sptr
> > overload, factor out some expressions as local variables, rely
> > on diff_to_be_reported to decide whether to emit a change,
> > fold together local/non-local change reporting using
> > local_changes to preserve current name formatting differences,
> > keep track explicitly of whether size information has been
> > emitted and ensure it happens if needed, make offset and size
> > change reporting for anonymous data members more meaningful.
> > * tests/test-abidiff-exit.cc: Run new test cases.
> > * tests/data/Makefile.am: Add new test files.
> > * tests/data/test-abidiff-exit/test-member-size-v0.cc: New
> > test.
> > * tests/data/test-abidiff-exit/test-member-size-v0.o: Ditto.
> > * tests/data/test-abidiff-exit/test-member-size-v1.cc: Ditto.
> > * tests/data/test-abidiff-exit/test-member-size-v1.o: Ditto.
> > * tests/data/test-abidiff-exit/test-member-size-report0.txt:
> > New test, default mode.
> > * tests/data/test-abidiff-exit/test-member-size-report1.txt:
> > New test, --leaf-changes-only mode.
> > * tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
> > Eliminate duplicate reporting of member sizes.
> > * tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
> > Ditto.
> > * tests/data/test-abidiff-exit/test-leaf1-report.txt: Ditto.
> > * tests/data/test-abidiff-exit/test-no-stray-comma-report.txt:
> > Ditto.
> > * tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
> > Add size report for anonymous data member.
> > * tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
> > Ditto.
> > * tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt:
> > Add missing size change report.
> > * tests/data/test-diff-suppr/test36-leaf-report-0.txt: Remove
> > size change report for previously reported type.
> > * tests/data/test-diff-suppr/test46-PR25128-report-1.txt:
> > Eliminate duplicate reporting of member size change.
> > * tests/data/test-diff-suppr/test46-PR25128-report-2.txt:
> > Ditto.
> >
>
> This broke one test case on my machine, that needs trivial adjusting:
This was due to commit ordering changing, it's a simple rebase and
reword to fix.
> test-leaf-peeling-report-indirect.txt:
>
> | @@ -9,7 +9,6 @@
> | type 'int' of 'foo::z' changed:
> | type name changed from 'int' to 'long int'
> | type size changed from 32 to 64 (in bits)
> | - and size changed from 32 to 64 (in bits) (by +32 bits)
> |
> | 'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
> | type size hasn't changed
> | FAIL runtestabidiffexit (exit status: 1)
>
>
> >Signed-off-by: Giuliano Procida <gprocida@google.com>
> >---
> > src/abg-reporter-priv.cc | 98 +++++++-----------
> > tests/data/Makefile.am | 6 ++
> > .../test-leaf-cxx-members-report.txt | 4 +-
> > .../test-leaf-peeling-report.txt | 1 -
> > .../test-abidiff-exit/test-leaf1-report.txt | 1 -
> > .../test-member-size-report0.txt | 27 +++++
> > .../test-member-size-report1.txt | 21 ++++
> > .../test-abidiff-exit/test-member-size-v0.cc | 26 +++++
> > .../test-abidiff-exit/test-member-size-v0.o | Bin 0 -> 3184 bytes
> > .../test-abidiff-exit/test-member-size-v1.cc | 27 +++++
> > .../test-abidiff-exit/test-member-size-v1.o | Bin 0 -> 3192 bytes
> > .../test-no-stray-comma-report.txt | 1 -
> > .../test45-anon-dm-change-report-0.txt | 1 +
> > .../test44-anonymous-data-member-report-0.txt | 1 +
> > ...-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt | 2 +-
> > .../test-diff-suppr/test36-leaf-report-0.txt | 1 -
> > .../test46-PR25128-report-1.txt | 1 -
> > .../test46-PR25128-report-2.txt | 1 -
> > tests/test-abidiff-exit.cc | 18 ++++
> > 19 files changed, 167 insertions(+), 70 deletions(-)
> > create mode 100644 tests/data/test-abidiff-exit/test-member-size-report0.txt
> > create mode 100644 tests/data/test-abidiff-exit/test-member-size-report1.txt
> > create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.cc
> > create mode 100644 tests/data/test-abidiff-exit/test-member-size-v0.o
> > create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.cc
> > create mode 100644 tests/data/test-abidiff-exit/test-member-size-v1.o
> >
> >diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
> >index 323de503..72181367 100644
> >--- a/src/abg-reporter-priv.cc
> >+++ b/src/abg-reporter-priv.cc
> >@@ -411,16 +411,26 @@ represent(const var_diff_sptr &diff,
> >
> > var_decl_sptr o = diff->first_var();
> > var_decl_sptr n = diff->second_var();
> >+ string name1 = o->get_qualified_name();
> >+ string name2 = n->get_qualified_name();
> >+ uint64_t size1 = get_var_size_in_bits(o);
> >+ uint64_t size2 = get_var_size_in_bits(n);
> >+ uint64_t offset1 = get_data_member_offset(o);
> >+ uint64_t offset2 = get_data_member_offset(n);
>
> Those are only used const, hence can be made const.
> Also, further down, offset1 feels like to much of an abbreviation.
Sure... I've tried to keep my changes consistent with the surrounding
code. In other places, first and second are used. Here we already had
name1 and name2, I just moved them for readability. Given we have
o(old) and n(new) already, how about having o_name, o_size, o_offset
etc.? There are some more like tr1 and tr2. Or I could rename o to
var1 and n to var2 and we have consistency the other way...
I'm happy to do this as a follow-up change to keep this one
self-contained and easier to read.
Giuliano.
> >
> > // Has the main diff text been output?
> > bool emitted = false;
> > // Are we continuing on a new line? (implies emitted)
> > bool begin_with_and = false;
> >- string name1 = o->get_qualified_name();
> >- string name2 = n->get_qualified_name();
> >- string pretty_representation = o->get_pretty_representation();
> >+ // Have we reported a size change already?
> >+ bool size_reported = false;
> >+ // Are we reporting a change to an anonymous member?
> > bool is_strict_anonymous_data_member_change = false;
> >
> >+ string pretty_representation = o->get_pretty_representation();
> >+ bool show_size_offset_changes = ctxt->get_allowed_category()
> >+ & SIZE_OR_OFFSET_CHANGE_CATEGORY;
> >+
> > if (is_anonymous_data_member(o) && is_anonymous_data_member(n))
> > {
> > is_strict_anonymous_data_member_change = true;
> >@@ -430,8 +440,7 @@ represent(const var_diff_sptr &diff,
> > if (tr1 != tr2)
> > {
> > show_offset_or_size(indent + "anonymous data member at offset",
> >- get_data_member_offset(o),
> >- *ctxt, out);
> >+ offset1, *ctxt, out);
> >
> > out << " changed from:\n"
> > << indent << " " << tr1 << "\n"
> >@@ -460,7 +469,6 @@ represent(const var_diff_sptr &diff,
> > begin_with_and = true;
> > emitted = true;
> > }
> >- // TODO: else ...?
> > }
> > else if (filtering::has_anonymous_data_member_change(diff))
> > {
> >@@ -472,9 +480,7 @@ represent(const var_diff_sptr &diff,
> > + o->get_pretty_representation()
> > + " at offset";
> >
> >- show_offset_or_size(indent + repr,
> >- get_data_member_offset(o),
> >- *ctxt, out);
> >+ show_offset_or_size(indent + repr, offset1, *ctxt, out);
> > out << " became data member '"
> > << n->get_pretty_representation()
> > << "'\n";
> >@@ -486,9 +492,7 @@ represent(const var_diff_sptr &diff,
> > + o->get_pretty_representation()
> > + " at offset";
> >
> >- show_offset_or_size(indent + repr,
> >- get_data_member_offset(o),
> >- *ctxt, out);
> >+ show_offset_or_size(indent + repr, offset1, *ctxt, out);
> > out << " became anonymous data member '"
> > << n->get_pretty_representation()
> > << "'\n";
> >@@ -499,11 +503,15 @@ represent(const var_diff_sptr &diff,
> > }
> > else if (diff_sptr d = diff->type_diff())
> > {
> >- if (local_only && d->has_local_changes())
> >+ if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
> > {
> >- out << indent << "type '" << get_pretty_representation(o->get_type())
> >- << "' of '" << o->get_qualified_name()
> >- << "' changed";
> >+ if (local_only)
> >+ out << indent << "type '" << get_pretty_representation(o->get_type())
> >+ << "' of '" << o->get_qualified_name()
> >+ << "' changed";
> >+ else
> >+ out << indent
> >+ << "type of '" << pretty_representation << "' changed";
> >
> > if (d->currently_reporting())
> > {
> >@@ -521,30 +529,7 @@ represent(const var_diff_sptr &diff,
> >
> > begin_with_and = true;
> > emitted = true;
> >- }
> >- else
> >- {
> >- if (ctxt->get_reporter()->diff_to_be_reported(d.get()))
> >- {
> >- out << indent
> >- << "type of '" << pretty_representation << "' changed";
> >- if (d->currently_reporting())
> >- {
> >- out << ", as being reported\n";
> >- }
> >- else if (d->reported_once())
> >- {
> >- out << ", as reported earlier\n";
> >- }
> >- else
> >- {
> >- out << ":\n";
> >- d->report(out, indent + " ");
> >- }
> >-
> >- begin_with_and = true;
> >- emitted = true;
> >- }
> >+ size_reported = true;
> > }
> > }
> >
> >@@ -589,9 +574,9 @@ represent(const var_diff_sptr &diff,
> > out << "now becomes laid out";
> > emitted = true;
> > }
> >- if ((ctxt->get_allowed_category() & SIZE_OR_OFFSET_CHANGE_CATEGORY))
> >+ if (show_size_offset_changes)
> > {
> >- if (get_data_member_offset(o) != get_data_member_offset(n))
> >+ if (offset1 != offset2)
> > {
> > if (begin_with_and)
> > {
> >@@ -600,26 +585,20 @@ represent(const var_diff_sptr &diff,
> > }
> > else if (!emitted)
> > {
> >+ out << indent;
> > if (is_strict_anonymous_data_member_change)
> >- out << indent;
> >- else
> >- out << indent << "'" << pretty_representation << "' ";
> >+ out << "anonymous data member ";
> >+ out << "'" << pretty_representation << "' ";
> > }
> > else
> > out << ", ";
> >
> >- show_numerical_change("offset",
> >- get_data_member_offset(o),
> >- get_data_member_offset(n),
> >- *ctxt, out);
> >+ show_numerical_change("offset", offset1, offset2, *ctxt, out);
> > maybe_show_relative_offset_change(diff, *ctxt, out);
> >-
> > emitted = true;
> > }
> >- if (// If we are not displaying only local changes, we must
> >- // have indicated the type size change already.
> >- local_only
> >- && (get_var_size_in_bits(o) != get_var_size_in_bits(n)))
> >+
> >+ if (!size_reported && size1 != size2)
> > {
> > if (begin_with_and)
> > {
> >@@ -628,18 +607,15 @@ represent(const var_diff_sptr &diff,
> > }
> > else if (!emitted)
> > {
> >+ out << indent;
> > if (is_strict_anonymous_data_member_change)
> >- out << indent;
> >- else
> >- out << indent << "'" << pretty_representation << "' ";
> >+ out << "anonymous data member ";
> >+ out << "'" << pretty_representation << "' ";
> > }
> > else
> > out << ", ";
> >
> >- show_numerical_change("size",
> >- get_var_size_in_bits(o),
> >- get_var_size_in_bits(n),
> >- *ctxt, out);
> >+ show_numerical_change("size", size1, size2, *ctxt, out);
> > maybe_show_relative_size_change(diff, *ctxt, out);
> > emitted = true;
> > }
> >diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> >index bb0fd83b..64e36b1a 100644
> >--- a/tests/data/Makefile.am
> >+++ b/tests/data/Makefile.am
> >@@ -140,6 +140,12 @@ test-abidiff-exit/test-leaf-cxx-members-v0.o \
> > test-abidiff-exit/test-leaf-cxx-members-v1.cc \
> > test-abidiff-exit/test-leaf-cxx-members-v1.o \
> > test-abidiff-exit/test-leaf-cxx-members-report.txt \
> >+test-abidiff-exit/test-member-size-v0.cc \
> >+test-abidiff-exit/test-member-size-v0.o \
> >+test-abidiff-exit/test-member-size-v1.cc \
> >+test-abidiff-exit/test-member-size-v1.o \
> >+test-abidiff-exit/test-member-size-report0.txt \
> >+test-abidiff-exit/test-member-size-report1.txt \
> > \
> > test-diff-dwarf/test0-v0.cc \
> > test-diff-dwarf/test0-v0.o \
> >diff --git a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
> >index 082db83e..eae74a3b 100644
> >--- a/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
> >+++ b/tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt
> >@@ -35,8 +35,8 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> > type 'int' of 'ops::deleted_var' changed:
> > type name changed from 'int' to 'long int'
> > type size changed from 32 to 64 (in bits)
> >- and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1, size changed from 32 to 64 (in bits) (by +32 bits)
> >+ and name of 'ops::deleted_var' changed to 'ops::added_var' at test-leaf-cxx-members-v1.cc:2:1
> > type 'int' of 'ops::changed_var' changed:
> > type name changed from 'int' to 'long int'
> > type size changed from 32 to 64 (in bits)
> >- and offset changed from 96 to 128 (in bits) (by +32 bits), size changed from 32 to 64 (in bits) (by +32 bits)
> >+ and offset changed from 96 to 128 (in bits) (by +32 bits)
> >diff --git a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
> >index bd5f36f8..7ab4dd6e 100644
> >--- a/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
> >+++ b/tests/data/test-abidiff-exit/test-leaf-peeling-report.txt
> >@@ -9,7 +9,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> > type 'int' of 'foo::z' changed:
> > type name changed from 'int' to 'long int'
> > type size changed from 32 to 64 (in bits)
> >- and size changed from 32 to 64 (in bits) (by +32 bits)
>
> As above, the same patch needs to be applied to
> test-leaf-peeling-report-indirect.txt.
>
> >
> > 'struct ops1 at test-leaf-peeling-v0.cc:5:1' changed:
> > type size hasn't changed
> >diff --git a/tests/data/test-abidiff-exit/test-leaf1-report.txt b/tests/data/test-abidiff-exit/test-leaf1-report.txt
> >index 9af6ccaa..aea08b81 100644
> >--- a/tests/data/test-abidiff-exit/test-leaf1-report.txt
> >+++ b/tests/data/test-abidiff-exit/test-leaf1-report.txt
> >@@ -40,4 +40,3 @@ Removed/Changed/Added variables summary: 1 Removed, 1 Changed, 1 Added variable
> > type 'int' of 'changed::foo' changed:
> > type name changed from 'int' to 'long int'
> > type size changed from 32 to 64 (in bits)
> >- and size changed from 32 to 64 (in bits) (by +32 bits)
> >diff --git a/tests/data/test-abidiff-exit/test-member-size-report0.txt b/tests/data/test-abidiff-exit/test-member-size-report0.txt
> >new file mode 100644
> >index 00000000..ff93dd50
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-member-size-report0.txt
> >@@ -0,0 +1,27 @@
> >+Functions changes summary: 0 Removed, 2 Changed, 0 Added functions
> >+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> >+
> >+2 functions with some indirect sub-type change:
> >+
> >+ [C] 'function void reg1(S*, T*, T*)' at test-member-size-v1.cc:26:1 has some indirect sub-type changes:
> >+ parameter 1 of type 'S*' has sub-type changes:
> >+ in pointed to type 'struct S' at test-member-size-v1.cc:3:1:
> >+ type size changed from 128 to 192 (in bits)
> >+ 1 data member insertion:
> >+ 'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
> >+ no data member change (1 filtered);
> >+ parameter 2 of type 'T*' has sub-type changes:
> >+ in pointed to type 'struct T' at test-member-size-v1.cc:14:1:
> >+ type size changed from 192 to 256 (in bits)
> >+ 2 data member changes:
> >+ 'S T::s' size changed from 128 to 192 (in bits) (by +64 bits)
> >+ 'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
> >+
> >+ [C] 'function void reg2(U*)' at test-member-size-v1.cc:27:1 has some indirect sub-type changes:
> >+ parameter 1 of type 'U*' has sub-type changes:
> >+ in pointed to type 'struct U' at test-member-size-v1.cc:19:1:
> >+ type size changed from 192 to 256 (in bits)
> >+ 2 data member changes:
> >+ anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
> >+ 'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
> >+
> >diff --git a/tests/data/test-abidiff-exit/test-member-size-report1.txt b/tests/data/test-abidiff-exit/test-member-size-report1.txt
> >new file mode 100644
> >index 00000000..dbad76be
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-member-size-report1.txt
> >@@ -0,0 +1,21 @@
> >+Leaf changes summary: 3 artifacts changed
> >+Changed leaf types summary: 3 leaf types changed
> >+Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
> >+Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> >+
> >+'struct S at test-member-size-v0.cc:3:1' changed:
> >+ type size changed from 128 to 192 (in bits)
> >+ 1 data member insertion:
> >+ 'int S::y', at offset 128 (in bits) at test-member-size-v1.cc:6:1
> >+
> >+'struct T at test-member-size-v0.cc:13:1' changed:
> >+ type size changed from 192 to 256 (in bits)
> >+ there are data member changes:
> >+ type 'struct S' of 'T::s' changed, as reported earlier
> >+ 'int T::a' offset changed from 128 to 192 (in bits) (by +64 bits)
> >+
> >+'struct U at test-member-size-v0.cc:18:1' changed:
> >+ type size changed from 192 to 256 (in bits)
> >+ there are data member changes:
> >+ anonymous data member 'struct {S s;}' size changed from 128 to 192 (in bits) (by +64 bits)
> >+ 'int U::r' offset changed from 128 to 192 (in bits) (by +64 bits)
> >diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.cc b/tests/data/test-abidiff-exit/test-member-size-v0.cc
> >new file mode 100644
> >index 00000000..f974a37f
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-member-size-v0.cc
> >@@ -0,0 +1,26 @@
> >+struct M;
> >+
> >+struct S {
> >+ int x;
> >+ M * m;
> >+};
> >+
> >+struct M {
> >+ int a;
> >+ S * s;
> >+};
> >+
> >+struct T {
> >+ S s;
> >+ int a;
> >+};
> >+
> >+struct U {
> >+ struct {
> >+ S s;
> >+ };
> >+ int r;
> >+};
> >+
> >+void reg1(S*, T*, T*) { }
> >+void reg2(U*) { }
> >diff --git a/tests/data/test-abidiff-exit/test-member-size-v0.o b/tests/data/test-abidiff-exit/test-member-size-v0.o
> >new file mode 100644
> >index 0000000000000000000000000000000000000000..e020bef77d57d601420f969061707bbf1486ac99
> >GIT binary patch
> >literal 3184
> >zcmbtWL2MgU5S{P!#_PCtoVcY4LC91fsLC!`E2`o`ifKxds<Z@BqY4SqwX=yW;&rs%
> >z6t@VFPzA~@m5>058xj(?a^Q}H#JR_E;{Xyz;Dp2hX8!(hHs8(#M)J=5H}hux|2zM$
> >zKYaO>ODQ3MMZkGD7%>WPKXYK$q*;R!OhJ8R@7I;R_y1bi+x>lIum0QqTR$*_<}#*A
> >z%V<j`jqQU>@OZ*btwB*3)~g5^eGuskSzvTPn3+CW7NpZ=JBT246$34cFvA0pRe?Pa
> >z<Cza+plQ8<+O%wZrmn-Z$Xj1iG=>yR0wr-m5{r<kW5ks85aF?+D2@N9LXT%;C}h5s
> >zj1a^lieV8T9vvgal2eBG(!jN5>5;nE!bwo&m@spQP#O>}5`kupge(nG_iieE0~6e(
> >zKrtzvDwJ|YVYV<SrZUBo7*)dU%H_`Gp3hZt0@E3JFsyiZxwL>2N<Rs?-1(e=<;@8+
> >zFJsVHNhCR$JjAh<BVL>_3Nby9MQYQZU6`T7Srda&(>RT1X_hpfcuCTtNAg7uKr~*%
> >zp8>RwLi}CG<5M;pu2Deb_aeLFcN%_Z_u6-S`_`P(Y)ZCy60&g_;!g{6=1+5C(VYA!
> >zWnhXUi0g08hyGS&)vW_+mmU>(F6!(+>~UA!wN>}}T4f!Uuhq*-XU@3u<*HM0+;U~k
> >zt-9`9wQP5y-N0Ym@&i9?H*x5N&DLUn;jDdjzHD!m?M<(DJ7~7TZqVKB*}K8J?O?->
> >zyvDZQ16W>KI#+(?qTgtHL7drktEaUZa)HO?6L;ax>md3?F?%dy?c=UIhB*C5aN_}9
> >zDil9XE#=3qVE_)v_X6Q4vg6o)DfY*n;U}iXllaF!!ybA+<Q*16y=mQJAY+sm>a6QN
> >z0}l25z->U{QW>B$pKM}uU>fm?4*+EukRQSbFraV*eryP*-kwVAgWP+>PAi<pe9Dh%
> >z-l0xQG$j?~ofuEpg!7CDr`2B|5M?q}z_=R12@R)nNiq{Fxst&FLX6fyLV4t6Bu4;0
> >zDb4wxR^y*F9IqQGzbH)|L-~1M##IY(o>MArQo~(^pHMjGQShPtF&RW%RJI<^XF<oy
> >zDxT(;+=rTm^Zdc--R|HqMjeH*Zm|{l{Ro^5FY>@?^m^cg{<i0A_>JAI^>(n?MW3|2
> >zMkDlZ9ddi27i{6pkfdz41D`2a0-R>I)A8}U<^<iyckpVmdy&_CN0I#2`ex{Ld^Pd^
> >zsb-N+d&Pao>1xW*?{SJv$`d#sOuqr%P6j1xolpHm{r6w_)IWMoD#!J&XijY?zhCq(
> >zq(-EK?5JidD+2W!#qn98`+;N;e|k>Bzfs^PG)RaZyNW&?qwqP^q;m-_nR@IM#7513
> >zU%gP~)BNdn)#tx4A}2y{Q`N82AR&7Fb@Yv@|Glcu`$YBW9@FdZAVxWPo@h$>PSyXM
> >z0j0S<>-P{J#lNZ=itjH@!#vh^6`%dABfTM&gLxeH1sa5@=`sv`#lJB?VuRPu@TX{u
> >z+P{Y*-XDFvn9ut<sy5|*pz62Ogz39MUqziy?_|_c7gYbxqdzJ7`YG=yzD3Q3@>Q$`
> >HPo4iS_W$I+
> >
> >literal 0
> >HcmV?d00001
> >
> >diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.cc b/tests/data/test-abidiff-exit/test-member-size-v1.cc
> >new file mode 100644
> >index 00000000..b3bd5988
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-member-size-v1.cc
> >@@ -0,0 +1,27 @@
> >+struct M;
> >+
> >+struct S {
> >+ int x;
> >+ M * m;
> >+ int y;
> >+};
> >+
> >+struct M {
> >+ int a;
> >+ S * s;
> >+};
> >+
> >+struct T {
> >+ S s;
> >+ int a;
> >+};
> >+
> >+struct U {
> >+ struct {
> >+ S s;
> >+ };
> >+ int r;
> >+};
> >+
> >+void reg1(S*, T*, T*) { }
> >+void reg2(U*) { }
>
> While these are only test cases, I would like for them to have a bit
> more descriptive names or a comment for each of them to make it easier
> to read the test case.
>
> Other than those little nits, this is a very nice patch:
>
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
> Tested-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >diff --git a/tests/data/test-abidiff-exit/test-member-size-v1.o b/tests/data/test-abidiff-exit/test-member-size-v1.o
> >new file mode 100644
> >index 0000000000000000000000000000000000000000..eaec9ebead662bc23c00130c537133a5be5c2019
> >GIT binary patch
> >literal 3192
> >zcmbtW&u<%55T57F#_Me3IB}ySq(VjlK~;9iMp4zYq?iWUR6q$$jiO2r*3QPZ8gHug
> >zx;RCs2oT68S_z3FapNyQ;?M&JBraUJaYO<nBo6#IA#s42w{Psti*tdIyfg2c`DWgG
> >zJMXRUJpbawj1a&g;5-aQi~>B!4(zft%TR<VsI2vVUhBR6*IKXh`&zH^%iY($VF=Bu
> >zm}*2uTQX^44>G~SDLb<Pv%;{hBV=?zjAY3IqXojuzK51&?1LDUXaPn>%w`xv<|QbI
> >zoJDTK2Qj8NZ4h?0M;tIMAGK*Y_{>~`lE_<MP&9^2m;_2<K@w*nQ^AO$^&`Tgv*P&J
> >zcPex`D?=gsm1IO9W)#CBK+KPlVzHGWKFi~>bM(mEYv3fPazc3M0HN3?yi5d|ITCU-
> >z$lQBF=}k=V5e14#@mRiS8~OSCq?pPUW-zLVJ7?Qx?Q{0BEij!i4u+KsFBi|?ghn0(
> >z+dgj_Sl*mK^D+jF7Dbwq9fu^=a>X-qMn0j3vPgOMlM8c{IA>x|W)`RMB+ZiMlPpPE
> >z^hm$cK8VIE_%nd^QHa0u<M@<)hieoN2kqEt1+8ijIql}{z`5nQwVGs`GmuNl5PwdX
> >zbAOr>E9T_883R)sL5zaB2V2*dP%o}~6(9{y#jTqP^wzzNb?@57;uc)GQYo#TI^`{u
> >zmfc0yD=jW~%bvHeTyk1*Ck$5VK^R2M8V>!a)>!GDIqjTYDmnF%v*Wk-!&)QS3->y0
> >zrxU)}47Z)wukHqIfJ>{ZXG>2!7gU>mm}GXm<&#<sfa7wFxCOVbg6I~+{IRTc7q{dD
> >z;`AfIT?cqJU-%%iIzD;@18_*brwK=q?Z*C#i9hiieqw4ok$?Pi*h4Reyobe5uUYpP
> >z$QUMuI`g{EfCEk6cN>tpT>2=@Czl!xOe0zG08o|z<A*Q;49FjWA3KCoV^5{_LGC?b
> >zCl$_PKH^7pZqj?HHcK@l73G~6OWB0;j0mUIUmy@=GEu;|8p5=O)1f4pOO#wm|MVb6
> >z>mZ>#@+OiafS;7+{Ew^guNuxh`J2M2V<<oG%d~1C&VzT8loJ~6DZHd`&ZFQ1`(rYQ
> >zx}a=5p3j1gmsC8>F})9E4d?lT+um>C8AcsPiEgkJ2i+LlZ9n$Gt+w0XM!~M{ZU@y)
> >zeXAMn?4eKEezh6}w+^`d$PepyEu<;C%`jjJmH@Z5*J=g$QFFt+IB@Y^a@w(9yP-%y
> >zV{0e!TY;MR|5WqHr@i7n<aA|a=$AOfrsWYF5T@S%ucw1jw$7*iqW=4@eCi)PCza#+
> >zuWC+hC_i5GZ=_75lyp_IabHruQ5>HYx*teZ@Tcb_{7VIXNQ0E<vFqs5F$zDYnsgz>
> >zB~y=m0kL88zpq{>^J)HcyXy1b9+DFwxS{G-Xpj=U{ucU%)&Ex2=Y693bdTxvZz4uH
> >zd7fxW`C8S#&w$cgpY>aa594pDhT{8+(=d<q`-;zg)_3%VR1W5G+^1*|rl!j<yrcM=
> >zeIzk>{S1GM#<2bSVaWTVuNU)qUx(GE+~2GE4K-o<ZqQdz=hHhGw$!rf|0VRNMPEPV
> >R9mX$Ev!Q&RsQy#u{|i5f=e7U<
> >
> >literal 0
> >HcmV?d00001
> >
> >diff --git a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
> >index 1583a40b..52c48b3d 100644
> >--- a/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
> >+++ b/tests/data/test-abidiff-exit/test-no-stray-comma-report.txt
> >@@ -10,4 +10,3 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> > type name changed from 'int[1]' to 'int[2]'
> > array type size changed from 32 to 64
> > array type subrange 1 changed length from 1 to 2
> >- and size changed from 32 to 64 (in bits) (by +32 bits)
> >diff --git a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
> >index 12af287b..2f8f31d0 100644
> >--- a/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
> >+++ b/tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt
> >@@ -13,6 +13,7 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> > 'char S1::m1' offset changed from 64 to 40 (in bits) (by -24 bits)
> > 1 data member change:
> > anonymous data member struct {int m0; char m01;} at offset 0 (in bits) became data member 'int S1::m0'
> >+ and size changed from 64 to 32 (in bits) (by -32 bits)
> >
> > [C] 'function void foo(S0&)' has some indirect sub-type changes:
> > parameter 1 of type 'S0&' has sub-type changes:
> >diff --git a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
> >index b8bb951c..ed68b550 100644
> >--- a/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
> >+++ b/tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt
> >@@ -12,5 +12,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
> > struct {int b; float c;}
> > to:
> > struct {int b; float c; char e;}
> >+ and size changed from 64 to 96 (in bits) (by +32 bits)
> > 'int S2::d' offset changed from 96 to 128 (in bits) (by +32 bits)
> >
> >diff --git a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
> >index b1611303..2a679d60 100644
> >--- a/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
> >+++ b/tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt
> >@@ -78,7 +78,7 @@
> > type name changed from 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl' to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl'
> > type size changed from 128 to 192 (in bits)
> > 1 data member change:
> >- name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node'
> >+ name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_List_impl::_M_node', size changed from 128 to 192 (in bits) (by +64 bits)
> > and name of 'std::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl' changed to 'std::__cxx11::_List_base<sigc::slot_base, std::allocator<sigc::slot_base> >::_M_impl'
> >
> > ================ end of changes of 'libsigc-2.0.so.0.0.0'===============
> >diff --git a/tests/data/test-diff-suppr/test36-leaf-report-0.txt b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
> >index 9caa9428..fcbb9232 100644
> >--- a/tests/data/test-diff-suppr/test36-leaf-report-0.txt
> >+++ b/tests/data/test-diff-suppr/test36-leaf-report-0.txt
> >@@ -16,7 +16,6 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> > type size changed from 64 to 96 (in bits)
> > there are data member changes:
> > type 'struct leaf1' of 'leaf2::member0' changed, as reported earlier
> >- and size changed from 32 to 64 (in bits) (by +32 bits)
> > 'char leaf2::member1' offset changed from 32 to 64 (in bits) (by +32 bits)
> > 3 impacted interfaces:
> > function void interface1(struct_type*)
> >diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
> >index 36439ce7..acff860c 100644
> >--- a/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
> >+++ b/tests/data/test-diff-suppr/test46-PR25128-report-1.txt
> >@@ -13,5 +13,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> > type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
> > entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
> > type size changed from 64 to 192 (in bits)
> >- and size changed from 64 to 192 (in bits) (by +128 bits)
> > 'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
> >diff --git a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
> >index 67dbf58f..79827103 100644
> >--- a/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
> >+++ b/tests/data/test-diff-suppr/test46-PR25128-report-2.txt
> >@@ -9,5 +9,4 @@ Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable
> > type 'unsigned long int' of 'root_domain::max_cpu_capacity' changed:
> > entity changed from 'unsigned long int' to 'struct max_cpu_capacity' at sched.h:722:1
> > type size changed from 64 to 192 (in bits)
> >- and size changed from 64 to 192 (in bits) (by +128 bits)
> > 'perf_domain* root_domain::pd' offset changed from 14528 to 14656 (in bits) (by +128 bits)
> >diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
> >index 17523afa..97981a69 100644
> >--- a/tests/test-abidiff-exit.cc
> >+++ b/tests/test-abidiff-exit.cc
> >@@ -177,6 +177,24 @@ InOutSpec in_out_specs[] =
> > "data/test-abidiff-exit/test-leaf-cxx-members-report.txt",
> > "output/test-abidiff-exit/test-leaf-cxx-members-report.txt"
> > },
> >+ {
> >+ "data/test-abidiff-exit/test-member-size-v0.o",
> >+ "data/test-abidiff-exit/test-member-size-v1.o",
> >+ "",
> >+ "",
> >+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
> >+ "data/test-abidiff-exit/test-member-size-report0.txt",
> >+ "output/test-abidiff-exit/test-member-size-report0.txt"
> >+ },
> >+ {
> >+ "data/test-abidiff-exit/test-member-size-v0.o",
> >+ "data/test-abidiff-exit/test-member-size-v1.o",
> >+ "",
> >+ "--leaf-changes-only",
> >+ abigail::tools_utils::ABIDIFF_ABI_CHANGE,
> >+ "data/test-abidiff-exit/test-member-size-report1.txt",
> >+ "output/test-abidiff-exit/test-member-size-report1.txt"
> >+ },
> > {0, 0, 0 ,0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
> > };
> >
> >--
> >2.26.0.292.g33ef6b2f38-goog
> >
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] abidiff: More compact references to prior diffs.
2020-04-07 21:33 ` [PATCH v2 " Giuliano Procida
2020-04-07 21:33 ` [PATCH v2 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
@ 2020-04-14 10:42 ` Dodji Seketeli
1 sibling, 0 replies; 13+ messages in thread
From: Dodji Seketeli @ 2020-04-14 10:42 UTC (permalink / raw)
To: Giuliano Procida; +Cc: libabigail, kernel-team
Hello Giuliano,
Giuliano Procida <gprocida@google.com> a écrit:
[...]
>
> * src/abg-reporter-priv.cc (represent): In the var_diff_sptr
> overload, use consistent punctuation and keep to a single line
> of output when referring back to an existing type diff report.
> Remove unnecessary braces around single line conditional
> blocks.
> * src/abg-reporter-priv.h: In the macro
> RETURN_IF_BEING_REPORTED_OR_WAS_REPORTED_EARLIER, use
> consistent punctuation when referring back to an existing type
> diff report.
> * tests/data/test-abidiff/test-PR18791-report0.txt: Adjust
> formatting of back references to existing type diff reports.
> * tests/data/test-diff-dwarf/PR25058-liblttng-ctl-report-1.txt:
> Ditto.
> * tests/data/test-diff-filter/test16-report-2.txt: Ditto.
> * tests/data/test-diff-filter/test17-1-report.txt: Ditto.
> * tests/data/test-diff-filter/test25-cyclic-type-report-1.txt:
> Ditto.
> * tests/data/test-diff-pkg/spice-server-0.12.4-19.el7.x86_64-0.12.8-1.el7.x86_64-report-2.txt:
> Ditto.
> * tests/data/test-diff-suppr/test36-leaf-report-0.txt: Ditto.
Applied to master, thanks!
Cheers,
--
Dodji
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] abidiff: Fix anonymous member size change reports.
2020-04-07 21:33 ` [PATCH v2 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
@ 2020-04-14 10:42 ` Dodji Seketeli
0 siblings, 0 replies; 13+ messages in thread
From: Dodji Seketeli @ 2020-04-14 10:42 UTC (permalink / raw)
To: Giuliano Procida; +Cc: libabigail, kernel-team
Giuliano Procida <gprocida@google.com> a écrit:
[...]
>
> * src/abg-reporter-priv.cc (represent): In the var_diff_sptr
> overload, factor out some expressions as local variables, rely
> on diff_to_be_reported to decide whether to emit a change,
> fold together local/non-local change reporting using
> local_changes to preserve current name formatting differences,
> keep track explicitly of whether size information has been
> emitted and ensure it happens if needed, make offset and size
> change reporting for anonymous data members more meaningful.
> * tests/test-abidiff-exit.cc: Run new test cases.
> * tests/data/Makefile.am: Add new test files.
> * tests/data/test-abidiff-exit/test-member-size-v0.cc: New
> test.
> * tests/data/test-abidiff-exit/test-member-size-v0.o: Ditto.
> * tests/data/test-abidiff-exit/test-member-size-v1.cc: Ditto.
> * tests/data/test-abidiff-exit/test-member-size-v1.o: Ditto.
> * tests/data/test-abidiff-exit/test-member-size-report0.txt:
> New test, default mode.
> * tests/data/test-abidiff-exit/test-member-size-report1.txt:
> New test, --leaf-changes-only mode.
> * tests/data/test-abidiff-exit/test-leaf-cxx-members-report.txt:
> Eliminate duplicate reporting of member sizes.
> * tests/data/test-abidiff-exit/test-leaf-more-report.txt: Ditto.
> * tests/data/test-abidiff-exit/test-leaf-peeling-report.txt:
> Ditto.
> * tests/data/test-abidiff-exit/test-no-stray-comma-report.txt:
> Ditto.
> * tests/data/test-diff-dwarf/test45-anon-dm-change-report-0.txt:
> Add size report for anonymous data member.
> * tests/data/test-diff-filter/test44-anonymous-data-member-report-0.txt:
> Ditto.
> * tests/data/test-diff-pkg/libsigc++-2.0-0c2a_2.4.0-1_amd64--libsigc++-2.0-0v5_2.4.1-1ubuntu2_amd64-report-0.txt:
> Add missing size change report.
> * tests/data/test-diff-suppr/test36-leaf-report-0.txt: Remove
> size change report for previously reported type.
> * tests/data/test-diff-suppr/test46-PR25128-report-1.txt:
> Eliminate duplicate reporting of member size change.
> * tests/data/test-diff-suppr/test46-PR25128-report-2.txt:
> Ditto.
Applied to master, thanks!
Cheers,
--
Dodji
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-04-14 10:43 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 21:53 [PATCH 1/2] abidiff: More compact references to prior diffs Giuliano Procida
2020-04-03 21:53 ` [PATCH 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
2020-04-06 16:08 ` Matthias Maennich
2020-04-07 13:55 ` Dodji Seketeli
2020-04-07 21:31 ` Giuliano Procida
2020-04-07 21:45 ` Giuliano Procida
2020-04-06 15:29 ` [PATCH 1/2] abidiff: More compact references to prior diffs Matthias Maennich
2020-04-07 13:47 ` Dodji Seketeli
2020-04-07 21:32 ` Giuliano Procida
2020-04-07 21:33 ` [PATCH v2 " Giuliano Procida
2020-04-07 21:33 ` [PATCH v2 2/2] abidiff: Fix anonymous member size change reports Giuliano Procida
2020-04-14 10:42 ` Dodji Seketeli
2020-04-14 10:42 ` [PATCH v2 1/2] abidiff: More compact references to prior diffs Dodji Seketeli
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).