* [PATCH 0/1] Add functionality to disable test for specific architecture @ 2023-02-23 14:18 Stephan Rohr 2023-02-23 14:18 ` [PATCH 1/1] gdb: " Stephan Rohr 0 siblings, 1 reply; 4+ messages in thread From: Stephan Rohr @ 2023-02-23 14:18 UTC (permalink / raw) To: gdb-patches From: "Rohr, Stephan" <stephan.rohr@intel.com> Hi all, the proposed patch adds functionality to disable a test for an architecture as specified by the user. The current 'register_test_foreach_arch' implementation skips all tests if the architecture is listed inside the 'skip_arch' function. However, if the user wants to skip a limited set of tests for a given architecture, this is currently not supported by the interface. The proposed patch extends the 'register_test_foreach_arch' interface and allows the user to provide a set of architectures for which the test shall not be executed, e.g., selftests::register_test_foreach_arch ("test_name", function_name, std::set<std::string> ({"arch"})); I tested this patch with the following targets: * unix * unix/-m32 * native-gdbserver * native-extended-gdbserver No regressions are identified. Rohr, Stephan (1): gdb: Add functionality to disable test for specific architecture gdb/selftest-arch.c | 11 ++++++++--- gdb/selftest-arch.h | 5 ++++- 2 files changed, 12 insertions(+), 4 deletions(-) -- 2.25.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] gdb: Add functionality to disable test for specific architecture 2023-02-23 14:18 [PATCH 0/1] Add functionality to disable test for specific architecture Stephan Rohr @ 2023-02-23 14:18 ` Stephan Rohr 2023-02-23 17:51 ` Andrew Burgess 0 siblings, 1 reply; 4+ messages in thread From: Stephan Rohr @ 2023-02-23 14:18 UTC (permalink / raw) To: gdb-patches From: "Rohr, Stephan" <stephan.rohr@intel.com> A set of not supported architectures can be provided to the 'register_test_for_earch_arch' function such that this test is skipped for all architectures contained in this set. --- gdb/selftest-arch.c | 11 ++++++++--- gdb/selftest-arch.h | 5 ++++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c index 691c40c14d7..d8fe49a1859 100644 --- a/gdb/selftest-arch.c +++ b/gdb/selftest-arch.c @@ -47,7 +47,8 @@ static bool skip_arch (const char *arch) static std::vector<selftest> foreach_arch_test_generator (const std::string &name, - self_test_foreach_arch_function *function) + self_test_foreach_arch_function *function, + const std::set<std::string> &disabled_arch) { std::vector<selftest> tests; std::vector<const char *> arches = gdbarch_printable_names (); @@ -57,6 +58,9 @@ foreach_arch_test_generator (const std::string &name, if (skip_arch (arch)) continue; + if (disabled_arch.count (arch) > 0) + continue; + struct gdbarch_info info; info.bfd_arch_info = bfd_scan_arch (arch); info.osabi = GDB_OSABI_NONE; @@ -96,11 +100,12 @@ foreach_arch_test_generator (const std::string &name, void register_test_foreach_arch (const std::string &name, - self_test_foreach_arch_function *function) + self_test_foreach_arch_function *function, + const std::set<std::string> &disabled_arch) { add_lazy_generator ([=] () { - return foreach_arch_test_generator (name, function); + return foreach_arch_test_generator (name, function, disabled_arch); }); } diff --git a/gdb/selftest-arch.h b/gdb/selftest-arch.h index a925e5a00dc..70b7f5224f8 100644 --- a/gdb/selftest-arch.h +++ b/gdb/selftest-arch.h @@ -19,6 +19,8 @@ #ifndef SELFTEST_ARCH_H #define SELFTEST_ARCH_H +#include <set> + typedef void self_test_foreach_arch_function (struct gdbarch *); namespace selftests @@ -28,7 +30,8 @@ namespace selftests extern void register_test_foreach_arch (const std::string &name, - self_test_foreach_arch_function *function); + self_test_foreach_arch_function *function, + const std::set<std::string> &disabled_arch = {}); } #endif /* SELFTEST_ARCH_H */ -- 2.25.1 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] gdb: Add functionality to disable test for specific architecture 2023-02-23 14:18 ` [PATCH 1/1] gdb: " Stephan Rohr @ 2023-02-23 17:51 ` Andrew Burgess 2023-03-01 7:42 ` Rohr, Stephan 0 siblings, 1 reply; 4+ messages in thread From: Andrew Burgess @ 2023-02-23 17:51 UTC (permalink / raw) To: Stephan Rohr, gdb-patches Stephan Rohr via Gdb-patches <gdb-patches@sourceware.org> writes: > From: "Rohr, Stephan" <stephan.rohr@intel.com> > > A set of not supported architectures can be provided to the > 'register_test_for_earch_arch' function such that this test > is skipped for all architectures contained in this set. Generally we avoid having unused code in upstream. That doesn't mean this patch couldn't go in, but I think you'd need to make a compelling argument for it in the commit message. Also, I'd be interested to understand more why this is needed. The current skip_arch seems to cover a set of architectures that GDB just doesn't support, so we wouldn't expect any tests to work. This change would seem to indicate you have an architecture for which some tests pass, but others don't. Yet you feel that's not a bug in either the test, or how GDB handles the architecture. It would be great to give some examples of this in the commit message. Thanks, Andrew > --- > gdb/selftest-arch.c | 11 ++++++++--- > gdb/selftest-arch.h | 5 ++++- > 2 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c > index 691c40c14d7..d8fe49a1859 100644 > --- a/gdb/selftest-arch.c > +++ b/gdb/selftest-arch.c > @@ -47,7 +47,8 @@ static bool skip_arch (const char *arch) > > static std::vector<selftest> > foreach_arch_test_generator (const std::string &name, > - self_test_foreach_arch_function *function) > + self_test_foreach_arch_function *function, > + const std::set<std::string> &disabled_arch) > { > std::vector<selftest> tests; > std::vector<const char *> arches = gdbarch_printable_names (); > @@ -57,6 +58,9 @@ foreach_arch_test_generator (const std::string &name, > if (skip_arch (arch)) > continue; > > + if (disabled_arch.count (arch) > 0) > + continue; > + > struct gdbarch_info info; > info.bfd_arch_info = bfd_scan_arch (arch); > info.osabi = GDB_OSABI_NONE; > @@ -96,11 +100,12 @@ foreach_arch_test_generator (const std::string &name, > > void > register_test_foreach_arch (const std::string &name, > - self_test_foreach_arch_function *function) > + self_test_foreach_arch_function *function, > + const std::set<std::string> &disabled_arch) > { > add_lazy_generator ([=] () > { > - return foreach_arch_test_generator (name, function); > + return foreach_arch_test_generator (name, function, disabled_arch); > }); > } > > diff --git a/gdb/selftest-arch.h b/gdb/selftest-arch.h > index a925e5a00dc..70b7f5224f8 100644 > --- a/gdb/selftest-arch.h > +++ b/gdb/selftest-arch.h > @@ -19,6 +19,8 @@ > #ifndef SELFTEST_ARCH_H > #define SELFTEST_ARCH_H > > +#include <set> > + > typedef void self_test_foreach_arch_function (struct gdbarch *); > > namespace selftests > @@ -28,7 +30,8 @@ namespace selftests > > extern void > register_test_foreach_arch (const std::string &name, > - self_test_foreach_arch_function *function); > + self_test_foreach_arch_function *function, > + const std::set<std::string> &disabled_arch = {}); > } > > #endif /* SELFTEST_ARCH_H */ > -- > 2.25.1 > > Intel Deutschland GmbH > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva > Chairperson of the Supervisory Board: Nicole Lau > Registered Office: Munich > Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH 1/1] gdb: Add functionality to disable test for specific architecture 2023-02-23 17:51 ` Andrew Burgess @ 2023-03-01 7:42 ` Rohr, Stephan 0 siblings, 0 replies; 4+ messages in thread From: Rohr, Stephan @ 2023-03-01 7:42 UTC (permalink / raw) To: Andrew Burgess, gdb-patches Hi Andrew, Thanks for you feedback. Please see my answers below. Thanks Stephan > -----Original Message----- > From: Andrew Burgess <aburgess@redhat.com> > Sent: Thursday, February 23, 2023 6:51 PM > To: Rohr, Stephan <stephan.rohr@intel.com>; gdb-patches@sourceware.org > Subject: Re: [PATCH 1/1] gdb: Add functionality to disable test for specific > architecture > > Stephan Rohr via Gdb-patches <gdb-patches@sourceware.org> writes: > > > From: "Rohr, Stephan" <stephan.rohr@intel.com> > > > > A set of not supported architectures can be provided to the > > 'register_test_for_earch_arch' function such that this test > > is skipped for all architectures contained in this set. > > Generally we avoid having unused code in upstream. That doesn't mean > this patch couldn't go in, but I think you'd need to make a compelling > argument for it in the commit message. > We have a few self-tests where some tests pass whilst other fails for our internal architecture. I understand this use case is currently not relevant for the GDB master. I created this patch to prepare for our upstreaming work and it might also become relevant for others. I understand if the recommendation is to postpone this patch until there is a real need on the master. I'd appreciate additional feedback. > Also, I'd be interested to understand more why this is needed. The > current skip_arch seems to cover a set of architectures that GDB just > doesn't support, so we wouldn't expect any tests to work. > I agree here, the current skip_arch implementation allows to skip all tests for architectures that are not supported by GDB. However, if some tests are supported whilst others are not, this use case is currently not supported by GDB. > This change would seem to indicate you have an architecture for which > some tests pass, but others don't. Yet you feel that's not a bug in > either the test, or how GDB handles the architecture. It would be great > to give some examples of this in the commit message. > Your understanding is correct, we have some self-tests passing whilst others are known to fail. I will update the commit message is there is a consent on the acceptance of the patch. > Thanks, > Andrew > > > > > > --- > > gdb/selftest-arch.c | 11 ++++++++--- > > gdb/selftest-arch.h | 5 ++++- > > 2 files changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c > > index 691c40c14d7..d8fe49a1859 100644 > > --- a/gdb/selftest-arch.c > > +++ b/gdb/selftest-arch.c > > @@ -47,7 +47,8 @@ static bool skip_arch (const char *arch) > > > > static std::vector<selftest> > > foreach_arch_test_generator (const std::string &name, > > - self_test_foreach_arch_function *function) > > + self_test_foreach_arch_function *function, > > + const std::set<std::string> &disabled_arch) > > { > > std::vector<selftest> tests; > > std::vector<const char *> arches = gdbarch_printable_names (); > > @@ -57,6 +58,9 @@ foreach_arch_test_generator (const std::string > &name, > > if (skip_arch (arch)) > > continue; > > > > + if (disabled_arch.count (arch) > 0) > > + continue; > > + > > struct gdbarch_info info; > > info.bfd_arch_info = bfd_scan_arch (arch); > > info.osabi = GDB_OSABI_NONE; > > @@ -96,11 +100,12 @@ foreach_arch_test_generator (const std::string > &name, > > > > void > > register_test_foreach_arch (const std::string &name, > > - self_test_foreach_arch_function *function) > > + self_test_foreach_arch_function *function, > > + const std::set<std::string> &disabled_arch) > > { > > add_lazy_generator ([=] () > > { > > - return foreach_arch_test_generator (name, function); > > + return foreach_arch_test_generator (name, > function, disabled_arch); > > }); > > } > > > > diff --git a/gdb/selftest-arch.h b/gdb/selftest-arch.h > > index a925e5a00dc..70b7f5224f8 100644 > > --- a/gdb/selftest-arch.h > > +++ b/gdb/selftest-arch.h > > @@ -19,6 +19,8 @@ > > #ifndef SELFTEST_ARCH_H > > #define SELFTEST_ARCH_H > > > > +#include <set> > > + > > typedef void self_test_foreach_arch_function (struct gdbarch *); > > > > namespace selftests > > @@ -28,7 +30,8 @@ namespace selftests > > > > extern void > > register_test_foreach_arch (const std::string &name, > > - self_test_foreach_arch_function *function); > > + self_test_foreach_arch_function *function, > > + const std::set<std::string> &disabled_arch = {}); > > } > > > > #endif /* SELFTEST_ARCH_H */ > > -- > > 2.25.1 > > > > Intel Deutschland GmbH > > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany > > Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> > > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva > > Chairperson of the Supervisory Board: Nicole Lau > > Registered Office: Munich > > Commercial Register: Amtsgericht Muenchen HRB 186928 Intel Deutschland GmbH Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-03-01 7:42 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-23 14:18 [PATCH 0/1] Add functionality to disable test for specific architecture Stephan Rohr 2023-02-23 14:18 ` [PATCH 1/1] gdb: " Stephan Rohr 2023-02-23 17:51 ` Andrew Burgess 2023-03-01 7:42 ` Rohr, Stephan
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).