* [PATCH 0/1] Bug 28319 - abidw - regression in treatment of anonymous enums in structs
@ 2022-01-11 16:10 Giuliano Procida
2022-01-11 16:10 ` [PATCH 1/1] abidw: add --consider-enumerator-names flag Giuliano Procida
0 siblings, 1 reply; 4+ messages in thread
From: Giuliano Procida @ 2022-01-11 16:10 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich
Hi Dodji.
This is a patch to add some flag control as I mentioned in
https://sourceware.org/bugzilla/show_bug.cgi?id=28319#c4
It may be that the better way forward is to change the default
behaviour and/or rename the flag to something like
"--ignore-enumerator-names".
Do let me know your thoughts.
Regards,
Giuliano.
Giuliano Procida (1):
abidw: add --consider-enumerator-names flag
doc/manuals/abidw.rst | 10 ++++++++
include/abg-dwarf-reader.h | 7 ++++++
src/abg-dwarf-reader.cc | 47 ++++++++++++++++++++++++++++++++++++--
tools/abidw.cc | 5 ++++
4 files changed, 67 insertions(+), 2 deletions(-)
--
2.34.1.575.g55b058a8bb-goog
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] abidw: add --consider-enumerator-names flag
2022-01-11 16:10 [PATCH 0/1] Bug 28319 - abidw - regression in treatment of anonymous enums in structs Giuliano Procida
@ 2022-01-11 16:10 ` Giuliano Procida
2022-01-12 10:40 ` [PATCH v2] abidw: add --ignore-enumerator-names flag Giuliano Procida
0 siblings, 1 reply; 4+ messages in thread
From: Giuliano Procida @ 2022-01-11 16:10 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich
Bug 28319 - abidw - regression in treatment of anonymous enums in structs
This flag reinstates the behaviour of comparing enumerator names
during duplicate enum type resolution which was lost by default as of
commit 1cfbff1b3037d1049bdff7e86de27c3a86af23b3.
That commit addressed an issue with a certain library that contained
duplicate enum type definitions with variations in the enumerator
names but with the same enumerator values.
Many projects will be able to avoid such issues by always compiling
from fresh and consistent sources. Using this flag then avoids the
risk of conflating unrelated anonymous enum types.
* doc/manuals/abidw.rst: Document --consider-enumerator-names.
* include/abg-dwarf-reader.h (get_consider_enumerator_names):
New function.
(set_consider_enumerator_names): Likewise.
* src/abg-dwarf-reader.cc (read_context): Fix documentation
typos. Add consider_enumerator_names_ member variable.
(read_context::initialize): Set consider_enumerator_names_ to
false.
(read_context::consider_enumerator_names): New getter and
setter methods.
(compare_before_canonicalisation): Set environment flag
use_enum_binary_only_equality according to the value of
consider_enumerator_names_.
(get_consider_enumerator_names): New function.
(set_consider_enumerator_names): Likewise.
* tools/abidw.cc (options): Add consider_enumerator_names_
option, defaulted to false.
(parse_command_line): Parse --consider-enumerator-names flag.
(load_corpus_and_write_abixml): Set consider_enumerator_names
flag in DWARF reader context.
No tests are affected. There is no change to default behaviour.
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
doc/manuals/abidw.rst | 10 ++++++++
include/abg-dwarf-reader.h | 7 ++++++
src/abg-dwarf-reader.cc | 47 ++++++++++++++++++++++++++++++++++++--
tools/abidw.cc | 5 ++++
4 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index bdd6204d..f3d8e097 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -182,6 +182,16 @@ Options
ELF binary. It thus considers functions and variables which are
defined and exported in the ELF sense.
+ * ``--consider-enumerator-names``
+
+ During resolution of duplicate enum types, abidw can consider both
+ enumerator names and values, or just the set of enumerator values.
+ The default is the latter.
+
+ With this option, abidw will also take into account enumerator
+ names and will consider enums, including anonymous enums, to be
+ distinct if there any differences in their enumerators' names.
+
* ``--check-alternate-debug-info`` <*elf-path*>
If the debug info for the file *elf-path* contains a reference to
diff --git a/include/abg-dwarf-reader.h b/include/abg-dwarf-reader.h
index 4c2de779..d2f4be1f 100644
--- a/include/abg-dwarf-reader.h
+++ b/include/abg-dwarf-reader.h
@@ -142,6 +142,13 @@ void
set_drop_undefined_syms(read_context& ctxt,
bool f);
+bool
+get_consider_enumerator_names(read_context& ctxt);
+
+void
+set_consider_enumerator_names(read_context& ctxt,
+ bool f);
+
void
set_do_log(read_context& ctxt, bool f);
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 3f716944..bdd53b35 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -1897,7 +1897,7 @@ struct dwarf_expr_eval_context
/// get some important data from it.
///
/// When a new data member is added to this context, it must be
-/// initiliazed by the read_context::initiliaze() function. So please
+/// initialized by the read_context::initialize() function. So please
/// do not forget.
class read_context
{
@@ -2116,6 +2116,7 @@ public:
corpus::exported_decls_builder* exported_decls_builder_;
options_type options_;
bool drop_undefined_syms_;
+ bool consider_enumerator_names_;
read_context();
private:
@@ -2259,6 +2260,7 @@ public:
options_.load_in_linux_kernel_mode = linux_kernel_mode;
options_.load_all_types = load_all_types;
drop_undefined_syms_ = false;
+ consider_enumerator_names_ = false;
load_in_linux_kernel_mode(linux_kernel_mode);
}
@@ -2346,6 +2348,23 @@ public:
drop_undefined_syms(bool f)
{drop_undefined_syms_ = f;}
+ /// Getter for the flag that tells if we should consider enumerator
+ /// names during duplicate enum type resolution.
+ ///
+ /// @return true iff we should consider enumerator names during
+ /// duplicate enum type resolution.
+ bool
+ consider_enumerator_names() const
+ {return consider_enumerator_names_;}
+
+ /// Setter for the flag that tells if we should consider enumerator
+ /// names during duplicate enum type resolution.
+ ///
+ /// @param f the new value of the flag.
+ void
+ consider_enumerator_names(bool f)
+ {consider_enumerator_names_ = f;}
+
/// Getter of the suppression specifications to be used during
/// ELF/DWARF parsing.
///
@@ -4151,7 +4170,7 @@ public:
bool s0 = e->decl_only_class_equals_definition();
bool s1 = e->use_enum_binary_only_equality();
e->decl_only_class_equals_definition(true);
- e->use_enum_binary_only_equality(true);
+ e->use_enum_binary_only_equality(!consider_enumerator_names_);
bool equal = l == r;
e->decl_only_class_equals_definition(s0);
e->use_enum_binary_only_equality(s1);
@@ -6221,6 +6240,30 @@ void
set_drop_undefined_syms(read_context& ctxt, bool f)
{ctxt.drop_undefined_syms(f);}
+/// Getter of the "consider_enumerator_names" flag.
+///
+/// This flag tells if we should consider enumerator names during
+/// duplicate enum type resolution.
+///
+/// @param ctx the read context to consider for this flag.
+///
+/// @return the value of the flag.
+bool
+get_consider_enumerator_names(read_context& ctxt)
+{return ctxt.consider_enumerator_names();}
+
+/// Setter of the "consider_enumerator_names" flag.
+///
+/// This flag tells if we should consider enumerator names during
+/// duplicate enum type resolution.
+///
+/// @param ctxt the read context to consider for this flag.
+///
+/// @param f the value of the flag.
+void
+set_consider_enumerator_names(read_context& ctxt, bool f)
+{ctxt.consider_enumerator_names(f);}
+
/// Setter of the "do_log" flag.
///
/// This flag tells if we should emit verbose logs for various
diff --git a/tools/abidw.cc b/tools/abidw.cc
index f7a8937d..34d0f1a2 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -95,6 +95,7 @@ struct options
bool default_sizes;
bool load_all_types;
bool linux_kernel_mode;
+ bool consider_enumerator_names;
bool corpus_group_for_linux;
bool show_stats;
bool noout;
@@ -132,6 +133,7 @@ struct options
default_sizes(true),
load_all_types(),
linux_kernel_mode(true),
+ consider_enumerator_names(false),
corpus_group_for_linux(false),
show_stats(),
noout(),
@@ -367,6 +369,8 @@ parse_command_line(int argc, char* argv[], options& opts)
opts.drop_undefined_syms = true;
else if (!strcmp(argv[i], "--no-linux-kernel-mode"))
opts.linux_kernel_mode = false;
+ else if (!strcmp(argv[i], "--consider-enumerator-names"))
+ opts.consider_enumerator_names = true;
else if (!strcmp(argv[i], "--abidiff"))
opts.abidiff = true;
#ifdef WITH_DEBUG_SELF_COMPARISON
@@ -562,6 +566,7 @@ load_corpus_and_write_abixml(char* argv[],
opts.linux_kernel_mode);
dwarf_reader::read_context& ctxt = *c;
set_drop_undefined_syms(ctxt, opts.drop_undefined_syms);
+ set_consider_enumerator_names(ctxt, opts.consider_enumerator_names);
set_show_stats(ctxt, opts.show_stats);
set_suppressions(ctxt, opts);
abigail::dwarf_reader::set_do_log(ctxt, opts.do_log);
--
2.34.1.575.g55b058a8bb-goog
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] abidw: add --ignore-enumerator-names flag
2022-01-11 16:10 ` [PATCH 1/1] abidw: add --consider-enumerator-names flag Giuliano Procida
@ 2022-01-12 10:40 ` Giuliano Procida
2022-01-12 11:06 ` Giuliano Procida
0 siblings, 1 reply; 4+ messages in thread
From: Giuliano Procida @ 2022-01-12 10:40 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich
Bug 28319 - abidw - regression in treatment of anonymous enums in structs
This commit reinstates the abidw behaviour of comparing enumerator
names during duplicate enum type resolution which was lost by default
as of commit 1cfbff1b3037d1049bdff7e86de27c3a86af23b3.
That commit addressed an issue with a certain library that contained
duplicate enum type definitions with variations in the enumerator
names but with the same enumerator values.
Many projects will be able to avoid this issue by always compiling
from fresh and consistent sources. This commit avoids the risk of
conflating unrelated anonymous enum types.
The previous behaviour can be requested with --ignore-enumerator-names
and this flag is now used for that specific library test case.
Note that code that uses libabigail library facilities directly will
still continue to ignore enumerator names by default. This means that
all other tests are also unaffected by this code change.
* doc/manuals/abidw.rst: Document --ignore-enumerator-names.
* include/abg-dwarf-reader.h (get_ignore_enumerator_names):
New function.
(set_ignore_enumerator_names): Likewise.
* src/abg-dwarf-reader.cc (read_context): Fix documentation
typos. Add ignore_enumerator_names_ member variable.
(read_context::initialize): Set ignore_enumerator_names_ to
false.
(read_context::ignore_enumerator_names): New getter and
setter methods.
(compare_before_canonicalisation): Set environment flag
use_enum_binary_only_equality according to the value of
ignore_enumerator_names_.
(get_ignore_enumerator_names): New function.
(set_ignore_enumerator_names): Likewise.
* src/abg-ir-priv.h (environment::priv::priv): Add comment to
default use_enum_binary_only_equality value.
* tests/test-alt-dwarf-file.cc: Add --ignore-enumerator-names
to rhbz1951526 test.
* tools/abidw.cc (options): Add ignore_enumerator_names_
option, defaulted to false.
(parse_command_line): Parse --ignore-enumerator-names flag.
(load_corpus_and_write_abixml): Set ignore_enumerator_names
flag in DWARF reader context.
Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
This is really just for-the-record. This is the commit we are going to
land in AOSP. I've kept the impact on tests minimal to ease rebase,
revert etc.
If you cannot find a better way forward on Bug 28319, then perhaps
take this commit, flip the default in src/abg-ir-priv.h and refresh
the few tests affected?
That still leaves whoever cares about oddly-compiled libraries having
to pass a flag manually though. Could something be done with abidiff
suppressions instead?
Regards,
Giuliano.
doc/manuals/abidw.rst | 12 +++++++++
include/abg-dwarf-reader.h | 7 ++++++
src/abg-dwarf-reader.cc | 47 ++++++++++++++++++++++++++++++++++--
src/abg-ir-priv.h | 2 +-
tests/test-alt-dwarf-file.cc | 2 +-
tools/abidw.cc | 7 ++++++
6 files changed, 73 insertions(+), 4 deletions(-)
diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
index bdd6204d..53a0a18f 100644
--- a/doc/manuals/abidw.rst
+++ b/doc/manuals/abidw.rst
@@ -182,6 +182,18 @@ Options
ELF binary. It thus considers functions and variables which are
defined and exported in the ELF sense.
+ * ``--ignore-enumerator-names``
+
+ During resolution of duplicate enum types, abidw can consider both
+ enumerator names and values, or just enumerator values.
+
+ The default is to consider enums, including anonymous enums, to be
+ distinct if there any differences in their enumerators' names or
+ values.
+
+ This option causes abidw to only check for differences between
+ sets of enumerator values.
+
* ``--check-alternate-debug-info`` <*elf-path*>
If the debug info for the file *elf-path* contains a reference to
diff --git a/include/abg-dwarf-reader.h b/include/abg-dwarf-reader.h
index 4c2de779..1ecc7ac4 100644
--- a/include/abg-dwarf-reader.h
+++ b/include/abg-dwarf-reader.h
@@ -142,6 +142,13 @@ void
set_drop_undefined_syms(read_context& ctxt,
bool f);
+bool
+get_ignore_enumerator_names(read_context& ctxt);
+
+void
+set_ignore_enumerator_names(read_context& ctxt,
+ bool f);
+
void
set_do_log(read_context& ctxt, bool f);
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 3f716944..f34bc3ee 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -1897,7 +1897,7 @@ struct dwarf_expr_eval_context
/// get some important data from it.
///
/// When a new data member is added to this context, it must be
-/// initiliazed by the read_context::initiliaze() function. So please
+/// initialized by the read_context::initialize() function. So please
/// do not forget.
class read_context
{
@@ -2116,6 +2116,7 @@ public:
corpus::exported_decls_builder* exported_decls_builder_;
options_type options_;
bool drop_undefined_syms_;
+ bool ignore_enumerator_names_;
read_context();
private:
@@ -2259,6 +2260,7 @@ public:
options_.load_in_linux_kernel_mode = linux_kernel_mode;
options_.load_all_types = load_all_types;
drop_undefined_syms_ = false;
+ ignore_enumerator_names_ = false;
load_in_linux_kernel_mode(linux_kernel_mode);
}
@@ -2346,6 +2348,23 @@ public:
drop_undefined_syms(bool f)
{drop_undefined_syms_ = f;}
+ /// Getter for the flag that tells if we should ignore enumerator
+ /// names during duplicate enum type resolution.
+ ///
+ /// @return true iff we should ignore enumerator names during
+ /// duplicate enum type resolution.
+ bool
+ ignore_enumerator_names() const
+ {return ignore_enumerator_names_;}
+
+ /// Setter for the flag that tells if we should ignore enumerator
+ /// names during duplicate enum type resolution.
+ ///
+ /// @param f the new value of the flag.
+ void
+ ignore_enumerator_names(bool f)
+ {ignore_enumerator_names_ = f;}
+
/// Getter of the suppression specifications to be used during
/// ELF/DWARF parsing.
///
@@ -4151,7 +4170,7 @@ public:
bool s0 = e->decl_only_class_equals_definition();
bool s1 = e->use_enum_binary_only_equality();
e->decl_only_class_equals_definition(true);
- e->use_enum_binary_only_equality(true);
+ e->use_enum_binary_only_equality(ignore_enumerator_names_);
bool equal = l == r;
e->decl_only_class_equals_definition(s0);
e->use_enum_binary_only_equality(s1);
@@ -6221,6 +6240,30 @@ void
set_drop_undefined_syms(read_context& ctxt, bool f)
{ctxt.drop_undefined_syms(f);}
+/// Getter of the "ignore_enumerator_names" flag.
+///
+/// This flag tells if we should ignore enumerator names during
+/// duplicate enum type resolution.
+///
+/// @param ctx the read context to consider for this flag.
+///
+/// @return the value of the flag.
+bool
+get_ignore_enumerator_names(read_context& ctxt)
+{return ctxt.ignore_enumerator_names();}
+
+/// Setter of the "ignore_enumerator_names" flag.
+///
+/// This flag tells if we should ignore enumerator names during
+/// duplicate enum type resolution.
+///
+/// @param ctxt the read context to consider for this flag.
+///
+/// @param f the value of the flag.
+void
+set_ignore_enumerator_names(read_context& ctxt, bool f)
+{ctxt.ignore_enumerator_names(f);}
+
/// Setter of the "do_log" flag.
///
/// This flag tells if we should emit verbose logs for various
diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h
index a01a1b2c..79b5e2dd 100644
--- a/src/abg-ir-priv.h
+++ b/src/abg-ir-priv.h
@@ -409,7 +409,7 @@ struct environment::priv
: canonicalization_is_done_(),
do_on_the_fly_canonicalization_(true),
decl_only_class_equals_definition_(false),
- use_enum_binary_only_equality_(true)
+ use_enum_binary_only_equality_(true) // NOTE: != abidw default
#ifdef WITH_DEBUG_SELF_COMPARISON
,
self_comparison_debug_on_(false)
diff --git a/tests/test-alt-dwarf-file.cc b/tests/test-alt-dwarf-file.cc
index 315296ab..13ccb961 100644
--- a/tests/test-alt-dwarf-file.cc
+++ b/tests/test-alt-dwarf-file.cc
@@ -56,7 +56,7 @@ InOutSpec in_out_specs[] =
{
"data/test-alt-dwarf-file/rhbz1951526/usr/bin/gimp-2.10",
"data/test-alt-dwarf-file/rhbz1951526/usr/lib/debug",
- "--abidiff",
+ "--abidiff --ignore-enumerator-names",
"data/test-alt-dwarf-file/rhbz1951526/rhbz1951526-report-0.txt",
"output/test-alt-dwarf-file/rhbz1951526/rhbz1951526-report-0.txt"
},
diff --git a/tools/abidw.cc b/tools/abidw.cc
index f7a8937d..ea2a4ad5 100644
--- a/tools/abidw.cc
+++ b/tools/abidw.cc
@@ -95,6 +95,7 @@ struct options
bool default_sizes;
bool load_all_types;
bool linux_kernel_mode;
+ bool ignore_enumerator_names;
bool corpus_group_for_linux;
bool show_stats;
bool noout;
@@ -132,6 +133,7 @@ struct options
default_sizes(true),
load_all_types(),
linux_kernel_mode(true),
+ ignore_enumerator_names(false),
corpus_group_for_linux(false),
show_stats(),
noout(),
@@ -206,6 +208,8 @@ display_usage(const string& prog_name, ostream& out)
"vmlinux and its modules\n"
<< " --vmlinux <path> the path to the vmlinux binary to consider to emit "
"the ABI of the union of vmlinux and its modules\n"
+ << " --ignore-enumerator-names only consider enumerator value sets when "
+ "deciding whether identically-named or anonymous enums are the same\n"
<< " --abidiff compare the loaded ABI against itself\n"
#ifdef WITH_DEBUG_SELF_COMPARISON
<< " --debug-abidiff debug the process of comparing the loaded ABI against itself\n"
@@ -367,6 +371,8 @@ parse_command_line(int argc, char* argv[], options& opts)
opts.drop_undefined_syms = true;
else if (!strcmp(argv[i], "--no-linux-kernel-mode"))
opts.linux_kernel_mode = false;
+ else if (!strcmp(argv[i], "--ignore-enumerator-names"))
+ opts.ignore_enumerator_names = true;
else if (!strcmp(argv[i], "--abidiff"))
opts.abidiff = true;
#ifdef WITH_DEBUG_SELF_COMPARISON
@@ -562,6 +568,7 @@ load_corpus_and_write_abixml(char* argv[],
opts.linux_kernel_mode);
dwarf_reader::read_context& ctxt = *c;
set_drop_undefined_syms(ctxt, opts.drop_undefined_syms);
+ set_ignore_enumerator_names(ctxt, opts.ignore_enumerator_names);
set_show_stats(ctxt, opts.show_stats);
set_suppressions(ctxt, opts);
abigail::dwarf_reader::set_do_log(ctxt, opts.do_log);
--
2.34.1.575.g55b058a8bb-goog
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] abidw: add --ignore-enumerator-names flag
2022-01-12 10:40 ` [PATCH v2] abidw: add --ignore-enumerator-names flag Giuliano Procida
@ 2022-01-12 11:06 ` Giuliano Procida
0 siblings, 0 replies; 4+ messages in thread
From: Giuliano Procida @ 2022-01-12 11:06 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, maennich
This doesn't actually fix the problem - the environment default
matters. I will follow up on Bug 28319.
Giuliano.
On Wed, 12 Jan 2022 at 10:40, Giuliano Procida <gprocida@google.com> wrote:
>
> Bug 28319 - abidw - regression in treatment of anonymous enums in structs
>
> This commit reinstates the abidw behaviour of comparing enumerator
> names during duplicate enum type resolution which was lost by default
> as of commit 1cfbff1b3037d1049bdff7e86de27c3a86af23b3.
>
> That commit addressed an issue with a certain library that contained
> duplicate enum type definitions with variations in the enumerator
> names but with the same enumerator values.
>
> Many projects will be able to avoid this issue by always compiling
> from fresh and consistent sources. This commit avoids the risk of
> conflating unrelated anonymous enum types.
>
> The previous behaviour can be requested with --ignore-enumerator-names
> and this flag is now used for that specific library test case.
>
> Note that code that uses libabigail library facilities directly will
> still continue to ignore enumerator names by default. This means that
> all other tests are also unaffected by this code change.
>
> * doc/manuals/abidw.rst: Document --ignore-enumerator-names.
> * include/abg-dwarf-reader.h (get_ignore_enumerator_names):
> New function.
> (set_ignore_enumerator_names): Likewise.
> * src/abg-dwarf-reader.cc (read_context): Fix documentation
> typos. Add ignore_enumerator_names_ member variable.
> (read_context::initialize): Set ignore_enumerator_names_ to
> false.
> (read_context::ignore_enumerator_names): New getter and
> setter methods.
> (compare_before_canonicalisation): Set environment flag
> use_enum_binary_only_equality according to the value of
> ignore_enumerator_names_.
> (get_ignore_enumerator_names): New function.
> (set_ignore_enumerator_names): Likewise.
> * src/abg-ir-priv.h (environment::priv::priv): Add comment to
> default use_enum_binary_only_equality value.
> * tests/test-alt-dwarf-file.cc: Add --ignore-enumerator-names
> to rhbz1951526 test.
> * tools/abidw.cc (options): Add ignore_enumerator_names_
> option, defaulted to false.
> (parse_command_line): Parse --ignore-enumerator-names flag.
> (load_corpus_and_write_abixml): Set ignore_enumerator_names
> flag in DWARF reader context.
>
> Reviewed-by: Matthias Maennich <maennich@google.com>
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> ---
>
> This is really just for-the-record. This is the commit we are going to
> land in AOSP. I've kept the impact on tests minimal to ease rebase,
> revert etc.
>
> If you cannot find a better way forward on Bug 28319, then perhaps
> take this commit, flip the default in src/abg-ir-priv.h and refresh
> the few tests affected?
>
> That still leaves whoever cares about oddly-compiled libraries having
> to pass a flag manually though. Could something be done with abidiff
> suppressions instead?
>
> Regards,
> Giuliano.
>
> doc/manuals/abidw.rst | 12 +++++++++
> include/abg-dwarf-reader.h | 7 ++++++
> src/abg-dwarf-reader.cc | 47 ++++++++++++++++++++++++++++++++++--
> src/abg-ir-priv.h | 2 +-
> tests/test-alt-dwarf-file.cc | 2 +-
> tools/abidw.cc | 7 ++++++
> 6 files changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/doc/manuals/abidw.rst b/doc/manuals/abidw.rst
> index bdd6204d..53a0a18f 100644
> --- a/doc/manuals/abidw.rst
> +++ b/doc/manuals/abidw.rst
> @@ -182,6 +182,18 @@ Options
> ELF binary. It thus considers functions and variables which are
> defined and exported in the ELF sense.
>
> + * ``--ignore-enumerator-names``
> +
> + During resolution of duplicate enum types, abidw can consider both
> + enumerator names and values, or just enumerator values.
> +
> + The default is to consider enums, including anonymous enums, to be
> + distinct if there any differences in their enumerators' names or
> + values.
> +
> + This option causes abidw to only check for differences between
> + sets of enumerator values.
> +
> * ``--check-alternate-debug-info`` <*elf-path*>
>
> If the debug info for the file *elf-path* contains a reference to
> diff --git a/include/abg-dwarf-reader.h b/include/abg-dwarf-reader.h
> index 4c2de779..1ecc7ac4 100644
> --- a/include/abg-dwarf-reader.h
> +++ b/include/abg-dwarf-reader.h
> @@ -142,6 +142,13 @@ void
> set_drop_undefined_syms(read_context& ctxt,
> bool f);
>
> +bool
> +get_ignore_enumerator_names(read_context& ctxt);
> +
> +void
> +set_ignore_enumerator_names(read_context& ctxt,
> + bool f);
> +
> void
> set_do_log(read_context& ctxt, bool f);
>
> diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
> index 3f716944..f34bc3ee 100644
> --- a/src/abg-dwarf-reader.cc
> +++ b/src/abg-dwarf-reader.cc
> @@ -1897,7 +1897,7 @@ struct dwarf_expr_eval_context
> /// get some important data from it.
> ///
> /// When a new data member is added to this context, it must be
> -/// initiliazed by the read_context::initiliaze() function. So please
> +/// initialized by the read_context::initialize() function. So please
> /// do not forget.
> class read_context
> {
> @@ -2116,6 +2116,7 @@ public:
> corpus::exported_decls_builder* exported_decls_builder_;
> options_type options_;
> bool drop_undefined_syms_;
> + bool ignore_enumerator_names_;
> read_context();
>
> private:
> @@ -2259,6 +2260,7 @@ public:
> options_.load_in_linux_kernel_mode = linux_kernel_mode;
> options_.load_all_types = load_all_types;
> drop_undefined_syms_ = false;
> + ignore_enumerator_names_ = false;
> load_in_linux_kernel_mode(linux_kernel_mode);
> }
>
> @@ -2346,6 +2348,23 @@ public:
> drop_undefined_syms(bool f)
> {drop_undefined_syms_ = f;}
>
> + /// Getter for the flag that tells if we should ignore enumerator
> + /// names during duplicate enum type resolution.
> + ///
> + /// @return true iff we should ignore enumerator names during
> + /// duplicate enum type resolution.
> + bool
> + ignore_enumerator_names() const
> + {return ignore_enumerator_names_;}
> +
> + /// Setter for the flag that tells if we should ignore enumerator
> + /// names during duplicate enum type resolution.
> + ///
> + /// @param f the new value of the flag.
> + void
> + ignore_enumerator_names(bool f)
> + {ignore_enumerator_names_ = f;}
> +
> /// Getter of the suppression specifications to be used during
> /// ELF/DWARF parsing.
> ///
> @@ -4151,7 +4170,7 @@ public:
> bool s0 = e->decl_only_class_equals_definition();
> bool s1 = e->use_enum_binary_only_equality();
> e->decl_only_class_equals_definition(true);
> - e->use_enum_binary_only_equality(true);
> + e->use_enum_binary_only_equality(ignore_enumerator_names_);
> bool equal = l == r;
> e->decl_only_class_equals_definition(s0);
> e->use_enum_binary_only_equality(s1);
> @@ -6221,6 +6240,30 @@ void
> set_drop_undefined_syms(read_context& ctxt, bool f)
> {ctxt.drop_undefined_syms(f);}
>
> +/// Getter of the "ignore_enumerator_names" flag.
> +///
> +/// This flag tells if we should ignore enumerator names during
> +/// duplicate enum type resolution.
> +///
> +/// @param ctx the read context to consider for this flag.
> +///
> +/// @return the value of the flag.
> +bool
> +get_ignore_enumerator_names(read_context& ctxt)
> +{return ctxt.ignore_enumerator_names();}
> +
> +/// Setter of the "ignore_enumerator_names" flag.
> +///
> +/// This flag tells if we should ignore enumerator names during
> +/// duplicate enum type resolution.
> +///
> +/// @param ctxt the read context to consider for this flag.
> +///
> +/// @param f the value of the flag.
> +void
> +set_ignore_enumerator_names(read_context& ctxt, bool f)
> +{ctxt.ignore_enumerator_names(f);}
> +
> /// Setter of the "do_log" flag.
> ///
> /// This flag tells if we should emit verbose logs for various
> diff --git a/src/abg-ir-priv.h b/src/abg-ir-priv.h
> index a01a1b2c..79b5e2dd 100644
> --- a/src/abg-ir-priv.h
> +++ b/src/abg-ir-priv.h
> @@ -409,7 +409,7 @@ struct environment::priv
> : canonicalization_is_done_(),
> do_on_the_fly_canonicalization_(true),
> decl_only_class_equals_definition_(false),
> - use_enum_binary_only_equality_(true)
> + use_enum_binary_only_equality_(true) // NOTE: != abidw default
> #ifdef WITH_DEBUG_SELF_COMPARISON
> ,
> self_comparison_debug_on_(false)
> diff --git a/tests/test-alt-dwarf-file.cc b/tests/test-alt-dwarf-file.cc
> index 315296ab..13ccb961 100644
> --- a/tests/test-alt-dwarf-file.cc
> +++ b/tests/test-alt-dwarf-file.cc
> @@ -56,7 +56,7 @@ InOutSpec in_out_specs[] =
> {
> "data/test-alt-dwarf-file/rhbz1951526/usr/bin/gimp-2.10",
> "data/test-alt-dwarf-file/rhbz1951526/usr/lib/debug",
> - "--abidiff",
> + "--abidiff --ignore-enumerator-names",
> "data/test-alt-dwarf-file/rhbz1951526/rhbz1951526-report-0.txt",
> "output/test-alt-dwarf-file/rhbz1951526/rhbz1951526-report-0.txt"
> },
> diff --git a/tools/abidw.cc b/tools/abidw.cc
> index f7a8937d..ea2a4ad5 100644
> --- a/tools/abidw.cc
> +++ b/tools/abidw.cc
> @@ -95,6 +95,7 @@ struct options
> bool default_sizes;
> bool load_all_types;
> bool linux_kernel_mode;
> + bool ignore_enumerator_names;
> bool corpus_group_for_linux;
> bool show_stats;
> bool noout;
> @@ -132,6 +133,7 @@ struct options
> default_sizes(true),
> load_all_types(),
> linux_kernel_mode(true),
> + ignore_enumerator_names(false),
> corpus_group_for_linux(false),
> show_stats(),
> noout(),
> @@ -206,6 +208,8 @@ display_usage(const string& prog_name, ostream& out)
> "vmlinux and its modules\n"
> << " --vmlinux <path> the path to the vmlinux binary to consider to emit "
> "the ABI of the union of vmlinux and its modules\n"
> + << " --ignore-enumerator-names only consider enumerator value sets when "
> + "deciding whether identically-named or anonymous enums are the same\n"
> << " --abidiff compare the loaded ABI against itself\n"
> #ifdef WITH_DEBUG_SELF_COMPARISON
> << " --debug-abidiff debug the process of comparing the loaded ABI against itself\n"
> @@ -367,6 +371,8 @@ parse_command_line(int argc, char* argv[], options& opts)
> opts.drop_undefined_syms = true;
> else if (!strcmp(argv[i], "--no-linux-kernel-mode"))
> opts.linux_kernel_mode = false;
> + else if (!strcmp(argv[i], "--ignore-enumerator-names"))
> + opts.ignore_enumerator_names = true;
> else if (!strcmp(argv[i], "--abidiff"))
> opts.abidiff = true;
> #ifdef WITH_DEBUG_SELF_COMPARISON
> @@ -562,6 +568,7 @@ load_corpus_and_write_abixml(char* argv[],
> opts.linux_kernel_mode);
> dwarf_reader::read_context& ctxt = *c;
> set_drop_undefined_syms(ctxt, opts.drop_undefined_syms);
> + set_ignore_enumerator_names(ctxt, opts.ignore_enumerator_names);
> set_show_stats(ctxt, opts.show_stats);
> set_suppressions(ctxt, opts);
> abigail::dwarf_reader::set_do_log(ctxt, opts.do_log);
> --
> 2.34.1.575.g55b058a8bb-goog
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-12 11:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-11 16:10 [PATCH 0/1] Bug 28319 - abidw - regression in treatment of anonymous enums in structs Giuliano Procida
2022-01-11 16:10 ` [PATCH 1/1] abidw: add --consider-enumerator-names flag Giuliano Procida
2022-01-12 10:40 ` [PATCH v2] abidw: add --ignore-enumerator-names flag Giuliano Procida
2022-01-12 11:06 ` Giuliano Procida
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).