[v4,05/15] diff suppression: Fix handling of change kinds.
Commit Message
When parsing suppression specifications, libabigail attempts to detect
and ignore suppressions that are empty and so would match everything
in their category by default.
Unfortunately, with the way the parser currently works, these checks
have to be against an exhaustive list of fields that matter, rather
than by listing the fields that don't (like label). They are fragile
in the face of changes that add new fields.
Two of the short-cut checks are in fact buggy, missing out the
change_kind field. One of the checks also risks a null pointer
dereference as it doesn't actually trigger a return from the function.
This patch eliminates (rather than fixes up) this short-cutting on the
grounds that it is a maintenance burden and inconsistent behaviour.
Users should be able to do this:
[suppress_variable]
label = Suppress all changes to variables
We could reinstate the logic when the code has global knowledge of
which fields are present and which have no suppression (restriction)
semantics, or perhaps just emit a warning message to the user if they
have supplied a completely empty (no label even) specification.
The patch also corrects 4 affected test cases to reflect that
suppression is actually happening (according to change_kind).
* src/abg-suppression.cc (read_type_suppression): Remove
short-circuiting of useless suppressions.
(read_function_suppression): Ditto.
(read_variable_suppression: Ditto.
(read_file_suppression): Ditto.
tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt:
Fix test - something is actually suppressed.
* tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt:
Ditto.
* tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt:
Ditto.
* tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt:
Ditto.
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
src/abg-suppression.cc | 91 +++++--------------
.../test15-suppr-added-fn-report-5.txt | 6 +-
.../test16-suppr-removed-fn-report-5.txt | 15 +--
.../test17-suppr-added-var-report-5.txt | 15 +--
.../test18-suppr-removed-var-report-5.txt | 15 +--
5 files changed, 26 insertions(+), 116 deletions(-)
Comments
On Mon, May 04, 2020 at 01:34:06PM +0100, Giuliano Procida wrote:
>When parsing suppression specifications, libabigail attempts to detect
>and ignore suppressions that are empty and so would match everything
>in their category by default.
>
>Unfortunately, with the way the parser currently works, these checks
>have to be against an exhaustive list of fields that matter, rather
>than by listing the fields that don't (like label). They are fragile
>in the face of changes that add new fields.
>
>Two of the short-cut checks are in fact buggy, missing out the
>change_kind field. One of the checks also risks a null pointer
>dereference as it doesn't actually trigger a return from the function.
>
>This patch eliminates (rather than fixes up) this short-cutting on the
>grounds that it is a maintenance burden and inconsistent behaviour.
>Users should be able to do this:
>
>[suppress_variable]
> label = Suppress all changes to variables
>
>We could reinstate the logic when the code has global knowledge of
>which fields are present and which have no suppression (restriction)
>semantics, or perhaps just emit a warning message to the user if they
>have supplied a completely empty (no label even) specification.
>
>The patch also corrects 4 affected test cases to reflect that
>suppression is actually happening (according to change_kind).
>
> * src/abg-suppression.cc (read_type_suppression): Remove
> short-circuiting of useless suppressions.
> (read_function_suppression): Ditto.
> (read_variable_suppression: Ditto.
> (read_file_suppression): Ditto.
> tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt:
> Fix test - something is actually suppressed.
> * tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt:
> Ditto.
> * tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt:
> Ditto.
> * tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt:
> Ditto.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
Reviewed-by: Matthias Maennich <maennich@google.com>
Cheers,
Matthias
>---
> src/abg-suppression.cc | 91 +++++--------------
> .../test15-suppr-added-fn-report-5.txt | 6 +-
> .../test16-suppr-removed-fn-report-5.txt | 15 +--
> .../test17-suppr-added-var-report-5.txt | 15 +--
> .../test18-suppr-removed-var-report-5.txt | 15 +--
> 5 files changed, 26 insertions(+), 116 deletions(-)
>
>diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
>index 217ec5e9..51885cf2 100644
>--- a/src/abg-suppression.cc
>+++ b/src/abg-suppression.cc
>@@ -1835,19 +1835,6 @@ read_type_suppression(const ini::config::section& section)
> changed_enumerator_names.push_back(p->get_value()->as_string());
> }
>
>- if (file_name_regex_str.empty()
>- && file_name_not_regex_str.empty()
>- && soname_regex_str.empty()
>- && soname_not_regex_str.empty()
>- && (!name_regex_prop || name_regex_prop->get_value()->as_string().empty())
>- && (!name_not_regex_prop
>- || name_not_regex_prop->get_value()->as_string().empty())
>- && (!name_prop || name_prop->get_value()->as_string().empty())
>- && !consider_type_kind
>- && srcloc_not_regexp_str.empty()
>- && srcloc_not_in.empty())
>- return result;
>-
> result.reset(new type_suppression(label_str, name_regex_str, name_str));
>
> if (consider_type_kind)
>@@ -3283,32 +3270,16 @@ read_function_suppression(const ini::config::section& section)
> parms.push_back(parm);
> }
>
>- if (!label_str.empty()
>- || !name.empty()
>- || !name_regex_str.empty()
>- || !name_not_regex_str.empty()
>- || !file_name_regex_str.empty()
>- || !file_name_not_regex_str.empty()
>- || !soname_regex_str.empty()
>- || !soname_not_regex_str.empty()
>- || !return_type_name.empty()
>- || !return_type_regex_str.empty()
>- || !sym_name.empty()
>- || !sym_name_regex_str.empty()
>- || !sym_name_not_regex_str.empty()
>- || !sym_version.empty()
>- || !sym_ver_regex_str.empty()
>- || !parms.empty())
>-
>- result.reset(new function_suppression(label_str, name,
>- name_regex_str,
>- return_type_name,
>- return_type_regex_str,
>- parms,
>- sym_name,
>- sym_name_regex_str,
>- sym_version,
>- sym_ver_regex_str));
>+ result.reset(new function_suppression(label_str,
>+ name,
>+ name_regex_str,
>+ return_type_name,
>+ return_type_regex_str,
>+ parms,
>+ sym_name,
>+ sym_name_regex_str,
>+ sym_version,
>+ sym_ver_regex_str));
>
> if ((drop_artifact_str == "yes" || drop_artifact_str == "true")
> && (!name.empty()
>@@ -3319,11 +3290,11 @@ read_function_suppression(const ini::config::section& section)
> || !sym_name_not_regex_str.empty()))
> result->set_drops_artifact_from_ir(true);
>
>- if (result && !change_kind_str.empty())
>+ if (!change_kind_str.empty())
> result->set_change_kind
> (function_suppression::parse_change_kind(change_kind_str));
>
>- if (result && !allow_other_aliases.empty())
>+ if (!allow_other_aliases.empty())
> result->set_allow_other_aliases(allow_other_aliases == "yes"
> || allow_other_aliases == "true");
>
>@@ -4151,27 +4122,15 @@ read_variable_suppression(const ini::config::section& section)
> ? type_name_regex_prop->get_value()->as_string()
> : "";
>
>- if (label_str.empty()
>- && name_str.empty()
>- && name_regex_str.empty()
>- && name_not_regex_str.empty()
>- && file_name_regex_str.empty()
>- && file_name_not_regex_str.empty()
>- && soname_regex_str.empty()
>- && soname_not_regex_str.empty()
>- && symbol_name.empty()
>- && symbol_name_regex_str.empty()
>- && symbol_name_not_regex_str.empty()
>- && symbol_version.empty()
>- && symbol_version_regex_str.empty()
>- && type_name_str.empty()
>- && type_name_regex_str.empty())
>- return result;
>-
>- result.reset(new variable_suppression(label_str, name_str, name_regex_str,
>- symbol_name, symbol_name_regex_str,
>- symbol_version, symbol_version_regex_str,
>- type_name_str, type_name_regex_str));
>+ result.reset(new variable_suppression(label_str,
>+ name_str,
>+ name_regex_str,
>+ symbol_name,
>+ symbol_name_regex_str,
>+ symbol_version,
>+ symbol_version_regex_str,
>+ type_name_str,
>+ type_name_regex_str));
>
> if ((drop_artifact_str == "yes" || drop_artifact_str == "true")
> && (!name_str.empty()
>@@ -4188,7 +4147,7 @@ read_variable_suppression(const ini::config::section& section)
> if (!symbol_name_not_regex_str.empty())
> result->set_symbol_name_not_regex_str(symbol_name_not_regex_str);
>
>- if (result && !change_kind_str.empty())
>+ if (!change_kind_str.empty())
> result->set_change_kind
> (variable_suppression::parse_change_kind(change_kind_str));
>
>@@ -4331,12 +4290,6 @@ read_file_suppression(const ini::config::section& section)
> ? soname_not_regex_prop->get_value()->as_string()
> : "";
>
>- if (file_name_regex_str.empty()
>- && file_name_not_regex_str.empty()
>- && soname_regex_str.empty()
>- && soname_not_regex_str.empty())
>- return result;
>-
> result.reset(new file_suppression(label_str,
> file_name_regex_str,
> file_name_not_regex_str));
>diff --git a/tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt b/tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt
>index 4eaba5b7..83dfe326 100644
>--- a/tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt
>+++ b/tests/data/test-diff-suppr/test15-suppr-added-fn-report-5.txt
>@@ -1,10 +1,6 @@
>-Functions changes summary: 0 Removed, 1 Changed, 1 Added functions
>+Functions changes summary: 0 Removed, 1 Changed, 0 Added (1 filtered out) functions
> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>
>-1 Added function:
>-
>- [A] 'function void bar()' {_Z3barv}
>-
> 1 function with some indirect sub-type change:
>
> [C] 'function void bar(S&)' has some indirect sub-type changes:
>diff --git a/tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt b/tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt
>index b28fbd16..851f7728 100644
>--- a/tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt
>+++ b/tests/data/test-diff-suppr/test16-suppr-removed-fn-report-5.txt
>@@ -1,16 +1,3 @@
>-Functions changes summary: 1 Removed, 1 Changed, 0 Added functions
>+Functions changes summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added functions
> Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>
>-1 Removed function:
>-
>- [D] 'function void bar()' {_Z3barv}
>-
>-1 function with some indirect sub-type change:
>-
>- [C] 'function void bar(S*)' has some indirect sub-type changes:
>- parameter 1 of type 'S*' has sub-type changes:
>- in pointed to type 'struct S':
>- type size changed from 32 to 64 (in bits)
>- 1 data member insertion:
>- 'unsigned int S::bar', at offset 32 (in bits)
>-
>diff --git a/tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt b/tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt
>index 6965a151..f4e0aa29 100644
>--- a/tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt
>+++ b/tests/data/test-diff-suppr/test17-suppr-added-var-report-5.txt
>@@ -1,16 +1,3 @@
> Functions changes summary: 0 Removed, 0 Changed, 0 Added function
>-Variables changes summary: 0 Removed, 1 Changed, 1 Added variables
>-
>-1 Added variable:
>-
>- [A] 'int var1' {var1}
>-
>-1 Changed variable:
>-
>- [C] 'S* var0' was changed:
>- type of variable changed:
>- in pointed to type 'struct S':
>- type size changed from 32 to 64 (in bits)
>- 1 data member insertion:
>- 'char S::m1', at offset 32 (in bits)
>+Variables changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added (1 filtered out) variables
>
>diff --git a/tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt b/tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt
>index 3edf2bd1..ac380a4a 100644
>--- a/tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt
>+++ b/tests/data/test-diff-suppr/test18-suppr-removed-var-report-5.txt
>@@ -1,16 +1,3 @@
> Functions changes summary: 0 Removed, 0 Changed, 0 Added function
>-Variables changes summary: 1 Removed, 1 Changed, 0 Added variables
>-
>-1 Removed variable:
>-
>- [D] 'int var1' {var1}
>-
>-1 Changed variable:
>-
>- [C] 'S* var0' was changed:
>- type of variable changed:
>- in pointed to type 'struct S':
>- type size changed from 32 to 64 (in bits)
>- 1 data member insertion:
>- 'char S::m1', at offset 32 (in bits)
>+Variables changes summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added variables
>
>--
>2.26.2.526.g744177e7f7-goog
>
Giuliano Procida <gprocida@google.com> a ?crit:
> When parsing suppression specifications, libabigail attempts to detect
> and ignore suppressions that are empty and so would match everything
> in their category by default.
No. Libabigail doesn't try to detect suppressions that are empty.
Rather, it requires *some* properties to be present in the suppression.
Those mandatory properties are documented for each kind of suppression
at
https://sourceware.org/libabigail/manual/libabigail-concepts.html#suppress-type,
for instance.
The reason why there are mandatory properties is to somehow, to give
incentive to the the writer of the suppression to think and to minimise
the risk of having supprising false negatives due to suppression
specifications that are too "greedy" in a sense.
In other words, when in doubt, we choose to not suppress anything,
rather than suppress too much.
I agree that this is a "balanced" act.
> Unfortunately, with the way the parser currently works, these checks
> have to be against an exhaustive list of fields that matter, rather
> than by listing the fields that don't (like label).
What I said above is the reason why it works against an exhaustive
list. So that is by design.
> They are fragile in the face of changes that add new fields.
Well, new fields are not a problem because they won't change the set of
mandatory fields that have been chosen already.
> Two of the short-cut checks are in fact buggy, missing out the
> change_kind field.
That's by design. You don't need to specify the change_kind property
for the suppress_type directive to work because that property it's
considered advanced.
> One of the checks also risks a null pointer
> dereference as it doesn't actually trigger a return from the function.
That ought to be fixed, I guess.
> This patch eliminates (rather than fixes up) this short-cutting on the
> grounds that it is a maintenance burden and inconsistent behaviour.
> Users should be able to do this:
>
> [suppress_variable]
> label = Suppress all changes to variables
That's precisely what I don't want to allow, by design.
I'd rather have the user specify a regular expression that matches
everything rather than that.
So, in light of my explanations above, I'd prefer that we don't apply
this patch.
Does that make sense?
Cheers,
Hi Dodji.
On Mon, 11 May 2020 at 15:15, Dodji Seketeli <dodji@seketeli.org> wrote:
>
> Giuliano Procida <gprocida@google.com> a ?crit:
>
> > When parsing suppression specifications, libabigail attempts to detect
> > and ignore suppressions that are empty and so would match everything
> > in their category by default.
>
> No. Libabigail doesn't try to detect suppressions that are empty.
> Rather, it requires *some* properties to be present in the suppression.
>
> Those mandatory properties are documented for each kind of suppression
> at
>
https://sourceware.org/libabigail/manual/libabigail-concepts.html#suppress-type
,
> for instance.
>
> The reason why there are mandatory properties is to somehow, to give
> incentive to the the writer of the suppression to think and to minimise
> the risk of having supprising false negatives due to suppression
> specifications that are too "greedy" in a sense.
>
> In other words, when in doubt, we choose to not suppress anything,
> rather than suppress too much.
>
> I agree that this is a "balanced" act.
The code and documentation should agree, certainly! If we stick to current
implementation and documentation, I would feel better if the code noisily
ignored rather than silently ignored such suppressions.
> > Unfortunately, with the way the parser currently works, these checks
> > have to be against an exhaustive list of fields that matter, rather
> > than by listing the fields that don't (like label).
>
> What I said above is the reason why it works against an exhaustive
> list. So that is by design.
>
> > They are fragile in the face of changes that add new fields.
>
> Well, new fields are not a problem because they won't change the set of
> mandatory fields that have been chosen already.
Well, once the dust has settled, it should be very cheap to add
source_location_regexp, for example. But, the way I see the code working,
my fragility argument doesn't stand.
> > Two of the short-cut checks are in fact buggy, missing out the
> > change_kind field.
>
> That's by design. You don't need to specify the change_kind property
> for the suppress_type directive to work because that property it's
> considered advanced.
It's not that change_kind should be mandatory but that it should be one of
the fields considered sufficient for the specification not to be ignored.
The tests as written are very confusing then. A comment in the .suppr files
that only specify change_kind to say "this suppression will be ignored"
would be useful. It was these tests that led me to believe the code should
have included change_kind as one of the sufficient fields.
> > One of the checks also risks a null pointer
> > dereference as it doesn't actually trigger a return from the function.
>
> That ought to be fixed, I guess.
>
> > This patch eliminates (rather than fixes up) this short-cutting on the
> > grounds that it is a maintenance burden and inconsistent behaviour.
>
> > Users should be able to do this:
> >
> > [suppress_variable]
> > label = Suppress all changes to variables
>
> That's precisely what I don't want to allow, by design.
>
> I'd rather have the user specify a regular expression that matches
> everything rather than that.
To give you my perspective...
I have no idea what suppression specifications are being used in reality,
so I am looking at things fairly abstractly. I see things through a
mathematical viewpoint: a suppression is simply a predicate (function
returning a truth value) on a type of object.
libabigail supports multiple suppression specifications and ORs them
together. Within each suppression specification, things are (mostly) ANDed
together (and within ANDs there is some support for NOT as well via
_not_regex properties). [c.f. DNF
<https://en.wikipedia.org/wiki/Disjunctive_normal_form>]
So the user can construct certain types of predicates. More generally, the
predicates which always return true or always return false are perfectly
valid mathematically. The always false one (never suppress) is
constructible by supplying an empty OR - no predicates - and the always
true one would logically be constructible by supplying an empty AND - a
predicate with no properties. Treating empty AND as false is what I meant
by "inconsistent".
More practically though, what do we want to happen when the user does
something bad? The current behaviour is to silently ignore.
I think it would be more useful to generate warning messages. Then there is
no surprise to the user if their suppression is either ignored or
suppresses too much. I don't mind so much which behaviour is implemented
so long as there is this coverage. It would be nice if the user didn't have
to do regex matches to implement "true" or "only additions" as predicates,
but that's a smaller issue.
> So, in light of my explanations above, I'd prefer that we don't apply
> this patch.
Understood.
> Does that make sense?
I can preserve the existing behaviours, but I would really want to do both
of:
a) comment the tests that were surprising to me (or allow change_kind as a
sufficient property)
b) emit warnings to the user whenever a suppression is going to be ignored
because it doesn't have one of a list of properties
I'm happy to adapt the code and repost.
> Cheers,
>
> --
> Dodji
Regards,
Giuliano.
Giuliano Procida <gprocida@google.com> a ?crit:
> The code and documentation should agree, certainly! If we stick to current
> implementation and documentation, I would feel better if the code noisily
> ignored rather than silently ignored such suppressions.
In general, I chose to avoid emitting user messages unless those are
emitted in well defined places like special "passes" that are meant for
analysis and reporting.
So rather than scathering user messages left and right everywhere, I'd
prefer we properly fix this issue by adding a (set of) pass(es) that
would walk the parsed ini file, analyse it and emit warnings or errors
as necessary.
I understand that you've proposed something in that regard at
https://sourceware.org/pipermail/libabigail/2020q2/002160.html, so I
think the discussion will happen there.
So the more I think about this, the less I am in favour of handling that
here.
[...]
>> > Two of the short-cut checks are in fact buggy, missing out the
>> > change_kind field.
>>
>> That's by design. You don't need to specify the change_kind property
>> for the suppress_type directive to work because that property it's
>> considered advanced.
>
> It's not that change_kind should be mandatory but that it should be one of
> the fields considered sufficient for the specification not to be
> ignored.
When I was assessing what property should be part of the sufficient set,
it didn't seem to me that change_kind should be in there (because it did
sound too easy to make it match a super-broad set of things to
suppress). The goal, in my mind is *NOT* to strive for minimalism or
orthogonality. To the contrary. I'd rather require some redundancy to
be sure user think about this twice. Because, again, it's super easy to
come up with false negatives otherwise.
Nevertheless if you think it should, then it's just a discussion to be
held. As a matter of fact, I am not dead opposed to it. So in the
grand scheme of things, I don't see this as bug.
> The tests as written are very confusing then. A comment in the .suppr files
> that only specify change_kind to say "this suppression will be ignored"
> would be useful. It was these tests that led me to believe the code should
> have included change_kind as one of the sufficient fields.
Fair enough.
[...]
> To give you my perspective...
>
> I have no idea what suppression specifications are being used in reality,
> so I am looking at things fairly abstractly. I see things through a
> mathematical viewpoint: a suppression is simply a predicate (function
> returning a truth value) on a type of object.
>
> libabigail supports multiple suppression specifications and ORs them
> together. Within each suppression specification, things are (mostly) ANDed
> together (and within ANDs there is some support for NOT as well via
> _not_regex properties). [c.f. DNF
> <https://en.wikipedia.org/wiki/Disjunctive_normal_form>]
>
> So the user can construct certain types of predicates. More generally, the
> predicates which always return true or always return false are perfectly
> valid mathematically. The always false one (never suppress) is
> constructible by supplying an empty OR - no predicates - and the always
> true one would logically be constructible by supplying an empty AND - a
> predicate with no properties. Treating empty AND as false is what I meant
> by "inconsistent".
>
> More practically though, what do we want to happen when the user does
> something bad? The current behaviour is to silently ignore.
Yes. The "silent" part of the behaviour was not meant to be engraved in
stone forever :-) I still want to add a semantic pass to handle that
separately.
> I think it would be more useful to generate warning messages.
Agreed.
[...]
>> So, in light of my explanations above, I'd prefer that we don't apply
>> this patch.
>
> Understood.
>
>> Does that make sense?
>
> I can preserve the existing behaviours, but I would really want to do both
> of:
>
> a) comment the tests that were surprising to me (or allow change_kind as a
> sufficient property)
Agreed.
> b) emit warnings to the user whenever a suppression is going to be ignored
> because it doesn't have one of a list of properties
I am not opposed to this, but as I said above, I'd rather see this
happen as part of the separate discussion/project I mentioned earlier.
> I'm happy to adapt the code and repost.
I'd happily accept a patch that does a/ in this context.
I hope this helps.
Cheers,
@@ -1835,19 +1835,6 @@ read_type_suppression(const ini::config::section& section)
changed_enumerator_names.push_back(p->get_value()->as_string());
}
- if (file_name_regex_str.empty()
- && file_name_not_regex_str.empty()
- && soname_regex_str.empty()
- && soname_not_regex_str.empty()
- && (!name_regex_prop || name_regex_prop->get_value()->as_string().empty())
- && (!name_not_regex_prop
- || name_not_regex_prop->get_value()->as_string().empty())
- && (!name_prop || name_prop->get_value()->as_string().empty())
- && !consider_type_kind
- && srcloc_not_regexp_str.empty()
- && srcloc_not_in.empty())
- return result;
-
result.reset(new type_suppression(label_str, name_regex_str, name_str));
if (consider_type_kind)
@@ -3283,32 +3270,16 @@ read_function_suppression(const ini::config::section& section)
parms.push_back(parm);
}
- if (!label_str.empty()
- || !name.empty()
- || !name_regex_str.empty()
- || !name_not_regex_str.empty()
- || !file_name_regex_str.empty()
- || !file_name_not_regex_str.empty()
- || !soname_regex_str.empty()
- || !soname_not_regex_str.empty()
- || !return_type_name.empty()
- || !return_type_regex_str.empty()
- || !sym_name.empty()
- || !sym_name_regex_str.empty()
- || !sym_name_not_regex_str.empty()
- || !sym_version.empty()
- || !sym_ver_regex_str.empty()
- || !parms.empty())
-
- result.reset(new function_suppression(label_str, name,
- name_regex_str,
- return_type_name,
- return_type_regex_str,
- parms,
- sym_name,
- sym_name_regex_str,
- sym_version,
- sym_ver_regex_str));
+ result.reset(new function_suppression(label_str,
+ name,
+ name_regex_str,
+ return_type_name,
+ return_type_regex_str,
+ parms,
+ sym_name,
+ sym_name_regex_str,
+ sym_version,
+ sym_ver_regex_str));
if ((drop_artifact_str == "yes" || drop_artifact_str == "true")
&& (!name.empty()
@@ -3319,11 +3290,11 @@ read_function_suppression(const ini::config::section& section)
|| !sym_name_not_regex_str.empty()))
result->set_drops_artifact_from_ir(true);
- if (result && !change_kind_str.empty())
+ if (!change_kind_str.empty())
result->set_change_kind
(function_suppression::parse_change_kind(change_kind_str));
- if (result && !allow_other_aliases.empty())
+ if (!allow_other_aliases.empty())
result->set_allow_other_aliases(allow_other_aliases == "yes"
|| allow_other_aliases == "true");
@@ -4151,27 +4122,15 @@ read_variable_suppression(const ini::config::section& section)
? type_name_regex_prop->get_value()->as_string()
: "";
- if (label_str.empty()
- && name_str.empty()
- && name_regex_str.empty()
- && name_not_regex_str.empty()
- && file_name_regex_str.empty()
- && file_name_not_regex_str.empty()
- && soname_regex_str.empty()
- && soname_not_regex_str.empty()
- && symbol_name.empty()
- && symbol_name_regex_str.empty()
- && symbol_name_not_regex_str.empty()
- && symbol_version.empty()
- && symbol_version_regex_str.empty()
- && type_name_str.empty()
- && type_name_regex_str.empty())
- return result;
-
- result.reset(new variable_suppression(label_str, name_str, name_regex_str,
- symbol_name, symbol_name_regex_str,
- symbol_version, symbol_version_regex_str,
- type_name_str, type_name_regex_str));
+ result.reset(new variable_suppression(label_str,
+ name_str,
+ name_regex_str,
+ symbol_name,
+ symbol_name_regex_str,
+ symbol_version,
+ symbol_version_regex_str,
+ type_name_str,
+ type_name_regex_str));
if ((drop_artifact_str == "yes" || drop_artifact_str == "true")
&& (!name_str.empty()
@@ -4188,7 +4147,7 @@ read_variable_suppression(const ini::config::section& section)
if (!symbol_name_not_regex_str.empty())
result->set_symbol_name_not_regex_str(symbol_name_not_regex_str);
- if (result && !change_kind_str.empty())
+ if (!change_kind_str.empty())
result->set_change_kind
(variable_suppression::parse_change_kind(change_kind_str));
@@ -4331,12 +4290,6 @@ read_file_suppression(const ini::config::section& section)
? soname_not_regex_prop->get_value()->as_string()
: "";
- if (file_name_regex_str.empty()
- && file_name_not_regex_str.empty()
- && soname_regex_str.empty()
- && soname_not_regex_str.empty())
- return result;
-
result.reset(new file_suppression(label_str,
file_name_regex_str,
file_name_not_regex_str));
@@ -1,10 +1,6 @@
-Functions changes summary: 0 Removed, 1 Changed, 1 Added functions
+Functions changes summary: 0 Removed, 1 Changed, 0 Added (1 filtered out) functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
-1 Added function:
-
- [A] 'function void bar()' {_Z3barv}
-
1 function with some indirect sub-type change:
[C] 'function void bar(S&)' has some indirect sub-type changes:
@@ -1,16 +1,3 @@
-Functions changes summary: 1 Removed, 1 Changed, 0 Added functions
+Functions changes summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added functions
Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
-1 Removed function:
-
- [D] 'function void bar()' {_Z3barv}
-
-1 function with some indirect sub-type change:
-
- [C] 'function void bar(S*)' has some indirect sub-type changes:
- parameter 1 of type 'S*' has sub-type changes:
- in pointed to type 'struct S':
- type size changed from 32 to 64 (in bits)
- 1 data member insertion:
- 'unsigned int S::bar', at offset 32 (in bits)
-
@@ -1,16 +1,3 @@
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
-Variables changes summary: 0 Removed, 1 Changed, 1 Added variables
-
-1 Added variable:
-
- [A] 'int var1' {var1}
-
-1 Changed variable:
-
- [C] 'S* var0' was changed:
- type of variable changed:
- in pointed to type 'struct S':
- type size changed from 32 to 64 (in bits)
- 1 data member insertion:
- 'char S::m1', at offset 32 (in bits)
+Variables changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added (1 filtered out) variables
@@ -1,16 +1,3 @@
Functions changes summary: 0 Removed, 0 Changed, 0 Added function
-Variables changes summary: 1 Removed, 1 Changed, 0 Added variables
-
-1 Removed variable:
-
- [D] 'int var1' {var1}
-
-1 Changed variable:
-
- [C] 'S* var0' was changed:
- type of variable changed:
- in pointed to type 'struct S':
- type size changed from 32 to 64 (in bits)
- 1 data member insertion:
- 'char S::m1', at offset 32 (in bits)
+Variables changes summary: 0 Removed (1 filtered out), 0 Changed (1 filtered out), 0 Added variables