public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC/PATCH] Don't disable selftests in a non-development build
@ 2018-08-14  5:42 Sergio Durigan Junior
  2018-08-14 16:28 ` Sergio Durigan Junior
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Sergio Durigan Junior @ 2018-08-14  5:42 UTC (permalink / raw)
  To: GDB Patches; +Cc: Joel Brobecker, Sergio Durigan Junior

This is an idea I mentioned a few days ago in our IRC channel, and
Simon said he'd be open to it, so I'm send this RFC/PATCH to see what
others think.

Due to the many racy testcases and random failures we see when running
the GDB testsuite, it is unfortunately not possible to perform a full
test when one is building a downstream package.  As the Fedora GDB
maintainer and one of the Debian GDB uploaders, I feel like this
situation could be improved by, at least, executing our selftests
after the package has been built.  However, we currently (for some
reason that is not clear by reading the archives, but see more below)
disable selftests on non-development builds.  Therefore, this patch
aims to leave them enabled all the time, for everyone (including the
end users).

I understand that disabling the selftests in a non-development build
brings a few advantages.  For example, we ship less code (even though
the final difference may be small enough that it doesn't really have
any real benefit).  Another thing to consider is that we disable
portions of the codebase that could have latent bugs, making GDB a bit
safer.  I don't really have an argument to compete with this;
nonetheless, I think the benefit of having "batteries included" when
it comes to testing is a huge thing.  With our selftests, the user
doesn't need to install any external
programs/libraries (dejagnu/tcl/etc.), and they're stable enough that
we know that they don't suffer from the racyness present in our
current testsuite.

Anyway, here's my take at the problem.  The patches are mostly
mechanical (getting rid of the "GDB_SELF_TEST" define, which becomes
useless now that selftests are always present); the ones that aren't
deal with the configure/Makefile machinery to always compile the
selftests source files.

I've tested this patch on BuildBot, without regressions.  I'm taking
the liberty to Cc Joel, who is our release manager and might have
opinions about this.

OK?

gdb/ChangeLog:
2018-08-14  Sergio Durigan Junior  <sergiodj@redhat.com>

	* Makefile.in (COMMON_SFILES): Add 'common/selftest.c'
	and 'selftest-arch.c'.
	(SFILES): Add '$(SUBDIR_UNITTESTS_SRCS)'.
	(HFILES_NO_SRCDIR): Add 'common/selftest.h'.
	(COMMON_OBS): Add '$(SUBDIR_UNITTESTS_OBS)'.
	* aarch64-tdep.c: Remove '#if GDB_SELF_TEST' guards.
	* amd64-linux-tdep.c: Likewise.
	* amd64-tdep.c: Likewise.
	* arm-tdep.c: Likewise.
	* config.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Don't define 'GDB_SELF_TEST'; remove setting
	of 'CONFIG_OBS' and 'CONFIG_SRCS' with selftests
	objects/sources.
	* cp-support.c: Remove '#if GDB_SELF_TEST' guards.
	* disasm-selftests.c: Likewise.
	* dwarf-index-cache.c: Likewise.
	* dwarf2-frame.c: Likewise.
	* dwarf2loc.c: Likewise.
	* dwarf2read.c: Likewise.
	* findvar.c: Likewise.
	* gdbarch-selftests.c: Likewise.
	* i386-linux-tdep.c: Likewise.
	* i386-tdep.c: Likewise.
	* maint.c: Likewise.
	(maintenance_selftest): Remove unneded message about selftests
	not available in a non-development build.
	(maintenance_info_selftests): Likewise.
	* producer.c: Remove '#if GDB_SELF_TEST' guards.
	* regcache.c: Likewise.
	* rust-exp.y: Likewise.
	* selftest-arch.c: Likewise.
	* symfile.c: Likewise.
	* target-descriptions.c: Likewise.
	* target-descriptions.h: Likewise.
	* target.c: Likewise.
	* target.h: Likewise.
	* utils.c: Likewise.
	* value.c: Likewise.

gdb/gdbserver/ChangeLog:
2018-08-14  Sergio Durigan Junior  <sergiodj@redhat.com>

	* Makefile.in (SFILES): Add '$(srcdir)/common/selftest.c'.
	(OBS): Add 'common/selftest.o'.
	* config.in: Regenerate.
	* configure: Regenerate.
	* configure.ac: Don't define 'GDB_SELF_TEST'.  Don't add
	'$srv_selftest_obs' to 'GDBSERVER_DEPFILES'.
	* configure.srv: Always set 'srv_i386_linux_regobj' and
	'srv_amd64_linux_regobj' with selftest-related objects.
	Always set 'srv_regobj' with selftest-related objects.
	* linux-aarch64-low.c: Remove '#if GDB_SELF_TEST' guards.
	* linux-aarch64-tdesc.h: Likewise.
	* linux-tic6x-low.c: Likewise.
	* linux-x86-low.c: Likewise.
	* server.c: Likewise.
	(captured_main): Remove unneded message about selftests not
	available in a non-development build.
	*

gdb/testsuite/ChangeLog:
2018-08-14  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.gdb/unittest.exp: Remove match for "Selftests are not
	available in a non-development build."
	* gdb.server/unittest.exp: Likewise.
---
 gdb/Makefile.in                       |  9 +++++++--
 gdb/aarch64-tdep.c                    |  8 --------
 gdb/amd64-linux-tdep.c                |  2 --
 gdb/amd64-tdep.c                      |  2 --
 gdb/arm-tdep.c                        |  8 --------
 gdb/config.in                         |  3 ---
 gdb/configure                         |  8 --------
 gdb/configure.ac                      |  7 -------
 gdb/cp-support.c                      |  5 -----
 gdb/disasm-selftests.c                |  4 ----
 gdb/dwarf-index-cache.c               |  8 +++++---
 gdb/dwarf2-frame.c                    |  6 ------
 gdb/dwarf2loc.c                       |  4 ----
 gdb/dwarf2read.c                      |  4 ----
 gdb/findvar.c                         |  4 ----
 gdb/gdbarch-selftests.c               |  4 ----
 gdb/gdbserver/Makefile.in             |  2 ++
 gdb/gdbserver/config.in               |  3 ---
 gdb/gdbserver/configure               |  7 -------
 gdb/gdbserver/configure.ac            |  8 +-------
 gdb/gdbserver/configure.srv           | 25 +++++++------------------
 gdb/gdbserver/linux-aarch64-low.c     |  2 --
 gdb/gdbserver/linux-aarch64-tdesc.h   |  2 --
 gdb/gdbserver/linux-tic6x-low.c       |  4 ----
 gdb/gdbserver/linux-x86-low.c         |  2 --
 gdb/gdbserver/server.c                | 10 ----------
 gdb/i386-linux-tdep.c                 |  2 --
 gdb/i386-tdep.c                       |  2 --
 gdb/maint.c                           | 10 ----------
 gdb/producer.c                        |  4 ----
 gdb/regcache.c                        |  4 ----
 gdb/rust-exp.y                        |  4 ----
 gdb/selftest-arch.c                   |  2 --
 gdb/symfile.c                         |  4 ----
 gdb/target-descriptions.c             |  2 --
 gdb/target-descriptions.h             |  2 --
 gdb/target.c                          |  2 --
 gdb/target.h                          |  2 --
 gdb/testsuite/gdb.gdb/unittest.exp    |  4 ----
 gdb/testsuite/gdb.server/unittest.exp |  4 ----
 gdb/utils.c                           |  4 ----
 gdb/value.c                           |  4 ----
 42 files changed, 22 insertions(+), 185 deletions(-)

diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index 118c3c8062..8c8c23fd6a 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -966,6 +966,7 @@ COMMON_SFILES = \
 	common/rsp-low.c \
 	common/run-time-clock.c \
 	common/scoped_mmap.c \
+	common/selftest.c \
 	common/signals.c \
 	common/signals-state-save-restore.c \
 	common/tdesc.c \
@@ -1092,6 +1093,7 @@ COMMON_SFILES = \
 	remote-notif.c \
 	reverse.c \
 	rust-lang.c \
+	selftest-arch.c \
 	sentinel-frame.c \
 	ser-event.c \
 	serial.c \
@@ -1162,7 +1164,8 @@ SFILES = \
 	$(SUBDIR_CLI_SRCS) \
 	$(SUBDIR_TARGET_SRCS) \
 	$(COMMON_SFILES) \
-	$(SUBDIR_GCC_COMPILE_SRCS)
+	$(SUBDIR_GCC_COMPILE_SRCS) \
+	$(SUBDIR_UNITTESTS_SRCS)
 
 # Header files that need to have srcdir added.  Note that in the cases
 # where we use a macro like $(gdbcmd_h), things are carefully arranged
@@ -1452,6 +1455,7 @@ HFILES_NO_SRCDIR = \
 	common/queue.h \
 	common/rsp-low.h \
 	common/run-time-clock.h \
+	common/selftest.h \
 	common/signals-state-save-restore.h \
 	common/symbol.h \
 	common/tdesc.h \
@@ -1566,7 +1570,8 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
 	$(patsubst %.c,%.o,$(COMMON_SFILES)) \
 	$(SUBDIR_CLI_OBS) \
 	$(SUBDIR_TARGET_OBS) \
-	$(SUBDIR_GCC_COMPILE_OBS)
+	$(SUBDIR_GCC_COMPILE_OBS) \
+	$(SUBDIR_UNITTESTS_OBS)
 
 SUBDIRS = doc @subdirs@ data-directory $(GNULIB_BUILDDIR)
 CLEANDIRS = $(SUBDIRS)
diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
index 5c6eb98545..00f9456954 100644
--- a/gdb/aarch64-tdep.c
+++ b/gdb/aarch64-tdep.c
@@ -516,7 +516,6 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
 				   reader);
 }
 
-#if GDB_SELF_TEST
 
 namespace selftests {
 
@@ -640,7 +639,6 @@ aarch64_analyze_prologue_test (void)
   }
 }
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
 
 /* Implement the "skip_prologue" gdbarch method.  */
 
@@ -3154,12 +3152,10 @@ aarch64_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
 		      paddress (gdbarch, tdep->lowest_pc));
 }
 
-#if GDB_SELF_TEST
 namespace selftests
 {
 static void aarch64_process_record_test (void);
 }
-#endif
 
 void
 _initialize_aarch64_tdep (void)
@@ -3176,14 +3172,12 @@ When on, AArch64 specific debugging is enabled."),
 			    show_aarch64_debug,
 			    &setdebuglist, &showdebuglist);
 
-#if GDB_SELF_TEST
   selftests::register_test ("aarch64-analyze-prologue",
 			    selftests::aarch64_analyze_prologue_test);
   selftests::register_test ("aarch64-process-record",
 			    selftests::aarch64_process_record_test);
   selftests::record_xml_tdesc ("aarch64.xml",
 			       aarch64_create_target_description (0));
-#endif
 }
 
 /* AArch64 process record-replay related structures, defines etc.  */
@@ -4085,7 +4079,6 @@ deallocate_reg_mem (insn_decode_record *record)
   xfree (record->aarch64_mems);
 }
 
-#if GDB_SELF_TEST
 namespace selftests {
 
 static void
@@ -4118,7 +4111,6 @@ aarch64_process_record_test (void)
 }
 
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
 
 /* Parse the current instruction and record the values of the registers and
    memory that will be changed in current instruction to record_arch_list
diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
index 7ab43897ab..19432b3fda 100644
--- a/gdb/amd64-linux-tdep.c
+++ b/gdb/amd64-linux-tdep.c
@@ -2280,7 +2280,6 @@ _initialize_amd64_linux_tdep (void)
   gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x64_32,
 			  GDB_OSABI_LINUX, amd64_x32_linux_init_abi);
 
-#if GDB_SELF_TEST
   struct
   {
     const char *xml;
@@ -2306,5 +2305,4 @@ _initialize_amd64_linux_tdep (void)
 
       selftests::record_xml_tdesc (a.xml, tdesc);
     }
-#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
index 088542d72b..4c88ce66ca 100644
--- a/gdb/amd64-tdep.c
+++ b/gdb/amd64-tdep.c
@@ -3300,7 +3300,6 @@ _initialize_amd64_tdep (void)
   gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x64_32, GDB_OSABI_NONE,
  			  amd64_x32_none_init_abi);
 
-#if GDB_SELF_TEST
   struct
   {
     const char *xml;
@@ -3321,7 +3320,6 @@ _initialize_amd64_tdep (void)
 
       selftests::record_xml_tdesc (a.xml, tdesc);
     }
-#endif /* GDB_SELF_TEST */
 }
 \f
 
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index c3280ee211..ac63594e5f 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -69,9 +69,7 @@
 #include "features/arm/arm-with-vfpv3.c"
 #include "features/arm/arm-with-neon.c"
 
-#if GDB_SELF_TEST
 #include "selftest.h"
-#endif
 
 static int arm_debug;
 
@@ -9523,12 +9521,10 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
 		      (unsigned long) tdep->lowest_pc);
 }
 
-#if GDB_SELF_TEST
 namespace selftests
 {
 static void arm_record_test (void);
 }
-#endif
 
 void
 _initialize_arm_tdep (void)
@@ -9664,9 +9660,7 @@ vfp - VFP co-processor."),
 			   NULL, /* FIXME: i18n: "ARM debugging is %s.  */
 			   &setdebuglist, &showdebuglist);
 
-#if GDB_SELF_TEST
   selftests::register_test ("arm-record", selftests::arm_record_test);
-#endif
 
 }
 
@@ -13199,7 +13193,6 @@ decode_insn (abstract_memory_reader &reader, insn_decode_record *arm_record,
   return ret;
 }
 
-#if GDB_SELF_TEST
 namespace selftests {
 
 /* Provide both 16-bit and 32-bit thumb instructions.  */
@@ -13303,7 +13296,6 @@ arm_record_test (void)
   }
 }
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
 
 /* Cleans up local record registers and memory allocations.  */
 
diff --git a/gdb/config.in b/gdb/config.in
index 01acda124b..75c1dd2930 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -65,9 +65,6 @@
 /* Define to the default OS ABI for this configuration. */
 #undef GDB_OSABI_DEFAULT
 
-/* Define if self-testing features should be enabled */
-#undef GDB_SELF_TEST
-
 /* Define to 1 if you have `alloca', as a function or macro. */
 #undef HAVE_ALLOCA
 
diff --git a/gdb/configure b/gdb/configure
index 9cd0036848..e30e67034b 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -17740,14 +17740,6 @@ ac_config_links="$ac_config_links $ac_config_links_1"
 $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h
 
 
-if $development; then
-
-$as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
-
-  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
-  CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) common/selftest.c selftest-arch.c"
-fi
-
 
   gdb_ac_transform=`echo "$program_transform_name" | sed -e 's/\\$\\$/\\$/g'`
   GDB_TRANSFORM_NAME=`echo gdb | sed -e "$gdb_ac_transform"`
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 13bc5f9a8f..ac41b4595b 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -2279,13 +2279,6 @@ dnl  At the moment, we just assume it's UTF-8.
 AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8",
           [Define to be a string naming the default host character set.])
 
-if $development; then
-  AC_DEFINE(GDB_SELF_TEST, 1,
-            [Define if self-testing features should be enabled])
-  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
-  CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) common/selftest.c selftest-arch.c"
-fi
-
 GDB_AC_TRANSFORM([gdb], [GDB_TRANSFORM_NAME])
 GDB_AC_TRANSFORM([gcore], [GCORE_TRANSFORM_NAME])
 AC_CONFIG_FILES([gcore], [chmod +x gcore])
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 3ce5f60b12..074bc44263 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1801,7 +1801,6 @@ cp_get_symbol_name_matcher (const lookup_name_info &lookup_name)
   gdb_assert_not_reached ("");
 }
 
-#if GDB_SELF_TEST
 
 namespace selftests {
 
@@ -2114,8 +2113,6 @@ test_cp_remove_params ()
 
 } // namespace selftests
 
-#endif /* GDB_SELF_CHECK */
-
 /* Don't allow just "maintenance cplus".  */
 
 static  void
@@ -2199,10 +2196,8 @@ display the offending symbol."),
 			   &maintenance_show_cmdlist);
 #endif
 
-#if GDB_SELF_TEST
   selftests::register_test ("cp_symbol_name_matches",
 			    selftests::test_cp_symbol_name_matches);
   selftests::register_test ("cp_remove_params",
 			    selftests::test_cp_remove_params);
-#endif
 }
diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
index f7d0ecca0e..38a5e16e7e 100644
--- a/gdb/disasm-selftests.c
+++ b/gdb/disasm-selftests.c
@@ -20,7 +20,6 @@
 #include "defs.h"
 #include "disasm.h"
 
-#if GDB_SELF_TEST
 #include "selftest.h"
 #include "selftest-arch.h"
 
@@ -205,15 +204,12 @@ memory_error_test (struct gdbarch *gdbarch)
 }
 
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
 
 void
 _initialize_disasm_selftests (void)
 {
-#if GDB_SELF_TEST
   selftests::register_test_foreach_arch ("print_one_insn",
 					 selftests::print_one_insn_test);
   selftests::register_test_foreach_arch ("memory_error",
 					 selftests::memory_error_test);
-#endif
 }
diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
index 966630b60c..07ddf4f466 100644
--- a/gdb/dwarf-index-cache.c
+++ b/gdb/dwarf-index-cache.c
@@ -346,7 +346,8 @@ show_index_cache_stats_command (const char *arg, int from_tty)
 		     indent, global_index_cache.n_misses ());
 }
 
-#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
+#if defined (HAVE_MKDTEMP)
+
 namespace selftests
 {
 
@@ -400,7 +401,8 @@ test_mkdir_recursive ()
   SELF_CHECK (create_dir_and_check (dir.c_str ()));
 }
 }
-#endif /*  GDB_SELF_TEST && defined (HAVE_MKDTEMP) */
+
+#endif /* defined (HAVE_MKDTEMP) */
 
 void
 _initialize_index_cache ()
@@ -457,7 +459,7 @@ When non-zero, debugging output for the index cache is displayed."),
 			    NULL, NULL,
 			    &setdebuglist, &showdebuglist);
 
-#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
+#if defined (HAVE_MKDTEMP)
   selftests::register_test ("mkdir_recursive", selftests::test_mkdir_recursive);
 #endif
 }
diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
index 58f1ba4f2f..f648962fec 100644
--- a/gdb/dwarf2-frame.c
+++ b/gdb/dwarf2-frame.c
@@ -39,10 +39,8 @@
 #include "ax.h"
 #include "dwarf2loc.h"
 #include "dwarf2-frame-tailcall.h"
-#if GDB_SELF_TEST
 #include "selftest.h"
 #include "selftest-arch.h"
-#endif
 
 struct comp_unit;
 
@@ -607,7 +605,6 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
   return insn_ptr;
 }
 
-#if GDB_SELF_TEST
 
 namespace selftests {
 
@@ -665,7 +662,6 @@ execute_cfa_program_test (struct gdbarch *gdbarch)
 }
 
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
 
 \f
 
@@ -2431,8 +2427,6 @@ architecture that doesn't support them will have no effect."),
 			   &set_dwarf_cmdlist,
 			   &show_dwarf_cmdlist);
 
-#if GDB_SELF_TEST
   selftests::register_test_foreach_arch ("execute_cfa_program",
 					 selftests::execute_cfa_program_test);
-#endif
 }
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index a98b676b30..df5f0059f6 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1612,7 +1612,6 @@ copy_bitwise (gdb_byte *dest, ULONGEST dest_offset,
     }
 }
 
-#if GDB_SELF_TEST
 
 namespace selftests {
 
@@ -1745,7 +1744,6 @@ copy_bitwise_tests (void)
 
 } /* namespace selftests */
 
-#endif /* GDB_SELF_TEST */
 
 /* Return the number of bytes overlapping a contiguous chunk of N_BITS
    bits whose first bit is located at bit offset START.  */
@@ -4681,7 +4679,5 @@ _initialize_dwarf2loc (void)
 			     show_entry_values_debug,
 			     &setdebuglist, &showdebuglist);
 
-#if GDB_SELF_TEST
   selftests::register_test ("copy_bitwise", selftests::copy_bitwise_tests);
-#endif
 }
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 81a0087c26..55391cbd4a 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -4566,7 +4566,6 @@ dw2_expand_symtabs_matching_symbol
   static_assert (sizeof (prev) > sizeof (offset_type), "");
 }
 
-#if GDB_SELF_TEST
 
 namespace selftests { namespace dw2_expand_symtabs_matching {
 
@@ -4989,7 +4988,6 @@ run_test ()
 
 }} // namespace selftests::dw2_expand_symtabs_matching
 
-#endif /* GDB_SELF_TEST */
 
 /* If FILE_MATCHER is NULL or if PER_CU has
    dwarf2_per_cu_quick_data::MARK set (see
@@ -25593,8 +25591,6 @@ Warning: This option must be enabled before gdb reads the file."),
   dwarf2_loclist_block_index = register_symbol_block_impl (LOC_BLOCK,
 					&dwarf2_block_frame_base_loclist_funcs);
 
-#if GDB_SELF_TEST
   selftests::register_test ("dw2_expand_symtabs_matching",
 			    selftests::dw2_expand_symtabs_matching::run_test);
-#endif
 }
diff --git a/gdb/findvar.c b/gdb/findvar.c
index ebaff923a1..74df206660 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -1010,7 +1010,6 @@ address_from_register (int regnum, struct frame_info *frame)
   return result;
 }
 
-#if GDB_SELF_TEST
 namespace selftests {
 namespace findvar_tests {
 
@@ -1089,14 +1088,11 @@ copy_integer_to_size_test ()
 } // namespace findvar_test
 } // namespace selftests
 
-#endif
 
 void
 _initialize_findvar (void)
 {
-#if GDB_SELF_TEST
   selftests::register_test
     ("copy_integer_to_size",
      selftests::findvar_tests::copy_integer_to_size_test);
-#endif
 }
diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
index 73a31244b4..317cfa3b2f 100644
--- a/gdb/gdbarch-selftests.c
+++ b/gdb/gdbarch-selftests.c
@@ -18,7 +18,6 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
-#if GDB_SELF_TEST
 #include "selftest.h"
 #include "selftest-arch.h"
 #include "inferior.h"
@@ -168,13 +167,10 @@ register_to_value_test (struct gdbarch *gdbarch)
 }
 
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
 
 void
 _initialize_gdbarch_selftests (void)
 {
-#if GDB_SELF_TEST
   selftests::register_test_foreach_arch ("register_to_value",
 					 selftests::register_to_value_test);
-#endif
 }
diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
index f2f8a084bd..fcda9e725d 100644
--- a/gdb/gdbserver/Makefile.in
+++ b/gdb/gdbserver/Makefile.in
@@ -217,6 +217,7 @@ SFILES = \
 	$(srcdir)/common/print-utils.c \
 	$(srcdir)/common/ptid.c \
 	$(srcdir)/common/rsp-low.c \
+	$(srcdir)/common/selftest.c \
 	$(srcdir)/common/tdesc.c \
 	$(srcdir)/common/vec.c \
 	$(srcdir)/common/xml-utils.c \
@@ -261,6 +262,7 @@ OBS = \
 	common/print-utils.o \
 	common/ptid.o \
 	common/rsp-low.o \
+	common/selftest.o \
 	common/signals.o \
 	common/signals-state-save-restore.o \
 	common/tdesc.o \
diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
index 05537df81e..ac404afd04 100644
--- a/gdb/gdbserver/config.in
+++ b/gdb/gdbserver/config.in
@@ -8,9 +8,6 @@
 /* Define to 1 if using `alloca.c'. */
 #undef C_ALLOCA
 
-/* Define if self-testing features should be enabled */
-#undef GDB_SELF_TEST
-
 /* Define to 1 if you have `alloca', as a function or macro. */
 #undef HAVE_ALLOCA
 
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 043bc216e4..fbd59d7268 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -5889,13 +5889,6 @@ fi
   fi
 
 
-if $development; then
-  srv_selftest_objs="common/selftest.o"
-
-$as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
-
-fi
-
  case ${build_alias} in
   "") build_noncanonical=${build} ;;
   *) build_noncanonical=${build_alias} ;;
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 99bc46221c..6fc3cc1fa2 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -54,12 +54,6 @@ else
 fi
 GDB_AC_LIBMCHECK(${libmcheck_default})
 
-if $development; then
-  srv_selftest_objs="common/selftest.o"
-  AC_DEFINE(GDB_SELF_TEST, 1,
-            [Define if self-testing features should be enabled])
-fi
-
 ACX_NONCANONICAL_TARGET
 ACX_NONCANONICAL_HOST
 
@@ -410,7 +404,7 @@ if test "$srv_xmlfiles" != ""; then
   done
 fi
 
-GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs $srv_selftest_objs"
+GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs"
 GDBSERVER_LIBS="$srv_libs"
 
 dnl Check whether the target supports __sync_*_compare_and_swap.
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 72e6a0d87f..46ea3ff142 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -24,13 +24,8 @@
 # Default hostio_last_error implementation
 srv_hostio_err_objs="hostio-errno.o"
 
-if $development; then
-   srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
-   srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
-else
-   srv_i386_linux_regobj=""
-   srv_amd64_linux_regobj=""
-fi
+srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
+srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
 
 ipa_ppc_linux_regobj="powerpc-32l-ipa.o powerpc-altivec32l-ipa.o powerpc-cell32l-ipa.o powerpc-vsx32l-ipa.o powerpc-isa205-32l-ipa.o powerpc-isa205-altivec32l-ipa.o powerpc-isa205-vsx32l-ipa.o powerpc-e500l-ipa.o powerpc-64l-ipa.o powerpc-altivec64l-ipa.o powerpc-cell64l-ipa.o powerpc-vsx64l-ipa.o powerpc-isa205-64l-ipa.o powerpc-isa205-altivec64l-ipa.o powerpc-isa205-vsx64l-ipa.o"
 
@@ -43,10 +38,8 @@ srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-wa
 case "${target}" in
   aarch64*-*-linux*)
 			srv_regobj="arm-with-neon.o"
-			if $development; then
-			  srv_regobj="${srv_regobj} aarch64.o"
-			  srv_regobj="${srv_regobj} linux-aarch64-tdesc-selftest.o"
-                        fi
+			srv_regobj="${srv_regobj} aarch64.o"
+			srv_regobj="${srv_regobj} linux-aarch64-tdesc-selftest.o"
 			srv_tgtobj="linux-aarch64-low.o aarch64-linux-hw-point.o"
 			srv_tgtobj="$srv_tgtobj linux-aarch32-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/arm.o"
@@ -343,13 +336,9 @@ case "${target}" in
   spu*-*-*)		srv_regobj=reg-spu.o
 			srv_tgtobj="spu-low.o fork-child.o fork-inferior.o"
 			;;
-  tic6x-*-uclinux)	if $development; then
-			  srv_regobj="tic6x-c64xp-linux.o"
-			  srv_regobj="${srv_regobj} tic6x-c64x-linux.o"
-			  srv_regobj="${srv_regobj} tic6x-c62x-linux.o"
-                        else
-			  srv_regobj=""
-                        fi
+  tic6x-*-uclinux)	srv_regobj="tic6x-c64xp-linux.o"
+			srv_regobj="${srv_regobj} tic6x-c64x-linux.o"
+			srv_regobj="${srv_regobj} tic6x-c62x-linux.o"
 			srv_tgtobj="$srv_linux_obj linux-tic6x-low.o"
 			srv_tgtobj="${srv_tgtobj} arch/tic6x.o"
 			srv_linux_regsets=yes
diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
index 1d34e319df..b77e18d2f3 100644
--- a/gdb/gdbserver/linux-aarch64-low.c
+++ b/gdb/gdbserver/linux-aarch64-low.c
@@ -3085,7 +3085,5 @@ initialize_low_arch (void)
   initialize_regsets_info (&aarch64_regsets_info);
   initialize_regsets_info (&aarch64_sve_regsets_info);
 
-#if GDB_SELF_TEST
   initialize_low_tdesc ();
-#endif
 }
diff --git a/gdb/gdbserver/linux-aarch64-tdesc.h b/gdb/gdbserver/linux-aarch64-tdesc.h
index 4d2b883b55..51bc08bd2c 100644
--- a/gdb/gdbserver/linux-aarch64-tdesc.h
+++ b/gdb/gdbserver/linux-aarch64-tdesc.h
@@ -19,6 +19,4 @@
 
 const target_desc * aarch64_linux_read_description (uint64_t vq);
 
-#if GDB_SELF_TEST
 void initialize_low_tdesc ();
-#endif
diff --git a/gdb/gdbserver/linux-tic6x-low.c b/gdb/gdbserver/linux-tic6x-low.c
index d90bbcfe51..99f2f8cad0 100644
--- a/gdb/gdbserver/linux-tic6x-low.c
+++ b/gdb/gdbserver/linux-tic6x-low.c
@@ -423,7 +423,6 @@ struct linux_target_ops the_low_target = {
   tic6x_supports_hardware_single_step,
 };
 
-#if GDB_SELF_TEST
 #include "common/selftest.h"
 
 namespace selftests {
@@ -437,19 +436,16 @@ tic6x_tdesc_test ()
 }
 }
 }
-#endif
 
 void
 initialize_low_arch (void)
 {
-#if GDB_SELF_TEST
   /* Initialize the Linux target descriptions.  */
   init_registers_tic6x_c64xp_linux ();
   init_registers_tic6x_c64x_linux ();
   init_registers_tic6x_c62x_linux ();
 
   selftests::register_test ("tic6x-tdesc", selftests::tdesc::tic6x_tdesc_test);
-#endif
 
   initialize_regsets_info (&tic6x_regsets_info);
 }
diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
index 80b43802c7..daa603398a 100644
--- a/gdb/gdbserver/linux-x86-low.c
+++ b/gdb/gdbserver/linux-x86-low.c
@@ -2897,9 +2897,7 @@ initialize_low_arch (void)
   tdesc_amd64_linux_no_xml->xmltarget = xmltarget_amd64_linux_no_xml;
 #endif
 
-#if GDB_SELF_TEST
   initialize_low_tdesc ();
-#endif
 
   tdesc_i386_linux_no_xml = allocate_target_description ();
   copy_target_description (tdesc_i386_linux_no_xml,
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a491ae0257..8bc51c8e3c 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3573,9 +3573,7 @@ captured_main (int argc, char *argv[])
   volatile int attach = 0;
   int was_running;
   bool selftest = false;
-#if GDB_SELF_TEST
   const char *selftest_filter = NULL;
-#endif
 
   current_directory = getcwd (NULL, 0);
   client_state &cs = get_client_state ();
@@ -3708,9 +3706,7 @@ captured_main (int argc, char *argv[])
       else if (startswith (*next_arg, "--selftest="))
 	{
 	  selftest = true;
-#if GDB_SELF_TEST
 	  selftest_filter = *next_arg + strlen ("--selftest=");
-#endif
 	}
       else
 	{
@@ -3787,11 +3783,7 @@ captured_main (int argc, char *argv[])
 
   if (selftest)
     {
-#if GDB_SELF_TEST
       selftests::run_tests (selftest_filter);
-#else
-      printf (_("Selftests are not available in a non-development build.\n"));
-#endif
       throw_quit ("Quit");
     }
 
@@ -4495,7 +4487,6 @@ handle_target_event (int err, gdb_client_data client_data)
   return 0;
 }
 
-#if GDB_SELF_TEST
 namespace selftests
 {
 
@@ -4504,4 +4495,3 @@ reset ()
 {}
 
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
index 802c41fe72..efb9b1a7a1 100644
--- a/gdb/i386-linux-tdep.c
+++ b/gdb/i386-linux-tdep.c
@@ -1083,7 +1083,6 @@ _initialize_i386_linux_tdep (void)
   gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_LINUX,
 			  i386_linux_init_abi);
 
-#if GDB_SELF_TEST
   struct
   {
     const char *xml;
@@ -1105,5 +1104,4 @@ _initialize_i386_linux_tdep (void)
 
       selftests::record_xml_tdesc (a.xml, tdesc);
     }
-#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index a6994aaf12..4e8bd9caa4 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -9053,7 +9053,6 @@ Show Intel Memory Protection Extensions specific variables."),
   /* Tell remote stub that we support XML target description.  */
   register_remote_support_xml ("i386");
 
-#if GDB_SELF_TEST
   struct
   {
     const char *xml;
@@ -9075,5 +9074,4 @@ Show Intel Memory Protection Extensions specific variables."),
 
       selftests::record_xml_tdesc (a.xml, tdesc);
     }
-#endif /* GDB_SELF_TEST */
 }
diff --git a/gdb/maint.c b/gdb/maint.c
index 5d4701cfac..446047227b 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -939,26 +939,16 @@ show_per_command_cmd (const char *args, int from_tty)
 static void
 maintenance_selftest (const char *args, int from_tty)
 {
-#if GDB_SELF_TEST
   selftests::run_tests (args);
-#else
-  printf_filtered (_("\
-Selftests are not available in a non-development build.\n"));
-#endif
 }
 
 static void
 maintenance_info_selftests (const char *arg, int from_tty)
 {
-#if GDB_SELF_TEST
   printf_filtered ("Registered selftests:\n");
   selftests::for_each_selftest ([] (const std::string &name) {
     printf_filtered (" - %s\n", name.c_str ());
   });
-#else
-  printf_filtered (_("\
-Selftests are not available in a non-development build.\n"));
-#endif
 }
 
 \f
diff --git a/gdb/producer.c b/gdb/producer.c
index f21aae93ac..459ced494c 100644
--- a/gdb/producer.c
+++ b/gdb/producer.c
@@ -125,7 +125,6 @@ producer_is_icc (const char *producer, int *major, int *minor)
   return false;
 }
 
-#if defined GDB_SELF_TEST
 namespace selftests {
 namespace producer {
 
@@ -206,13 +205,10 @@ Version 18.0 Beta";
 }
 }
 }
-#endif
 
 void
 _initialize_producer ()
 {
-#if defined GDB_SELF_TEST
   selftests::register_test
     ("producer-parser", selftests::producer::producer_parsing_tests);
-#endif
 }
diff --git a/gdb/regcache.c b/gdb/regcache.c
index f3f845aad6..91561ad3fc 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1419,7 +1419,6 @@ register_dump::dump (ui_file *file)
 			footnote_register_type_name_null);
 }
 
-#if GDB_SELF_TEST
 #include "selftest.h"
 #include "selftest-arch.h"
 #include "gdbthread.h"
@@ -1846,7 +1845,6 @@ cooked_write_test (struct gdbarch *gdbarch)
 }
 
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
 
 void
 _initialize_regcache (void)
@@ -1861,12 +1859,10 @@ _initialize_regcache (void)
   add_com ("flushregs", class_maintenance, reg_flush_command,
 	   _("Force gdb to flush its register cache (maintainer command)"));
 
-#if GDB_SELF_TEST
   selftests::register_test ("current_regcache", selftests::current_regcache_test);
 
   selftests::register_test_foreach_arch ("regcache::cooked_read_test",
 					 selftests::cooked_read_test);
   selftests::register_test_foreach_arch ("regcache::cooked_write_test",
 					 selftests::cooked_write_test);
-#endif
 }
diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
index 248e558a54..70a8acf9ae 100644
--- a/gdb/rust-exp.y
+++ b/gdb/rust-exp.y
@@ -2545,7 +2545,6 @@ rustyyerror (rust_parser *parser, const char *msg)
 
 \f
 
-#if GDB_SELF_TEST
 
 /* Initialize the lexer for testing.  */
 
@@ -2803,7 +2802,6 @@ rust_lex_tests (void)
   rust_lex_test_push_back (&parser);
 }
 
-#endif /* GDB_SELF_TEST */
 
 void
 _initialize_rust_exp (void)
@@ -2813,7 +2811,5 @@ _initialize_rust_exp (void)
      error.  */
   gdb_assert (code == 0);
 
-#if GDB_SELF_TEST
   selftests::register_test ("rust-lex", rust_lex_tests);
-#endif
 }
diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
index 7045c45660..835145c939 100644
--- a/gdb/selftest-arch.c
+++ b/gdb/selftest-arch.c
@@ -18,7 +18,6 @@
 
 #include "defs.h"
 
-#if GDB_SELF_TEST
 #include "selftest.h"
 #include "selftest-arch.h"
 #include "arch-utils.h"
@@ -104,4 +103,3 @@ reset ()
   reinit_frame_cache ();
 }
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 20697d9dca..c61a6890ad 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -3791,7 +3791,6 @@ map_symbol_filenames (symbol_filename_ftype *fun, void *data,
   }
 }
 
-#if GDB_SELF_TEST
 
 namespace selftests {
 namespace filename_language {
@@ -3848,7 +3847,6 @@ test_set_ext_lang_command ()
 } /* namespace filename_language */
 } /* namespace selftests */
 
-#endif /* GDB_SELF_TEST */
 
 void
 _initialize_symfile (void)
@@ -3975,11 +3973,9 @@ Show printing of separate debug info file search debug."), _("\
 When on, GDB prints the searched locations while looking for separate debug \
 info files."), NULL, NULL, &setdebuglist, &showdebuglist);
 
-#if GDB_SELF_TEST
   selftests::register_test
     ("filename_language", selftests::filename_language::test_filename_language);
   selftests::register_test
     ("set_ext_lang_command",
      selftests::filename_language::test_set_ext_lang_command);
-#endif
 }
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 087de141f7..10f2f34eb8 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -1748,7 +1748,6 @@ struct xml_test_tdesc
 
 static std::vector<xml_test_tdesc> xml_tdesc;
 
-#if GDB_SELF_TEST
 
 /* See target-descritpions.h.  */
 
@@ -1757,7 +1756,6 @@ record_xml_tdesc (const char *xml_file, const struct target_desc *tdesc)
 {
   xml_tdesc.emplace_back (xml_file, std::unique_ptr<const target_desc> (tdesc));
 }
-#endif
 
 }
 
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 96290b7d97..1aa8f5c79c 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -210,7 +210,6 @@ void set_tdesc_property (struct target_desc *,
 void tdesc_add_compatible (struct target_desc *,
 			   const struct bfd_arch_info *);
 
-#if GDB_SELF_TEST
 namespace selftests {
 
 /* Record that XML_FILE should generate a target description that equals
@@ -220,6 +219,5 @@ namespace selftests {
 void record_xml_tdesc (const char *xml_file,
 		       const struct target_desc *tdesc);
 }
-#endif
 
 #endif /* TARGET_DESCRIPTIONS_H */
diff --git a/gdb/target.c b/gdb/target.c
index 2d98954b54..6f46575a19 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -192,7 +192,6 @@ target_command (const char *arg, int from_tty)
 		  gdb_stdout);
 }
 
-#if GDB_SELF_TEST
 namespace selftests {
 
 /* A mock process_stratum target_ops that doesn't read/write registers
@@ -211,7 +210,6 @@ test_target_ops::info () const
 }
 
 } /* namespace selftests */
-#endif /* GDB_SELF_TEST */
 
 /* Default target_has_* methods for process_stratum targets.  */
 
diff --git a/gdb/target.h b/gdb/target.h
index 39aa8c3c73..9c2cb38279 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -2543,7 +2543,6 @@ extern void target_prepare_to_generate_core (void);
 /* See to_done_generating_core.  */
 extern void target_done_generating_core (void);
 
-#if GDB_SELF_TEST
 namespace selftests {
 
 /* A mock process_stratum target_ops that doesn't read/write registers
@@ -2586,6 +2585,5 @@ public:
 
 
 } // namespace selftests
-#endif /* GDB_SELF_TEST */
 
 #endif /* !defined (TARGET_H) */
diff --git a/gdb/testsuite/gdb.gdb/unittest.exp b/gdb/testsuite/gdb.gdb/unittest.exp
index 1c835850b8..c962964284 100644
--- a/gdb/testsuite/gdb.gdb/unittest.exp
+++ b/gdb/testsuite/gdb.gdb/unittest.exp
@@ -23,10 +23,6 @@ gdb_test_multiple $test $test {
 	set num_ran $expect_out(1,string)
 	gdb_assert "$num_ran > 0" $test
   }
-
-  -re "Selftests are not available in a non-development build.\r\n$gdb_prompt $" {
-	unsupported $test
-  }
 }
 
 if { ![is_remote host] && $do_xml_test } {
diff --git a/gdb/testsuite/gdb.server/unittest.exp b/gdb/testsuite/gdb.server/unittest.exp
index e947ff2c30..c050989416 100644
--- a/gdb/testsuite/gdb.server/unittest.exp
+++ b/gdb/testsuite/gdb.server/unittest.exp
@@ -38,10 +38,6 @@ gdb_expect {
 	gdb_assert "$num_ran > 0" $test
     }
 
-    -re "Selftests are not available in a non-development build.\r\n$" {
-	unsupported $test
-    }
-
     default {
 	fail $test
     }
diff --git a/gdb/utils.c b/gdb/utils.c
index 7a8c80c64e..67850c93e7 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -2809,7 +2809,6 @@ string_to_core_addr (const char *my_string)
   return addr;
 }
 
-#if GDB_SELF_TEST
 
 static void
 gdb_realpath_check_trailer (const char *input, const char *trailer)
@@ -2843,7 +2842,6 @@ gdb_realpath_tests ()
   gdb_realpath_check_trailer ("", "");
 }
 
-#endif /* GDB_SELF_TEST */
 
 /* Allocation function for the libiberty hash table which uses an
    obstack.  The obstack is passed as DATA.  */
@@ -3239,7 +3237,5 @@ _initialize_utils (void)
   add_internal_problem_command (&internal_warning_problem);
   add_internal_problem_command (&demangler_warning_problem);
 
-#if GDB_SELF_TEST
   selftests::register_test ("gdb_realpath", gdb_realpath_tests);
-#endif
 }
diff --git a/gdb/value.c b/gdb/value.c
index b0f22f1117..0d2307ebdb 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3921,7 +3921,6 @@ isvoid_internal_fn (struct gdbarch *gdbarch,
   return value_from_longest (builtin_type (gdbarch)->builtin_int, ret);
 }
 
-#if GDB_SELF_TEST
 namespace selftests
 {
 
@@ -4061,7 +4060,6 @@ test_insert_into_bit_range_vector ()
 }
 
 } /* namespace selftests */
-#endif /* GDB_SELF_TEST */
 
 void
 _initialize_values (void)
@@ -4114,9 +4112,7 @@ prevents future values, larger than this size, from being allocated."),
 			    set_max_value_size,
 			    show_max_value_size,
 			    &setlist, &showlist);
-#if GDB_SELF_TEST
   selftests::register_test ("ranges_contain", selftests::test_ranges_contain);
   selftests::register_test ("insert_into_bit_range_vector",
 			    selftests::test_insert_into_bit_range_vector);
-#endif
 }
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH] Don't disable selftests in a non-development build
  2018-08-14  5:42 [RFC/PATCH] Don't disable selftests in a non-development build Sergio Durigan Junior
@ 2018-08-14 16:28 ` Sergio Durigan Junior
  2018-08-14 18:08 ` Pedro Alves
  2018-09-17 20:22 ` [PATCH] Add parameter to allow enabling/disabling selftests via configure Sergio Durigan Junior
  2 siblings, 0 replies; 11+ messages in thread
From: Sergio Durigan Junior @ 2018-08-14 16:28 UTC (permalink / raw)
  To: GDB Patches; +Cc: Joel Brobecker

On Tuesday, August 14 2018, I wrote:

> This is an idea I mentioned a few days ago in our IRC channel, and
> Simon said he'd be open to it, so I'm send this RFC/PATCH to see what
> others think.

Hm, the patch fails on Aarch64 (sorry, took a while for the builders to
notify about this).  I'll rework them and resubmit, but the idea stays
the same, so feel free to comment on it.

Thanks.

> Due to the many racy testcases and random failures we see when running
> the GDB testsuite, it is unfortunately not possible to perform a full
> test when one is building a downstream package.  As the Fedora GDB
> maintainer and one of the Debian GDB uploaders, I feel like this
> situation could be improved by, at least, executing our selftests
> after the package has been built.  However, we currently (for some
> reason that is not clear by reading the archives, but see more below)
> disable selftests on non-development builds.  Therefore, this patch
> aims to leave them enabled all the time, for everyone (including the
> end users).
>
> I understand that disabling the selftests in a non-development build
> brings a few advantages.  For example, we ship less code (even though
> the final difference may be small enough that it doesn't really have
> any real benefit).  Another thing to consider is that we disable
> portions of the codebase that could have latent bugs, making GDB a bit
> safer.  I don't really have an argument to compete with this;
> nonetheless, I think the benefit of having "batteries included" when
> it comes to testing is a huge thing.  With our selftests, the user
> doesn't need to install any external
> programs/libraries (dejagnu/tcl/etc.), and they're stable enough that
> we know that they don't suffer from the racyness present in our
> current testsuite.
>
> Anyway, here's my take at the problem.  The patches are mostly
> mechanical (getting rid of the "GDB_SELF_TEST" define, which becomes
> useless now that selftests are always present); the ones that aren't
> deal with the configure/Makefile machinery to always compile the
> selftests source files.
>
> I've tested this patch on BuildBot, without regressions.  I'm taking
> the liberty to Cc Joel, who is our release manager and might have
> opinions about this.
>
> OK?
>
> gdb/ChangeLog:
> 2018-08-14  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* Makefile.in (COMMON_SFILES): Add 'common/selftest.c'
> 	and 'selftest-arch.c'.
> 	(SFILES): Add '$(SUBDIR_UNITTESTS_SRCS)'.
> 	(HFILES_NO_SRCDIR): Add 'common/selftest.h'.
> 	(COMMON_OBS): Add '$(SUBDIR_UNITTESTS_OBS)'.
> 	* aarch64-tdep.c: Remove '#if GDB_SELF_TEST' guards.
> 	* amd64-linux-tdep.c: Likewise.
> 	* amd64-tdep.c: Likewise.
> 	* arm-tdep.c: Likewise.
> 	* config.in: Regenerate.
> 	* configure: Regenerate.
> 	* configure.ac: Don't define 'GDB_SELF_TEST'; remove setting
> 	of 'CONFIG_OBS' and 'CONFIG_SRCS' with selftests
> 	objects/sources.
> 	* cp-support.c: Remove '#if GDB_SELF_TEST' guards.
> 	* disasm-selftests.c: Likewise.
> 	* dwarf-index-cache.c: Likewise.
> 	* dwarf2-frame.c: Likewise.
> 	* dwarf2loc.c: Likewise.
> 	* dwarf2read.c: Likewise.
> 	* findvar.c: Likewise.
> 	* gdbarch-selftests.c: Likewise.
> 	* i386-linux-tdep.c: Likewise.
> 	* i386-tdep.c: Likewise.
> 	* maint.c: Likewise.
> 	(maintenance_selftest): Remove unneded message about selftests
> 	not available in a non-development build.
> 	(maintenance_info_selftests): Likewise.
> 	* producer.c: Remove '#if GDB_SELF_TEST' guards.
> 	* regcache.c: Likewise.
> 	* rust-exp.y: Likewise.
> 	* selftest-arch.c: Likewise.
> 	* symfile.c: Likewise.
> 	* target-descriptions.c: Likewise.
> 	* target-descriptions.h: Likewise.
> 	* target.c: Likewise.
> 	* target.h: Likewise.
> 	* utils.c: Likewise.
> 	* value.c: Likewise.
>
> gdb/gdbserver/ChangeLog:
> 2018-08-14  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* Makefile.in (SFILES): Add '$(srcdir)/common/selftest.c'.
> 	(OBS): Add 'common/selftest.o'.
> 	* config.in: Regenerate.
> 	* configure: Regenerate.
> 	* configure.ac: Don't define 'GDB_SELF_TEST'.  Don't add
> 	'$srv_selftest_obs' to 'GDBSERVER_DEPFILES'.
> 	* configure.srv: Always set 'srv_i386_linux_regobj' and
> 	'srv_amd64_linux_regobj' with selftest-related objects.
> 	Always set 'srv_regobj' with selftest-related objects.
> 	* linux-aarch64-low.c: Remove '#if GDB_SELF_TEST' guards.
> 	* linux-aarch64-tdesc.h: Likewise.
> 	* linux-tic6x-low.c: Likewise.
> 	* linux-x86-low.c: Likewise.
> 	* server.c: Likewise.
> 	(captured_main): Remove unneded message about selftests not
> 	available in a non-development build.
> 	*
>
> gdb/testsuite/ChangeLog:
> 2018-08-14  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* gdb.gdb/unittest.exp: Remove match for "Selftests are not
> 	available in a non-development build."
> 	* gdb.server/unittest.exp: Likewise.
> ---
>  gdb/Makefile.in                       |  9 +++++++--
>  gdb/aarch64-tdep.c                    |  8 --------
>  gdb/amd64-linux-tdep.c                |  2 --
>  gdb/amd64-tdep.c                      |  2 --
>  gdb/arm-tdep.c                        |  8 --------
>  gdb/config.in                         |  3 ---
>  gdb/configure                         |  8 --------
>  gdb/configure.ac                      |  7 -------
>  gdb/cp-support.c                      |  5 -----
>  gdb/disasm-selftests.c                |  4 ----
>  gdb/dwarf-index-cache.c               |  8 +++++---
>  gdb/dwarf2-frame.c                    |  6 ------
>  gdb/dwarf2loc.c                       |  4 ----
>  gdb/dwarf2read.c                      |  4 ----
>  gdb/findvar.c                         |  4 ----
>  gdb/gdbarch-selftests.c               |  4 ----
>  gdb/gdbserver/Makefile.in             |  2 ++
>  gdb/gdbserver/config.in               |  3 ---
>  gdb/gdbserver/configure               |  7 -------
>  gdb/gdbserver/configure.ac            |  8 +-------
>  gdb/gdbserver/configure.srv           | 25 +++++++------------------
>  gdb/gdbserver/linux-aarch64-low.c     |  2 --
>  gdb/gdbserver/linux-aarch64-tdesc.h   |  2 --
>  gdb/gdbserver/linux-tic6x-low.c       |  4 ----
>  gdb/gdbserver/linux-x86-low.c         |  2 --
>  gdb/gdbserver/server.c                | 10 ----------
>  gdb/i386-linux-tdep.c                 |  2 --
>  gdb/i386-tdep.c                       |  2 --
>  gdb/maint.c                           | 10 ----------
>  gdb/producer.c                        |  4 ----
>  gdb/regcache.c                        |  4 ----
>  gdb/rust-exp.y                        |  4 ----
>  gdb/selftest-arch.c                   |  2 --
>  gdb/symfile.c                         |  4 ----
>  gdb/target-descriptions.c             |  2 --
>  gdb/target-descriptions.h             |  2 --
>  gdb/target.c                          |  2 --
>  gdb/target.h                          |  2 --
>  gdb/testsuite/gdb.gdb/unittest.exp    |  4 ----
>  gdb/testsuite/gdb.server/unittest.exp |  4 ----
>  gdb/utils.c                           |  4 ----
>  gdb/value.c                           |  4 ----
>  42 files changed, 22 insertions(+), 185 deletions(-)
>
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 118c3c8062..8c8c23fd6a 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -966,6 +966,7 @@ COMMON_SFILES = \
>  	common/rsp-low.c \
>  	common/run-time-clock.c \
>  	common/scoped_mmap.c \
> +	common/selftest.c \
>  	common/signals.c \
>  	common/signals-state-save-restore.c \
>  	common/tdesc.c \
> @@ -1092,6 +1093,7 @@ COMMON_SFILES = \
>  	remote-notif.c \
>  	reverse.c \
>  	rust-lang.c \
> +	selftest-arch.c \
>  	sentinel-frame.c \
>  	ser-event.c \
>  	serial.c \
> @@ -1162,7 +1164,8 @@ SFILES = \
>  	$(SUBDIR_CLI_SRCS) \
>  	$(SUBDIR_TARGET_SRCS) \
>  	$(COMMON_SFILES) \
> -	$(SUBDIR_GCC_COMPILE_SRCS)
> +	$(SUBDIR_GCC_COMPILE_SRCS) \
> +	$(SUBDIR_UNITTESTS_SRCS)
>  
>  # Header files that need to have srcdir added.  Note that in the cases
>  # where we use a macro like $(gdbcmd_h), things are carefully arranged
> @@ -1452,6 +1455,7 @@ HFILES_NO_SRCDIR = \
>  	common/queue.h \
>  	common/rsp-low.h \
>  	common/run-time-clock.h \
> +	common/selftest.h \
>  	common/signals-state-save-restore.h \
>  	common/symbol.h \
>  	common/tdesc.h \
> @@ -1566,7 +1570,8 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
>  	$(patsubst %.c,%.o,$(COMMON_SFILES)) \
>  	$(SUBDIR_CLI_OBS) \
>  	$(SUBDIR_TARGET_OBS) \
> -	$(SUBDIR_GCC_COMPILE_OBS)
> +	$(SUBDIR_GCC_COMPILE_OBS) \
> +	$(SUBDIR_UNITTESTS_OBS)
>  
>  SUBDIRS = doc @subdirs@ data-directory $(GNULIB_BUILDDIR)
>  CLEANDIRS = $(SUBDIRS)
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 5c6eb98545..00f9456954 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -516,7 +516,6 @@ aarch64_analyze_prologue (struct gdbarch *gdbarch,
>  				   reader);
>  }
>  
> -#if GDB_SELF_TEST
>  
>  namespace selftests {
>  
> @@ -640,7 +639,6 @@ aarch64_analyze_prologue_test (void)
>    }
>  }
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
>  
>  /* Implement the "skip_prologue" gdbarch method.  */
>  
> @@ -3154,12 +3152,10 @@ aarch64_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
>  		      paddress (gdbarch, tdep->lowest_pc));
>  }
>  
> -#if GDB_SELF_TEST
>  namespace selftests
>  {
>  static void aarch64_process_record_test (void);
>  }
> -#endif
>  
>  void
>  _initialize_aarch64_tdep (void)
> @@ -3176,14 +3172,12 @@ When on, AArch64 specific debugging is enabled."),
>  			    show_aarch64_debug,
>  			    &setdebuglist, &showdebuglist);
>  
> -#if GDB_SELF_TEST
>    selftests::register_test ("aarch64-analyze-prologue",
>  			    selftests::aarch64_analyze_prologue_test);
>    selftests::register_test ("aarch64-process-record",
>  			    selftests::aarch64_process_record_test);
>    selftests::record_xml_tdesc ("aarch64.xml",
>  			       aarch64_create_target_description (0));
> -#endif
>  }
>  
>  /* AArch64 process record-replay related structures, defines etc.  */
> @@ -4085,7 +4079,6 @@ deallocate_reg_mem (insn_decode_record *record)
>    xfree (record->aarch64_mems);
>  }
>  
> -#if GDB_SELF_TEST
>  namespace selftests {
>  
>  static void
> @@ -4118,7 +4111,6 @@ aarch64_process_record_test (void)
>  }
>  
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
>  
>  /* Parse the current instruction and record the values of the registers and
>     memory that will be changed in current instruction to record_arch_list
> diff --git a/gdb/amd64-linux-tdep.c b/gdb/amd64-linux-tdep.c
> index 7ab43897ab..19432b3fda 100644
> --- a/gdb/amd64-linux-tdep.c
> +++ b/gdb/amd64-linux-tdep.c
> @@ -2280,7 +2280,6 @@ _initialize_amd64_linux_tdep (void)
>    gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x64_32,
>  			  GDB_OSABI_LINUX, amd64_x32_linux_init_abi);
>  
> -#if GDB_SELF_TEST
>    struct
>    {
>      const char *xml;
> @@ -2306,5 +2305,4 @@ _initialize_amd64_linux_tdep (void)
>  
>        selftests::record_xml_tdesc (a.xml, tdesc);
>      }
> -#endif /* GDB_SELF_TEST */
>  }
> diff --git a/gdb/amd64-tdep.c b/gdb/amd64-tdep.c
> index 088542d72b..4c88ce66ca 100644
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -3300,7 +3300,6 @@ _initialize_amd64_tdep (void)
>    gdbarch_register_osabi (bfd_arch_i386, bfd_mach_x64_32, GDB_OSABI_NONE,
>   			  amd64_x32_none_init_abi);
>  
> -#if GDB_SELF_TEST
>    struct
>    {
>      const char *xml;
> @@ -3321,7 +3320,6 @@ _initialize_amd64_tdep (void)
>  
>        selftests::record_xml_tdesc (a.xml, tdesc);
>      }
> -#endif /* GDB_SELF_TEST */
>  }
>  \f
>  
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index c3280ee211..ac63594e5f 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -69,9 +69,7 @@
>  #include "features/arm/arm-with-vfpv3.c"
>  #include "features/arm/arm-with-neon.c"
>  
> -#if GDB_SELF_TEST
>  #include "selftest.h"
> -#endif
>  
>  static int arm_debug;
>  
> @@ -9523,12 +9521,10 @@ arm_dump_tdep (struct gdbarch *gdbarch, struct ui_file *file)
>  		      (unsigned long) tdep->lowest_pc);
>  }
>  
> -#if GDB_SELF_TEST
>  namespace selftests
>  {
>  static void arm_record_test (void);
>  }
> -#endif
>  
>  void
>  _initialize_arm_tdep (void)
> @@ -9664,9 +9660,7 @@ vfp - VFP co-processor."),
>  			   NULL, /* FIXME: i18n: "ARM debugging is %s.  */
>  			   &setdebuglist, &showdebuglist);
>  
> -#if GDB_SELF_TEST
>    selftests::register_test ("arm-record", selftests::arm_record_test);
> -#endif
>  
>  }
>  
> @@ -13199,7 +13193,6 @@ decode_insn (abstract_memory_reader &reader, insn_decode_record *arm_record,
>    return ret;
>  }
>  
> -#if GDB_SELF_TEST
>  namespace selftests {
>  
>  /* Provide both 16-bit and 32-bit thumb instructions.  */
> @@ -13303,7 +13296,6 @@ arm_record_test (void)
>    }
>  }
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
>  
>  /* Cleans up local record registers and memory allocations.  */
>  
> diff --git a/gdb/config.in b/gdb/config.in
> index 01acda124b..75c1dd2930 100644
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -65,9 +65,6 @@
>  /* Define to the default OS ABI for this configuration. */
>  #undef GDB_OSABI_DEFAULT
>  
> -/* Define if self-testing features should be enabled */
> -#undef GDB_SELF_TEST
> -
>  /* Define to 1 if you have `alloca', as a function or macro. */
>  #undef HAVE_ALLOCA
>  
> diff --git a/gdb/configure b/gdb/configure
> index 9cd0036848..e30e67034b 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -17740,14 +17740,6 @@ ac_config_links="$ac_config_links $ac_config_links_1"
>  $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h
>  
>  
> -if $development; then
> -
> -$as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
> -
> -  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
> -  CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) common/selftest.c selftest-arch.c"
> -fi
> -
>  
>    gdb_ac_transform=`echo "$program_transform_name" | sed -e 's/\\$\\$/\\$/g'`
>    GDB_TRANSFORM_NAME=`echo gdb | sed -e "$gdb_ac_transform"`
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 13bc5f9a8f..ac41b4595b 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -2279,13 +2279,6 @@ dnl  At the moment, we just assume it's UTF-8.
>  AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8",
>            [Define to be a string naming the default host character set.])
>  
> -if $development; then
> -  AC_DEFINE(GDB_SELF_TEST, 1,
> -            [Define if self-testing features should be enabled])
> -  CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
> -  CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) common/selftest.c selftest-arch.c"
> -fi
> -
>  GDB_AC_TRANSFORM([gdb], [GDB_TRANSFORM_NAME])
>  GDB_AC_TRANSFORM([gcore], [GCORE_TRANSFORM_NAME])
>  AC_CONFIG_FILES([gcore], [chmod +x gcore])
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index 3ce5f60b12..074bc44263 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -1801,7 +1801,6 @@ cp_get_symbol_name_matcher (const lookup_name_info &lookup_name)
>    gdb_assert_not_reached ("");
>  }
>  
> -#if GDB_SELF_TEST
>  
>  namespace selftests {
>  
> @@ -2114,8 +2113,6 @@ test_cp_remove_params ()
>  
>  } // namespace selftests
>  
> -#endif /* GDB_SELF_CHECK */
> -
>  /* Don't allow just "maintenance cplus".  */
>  
>  static  void
> @@ -2199,10 +2196,8 @@ display the offending symbol."),
>  			   &maintenance_show_cmdlist);
>  #endif
>  
> -#if GDB_SELF_TEST
>    selftests::register_test ("cp_symbol_name_matches",
>  			    selftests::test_cp_symbol_name_matches);
>    selftests::register_test ("cp_remove_params",
>  			    selftests::test_cp_remove_params);
> -#endif
>  }
> diff --git a/gdb/disasm-selftests.c b/gdb/disasm-selftests.c
> index f7d0ecca0e..38a5e16e7e 100644
> --- a/gdb/disasm-selftests.c
> +++ b/gdb/disasm-selftests.c
> @@ -20,7 +20,6 @@
>  #include "defs.h"
>  #include "disasm.h"
>  
> -#if GDB_SELF_TEST
>  #include "selftest.h"
>  #include "selftest-arch.h"
>  
> @@ -205,15 +204,12 @@ memory_error_test (struct gdbarch *gdbarch)
>  }
>  
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
>  
>  void
>  _initialize_disasm_selftests (void)
>  {
> -#if GDB_SELF_TEST
>    selftests::register_test_foreach_arch ("print_one_insn",
>  					 selftests::print_one_insn_test);
>    selftests::register_test_foreach_arch ("memory_error",
>  					 selftests::memory_error_test);
> -#endif
>  }
> diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
> index 966630b60c..07ddf4f466 100644
> --- a/gdb/dwarf-index-cache.c
> +++ b/gdb/dwarf-index-cache.c
> @@ -346,7 +346,8 @@ show_index_cache_stats_command (const char *arg, int from_tty)
>  		     indent, global_index_cache.n_misses ());
>  }
>  
> -#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
> +#if defined (HAVE_MKDTEMP)
> +
>  namespace selftests
>  {
>  
> @@ -400,7 +401,8 @@ test_mkdir_recursive ()
>    SELF_CHECK (create_dir_and_check (dir.c_str ()));
>  }
>  }
> -#endif /*  GDB_SELF_TEST && defined (HAVE_MKDTEMP) */
> +
> +#endif /* defined (HAVE_MKDTEMP) */
>  
>  void
>  _initialize_index_cache ()
> @@ -457,7 +459,7 @@ When non-zero, debugging output for the index cache is displayed."),
>  			    NULL, NULL,
>  			    &setdebuglist, &showdebuglist);
>  
> -#if GDB_SELF_TEST && defined (HAVE_MKDTEMP)
> +#if defined (HAVE_MKDTEMP)
>    selftests::register_test ("mkdir_recursive", selftests::test_mkdir_recursive);
>  #endif
>  }
> diff --git a/gdb/dwarf2-frame.c b/gdb/dwarf2-frame.c
> index 58f1ba4f2f..f648962fec 100644
> --- a/gdb/dwarf2-frame.c
> +++ b/gdb/dwarf2-frame.c
> @@ -39,10 +39,8 @@
>  #include "ax.h"
>  #include "dwarf2loc.h"
>  #include "dwarf2-frame-tailcall.h"
> -#if GDB_SELF_TEST
>  #include "selftest.h"
>  #include "selftest-arch.h"
> -#endif
>  
>  struct comp_unit;
>  
> @@ -607,7 +605,6 @@ bad CFI data; mismatched DW_CFA_restore_state at %s"),
>    return insn_ptr;
>  }
>  
> -#if GDB_SELF_TEST
>  
>  namespace selftests {
>  
> @@ -665,7 +662,6 @@ execute_cfa_program_test (struct gdbarch *gdbarch)
>  }
>  
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
>  
>  \f
>  
> @@ -2431,8 +2427,6 @@ architecture that doesn't support them will have no effect."),
>  			   &set_dwarf_cmdlist,
>  			   &show_dwarf_cmdlist);
>  
> -#if GDB_SELF_TEST
>    selftests::register_test_foreach_arch ("execute_cfa_program",
>  					 selftests::execute_cfa_program_test);
> -#endif
>  }
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index a98b676b30..df5f0059f6 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -1612,7 +1612,6 @@ copy_bitwise (gdb_byte *dest, ULONGEST dest_offset,
>      }
>  }
>  
> -#if GDB_SELF_TEST
>  
>  namespace selftests {
>  
> @@ -1745,7 +1744,6 @@ copy_bitwise_tests (void)
>  
>  } /* namespace selftests */
>  
> -#endif /* GDB_SELF_TEST */
>  
>  /* Return the number of bytes overlapping a contiguous chunk of N_BITS
>     bits whose first bit is located at bit offset START.  */
> @@ -4681,7 +4679,5 @@ _initialize_dwarf2loc (void)
>  			     show_entry_values_debug,
>  			     &setdebuglist, &showdebuglist);
>  
> -#if GDB_SELF_TEST
>    selftests::register_test ("copy_bitwise", selftests::copy_bitwise_tests);
> -#endif
>  }
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 81a0087c26..55391cbd4a 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -4566,7 +4566,6 @@ dw2_expand_symtabs_matching_symbol
>    static_assert (sizeof (prev) > sizeof (offset_type), "");
>  }
>  
> -#if GDB_SELF_TEST
>  
>  namespace selftests { namespace dw2_expand_symtabs_matching {
>  
> @@ -4989,7 +4988,6 @@ run_test ()
>  
>  }} // namespace selftests::dw2_expand_symtabs_matching
>  
> -#endif /* GDB_SELF_TEST */
>  
>  /* If FILE_MATCHER is NULL or if PER_CU has
>     dwarf2_per_cu_quick_data::MARK set (see
> @@ -25593,8 +25591,6 @@ Warning: This option must be enabled before gdb reads the file."),
>    dwarf2_loclist_block_index = register_symbol_block_impl (LOC_BLOCK,
>  					&dwarf2_block_frame_base_loclist_funcs);
>  
> -#if GDB_SELF_TEST
>    selftests::register_test ("dw2_expand_symtabs_matching",
>  			    selftests::dw2_expand_symtabs_matching::run_test);
> -#endif
>  }
> diff --git a/gdb/findvar.c b/gdb/findvar.c
> index ebaff923a1..74df206660 100644
> --- a/gdb/findvar.c
> +++ b/gdb/findvar.c
> @@ -1010,7 +1010,6 @@ address_from_register (int regnum, struct frame_info *frame)
>    return result;
>  }
>  
> -#if GDB_SELF_TEST
>  namespace selftests {
>  namespace findvar_tests {
>  
> @@ -1089,14 +1088,11 @@ copy_integer_to_size_test ()
>  } // namespace findvar_test
>  } // namespace selftests
>  
> -#endif
>  
>  void
>  _initialize_findvar (void)
>  {
> -#if GDB_SELF_TEST
>    selftests::register_test
>      ("copy_integer_to_size",
>       selftests::findvar_tests::copy_integer_to_size_test);
> -#endif
>  }
> diff --git a/gdb/gdbarch-selftests.c b/gdb/gdbarch-selftests.c
> index 73a31244b4..317cfa3b2f 100644
> --- a/gdb/gdbarch-selftests.c
> +++ b/gdb/gdbarch-selftests.c
> @@ -18,7 +18,6 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> -#if GDB_SELF_TEST
>  #include "selftest.h"
>  #include "selftest-arch.h"
>  #include "inferior.h"
> @@ -168,13 +167,10 @@ register_to_value_test (struct gdbarch *gdbarch)
>  }
>  
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
>  
>  void
>  _initialize_gdbarch_selftests (void)
>  {
> -#if GDB_SELF_TEST
>    selftests::register_test_foreach_arch ("register_to_value",
>  					 selftests::register_to_value_test);
> -#endif
>  }
> diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> index f2f8a084bd..fcda9e725d 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -217,6 +217,7 @@ SFILES = \
>  	$(srcdir)/common/print-utils.c \
>  	$(srcdir)/common/ptid.c \
>  	$(srcdir)/common/rsp-low.c \
> +	$(srcdir)/common/selftest.c \
>  	$(srcdir)/common/tdesc.c \
>  	$(srcdir)/common/vec.c \
>  	$(srcdir)/common/xml-utils.c \
> @@ -261,6 +262,7 @@ OBS = \
>  	common/print-utils.o \
>  	common/ptid.o \
>  	common/rsp-low.o \
> +	common/selftest.o \
>  	common/signals.o \
>  	common/signals-state-save-restore.o \
>  	common/tdesc.o \
> diff --git a/gdb/gdbserver/config.in b/gdb/gdbserver/config.in
> index 05537df81e..ac404afd04 100644
> --- a/gdb/gdbserver/config.in
> +++ b/gdb/gdbserver/config.in
> @@ -8,9 +8,6 @@
>  /* Define to 1 if using `alloca.c'. */
>  #undef C_ALLOCA
>  
> -/* Define if self-testing features should be enabled */
> -#undef GDB_SELF_TEST
> -
>  /* Define to 1 if you have `alloca', as a function or macro. */
>  #undef HAVE_ALLOCA
>  
> diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
> index 043bc216e4..fbd59d7268 100755
> --- a/gdb/gdbserver/configure
> +++ b/gdb/gdbserver/configure
> @@ -5889,13 +5889,6 @@ fi
>    fi
>  
>  
> -if $development; then
> -  srv_selftest_objs="common/selftest.o"
> -
> -$as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
> -
> -fi
> -
>   case ${build_alias} in
>    "") build_noncanonical=${build} ;;
>    *) build_noncanonical=${build_alias} ;;
> diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
> index 99bc46221c..6fc3cc1fa2 100644
> --- a/gdb/gdbserver/configure.ac
> +++ b/gdb/gdbserver/configure.ac
> @@ -54,12 +54,6 @@ else
>  fi
>  GDB_AC_LIBMCHECK(${libmcheck_default})
>  
> -if $development; then
> -  srv_selftest_objs="common/selftest.o"
> -  AC_DEFINE(GDB_SELF_TEST, 1,
> -            [Define if self-testing features should be enabled])
> -fi
> -
>  ACX_NONCANONICAL_TARGET
>  ACX_NONCANONICAL_HOST
>  
> @@ -410,7 +404,7 @@ if test "$srv_xmlfiles" != ""; then
>    done
>  fi
>  
> -GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs $srv_selftest_objs"
> +GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_host_obs"
>  GDBSERVER_LIBS="$srv_libs"
>  
>  dnl Check whether the target supports __sync_*_compare_and_swap.
> diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
> index 72e6a0d87f..46ea3ff142 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -24,13 +24,8 @@
>  # Default hostio_last_error implementation
>  srv_hostio_err_objs="hostio-errno.o"
>  
> -if $development; then
> -   srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
> -   srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
> -else
> -   srv_i386_linux_regobj=""
> -   srv_amd64_linux_regobj=""
> -fi
> +srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
> +srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
>  
>  ipa_ppc_linux_regobj="powerpc-32l-ipa.o powerpc-altivec32l-ipa.o powerpc-cell32l-ipa.o powerpc-vsx32l-ipa.o powerpc-isa205-32l-ipa.o powerpc-isa205-altivec32l-ipa.o powerpc-isa205-vsx32l-ipa.o powerpc-e500l-ipa.o powerpc-64l-ipa.o powerpc-altivec64l-ipa.o powerpc-cell64l-ipa.o powerpc-vsx64l-ipa.o powerpc-isa205-64l-ipa.o powerpc-isa205-altivec64l-ipa.o powerpc-isa205-vsx64l-ipa.o"
>  
> @@ -43,10 +38,8 @@ srv_linux_obj="linux-low.o linux-osdata.o linux-procfs.o linux-ptrace.o linux-wa
>  case "${target}" in
>    aarch64*-*-linux*)
>  			srv_regobj="arm-with-neon.o"
> -			if $development; then
> -			  srv_regobj="${srv_regobj} aarch64.o"
> -			  srv_regobj="${srv_regobj} linux-aarch64-tdesc-selftest.o"
> -                        fi
> +			srv_regobj="${srv_regobj} aarch64.o"
> +			srv_regobj="${srv_regobj} linux-aarch64-tdesc-selftest.o"
>  			srv_tgtobj="linux-aarch64-low.o aarch64-linux-hw-point.o"
>  			srv_tgtobj="$srv_tgtobj linux-aarch32-low.o"
>  			srv_tgtobj="${srv_tgtobj} arch/arm.o"
> @@ -343,13 +336,9 @@ case "${target}" in
>    spu*-*-*)		srv_regobj=reg-spu.o
>  			srv_tgtobj="spu-low.o fork-child.o fork-inferior.o"
>  			;;
> -  tic6x-*-uclinux)	if $development; then
> -			  srv_regobj="tic6x-c64xp-linux.o"
> -			  srv_regobj="${srv_regobj} tic6x-c64x-linux.o"
> -			  srv_regobj="${srv_regobj} tic6x-c62x-linux.o"
> -                        else
> -			  srv_regobj=""
> -                        fi
> +  tic6x-*-uclinux)	srv_regobj="tic6x-c64xp-linux.o"
> +			srv_regobj="${srv_regobj} tic6x-c64x-linux.o"
> +			srv_regobj="${srv_regobj} tic6x-c62x-linux.o"
>  			srv_tgtobj="$srv_linux_obj linux-tic6x-low.o"
>  			srv_tgtobj="${srv_tgtobj} arch/tic6x.o"
>  			srv_linux_regsets=yes
> diff --git a/gdb/gdbserver/linux-aarch64-low.c b/gdb/gdbserver/linux-aarch64-low.c
> index 1d34e319df..b77e18d2f3 100644
> --- a/gdb/gdbserver/linux-aarch64-low.c
> +++ b/gdb/gdbserver/linux-aarch64-low.c
> @@ -3085,7 +3085,5 @@ initialize_low_arch (void)
>    initialize_regsets_info (&aarch64_regsets_info);
>    initialize_regsets_info (&aarch64_sve_regsets_info);
>  
> -#if GDB_SELF_TEST
>    initialize_low_tdesc ();
> -#endif
>  }
> diff --git a/gdb/gdbserver/linux-aarch64-tdesc.h b/gdb/gdbserver/linux-aarch64-tdesc.h
> index 4d2b883b55..51bc08bd2c 100644
> --- a/gdb/gdbserver/linux-aarch64-tdesc.h
> +++ b/gdb/gdbserver/linux-aarch64-tdesc.h
> @@ -19,6 +19,4 @@
>  
>  const target_desc * aarch64_linux_read_description (uint64_t vq);
>  
> -#if GDB_SELF_TEST
>  void initialize_low_tdesc ();
> -#endif
> diff --git a/gdb/gdbserver/linux-tic6x-low.c b/gdb/gdbserver/linux-tic6x-low.c
> index d90bbcfe51..99f2f8cad0 100644
> --- a/gdb/gdbserver/linux-tic6x-low.c
> +++ b/gdb/gdbserver/linux-tic6x-low.c
> @@ -423,7 +423,6 @@ struct linux_target_ops the_low_target = {
>    tic6x_supports_hardware_single_step,
>  };
>  
> -#if GDB_SELF_TEST
>  #include "common/selftest.h"
>  
>  namespace selftests {
> @@ -437,19 +436,16 @@ tic6x_tdesc_test ()
>  }
>  }
>  }
> -#endif
>  
>  void
>  initialize_low_arch (void)
>  {
> -#if GDB_SELF_TEST
>    /* Initialize the Linux target descriptions.  */
>    init_registers_tic6x_c64xp_linux ();
>    init_registers_tic6x_c64x_linux ();
>    init_registers_tic6x_c62x_linux ();
>  
>    selftests::register_test ("tic6x-tdesc", selftests::tdesc::tic6x_tdesc_test);
> -#endif
>  
>    initialize_regsets_info (&tic6x_regsets_info);
>  }
> diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
> index 80b43802c7..daa603398a 100644
> --- a/gdb/gdbserver/linux-x86-low.c
> +++ b/gdb/gdbserver/linux-x86-low.c
> @@ -2897,9 +2897,7 @@ initialize_low_arch (void)
>    tdesc_amd64_linux_no_xml->xmltarget = xmltarget_amd64_linux_no_xml;
>  #endif
>  
> -#if GDB_SELF_TEST
>    initialize_low_tdesc ();
> -#endif
>  
>    tdesc_i386_linux_no_xml = allocate_target_description ();
>    copy_target_description (tdesc_i386_linux_no_xml,
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index a491ae0257..8bc51c8e3c 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -3573,9 +3573,7 @@ captured_main (int argc, char *argv[])
>    volatile int attach = 0;
>    int was_running;
>    bool selftest = false;
> -#if GDB_SELF_TEST
>    const char *selftest_filter = NULL;
> -#endif
>  
>    current_directory = getcwd (NULL, 0);
>    client_state &cs = get_client_state ();
> @@ -3708,9 +3706,7 @@ captured_main (int argc, char *argv[])
>        else if (startswith (*next_arg, "--selftest="))
>  	{
>  	  selftest = true;
> -#if GDB_SELF_TEST
>  	  selftest_filter = *next_arg + strlen ("--selftest=");
> -#endif
>  	}
>        else
>  	{
> @@ -3787,11 +3783,7 @@ captured_main (int argc, char *argv[])
>  
>    if (selftest)
>      {
> -#if GDB_SELF_TEST
>        selftests::run_tests (selftest_filter);
> -#else
> -      printf (_("Selftests are not available in a non-development build.\n"));
> -#endif
>        throw_quit ("Quit");
>      }
>  
> @@ -4495,7 +4487,6 @@ handle_target_event (int err, gdb_client_data client_data)
>    return 0;
>  }
>  
> -#if GDB_SELF_TEST
>  namespace selftests
>  {
>  
> @@ -4504,4 +4495,3 @@ reset ()
>  {}
>  
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
> diff --git a/gdb/i386-linux-tdep.c b/gdb/i386-linux-tdep.c
> index 802c41fe72..efb9b1a7a1 100644
> --- a/gdb/i386-linux-tdep.c
> +++ b/gdb/i386-linux-tdep.c
> @@ -1083,7 +1083,6 @@ _initialize_i386_linux_tdep (void)
>    gdbarch_register_osabi (bfd_arch_i386, 0, GDB_OSABI_LINUX,
>  			  i386_linux_init_abi);
>  
> -#if GDB_SELF_TEST
>    struct
>    {
>      const char *xml;
> @@ -1105,5 +1104,4 @@ _initialize_i386_linux_tdep (void)
>  
>        selftests::record_xml_tdesc (a.xml, tdesc);
>      }
> -#endif /* GDB_SELF_TEST */
>  }
> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
> index a6994aaf12..4e8bd9caa4 100644
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -9053,7 +9053,6 @@ Show Intel Memory Protection Extensions specific variables."),
>    /* Tell remote stub that we support XML target description.  */
>    register_remote_support_xml ("i386");
>  
> -#if GDB_SELF_TEST
>    struct
>    {
>      const char *xml;
> @@ -9075,5 +9074,4 @@ Show Intel Memory Protection Extensions specific variables."),
>  
>        selftests::record_xml_tdesc (a.xml, tdesc);
>      }
> -#endif /* GDB_SELF_TEST */
>  }
> diff --git a/gdb/maint.c b/gdb/maint.c
> index 5d4701cfac..446047227b 100644
> --- a/gdb/maint.c
> +++ b/gdb/maint.c
> @@ -939,26 +939,16 @@ show_per_command_cmd (const char *args, int from_tty)
>  static void
>  maintenance_selftest (const char *args, int from_tty)
>  {
> -#if GDB_SELF_TEST
>    selftests::run_tests (args);
> -#else
> -  printf_filtered (_("\
> -Selftests are not available in a non-development build.\n"));
> -#endif
>  }
>  
>  static void
>  maintenance_info_selftests (const char *arg, int from_tty)
>  {
> -#if GDB_SELF_TEST
>    printf_filtered ("Registered selftests:\n");
>    selftests::for_each_selftest ([] (const std::string &name) {
>      printf_filtered (" - %s\n", name.c_str ());
>    });
> -#else
> -  printf_filtered (_("\
> -Selftests are not available in a non-development build.\n"));
> -#endif
>  }
>  
>  \f
> diff --git a/gdb/producer.c b/gdb/producer.c
> index f21aae93ac..459ced494c 100644
> --- a/gdb/producer.c
> +++ b/gdb/producer.c
> @@ -125,7 +125,6 @@ producer_is_icc (const char *producer, int *major, int *minor)
>    return false;
>  }
>  
> -#if defined GDB_SELF_TEST
>  namespace selftests {
>  namespace producer {
>  
> @@ -206,13 +205,10 @@ Version 18.0 Beta";
>  }
>  }
>  }
> -#endif
>  
>  void
>  _initialize_producer ()
>  {
> -#if defined GDB_SELF_TEST
>    selftests::register_test
>      ("producer-parser", selftests::producer::producer_parsing_tests);
> -#endif
>  }
> diff --git a/gdb/regcache.c b/gdb/regcache.c
> index f3f845aad6..91561ad3fc 100644
> --- a/gdb/regcache.c
> +++ b/gdb/regcache.c
> @@ -1419,7 +1419,6 @@ register_dump::dump (ui_file *file)
>  			footnote_register_type_name_null);
>  }
>  
> -#if GDB_SELF_TEST
>  #include "selftest.h"
>  #include "selftest-arch.h"
>  #include "gdbthread.h"
> @@ -1846,7 +1845,6 @@ cooked_write_test (struct gdbarch *gdbarch)
>  }
>  
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
>  
>  void
>  _initialize_regcache (void)
> @@ -1861,12 +1859,10 @@ _initialize_regcache (void)
>    add_com ("flushregs", class_maintenance, reg_flush_command,
>  	   _("Force gdb to flush its register cache (maintainer command)"));
>  
> -#if GDB_SELF_TEST
>    selftests::register_test ("current_regcache", selftests::current_regcache_test);
>  
>    selftests::register_test_foreach_arch ("regcache::cooked_read_test",
>  					 selftests::cooked_read_test);
>    selftests::register_test_foreach_arch ("regcache::cooked_write_test",
>  					 selftests::cooked_write_test);
> -#endif
>  }
> diff --git a/gdb/rust-exp.y b/gdb/rust-exp.y
> index 248e558a54..70a8acf9ae 100644
> --- a/gdb/rust-exp.y
> +++ b/gdb/rust-exp.y
> @@ -2545,7 +2545,6 @@ rustyyerror (rust_parser *parser, const char *msg)
>  
>  \f
>  
> -#if GDB_SELF_TEST
>  
>  /* Initialize the lexer for testing.  */
>  
> @@ -2803,7 +2802,6 @@ rust_lex_tests (void)
>    rust_lex_test_push_back (&parser);
>  }
>  
> -#endif /* GDB_SELF_TEST */
>  
>  void
>  _initialize_rust_exp (void)
> @@ -2813,7 +2811,5 @@ _initialize_rust_exp (void)
>       error.  */
>    gdb_assert (code == 0);
>  
> -#if GDB_SELF_TEST
>    selftests::register_test ("rust-lex", rust_lex_tests);
> -#endif
>  }
> diff --git a/gdb/selftest-arch.c b/gdb/selftest-arch.c
> index 7045c45660..835145c939 100644
> --- a/gdb/selftest-arch.c
> +++ b/gdb/selftest-arch.c
> @@ -18,7 +18,6 @@
>  
>  #include "defs.h"
>  
> -#if GDB_SELF_TEST
>  #include "selftest.h"
>  #include "selftest-arch.h"
>  #include "arch-utils.h"
> @@ -104,4 +103,3 @@ reset ()
>    reinit_frame_cache ();
>  }
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 20697d9dca..c61a6890ad 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -3791,7 +3791,6 @@ map_symbol_filenames (symbol_filename_ftype *fun, void *data,
>    }
>  }
>  
> -#if GDB_SELF_TEST
>  
>  namespace selftests {
>  namespace filename_language {
> @@ -3848,7 +3847,6 @@ test_set_ext_lang_command ()
>  } /* namespace filename_language */
>  } /* namespace selftests */
>  
> -#endif /* GDB_SELF_TEST */
>  
>  void
>  _initialize_symfile (void)
> @@ -3975,11 +3973,9 @@ Show printing of separate debug info file search debug."), _("\
>  When on, GDB prints the searched locations while looking for separate debug \
>  info files."), NULL, NULL, &setdebuglist, &showdebuglist);
>  
> -#if GDB_SELF_TEST
>    selftests::register_test
>      ("filename_language", selftests::filename_language::test_filename_language);
>    selftests::register_test
>      ("set_ext_lang_command",
>       selftests::filename_language::test_set_ext_lang_command);
> -#endif
>  }
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index 087de141f7..10f2f34eb8 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -1748,7 +1748,6 @@ struct xml_test_tdesc
>  
>  static std::vector<xml_test_tdesc> xml_tdesc;
>  
> -#if GDB_SELF_TEST
>  
>  /* See target-descritpions.h.  */
>  
> @@ -1757,7 +1756,6 @@ record_xml_tdesc (const char *xml_file, const struct target_desc *tdesc)
>  {
>    xml_tdesc.emplace_back (xml_file, std::unique_ptr<const target_desc> (tdesc));
>  }
> -#endif
>  
>  }
>  
> diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
> index 96290b7d97..1aa8f5c79c 100644
> --- a/gdb/target-descriptions.h
> +++ b/gdb/target-descriptions.h
> @@ -210,7 +210,6 @@ void set_tdesc_property (struct target_desc *,
>  void tdesc_add_compatible (struct target_desc *,
>  			   const struct bfd_arch_info *);
>  
> -#if GDB_SELF_TEST
>  namespace selftests {
>  
>  /* Record that XML_FILE should generate a target description that equals
> @@ -220,6 +219,5 @@ namespace selftests {
>  void record_xml_tdesc (const char *xml_file,
>  		       const struct target_desc *tdesc);
>  }
> -#endif
>  
>  #endif /* TARGET_DESCRIPTIONS_H */
> diff --git a/gdb/target.c b/gdb/target.c
> index 2d98954b54..6f46575a19 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -192,7 +192,6 @@ target_command (const char *arg, int from_tty)
>  		  gdb_stdout);
>  }
>  
> -#if GDB_SELF_TEST
>  namespace selftests {
>  
>  /* A mock process_stratum target_ops that doesn't read/write registers
> @@ -211,7 +210,6 @@ test_target_ops::info () const
>  }
>  
>  } /* namespace selftests */
> -#endif /* GDB_SELF_TEST */
>  
>  /* Default target_has_* methods for process_stratum targets.  */
>  
> diff --git a/gdb/target.h b/gdb/target.h
> index 39aa8c3c73..9c2cb38279 100644
> --- a/gdb/target.h
> +++ b/gdb/target.h
> @@ -2543,7 +2543,6 @@ extern void target_prepare_to_generate_core (void);
>  /* See to_done_generating_core.  */
>  extern void target_done_generating_core (void);
>  
> -#if GDB_SELF_TEST
>  namespace selftests {
>  
>  /* A mock process_stratum target_ops that doesn't read/write registers
> @@ -2586,6 +2585,5 @@ public:
>  
>  
>  } // namespace selftests
> -#endif /* GDB_SELF_TEST */
>  
>  #endif /* !defined (TARGET_H) */
> diff --git a/gdb/testsuite/gdb.gdb/unittest.exp b/gdb/testsuite/gdb.gdb/unittest.exp
> index 1c835850b8..c962964284 100644
> --- a/gdb/testsuite/gdb.gdb/unittest.exp
> +++ b/gdb/testsuite/gdb.gdb/unittest.exp
> @@ -23,10 +23,6 @@ gdb_test_multiple $test $test {
>  	set num_ran $expect_out(1,string)
>  	gdb_assert "$num_ran > 0" $test
>    }
> -
> -  -re "Selftests are not available in a non-development build.\r\n$gdb_prompt $" {
> -	unsupported $test
> -  }
>  }
>  
>  if { ![is_remote host] && $do_xml_test } {
> diff --git a/gdb/testsuite/gdb.server/unittest.exp b/gdb/testsuite/gdb.server/unittest.exp
> index e947ff2c30..c050989416 100644
> --- a/gdb/testsuite/gdb.server/unittest.exp
> +++ b/gdb/testsuite/gdb.server/unittest.exp
> @@ -38,10 +38,6 @@ gdb_expect {
>  	gdb_assert "$num_ran > 0" $test
>      }
>  
> -    -re "Selftests are not available in a non-development build.\r\n$" {
> -	unsupported $test
> -    }
> -
>      default {
>  	fail $test
>      }
> diff --git a/gdb/utils.c b/gdb/utils.c
> index 7a8c80c64e..67850c93e7 100644
> --- a/gdb/utils.c
> +++ b/gdb/utils.c
> @@ -2809,7 +2809,6 @@ string_to_core_addr (const char *my_string)
>    return addr;
>  }
>  
> -#if GDB_SELF_TEST
>  
>  static void
>  gdb_realpath_check_trailer (const char *input, const char *trailer)
> @@ -2843,7 +2842,6 @@ gdb_realpath_tests ()
>    gdb_realpath_check_trailer ("", "");
>  }
>  
> -#endif /* GDB_SELF_TEST */
>  
>  /* Allocation function for the libiberty hash table which uses an
>     obstack.  The obstack is passed as DATA.  */
> @@ -3239,7 +3237,5 @@ _initialize_utils (void)
>    add_internal_problem_command (&internal_warning_problem);
>    add_internal_problem_command (&demangler_warning_problem);
>  
> -#if GDB_SELF_TEST
>    selftests::register_test ("gdb_realpath", gdb_realpath_tests);
> -#endif
>  }
> diff --git a/gdb/value.c b/gdb/value.c
> index b0f22f1117..0d2307ebdb 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -3921,7 +3921,6 @@ isvoid_internal_fn (struct gdbarch *gdbarch,
>    return value_from_longest (builtin_type (gdbarch)->builtin_int, ret);
>  }
>  
> -#if GDB_SELF_TEST
>  namespace selftests
>  {
>  
> @@ -4061,7 +4060,6 @@ test_insert_into_bit_range_vector ()
>  }
>  
>  } /* namespace selftests */
> -#endif /* GDB_SELF_TEST */
>  
>  void
>  _initialize_values (void)
> @@ -4114,9 +4112,7 @@ prevents future values, larger than this size, from being allocated."),
>  			    set_max_value_size,
>  			    show_max_value_size,
>  			    &setlist, &showlist);
> -#if GDB_SELF_TEST
>    selftests::register_test ("ranges_contain", selftests::test_ranges_contain);
>    selftests::register_test ("insert_into_bit_range_vector",
>  			    selftests::test_insert_into_bit_range_vector);
> -#endif
>  }
> -- 
> 2.17.1

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH] Don't disable selftests in a non-development build
  2018-08-14  5:42 [RFC/PATCH] Don't disable selftests in a non-development build Sergio Durigan Junior
  2018-08-14 16:28 ` Sergio Durigan Junior
@ 2018-08-14 18:08 ` Pedro Alves
  2018-08-14 18:42   ` Philippe Waroquiers
  2018-08-14 20:12   ` Sergio Durigan Junior
  2018-09-17 20:22 ` [PATCH] Add parameter to allow enabling/disabling selftests via configure Sergio Durigan Junior
  2 siblings, 2 replies; 11+ messages in thread
From: Pedro Alves @ 2018-08-14 18:08 UTC (permalink / raw)
  To: Sergio Durigan Junior, GDB Patches; +Cc: Joel Brobecker

On 08/14/2018 06:42 AM, Sergio Durigan Junior wrote:
> This is an idea I mentioned a few days ago in our IRC channel, and
> Simon said he'd be open to it, so I'm send this RFC/PATCH to see what
> others think.
> 
> Due to the many racy testcases and random failures we see when running
> the GDB testsuite, it is unfortunately not possible to perform a full
> test when one is building a downstream package.  As the Fedora GDB
> maintainer and one of the Debian GDB uploaders, I feel like this
> situation could be improved by, at least, executing our selftests
> after the package has been built.  However, we currently (for some
> reason that is not clear by reading the archives, but see more below)
> disable selftests on non-development builds.  Therefore, this patch
> aims to leave them enabled all the time, for everyone (including the
> end users).
> 
> I understand that disabling the selftests in a non-development build
> brings a few advantages.  For example, we ship less code (even though
> the final difference may be small enough that it doesn't really have
> any real benefit).  

At least for now.  It's plausible that the tests grow substantially
over time.  Also remember that we include unit tests in gdbserver
too.  From this angle, I'd think a configure switch would be more
appropriate.

> Another thing to consider is that we disable
> portions of the codebase that could have latent bugs, making GDB a bit
> safer.  I don't really have an argument to compete with this;
> nonetheless, I think the benefit of having "batteries included" when
> it comes to testing is a huge thing.  With our selftests, the user
> doesn't need to install any external
> programs/libraries (dejagnu/tcl/etc.), and they're stable enough that
> we know that they don't suffer from the racyness present in our
> current testsuite.
> 
> Anyway, here's my take at the problem.  The patches are mostly
> mechanical (getting rid of the "GDB_SELF_TEST" define, which becomes
> useless now that selftests are always present); the ones that aren't
> deal with the configure/Makefile machinery to always compile the
> selftests source files.
> 
> I've tested this patch on BuildBot, without regressions.  I'm taking
> the liberty to Cc Joel, who is our release manager and might have
> opinions about this.

As I mentioned in an internal discussion, I suspect that this must
have been discussed while the original unit testing patch was added,
since, well, we sprinkle #ifdef GDB_SELF_TEST throughout, so it
wasn't done by chance.

One point I'd like to raise is that the fact that GDB's unit tests are built
and bolted into GDB-the-binary is more of a technical consequence than
a design goal.  It just so happens that it was much easier to build unit
tests this way.  But this is not the only way.  In fact, for fairly isolated
generic utilities, the corresponding unit tests are pretty self
contained -- I'm talking about the src/gdb/unittests/ tests -- 
I've considered creating a small unit tests driver binary for them, independent
of GDB.  Imagine we get to a point where we want to
share the code that is tested by those tests with another project like
e.g., GCC (e.g., by moving the code to a new libiberty-like library or something
like that).  It would be totally doable to build a separate small binary other
than build/gdb/gdb whose only purpose would be to drive the unit tests (like,
e.g., build/gdb/unittests-driver).  If we do that, then gdb's own built-in
unit tests start disappearing...

Last year, Yao was pondering/investigating using some off-the-shelf
unit tests framework, like Google Test.  The result would be same.

I would hope that going in the direction of having unit tests
in released binaries would not prevent exploration or design
changes in the unit testing space.

Of course, if we had a separate unit tests driver, then there would
be nothing preventing building it in release mode, since by design
it would not affect gdb's binary.

Another approach to addressing this issue here:

> Due to the many racy testcases and random failures we see when running
> the GDB testsuite, it is unfortunately not possible to perform a full
> test when one is building a downstream package.  As the Fedora GDB
> maintainer and one of the Debian GDB uploaders, I feel like this
> situation could be improved by, at least, executing our selftests
> after the package has been built.  However, we currently (for some
> reason that is not clear by reading the archives, but see more below)
> disable selftests on non-development builds.  Therefore, this patch
> aims to leave them enabled all the time, for everyone (including the
> end users).

... is to come up with some small set of stable testcases that
are considered the "smoke tests" and add a mechanism to run them.
Could be just a list of testcases in a file that is passed to
  make check TESTS="list of basic tests here"
or some make target like "make check-smoke", or something
else even.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH] Don't disable selftests in a non-development build
  2018-08-14 18:08 ` Pedro Alves
@ 2018-08-14 18:42   ` Philippe Waroquiers
  2018-08-14 20:17     ` Sergio Durigan Junior
  2018-08-14 20:12   ` Sergio Durigan Junior
  1 sibling, 1 reply; 11+ messages in thread
From: Philippe Waroquiers @ 2018-08-14 18:42 UTC (permalink / raw)
  To: Pedro Alves, Sergio Durigan Junior, GDB Patches; +Cc: Joel Brobecker

On Tue, 2018-08-14 at 19:08 +0100, Pedro Alves wrote:
> Another approach to addressing this issue here:
> 
> > Due to the many racy testcases and random failures we see when running
> > the GDB testsuite, it is unfortunately not possible to perform a full
> > test when one is building a downstream package.  As the Fedora GDB
> > maintainer and one of the Debian GDB uploaders, I feel like this
> > situation could be improved by, at least, executing our selftests
> > after the package has been built.  However, we currently (for some
> > reason that is not clear by reading the archives, but see more below)
> > disable selftests on non-development builds.  Therefore, this patch
> > aims to leave them enabled all the time, for everyone (including the
> > end users).
> 
> ... is to come up with some small set of stable testcases that
> are considered the "smoke tests" and add a mechanism to run them.
> Could be just a list of testcases in a file that is passed to
>   make check TESTS="list of basic tests here"
> or some make target like "make check-smoke", or something
> else even.

Instead of (or in addition to) some list of tests that are known to be
non-racy/alwayd ok,
maybe it would be nice to have a list of tests that are known to
be racy/sometimes wrong ?
We e.g. could mark these racy tests as part of the test itself,
with some pseudo dejagnu code like :

if [do_not_run_racy_or_sometimes_wrong_tests] {
  return
}

# or, if the test is only racy on some platform
if [do_not_run_racy_or_sometimes_wrong_tests && istarget x86_64-*-*] {
  return
}

The above would also help the gdb developers to interpret
the results of a test run.
I am always wondering if a failure I see can (or cannot) be
explained by the change I just did.

Philippe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH] Don't disable selftests in a non-development build
  2018-08-14 18:08 ` Pedro Alves
  2018-08-14 18:42   ` Philippe Waroquiers
@ 2018-08-14 20:12   ` Sergio Durigan Junior
  1 sibling, 0 replies; 11+ messages in thread
From: Sergio Durigan Junior @ 2018-08-14 20:12 UTC (permalink / raw)
  To: Pedro Alves; +Cc: GDB Patches, Joel Brobecker

Thanks for the comments.

On Tuesday, August 14 2018, Pedro Alves wrote:

> On 08/14/2018 06:42 AM, Sergio Durigan Junior wrote:
>> This is an idea I mentioned a few days ago in our IRC channel, and
>> Simon said he'd be open to it, so I'm send this RFC/PATCH to see what
>> others think.
>> 
>> Due to the many racy testcases and random failures we see when running
>> the GDB testsuite, it is unfortunately not possible to perform a full
>> test when one is building a downstream package.  As the Fedora GDB
>> maintainer and one of the Debian GDB uploaders, I feel like this
>> situation could be improved by, at least, executing our selftests
>> after the package has been built.  However, we currently (for some
>> reason that is not clear by reading the archives, but see more below)
>> disable selftests on non-development builds.  Therefore, this patch
>> aims to leave them enabled all the time, for everyone (including the
>> end users).
>> 
>> I understand that disabling the selftests in a non-development build
>> brings a few advantages.  For example, we ship less code (even though
>> the final difference may be small enough that it doesn't really have
>> any real benefit).  
>
> At least for now.  It's plausible that the tests grow substantially
> over time.

I agree.  I think we could always revisit the decision if the unittests
become too big.

> Also remember that we include unit tests in gdbserver
> too.  From this angle, I'd think a configure switch would be more
> appropriate.

I thought about adding a configure switch to control whether
GDB_SELF_TEST is defined or not.  In the end, it seemed to me that
always enabling the tests made more sense.  But I can certainly add a
configure switch, both on GDB and gdbserver, of course.

>> Another thing to consider is that we disable
>> portions of the codebase that could have latent bugs, making GDB a bit
>> safer.  I don't really have an argument to compete with this;
>> nonetheless, I think the benefit of having "batteries included" when
>> it comes to testing is a huge thing.  With our selftests, the user
>> doesn't need to install any external
>> programs/libraries (dejagnu/tcl/etc.), and they're stable enough that
>> we know that they don't suffer from the racyness present in our
>> current testsuite.
>> 
>> Anyway, here's my take at the problem.  The patches are mostly
>> mechanical (getting rid of the "GDB_SELF_TEST" define, which becomes
>> useless now that selftests are always present); the ones that aren't
>> deal with the configure/Makefile machinery to always compile the
>> selftests source files.
>> 
>> I've tested this patch on BuildBot, without regressions.  I'm taking
>> the liberty to Cc Joel, who is our release manager and might have
>> opinions about this.
>
> As I mentioned in an internal discussion, I suspect that this must
> have been discussed while the original unit testing patch was added,
> since, well, we sprinkle #ifdef GDB_SELF_TEST throughout, so it
> wasn't done by chance.

I thought so too, and I looked in the archives and tried to search this
discussion, but nothing came up.  The first message I found was:

  https://sourceware.org/ml/gdb-patches/2016-04/msg00569.html

and it already proposed that the feature should be disabled in
non-development builds.  I couldn't find any discussions specific to
this, so I think people just accepted this was the better way to do it.

> One point I'd like to raise is that the fact that GDB's unit tests are built
> and bolted into GDB-the-binary is more of a technical consequence than
> a design goal.  It just so happens that it was much easier to build unit
> tests this way.  But this is not the only way.  In fact, for fairly isolated
> generic utilities, the corresponding unit tests are pretty self
> contained -- I'm talking about the src/gdb/unittests/ tests -- 
> I've considered creating a small unit tests driver binary for them, independent
> of GDB.  Imagine we get to a point where we want to
> share the code that is tested by those tests with another project like
> e.g., GCC (e.g., by moving the code to a new libiberty-like library or something
> like that).  It would be totally doable to build a separate small binary other
> than build/gdb/gdb whose only purpose would be to drive the unit tests (like,
> e.g., build/gdb/unittests-driver).  If we do that, then gdb's own built-in
> unit tests start disappearing...
>
> Last year, Yao was pondering/investigating using some off-the-shelf
> unit tests framework, like Google Test.  The result would be same.
>
> I would hope that going in the direction of having unit tests
> in released binaries would not prevent exploration or design
> changes in the unit testing space.

I appreciate the explanation, but it seems to me that this is another
problem.  If my proposal is accepted, there will be nothing preventing
us to go forward with this idea (which I think is a very good idea, of
course).

> Of course, if we had a separate unit tests driver, then there would
> be nothing preventing building it in release mode, since by design
> it would not affect gdb's binary.
>
> Another approach to addressing this issue here:
>
>> Due to the many racy testcases and random failures we see when running
>> the GDB testsuite, it is unfortunately not possible to perform a full
>> test when one is building a downstream package.  As the Fedora GDB
>> maintainer and one of the Debian GDB uploaders, I feel like this
>> situation could be improved by, at least, executing our selftests
>> after the package has been built.  However, we currently (for some
>> reason that is not clear by reading the archives, but see more below)
>> disable selftests on non-development builds.  Therefore, this patch
>> aims to leave them enabled all the time, for everyone (including the
>> end users).
>
> ... is to come up with some small set of stable testcases that
> are considered the "smoke tests" and add a mechanism to run them.
> Could be just a list of testcases in a file that is passed to
>   make check TESTS="list of basic tests here"
> or some make target like "make check-smoke", or something
> else even.

Sure, that works too.  This is somewhat related to your idea of marking
the racy tests in our testsuite (a la "setup_kfail"), which I think is
also good, and it's on my long-term TODO list.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH] Don't disable selftests in a non-development build
  2018-08-14 18:42   ` Philippe Waroquiers
@ 2018-08-14 20:17     ` Sergio Durigan Junior
  0 siblings, 0 replies; 11+ messages in thread
From: Sergio Durigan Junior @ 2018-08-14 20:17 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: Pedro Alves, GDB Patches, Joel Brobecker

On Tuesday, August 14 2018, Philippe Waroquiers wrote:

> On Tue, 2018-08-14 at 19:08 +0100, Pedro Alves wrote:
>> Another approach to addressing this issue here:
>> 
>> > Due to the many racy testcases and random failures we see when running
>> > the GDB testsuite, it is unfortunately not possible to perform a full
>> > test when one is building a downstream package.  As the Fedora GDB
>> > maintainer and one of the Debian GDB uploaders, I feel like this
>> > situation could be improved by, at least, executing our selftests
>> > after the package has been built.  However, we currently (for some
>> > reason that is not clear by reading the archives, but see more below)
>> > disable selftests on non-development builds.  Therefore, this patch
>> > aims to leave them enabled all the time, for everyone (including the
>> > end users).
>> 
>> ... is to come up with some small set of stable testcases that
>> are considered the "smoke tests" and add a mechanism to run them.
>> Could be just a list of testcases in a file that is passed to
>>   make check TESTS="list of basic tests here"
>> or some make target like "make check-smoke", or something
>> else even.
>
> Instead of (or in addition to) some list of tests that are known to be
> non-racy/alwayd ok,
> maybe it would be nice to have a list of tests that are known to
> be racy/sometimes wrong ?
> We e.g. could mark these racy tests as part of the test itself,
> with some pseudo dejagnu code like :
>
> if [do_not_run_racy_or_sometimes_wrong_tests] {
>   return
> }
>
> # or, if the test is only racy on some platform
> if [do_not_run_racy_or_sometimes_wrong_tests && istarget x86_64-*-*] {
>   return
> }

Yes, this is a good idea.  Pedro suggested something very similar (if
not equal) a while ago, when I was doing some work on detecting racy
tests for our BuildBot.  The idea is to mark the known racy tests using
something a la "setup_kfail".  I haven't gotten the time to work on this
yet, but it's on my TODO list.

> The above would also help the gdb developers to interpret
> the results of a test run.
> I am always wondering if a failure I see can (or cannot) be
> explained by the change I just did.

It's a PITA, indeed.  If it helps, the BuildBot automatically refreshes
the list of known racy tests every week (or every day, for x86*
builders).  You can have a look at the lists here:

  https://git.sergiodj.net/gdb-xfails.git/tree/xfails

So, for a list of known racy tests for Fedora-x86-64-m64:

  https://git.sergiodj.net/gdb-xfails.git/tree/xfails/Fedora-x86_64-m64/xfails/master/xfail.table

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Add parameter to allow enabling/disabling selftests via configure
  2018-08-14  5:42 [RFC/PATCH] Don't disable selftests in a non-development build Sergio Durigan Junior
  2018-08-14 16:28 ` Sergio Durigan Junior
  2018-08-14 18:08 ` Pedro Alves
@ 2018-09-17 20:22 ` Sergio Durigan Junior
  2018-09-23  3:56   ` Sergio Durigan Junior
  2018-10-06  1:19   ` Simon Marchi
  2 siblings, 2 replies; 11+ messages in thread
From: Sergio Durigan Junior @ 2018-09-17 20:22 UTC (permalink / raw)
  To: GDB Patches; +Cc: Pedro Alves, Sergio Durigan Junior

This is a follow-up of:

  https://sourceware.org/ml/gdb-patches/2018-08/msg00347.html

Instead of going throttle and always enabling our selftests (even in
non-development builds), this patch is a bit more conservative and
introduces a configure option ("--enable-unit-tests") that allows the
user to choose whether she wants unit tests in the build or not.  Note
that the current behaviour is retained: if no option is provided, GDB
will have selftests included in a development build, and will *not*
have selftests included in a non-development build.

The rationale for having this option is still the same: due to the
many racy testcases and random failures we see when running the GDB
testsuite, it is unfortunately not possible to perform a full test
when one is building a downstream package.  As the Fedora GDB
maintainer and one of the Debian GDB uploaders, I feel like this
situation could be improved by, at least, executing our selftests
after the package has been built.

This patch introduces no regressions to our build.

OK?

gdb/ChangeLog:
2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>

	* README (`configure' options): Add documentation for new
	"--enable-unit-tests" option.
	* configure: Regenerate.
	* configure.ac: Add "--enable-unit-tests" option.
	* maint.c (maintenance_selftest): Update message informing
	that selftests have been disabled.
	(maintenance_info_selftests): Likewise.

gdb/gdbserver/ChangeLog:
2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>

	* configure: Regenerate.
	* configure.ac: Add "--enable-unit-tests" option.
	* configure.srv: Use "$enable_unittests" instead of
	"$development" when checking whether unit tests have been
	enabled.
	* server.c (captured_main): Update message informing that
	selftests have been disabled.

gdb/testsuite/ChangeLog:
2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>

	* gdb.gdb/unittest.exp: Update expected message informing that
	selftests have been disabled.
	* gdb.server/unittest.exp: Likewise.
---
 gdb/README                            |  6 ++++++
 gdb/configure                         | 23 ++++++++++++++++++++++-
 gdb/configure.ac                      | 17 ++++++++++++++++-
 gdb/gdbserver/configure               | 23 ++++++++++++++++++++++-
 gdb/gdbserver/configure.ac            | 17 ++++++++++++++++-
 gdb/gdbserver/configure.srv           |  2 +-
 gdb/gdbserver/server.c                |  2 +-
 gdb/maint.c                           |  4 ++--
 gdb/testsuite/gdb.gdb/unittest.exp    |  2 +-
 gdb/testsuite/gdb.server/unittest.exp |  2 +-
 10 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/gdb/README b/gdb/README
index e43887ffcd..e6fe3b183d 100644
--- a/gdb/README
+++ b/gdb/README
@@ -524,6 +524,12 @@ prefer; but you may abbreviate option names if you use `--'.
      after being built, the location of the system-wide init file will
      be adjusted accordingly. 
 
+`--enable-unit-tests[=yes|no]'
+     Enable (i.e., include) support for unit tests when compiling GDB
+     and GDBServer.  Note that if this option is not passed, GDB will
+     have selftests if it is a development build, and will *not* have
+     selftests if it a non-development build.
+
 `configure' accepts other options, for compatibility with configuring
 other GNU tools recursively; but these are the only options that affect
 GDB or its supporting libraries.
diff --git a/gdb/configure b/gdb/configure
index d92a256f1f..18e04c0c50 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -895,6 +895,7 @@ enable_sim
 enable_gdbserver
 with_babeltrace
 with_libbabeltrace_prefix
+enable_unit_tests
 '
       ac_precious_vars='build_alias
 host_alias
@@ -1559,6 +1560,8 @@ Optional Features:
   --enable-sim            link gdb with simulator
   --enable-gdbserver      automatically build gdbserver (yes/no/auto, default
                           is auto)
+  --enable-unit-tests     Enable the inclusion of unit tests when compiling
+                          GDB
 
 Optional Packages:
   --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
@@ -17731,7 +17734,25 @@ ac_config_links="$ac_config_links $ac_config_links_1"
 $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h
 
 
-if $development; then
+# Check whether we will enable the inclusion of unit tests when
+# compiling GDB.
+#
+# The default value of this option changes depending whether we're on
+# development mode (in which case it's "true") or not (in which case
+# it's "false").
+# Check whether --enable-unit-tests was given.
+if test "${enable_unit_tests+set}" = set; then :
+  enableval=$enable_unit_tests; case "${enableval}" in
+  yes)  enable_unittests=true  ;;
+  no)   enable_unittests=false ;;
+  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
+esac
+else
+  enable_unittests=$development
+fi
+
+
+if $enable_unittests; then
 
 $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
 
diff --git a/gdb/configure.ac b/gdb/configure.ac
index e38604cb65..3ef43a95f4 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -2267,7 +2267,22 @@ dnl  At the moment, we just assume it's UTF-8.
 AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8",
           [Define to be a string naming the default host character set.])
 
-if $development; then
+# Check whether we will enable the inclusion of unit tests when
+# compiling GDB.
+#
+# The default value of this option changes depending whether we're on
+# development mode (in which case it's "true") or not (in which case
+# it's "false").
+AC_ARG_ENABLE(unit-tests,
+AS_HELP_STRING([--enable-unit-tests],
+[Enable the inclusion of unit tests when compiling GDB]),
+[case "${enableval}" in
+  yes)  enable_unittests=true  ;;
+  no)   enable_unittests=false ;;
+  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
+esac], [enable_unittests=$development])
+
+if $enable_unittests; then
   AC_DEFINE(GDB_SELF_TEST, 1,
             [Define if self-testing features should be enabled])
   CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index f5cbbaea78..9915e212d7 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -722,6 +722,7 @@ enable_option_checking
 enable_maintainer_mode
 enable_largefile
 enable_libmcheck
+enable_unit_tests
 with_ust
 with_ust_include
 with_ust_lib
@@ -1367,6 +1368,8 @@ Optional Features:
                           sometimes confusing) to the casual installer
   --disable-largefile     omit support for large files
   --enable-libmcheck      Try linking with -lmcheck if available
+  --enable-unit-tests     Enable the inclusion of unit tests when compiling
+                          GDB
   --enable-werror         treat compile warnings as errors
   --enable-build-warnings enable build-time compiler warnings if gcc is used
   --enable-gdb-build-warnings
@@ -5889,7 +5892,25 @@ fi
   fi
 
 
-if $development; then
+# Check whether we will enable the inclusion of unit tests when
+# compiling GDB.
+#
+# The default value of this option changes depending whether we're on
+# development mode (in which case it's "true") or not (in which case
+# it's "false").
+# Check whether --enable-unit-tests was given.
+if test "${enable_unit_tests+set}" = set; then :
+  enableval=$enable_unit_tests; case "${enableval}" in
+  yes)  enable_unittests=true  ;;
+  no)   enable_unittests=false ;;
+  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
+esac
+else
+  enable_unittests=$development
+fi
+
+
+if $enable_unittests; then
   srv_selftest_objs="common/selftest.o"
 
 $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index 99bc46221c..3c6ed9143f 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -54,7 +54,22 @@ else
 fi
 GDB_AC_LIBMCHECK(${libmcheck_default})
 
-if $development; then
+# Check whether we will enable the inclusion of unit tests when
+# compiling GDB.
+#
+# The default value of this option changes depending whether we're on
+# development mode (in which case it's "true") or not (in which case
+# it's "false").
+AC_ARG_ENABLE(unit-tests,
+AS_HELP_STRING([--enable-unit-tests],
+[Enable the inclusion of unit tests when compiling GDB]),
+[case "${enableval}" in
+  yes)  enable_unittests=true  ;;
+  no)   enable_unittests=false ;;
+  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
+esac], [enable_unittests=$development])
+
+if $enable_unittests; then
   srv_selftest_objs="common/selftest.o"
   AC_DEFINE(GDB_SELF_TEST, 1,
             [Define if self-testing features should be enabled])
diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
index 72e6a0d87f..636d830f1a 100644
--- a/gdb/gdbserver/configure.srv
+++ b/gdb/gdbserver/configure.srv
@@ -24,7 +24,7 @@
 # Default hostio_last_error implementation
 srv_hostio_err_objs="hostio-errno.o"
 
-if $development; then
+if $enable_unittests; then
    srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
    srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
 else
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index a491ae0257..f2f0d569ab 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -3790,7 +3790,7 @@ captured_main (int argc, char *argv[])
 #if GDB_SELF_TEST
       selftests::run_tests (selftest_filter);
 #else
-      printf (_("Selftests are not available in a non-development build.\n"));
+      printf (_("Selftests have been disabled for this build.\n"));
 #endif
       throw_quit ("Quit");
     }
diff --git a/gdb/maint.c b/gdb/maint.c
index 19db8a850b..01a80f5d73 100644
--- a/gdb/maint.c
+++ b/gdb/maint.c
@@ -943,7 +943,7 @@ maintenance_selftest (const char *args, int from_tty)
   selftests::run_tests (args);
 #else
   printf_filtered (_("\
-Selftests are not available in a non-development build.\n"));
+Selftests have been disabled for this build.\n"));
 #endif
 }
 
@@ -957,7 +957,7 @@ maintenance_info_selftests (const char *arg, int from_tty)
   });
 #else
   printf_filtered (_("\
-Selftests are not available in a non-development build.\n"));
+Selftests have been disabled for this build.\n"));
 #endif
 }
 
diff --git a/gdb/testsuite/gdb.gdb/unittest.exp b/gdb/testsuite/gdb.gdb/unittest.exp
index 1c835850b8..8e3e9a1761 100644
--- a/gdb/testsuite/gdb.gdb/unittest.exp
+++ b/gdb/testsuite/gdb.gdb/unittest.exp
@@ -24,7 +24,7 @@ gdb_test_multiple $test $test {
 	gdb_assert "$num_ran > 0" $test
   }
 
-  -re "Selftests are not available in a non-development build.\r\n$gdb_prompt $" {
+  -re "Selftests have been disabled for this build.\r\n$gdb_prompt $" {
 	unsupported $test
   }
 }
diff --git a/gdb/testsuite/gdb.server/unittest.exp b/gdb/testsuite/gdb.server/unittest.exp
index e947ff2c30..b0a7c6ae56 100644
--- a/gdb/testsuite/gdb.server/unittest.exp
+++ b/gdb/testsuite/gdb.server/unittest.exp
@@ -38,7 +38,7 @@ gdb_expect {
 	gdb_assert "$num_ran > 0" $test
     }
 
-    -re "Selftests are not available in a non-development build.\r\n$" {
+    -re "Selftests have been disabled for this build.\r\n$" {
 	unsupported $test
     }
 
-- 
2.17.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Add parameter to allow enabling/disabling selftests via configure
  2018-09-17 20:22 ` [PATCH] Add parameter to allow enabling/disabling selftests via configure Sergio Durigan Junior
@ 2018-09-23  3:56   ` Sergio Durigan Junior
  2018-10-05 21:42     ` Sergio Durigan Junior
  2018-10-06  1:19   ` Simon Marchi
  1 sibling, 1 reply; 11+ messages in thread
From: Sergio Durigan Junior @ 2018-09-23  3:56 UTC (permalink / raw)
  To: GDB Patches; +Cc: Pedro Alves

On Monday, September 17 2018, I wrote:

> This is a follow-up of:
>
>   https://sourceware.org/ml/gdb-patches/2018-08/msg00347.html

Ping.

> Instead of going throttle and always enabling our selftests (even in
> non-development builds), this patch is a bit more conservative and
> introduces a configure option ("--enable-unit-tests") that allows the
> user to choose whether she wants unit tests in the build or not.  Note
> that the current behaviour is retained: if no option is provided, GDB
> will have selftests included in a development build, and will *not*
> have selftests included in a non-development build.
>
> The rationale for having this option is still the same: due to the
> many racy testcases and random failures we see when running the GDB
> testsuite, it is unfortunately not possible to perform a full test
> when one is building a downstream package.  As the Fedora GDB
> maintainer and one of the Debian GDB uploaders, I feel like this
> situation could be improved by, at least, executing our selftests
> after the package has been built.
>
> This patch introduces no regressions to our build.
>
> OK?
>
> gdb/ChangeLog:
> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* README (`configure' options): Add documentation for new
> 	"--enable-unit-tests" option.
> 	* configure: Regenerate.
> 	* configure.ac: Add "--enable-unit-tests" option.
> 	* maint.c (maintenance_selftest): Update message informing
> 	that selftests have been disabled.
> 	(maintenance_info_selftests): Likewise.
>
> gdb/gdbserver/ChangeLog:
> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* configure: Regenerate.
> 	* configure.ac: Add "--enable-unit-tests" option.
> 	* configure.srv: Use "$enable_unittests" instead of
> 	"$development" when checking whether unit tests have been
> 	enabled.
> 	* server.c (captured_main): Update message informing that
> 	selftests have been disabled.
>
> gdb/testsuite/ChangeLog:
> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* gdb.gdb/unittest.exp: Update expected message informing that
> 	selftests have been disabled.
> 	* gdb.server/unittest.exp: Likewise.
> ---
>  gdb/README                            |  6 ++++++
>  gdb/configure                         | 23 ++++++++++++++++++++++-
>  gdb/configure.ac                      | 17 ++++++++++++++++-
>  gdb/gdbserver/configure               | 23 ++++++++++++++++++++++-
>  gdb/gdbserver/configure.ac            | 17 ++++++++++++++++-
>  gdb/gdbserver/configure.srv           |  2 +-
>  gdb/gdbserver/server.c                |  2 +-
>  gdb/maint.c                           |  4 ++--
>  gdb/testsuite/gdb.gdb/unittest.exp    |  2 +-
>  gdb/testsuite/gdb.server/unittest.exp |  2 +-
>  10 files changed, 88 insertions(+), 10 deletions(-)
>
> diff --git a/gdb/README b/gdb/README
> index e43887ffcd..e6fe3b183d 100644
> --- a/gdb/README
> +++ b/gdb/README
> @@ -524,6 +524,12 @@ prefer; but you may abbreviate option names if you use `--'.
>       after being built, the location of the system-wide init file will
>       be adjusted accordingly. 
>  
> +`--enable-unit-tests[=yes|no]'
> +     Enable (i.e., include) support for unit tests when compiling GDB
> +     and GDBServer.  Note that if this option is not passed, GDB will
> +     have selftests if it is a development build, and will *not* have
> +     selftests if it a non-development build.
> +
>  `configure' accepts other options, for compatibility with configuring
>  other GNU tools recursively; but these are the only options that affect
>  GDB or its supporting libraries.
> diff --git a/gdb/configure b/gdb/configure
> index d92a256f1f..18e04c0c50 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -895,6 +895,7 @@ enable_sim
>  enable_gdbserver
>  with_babeltrace
>  with_libbabeltrace_prefix
> +enable_unit_tests
>  '
>        ac_precious_vars='build_alias
>  host_alias
> @@ -1559,6 +1560,8 @@ Optional Features:
>    --enable-sim            link gdb with simulator
>    --enable-gdbserver      automatically build gdbserver (yes/no/auto, default
>                            is auto)
> +  --enable-unit-tests     Enable the inclusion of unit tests when compiling
> +                          GDB
>  
>  Optional Packages:
>    --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
> @@ -17731,7 +17734,25 @@ ac_config_links="$ac_config_links $ac_config_links_1"
>  $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h
>  
>  
> -if $development; then
> +# Check whether we will enable the inclusion of unit tests when
> +# compiling GDB.
> +#
> +# The default value of this option changes depending whether we're on
> +# development mode (in which case it's "true") or not (in which case
> +# it's "false").
> +# Check whether --enable-unit-tests was given.
> +if test "${enable_unit_tests+set}" = set; then :
> +  enableval=$enable_unit_tests; case "${enableval}" in
> +  yes)  enable_unittests=true  ;;
> +  no)   enable_unittests=false ;;
> +  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
> +esac
> +else
> +  enable_unittests=$development
> +fi
> +
> +
> +if $enable_unittests; then
>  
>  $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
>  
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index e38604cb65..3ef43a95f4 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -2267,7 +2267,22 @@ dnl  At the moment, we just assume it's UTF-8.
>  AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8",
>            [Define to be a string naming the default host character set.])
>  
> -if $development; then
> +# Check whether we will enable the inclusion of unit tests when
> +# compiling GDB.
> +#
> +# The default value of this option changes depending whether we're on
> +# development mode (in which case it's "true") or not (in which case
> +# it's "false").
> +AC_ARG_ENABLE(unit-tests,
> +AS_HELP_STRING([--enable-unit-tests],
> +[Enable the inclusion of unit tests when compiling GDB]),
> +[case "${enableval}" in
> +  yes)  enable_unittests=true  ;;
> +  no)   enable_unittests=false ;;
> +  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
> +esac], [enable_unittests=$development])
> +
> +if $enable_unittests; then
>    AC_DEFINE(GDB_SELF_TEST, 1,
>              [Define if self-testing features should be enabled])
>    CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
> diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
> index f5cbbaea78..9915e212d7 100755
> --- a/gdb/gdbserver/configure
> +++ b/gdb/gdbserver/configure
> @@ -722,6 +722,7 @@ enable_option_checking
>  enable_maintainer_mode
>  enable_largefile
>  enable_libmcheck
> +enable_unit_tests
>  with_ust
>  with_ust_include
>  with_ust_lib
> @@ -1367,6 +1368,8 @@ Optional Features:
>                            sometimes confusing) to the casual installer
>    --disable-largefile     omit support for large files
>    --enable-libmcheck      Try linking with -lmcheck if available
> +  --enable-unit-tests     Enable the inclusion of unit tests when compiling
> +                          GDB
>    --enable-werror         treat compile warnings as errors
>    --enable-build-warnings enable build-time compiler warnings if gcc is used
>    --enable-gdb-build-warnings
> @@ -5889,7 +5892,25 @@ fi
>    fi
>  
>  
> -if $development; then
> +# Check whether we will enable the inclusion of unit tests when
> +# compiling GDB.
> +#
> +# The default value of this option changes depending whether we're on
> +# development mode (in which case it's "true") or not (in which case
> +# it's "false").
> +# Check whether --enable-unit-tests was given.
> +if test "${enable_unit_tests+set}" = set; then :
> +  enableval=$enable_unit_tests; case "${enableval}" in
> +  yes)  enable_unittests=true  ;;
> +  no)   enable_unittests=false ;;
> +  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
> +esac
> +else
> +  enable_unittests=$development
> +fi
> +
> +
> +if $enable_unittests; then
>    srv_selftest_objs="common/selftest.o"
>  
>  $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
> diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
> index 99bc46221c..3c6ed9143f 100644
> --- a/gdb/gdbserver/configure.ac
> +++ b/gdb/gdbserver/configure.ac
> @@ -54,7 +54,22 @@ else
>  fi
>  GDB_AC_LIBMCHECK(${libmcheck_default})
>  
> -if $development; then
> +# Check whether we will enable the inclusion of unit tests when
> +# compiling GDB.
> +#
> +# The default value of this option changes depending whether we're on
> +# development mode (in which case it's "true") or not (in which case
> +# it's "false").
> +AC_ARG_ENABLE(unit-tests,
> +AS_HELP_STRING([--enable-unit-tests],
> +[Enable the inclusion of unit tests when compiling GDB]),
> +[case "${enableval}" in
> +  yes)  enable_unittests=true  ;;
> +  no)   enable_unittests=false ;;
> +  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
> +esac], [enable_unittests=$development])
> +
> +if $enable_unittests; then
>    srv_selftest_objs="common/selftest.o"
>    AC_DEFINE(GDB_SELF_TEST, 1,
>              [Define if self-testing features should be enabled])
> diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
> index 72e6a0d87f..636d830f1a 100644
> --- a/gdb/gdbserver/configure.srv
> +++ b/gdb/gdbserver/configure.srv
> @@ -24,7 +24,7 @@
>  # Default hostio_last_error implementation
>  srv_hostio_err_objs="hostio-errno.o"
>  
> -if $development; then
> +if $enable_unittests; then
>     srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
>     srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
>  else
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index a491ae0257..f2f0d569ab 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -3790,7 +3790,7 @@ captured_main (int argc, char *argv[])
>  #if GDB_SELF_TEST
>        selftests::run_tests (selftest_filter);
>  #else
> -      printf (_("Selftests are not available in a non-development build.\n"));
> +      printf (_("Selftests have been disabled for this build.\n"));
>  #endif
>        throw_quit ("Quit");
>      }
> diff --git a/gdb/maint.c b/gdb/maint.c
> index 19db8a850b..01a80f5d73 100644
> --- a/gdb/maint.c
> +++ b/gdb/maint.c
> @@ -943,7 +943,7 @@ maintenance_selftest (const char *args, int from_tty)
>    selftests::run_tests (args);
>  #else
>    printf_filtered (_("\
> -Selftests are not available in a non-development build.\n"));
> +Selftests have been disabled for this build.\n"));
>  #endif
>  }
>  
> @@ -957,7 +957,7 @@ maintenance_info_selftests (const char *arg, int from_tty)
>    });
>  #else
>    printf_filtered (_("\
> -Selftests are not available in a non-development build.\n"));
> +Selftests have been disabled for this build.\n"));
>  #endif
>  }
>  
> diff --git a/gdb/testsuite/gdb.gdb/unittest.exp b/gdb/testsuite/gdb.gdb/unittest.exp
> index 1c835850b8..8e3e9a1761 100644
> --- a/gdb/testsuite/gdb.gdb/unittest.exp
> +++ b/gdb/testsuite/gdb.gdb/unittest.exp
> @@ -24,7 +24,7 @@ gdb_test_multiple $test $test {
>  	gdb_assert "$num_ran > 0" $test
>    }
>  
> -  -re "Selftests are not available in a non-development build.\r\n$gdb_prompt $" {
> +  -re "Selftests have been disabled for this build.\r\n$gdb_prompt $" {
>  	unsupported $test
>    }
>  }
> diff --git a/gdb/testsuite/gdb.server/unittest.exp b/gdb/testsuite/gdb.server/unittest.exp
> index e947ff2c30..b0a7c6ae56 100644
> --- a/gdb/testsuite/gdb.server/unittest.exp
> +++ b/gdb/testsuite/gdb.server/unittest.exp
> @@ -38,7 +38,7 @@ gdb_expect {
>  	gdb_assert "$num_ran > 0" $test
>      }
>  
> -    -re "Selftests are not available in a non-development build.\r\n$" {
> +    -re "Selftests have been disabled for this build.\r\n$" {
>  	unsupported $test
>      }
>  
> -- 
> 2.17.1

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Add parameter to allow enabling/disabling selftests via configure
  2018-09-23  3:56   ` Sergio Durigan Junior
@ 2018-10-05 21:42     ` Sergio Durigan Junior
  0 siblings, 0 replies; 11+ messages in thread
From: Sergio Durigan Junior @ 2018-10-05 21:42 UTC (permalink / raw)
  To: GDB Patches; +Cc: Pedro Alves

On Saturday, September 22 2018, I wrote:

> On Monday, September 17 2018, I wrote:
>
>> This is a follow-up of:
>>
>>   https://sourceware.org/ml/gdb-patches/2018-08/msg00347.html
>
> Ping.

Ping^2.

>> Instead of going throttle and always enabling our selftests (even in
>> non-development builds), this patch is a bit more conservative and
>> introduces a configure option ("--enable-unit-tests") that allows the
>> user to choose whether she wants unit tests in the build or not.  Note
>> that the current behaviour is retained: if no option is provided, GDB
>> will have selftests included in a development build, and will *not*
>> have selftests included in a non-development build.
>>
>> The rationale for having this option is still the same: due to the
>> many racy testcases and random failures we see when running the GDB
>> testsuite, it is unfortunately not possible to perform a full test
>> when one is building a downstream package.  As the Fedora GDB
>> maintainer and one of the Debian GDB uploaders, I feel like this
>> situation could be improved by, at least, executing our selftests
>> after the package has been built.
>>
>> This patch introduces no regressions to our build.
>>
>> OK?
>>
>> gdb/ChangeLog:
>> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>>
>> 	* README (`configure' options): Add documentation for new
>> 	"--enable-unit-tests" option.
>> 	* configure: Regenerate.
>> 	* configure.ac: Add "--enable-unit-tests" option.
>> 	* maint.c (maintenance_selftest): Update message informing
>> 	that selftests have been disabled.
>> 	(maintenance_info_selftests): Likewise.
>>
>> gdb/gdbserver/ChangeLog:
>> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>>
>> 	* configure: Regenerate.
>> 	* configure.ac: Add "--enable-unit-tests" option.
>> 	* configure.srv: Use "$enable_unittests" instead of
>> 	"$development" when checking whether unit tests have been
>> 	enabled.
>> 	* server.c (captured_main): Update message informing that
>> 	selftests have been disabled.
>>
>> gdb/testsuite/ChangeLog:
>> 2018-09-17  Sergio Durigan Junior  <sergiodj@redhat.com>
>>
>> 	* gdb.gdb/unittest.exp: Update expected message informing that
>> 	selftests have been disabled.
>> 	* gdb.server/unittest.exp: Likewise.
>> ---
>>  gdb/README                            |  6 ++++++
>>  gdb/configure                         | 23 ++++++++++++++++++++++-
>>  gdb/configure.ac                      | 17 ++++++++++++++++-
>>  gdb/gdbserver/configure               | 23 ++++++++++++++++++++++-
>>  gdb/gdbserver/configure.ac            | 17 ++++++++++++++++-
>>  gdb/gdbserver/configure.srv           |  2 +-
>>  gdb/gdbserver/server.c                |  2 +-
>>  gdb/maint.c                           |  4 ++--
>>  gdb/testsuite/gdb.gdb/unittest.exp    |  2 +-
>>  gdb/testsuite/gdb.server/unittest.exp |  2 +-
>>  10 files changed, 88 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdb/README b/gdb/README
>> index e43887ffcd..e6fe3b183d 100644
>> --- a/gdb/README
>> +++ b/gdb/README
>> @@ -524,6 +524,12 @@ prefer; but you may abbreviate option names if you use `--'.
>>       after being built, the location of the system-wide init file will
>>       be adjusted accordingly. 
>>  
>> +`--enable-unit-tests[=yes|no]'
>> +     Enable (i.e., include) support for unit tests when compiling GDB
>> +     and GDBServer.  Note that if this option is not passed, GDB will
>> +     have selftests if it is a development build, and will *not* have
>> +     selftests if it a non-development build.
>> +
>>  `configure' accepts other options, for compatibility with configuring
>>  other GNU tools recursively; but these are the only options that affect
>>  GDB or its supporting libraries.
>> diff --git a/gdb/configure b/gdb/configure
>> index d92a256f1f..18e04c0c50 100755
>> --- a/gdb/configure
>> +++ b/gdb/configure
>> @@ -895,6 +895,7 @@ enable_sim
>>  enable_gdbserver
>>  with_babeltrace
>>  with_libbabeltrace_prefix
>> +enable_unit_tests
>>  '
>>        ac_precious_vars='build_alias
>>  host_alias
>> @@ -1559,6 +1560,8 @@ Optional Features:
>>    --enable-sim            link gdb with simulator
>>    --enable-gdbserver      automatically build gdbserver (yes/no/auto, default
>>                            is auto)
>> +  --enable-unit-tests     Enable the inclusion of unit tests when compiling
>> +                          GDB
>>  
>>  Optional Packages:
>>    --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
>> @@ -17731,7 +17734,25 @@ ac_config_links="$ac_config_links $ac_config_links_1"
>>  $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h
>>  
>>  
>> -if $development; then
>> +# Check whether we will enable the inclusion of unit tests when
>> +# compiling GDB.
>> +#
>> +# The default value of this option changes depending whether we're on
>> +# development mode (in which case it's "true") or not (in which case
>> +# it's "false").
>> +# Check whether --enable-unit-tests was given.
>> +if test "${enable_unit_tests+set}" = set; then :
>> +  enableval=$enable_unit_tests; case "${enableval}" in
>> +  yes)  enable_unittests=true  ;;
>> +  no)   enable_unittests=false ;;
>> +  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
>> +esac
>> +else
>> +  enable_unittests=$development
>> +fi
>> +
>> +
>> +if $enable_unittests; then
>>  
>>  $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
>>  
>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>> index e38604cb65..3ef43a95f4 100644
>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -2267,7 +2267,22 @@ dnl  At the moment, we just assume it's UTF-8.
>>  AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8",
>>            [Define to be a string naming the default host character set.])
>>  
>> -if $development; then
>> +# Check whether we will enable the inclusion of unit tests when
>> +# compiling GDB.
>> +#
>> +# The default value of this option changes depending whether we're on
>> +# development mode (in which case it's "true") or not (in which case
>> +# it's "false").
>> +AC_ARG_ENABLE(unit-tests,
>> +AS_HELP_STRING([--enable-unit-tests],
>> +[Enable the inclusion of unit tests when compiling GDB]),
>> +[case "${enableval}" in
>> +  yes)  enable_unittests=true  ;;
>> +  no)   enable_unittests=false ;;
>> +  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
>> +esac], [enable_unittests=$development])
>> +
>> +if $enable_unittests; then
>>    AC_DEFINE(GDB_SELF_TEST, 1,
>>              [Define if self-testing features should be enabled])
>>    CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
>> diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
>> index f5cbbaea78..9915e212d7 100755
>> --- a/gdb/gdbserver/configure
>> +++ b/gdb/gdbserver/configure
>> @@ -722,6 +722,7 @@ enable_option_checking
>>  enable_maintainer_mode
>>  enable_largefile
>>  enable_libmcheck
>> +enable_unit_tests
>>  with_ust
>>  with_ust_include
>>  with_ust_lib
>> @@ -1367,6 +1368,8 @@ Optional Features:
>>                            sometimes confusing) to the casual installer
>>    --disable-largefile     omit support for large files
>>    --enable-libmcheck      Try linking with -lmcheck if available
>> +  --enable-unit-tests     Enable the inclusion of unit tests when compiling
>> +                          GDB
>>    --enable-werror         treat compile warnings as errors
>>    --enable-build-warnings enable build-time compiler warnings if gcc is used
>>    --enable-gdb-build-warnings
>> @@ -5889,7 +5892,25 @@ fi
>>    fi
>>  
>>  
>> -if $development; then
>> +# Check whether we will enable the inclusion of unit tests when
>> +# compiling GDB.
>> +#
>> +# The default value of this option changes depending whether we're on
>> +# development mode (in which case it's "true") or not (in which case
>> +# it's "false").
>> +# Check whether --enable-unit-tests was given.
>> +if test "${enable_unit_tests+set}" = set; then :
>> +  enableval=$enable_unit_tests; case "${enableval}" in
>> +  yes)  enable_unittests=true  ;;
>> +  no)   enable_unittests=false ;;
>> +  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
>> +esac
>> +else
>> +  enable_unittests=$development
>> +fi
>> +
>> +
>> +if $enable_unittests; then
>>    srv_selftest_objs="common/selftest.o"
>>  
>>  $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
>> diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
>> index 99bc46221c..3c6ed9143f 100644
>> --- a/gdb/gdbserver/configure.ac
>> +++ b/gdb/gdbserver/configure.ac
>> @@ -54,7 +54,22 @@ else
>>  fi
>>  GDB_AC_LIBMCHECK(${libmcheck_default})
>>  
>> -if $development; then
>> +# Check whether we will enable the inclusion of unit tests when
>> +# compiling GDB.
>> +#
>> +# The default value of this option changes depending whether we're on
>> +# development mode (in which case it's "true") or not (in which case
>> +# it's "false").
>> +AC_ARG_ENABLE(unit-tests,
>> +AS_HELP_STRING([--enable-unit-tests],
>> +[Enable the inclusion of unit tests when compiling GDB]),
>> +[case "${enableval}" in
>> +  yes)  enable_unittests=true  ;;
>> +  no)   enable_unittests=false ;;
>> +  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
>> +esac], [enable_unittests=$development])
>> +
>> +if $enable_unittests; then
>>    srv_selftest_objs="common/selftest.o"
>>    AC_DEFINE(GDB_SELF_TEST, 1,
>>              [Define if self-testing features should be enabled])
>> diff --git a/gdb/gdbserver/configure.srv b/gdb/gdbserver/configure.srv
>> index 72e6a0d87f..636d830f1a 100644
>> --- a/gdb/gdbserver/configure.srv
>> +++ b/gdb/gdbserver/configure.srv
>> @@ -24,7 +24,7 @@
>>  # Default hostio_last_error implementation
>>  srv_hostio_err_objs="hostio-errno.o"
>>  
>> -if $development; then
>> +if $enable_unittests; then
>>     srv_i386_linux_regobj="i386-linux.o i386-avx-linux.o i386-avx-avx512-linux.o i386-avx-mpx-avx512-pku-linux.o i386-mpx-linux.o i386-avx-mpx-linux.o i386-mmx-linux.o linux-x86-tdesc-selftest.o"
>>     srv_amd64_linux_regobj="amd64-linux.o amd64-avx-linux.o amd64-avx-avx512-linux.o amd64-avx-mpx-avx512-pku-linux.o amd64-mpx-linux.o amd64-avx-mpx-linux.o x32-linux.o x32-avx-linux.o x32-avx-avx512-linux.o"
>>  else
>> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
>> index a491ae0257..f2f0d569ab 100644
>> --- a/gdb/gdbserver/server.c
>> +++ b/gdb/gdbserver/server.c
>> @@ -3790,7 +3790,7 @@ captured_main (int argc, char *argv[])
>>  #if GDB_SELF_TEST
>>        selftests::run_tests (selftest_filter);
>>  #else
>> -      printf (_("Selftests are not available in a non-development build.\n"));
>> +      printf (_("Selftests have been disabled for this build.\n"));
>>  #endif
>>        throw_quit ("Quit");
>>      }
>> diff --git a/gdb/maint.c b/gdb/maint.c
>> index 19db8a850b..01a80f5d73 100644
>> --- a/gdb/maint.c
>> +++ b/gdb/maint.c
>> @@ -943,7 +943,7 @@ maintenance_selftest (const char *args, int from_tty)
>>    selftests::run_tests (args);
>>  #else
>>    printf_filtered (_("\
>> -Selftests are not available in a non-development build.\n"));
>> +Selftests have been disabled for this build.\n"));
>>  #endif
>>  }
>>  
>> @@ -957,7 +957,7 @@ maintenance_info_selftests (const char *arg, int from_tty)
>>    });
>>  #else
>>    printf_filtered (_("\
>> -Selftests are not available in a non-development build.\n"));
>> +Selftests have been disabled for this build.\n"));
>>  #endif
>>  }
>>  
>> diff --git a/gdb/testsuite/gdb.gdb/unittest.exp b/gdb/testsuite/gdb.gdb/unittest.exp
>> index 1c835850b8..8e3e9a1761 100644
>> --- a/gdb/testsuite/gdb.gdb/unittest.exp
>> +++ b/gdb/testsuite/gdb.gdb/unittest.exp
>> @@ -24,7 +24,7 @@ gdb_test_multiple $test $test {
>>  	gdb_assert "$num_ran > 0" $test
>>    }
>>  
>> -  -re "Selftests are not available in a non-development build.\r\n$gdb_prompt $" {
>> +  -re "Selftests have been disabled for this build.\r\n$gdb_prompt $" {
>>  	unsupported $test
>>    }
>>  }
>> diff --git a/gdb/testsuite/gdb.server/unittest.exp b/gdb/testsuite/gdb.server/unittest.exp
>> index e947ff2c30..b0a7c6ae56 100644
>> --- a/gdb/testsuite/gdb.server/unittest.exp
>> +++ b/gdb/testsuite/gdb.server/unittest.exp
>> @@ -38,7 +38,7 @@ gdb_expect {
>>  	gdb_assert "$num_ran > 0" $test
>>      }
>>  
>> -    -re "Selftests are not available in a non-development build.\r\n$" {
>> +    -re "Selftests have been disabled for this build.\r\n$" {
>>  	unsupported $test
>>      }
>>  
>> -- 
>> 2.17.1
>
> -- 
> Sergio
> GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
> Please send encrypted e-mail if possible
> http://sergiodj.net/

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Add parameter to allow enabling/disabling selftests via configure
  2018-09-17 20:22 ` [PATCH] Add parameter to allow enabling/disabling selftests via configure Sergio Durigan Junior
  2018-09-23  3:56   ` Sergio Durigan Junior
@ 2018-10-06  1:19   ` Simon Marchi
  2018-10-10 20:31     ` Sergio Durigan Junior
  1 sibling, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2018-10-06  1:19 UTC (permalink / raw)
  To: Sergio Durigan Junior, GDB Patches; +Cc: Pedro Alves

On 2018-09-17 4:22 p.m., Sergio Durigan Junior wrote:
> This is a follow-up of:
> 
>   https://sourceware.org/ml/gdb-patches/2018-08/msg00347.html
> 
> Instead of going throttle and always enabling our selftests (even in
> non-development builds), this patch is a bit more conservative and
> introduces a configure option ("--enable-unit-tests") that allows the
> user to choose whether she wants unit tests in the build or not.  Note
> that the current behaviour is retained: if no option is provided, GDB
> will have selftests included in a development build, and will *not*
> have selftests included in a non-development build.
> 
> The rationale for having this option is still the same: due to the
> many racy testcases and random failures we see when running the GDB
> testsuite, it is unfortunately not possible to perform a full test
> when one is building a downstream package.  As the Fedora GDB
> maintainer and one of the Debian GDB uploaders, I feel like this
> situation could be improved by, at least, executing our selftests
> after the package has been built.
> 
> This patch introduces no regressions to our build.

Hi Sergio,

I think having an configure switch with that default value makes sense.

I have two suggestions:

- Mention the switch explicitly in the error message (--{enable,disable}-unit-tests)
  so the user knows exactly what you are talking about.
- Share the code between gdb's and gdbserver's configure script

> +`--enable-unit-tests[=yes|no]'
> +     Enable (i.e., include) support for unit tests when compiling GDB
> +     and GDBServer.  Note that if this option is not passed, GDB will
> +     have selftests if it is a development build, and will *not* have
> +     selftests if it a non-development build.

Missing a word: "if it is a".

Here's a patch that goes on top of yours that does what I suggest (I had to try it
before suggesting it to make sure I don't ask for the impossible, so I might as well
share it).  With this (especially the code-sharing part), the patch LGTM (the
ChangeLog entry would need to be updated to account for the new changes).

Thanks,

Simon


From 8721a296c564b572c03e20031b33151fc3e9b7a8 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Fri, 5 Oct 2018 21:04:38 -0400
Subject: [PATCH] Suggestions

---
 gdb/README                 |  2 +-
 gdb/acinclude.m4           |  3 +++
 gdb/configure              |  6 ++++-
 gdb/configure.ac           | 21 ++----------------
 gdb/gdbserver/acinclude.m4 |  3 +++
 gdb/gdbserver/configure    |  8 +++++--
 gdb/gdbserver/configure.ac | 21 ++----------------
 gdb/selftest.m4            | 45 ++++++++++++++++++++++++++++++++++++++
 8 files changed, 67 insertions(+), 42 deletions(-)
 create mode 100644 gdb/selftest.m4

diff --git a/gdb/README b/gdb/README
index 4639d76c4e83..8a91aab2a4c6 100644
--- a/gdb/README
+++ b/gdb/README
@@ -549,7 +549,7 @@ more obscure GDB `configure' options are not listed here.
      Enable (i.e., include) support for unit tests when compiling GDB
      and GDBServer.  Note that if this option is not passed, GDB will
      have selftests if it is a development build, and will *not* have
-     selftests if it a non-development build.
+     selftests if it is a non-development build.

 `configure' accepts other options, for compatibility with configuring
 other GNU tools recursively.
diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
index 52ba3f9ed6e1..d21dd76fab9c 100644
--- a/gdb/acinclude.m4
+++ b/gdb/acinclude.m4
@@ -18,6 +18,9 @@ sinclude(warning.m4)
 # AM_GDB_UBSAN
 sinclude(sanitize.m4)

+# This gets GDB_AC_SELFTEST.
+sinclude(selftest.m4)
+
 dnl gdb/configure.in uses BFD_NEED_DECLARATION, so get its definition.
 sinclude(../bfd/bfd.m4)

diff --git a/gdb/configure b/gdb/configure
index 7a9295459692..47319bf36a3e 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -17841,6 +17841,7 @@ ac_config_links="$ac_config_links $ac_config_links_1"
 $as_echo "#define GDB_DEFAULT_HOST_CHARSET \"UTF-8\"" >>confdefs.h


+
 # Check whether we will enable the inclusion of unit tests when
 # compiling GDB.
 #
@@ -17852,7 +17853,7 @@ if test "${enable_unit_tests+set}" = set; then :
   enableval=$enable_unit_tests; case "${enableval}" in
   yes)  enable_unittests=true  ;;
   no)   enable_unittests=false ;;
-  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
+  *)    as_fn_error $? "bad value ${enableval} for --{enable,disable}-unit-tests option" "$LINENO" 5 ;;
 esac
 else
   enable_unittests=$development
@@ -17863,11 +17864,14 @@ if $enable_unittests; then

 $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h

+
   CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
   CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) common/selftest.c selftest-arch.c"
+
 fi


+
   gdb_ac_transform=`echo "$program_transform_name" | sed -e 's/\\$\\$/\\$/g'`
   GDB_TRANSFORM_NAME=`echo gdb | sed -e "$gdb_ac_transform"`
   if test "x$GDB_TRANSFORM_NAME" = x; then
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 987bf01df86c..b2343a90e5fa 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -2247,27 +2247,10 @@ dnl  At the moment, we just assume it's UTF-8.
 AC_DEFINE(GDB_DEFAULT_HOST_CHARSET, "UTF-8",
           [Define to be a string naming the default host character set.])

-# Check whether we will enable the inclusion of unit tests when
-# compiling GDB.
-#
-# The default value of this option changes depending whether we're on
-# development mode (in which case it's "true") or not (in which case
-# it's "false").
-AC_ARG_ENABLE(unit-tests,
-AS_HELP_STRING([--enable-unit-tests],
-[Enable the inclusion of unit tests when compiling GDB]),
-[case "${enableval}" in
-  yes)  enable_unittests=true  ;;
-  no)   enable_unittests=false ;;
-  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
-esac], [enable_unittests=$development])
-
-if $enable_unittests; then
-  AC_DEFINE(GDB_SELF_TEST, 1,
-            [Define if self-testing features should be enabled])
+GDB_AC_SELFTEST([
   CONFIG_OBS="$CONFIG_OBS \$(SUBDIR_UNITTESTS_OBS) common/selftest.o selftest-arch.o"
   CONFIG_SRCS="$CONFIG_SRCS \$(SUBDIR_UNITTESTS_SRCS) common/selftest.c selftest-arch.c"
-fi
+])

 GDB_AC_TRANSFORM([gdb], [GDB_TRANSFORM_NAME])
 GDB_AC_TRANSFORM([gcore], [GCORE_TRANSFORM_NAME])
diff --git a/gdb/gdbserver/acinclude.m4 b/gdb/gdbserver/acinclude.m4
index c75d783f57bb..fc3e344a611a 100644
--- a/gdb/gdbserver/acinclude.m4
+++ b/gdb/gdbserver/acinclude.m4
@@ -31,6 +31,9 @@ m4_include(../ptrace.m4)

 m4_include(../ax_cxx_compile_stdcxx.m4)

+dnl For GDB_AC_SELFTEST.
+m4_include(../selftest.m4)
+
 dnl Check for existence of a type $1 in libthread_db.h
 dnl Based on BFD_HAVE_SYS_PROCFS_TYPE in bfd/bfd.m4.

diff --git a/gdb/gdbserver/configure b/gdb/gdbserver/configure
index 3df3111259fd..1ddbd6b27e0a 100755
--- a/gdb/gdbserver/configure
+++ b/gdb/gdbserver/configure
@@ -5892,6 +5892,7 @@ fi
   fi


+
 # Check whether we will enable the inclusion of unit tests when
 # compiling GDB.
 #
@@ -5903,7 +5904,7 @@ if test "${enable_unit_tests+set}" = set; then :
   enableval=$enable_unit_tests; case "${enableval}" in
   yes)  enable_unittests=true  ;;
   no)   enable_unittests=false ;;
-  *)    as_fn_error $? "bad value ${enableval} for enable unit tests option" "$LINENO" 5 ;;
+  *)    as_fn_error $? "bad value ${enableval} for --{enable,disable}-unit-tests option" "$LINENO" 5 ;;
 esac
 else
   enable_unittests=$development
@@ -5911,12 +5912,15 @@ fi


 if $enable_unittests; then
-  srv_selftest_objs="common/selftest.o"

 $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h

+
+  srv_selftest_objs="common/selftest.o"
+
 fi

+
  case ${build_alias} in
   "") build_noncanonical=${build} ;;
   *) build_noncanonical=${build_alias} ;;
diff --git a/gdb/gdbserver/configure.ac b/gdb/gdbserver/configure.ac
index f1e7086662af..f1dfa4546cce 100644
--- a/gdb/gdbserver/configure.ac
+++ b/gdb/gdbserver/configure.ac
@@ -54,26 +54,9 @@ else
 fi
 GDB_AC_LIBMCHECK(${libmcheck_default})

-# Check whether we will enable the inclusion of unit tests when
-# compiling GDB.
-#
-# The default value of this option changes depending whether we're on
-# development mode (in which case it's "true") or not (in which case
-# it's "false").
-AC_ARG_ENABLE(unit-tests,
-AS_HELP_STRING([--enable-unit-tests],
-[Enable the inclusion of unit tests when compiling GDB]),
-[case "${enableval}" in
-  yes)  enable_unittests=true  ;;
-  no)   enable_unittests=false ;;
-  *)    AC_MSG_ERROR(bad value ${enableval} for enable unit tests option) ;;
-esac], [enable_unittests=$development])
-
-if $enable_unittests; then
+GDB_AC_SELFTEST([
   srv_selftest_objs="common/selftest.o"
-  AC_DEFINE(GDB_SELF_TEST, 1,
-            [Define if self-testing features should be enabled])
-fi
+])

 ACX_NONCANONICAL_TARGET
 ACX_NONCANONICAL_HOST
diff --git a/gdb/selftest.m4 b/gdb/selftest.m4
new file mode 100644
index 000000000000..acf4050bde85
--- /dev/null
+++ b/gdb/selftest.m4
@@ -0,0 +1,45 @@
+dnl Copyright (C) 2018 Free Software Foundation, Inc.
+dnl
+dnl This file is part of GDB.
+dnl
+dnl This program is free software; you can redistribute it and/or modify
+dnl it under the terms of the GNU General Public License as published by
+dnl the Free Software Foundation; either version 3 of the License, or
+dnl (at your option) any later version.
+dnl
+dnl This program is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+dnl GNU General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU General Public License
+dnl along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+dnl GDB_AC_SELFTEST(ACTION-IF-ENABLED)
+dnl
+dnl Enable the unit/self tests if needed.  If they are enabled, AC_DEFINE
+dnl the GDB_SELF_TEST macro, and execute ACTION-IF-ENABLED.
+
+AC_DEFUN([GDB_AC_SELFTEST],[
+# Check whether we will enable the inclusion of unit tests when
+# compiling GDB.
+#
+# The default value of this option changes depending whether we're on
+# development mode (in which case it's "true") or not (in which case
+# it's "false").
+AC_ARG_ENABLE(unit-tests,
+AS_HELP_STRING([--enable-unit-tests],
+[Enable the inclusion of unit tests when compiling GDB]),
+[case "${enableval}" in
+  yes)  enable_unittests=true  ;;
+  no)   enable_unittests=false ;;
+  *)    AC_MSG_ERROR(
+[bad value ${enableval} for --{enable,disable}-unit-tests option]) ;;
+esac], [enable_unittests=$development])
+
+if $enable_unittests; then
+  AC_DEFINE(GDB_SELF_TEST, 1,
+            [Define if self-testing features should be enabled])
+  $1
+fi
+])
-- 
2.19.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Add parameter to allow enabling/disabling selftests via configure
  2018-10-06  1:19   ` Simon Marchi
@ 2018-10-10 20:31     ` Sergio Durigan Junior
  0 siblings, 0 replies; 11+ messages in thread
From: Sergio Durigan Junior @ 2018-10-10 20:31 UTC (permalink / raw)
  To: Simon Marchi; +Cc: GDB Patches, Pedro Alves

On Friday, October 05 2018, Simon Marchi wrote:

> On 2018-09-17 4:22 p.m., Sergio Durigan Junior wrote:
>> This is a follow-up of:
>> 
>>   https://sourceware.org/ml/gdb-patches/2018-08/msg00347.html
>> 
>> Instead of going throttle and always enabling our selftests (even in
>> non-development builds), this patch is a bit more conservative and
>> introduces a configure option ("--enable-unit-tests") that allows the
>> user to choose whether she wants unit tests in the build or not.  Note
>> that the current behaviour is retained: if no option is provided, GDB
>> will have selftests included in a development build, and will *not*
>> have selftests included in a non-development build.
>> 
>> The rationale for having this option is still the same: due to the
>> many racy testcases and random failures we see when running the GDB
>> testsuite, it is unfortunately not possible to perform a full test
>> when one is building a downstream package.  As the Fedora GDB
>> maintainer and one of the Debian GDB uploaders, I feel like this
>> situation could be improved by, at least, executing our selftests
>> after the package has been built.
>> 
>> This patch introduces no regressions to our build.
>
> Hi Sergio,

Hi Simon,

Thanks for the review.

> I think having an configure switch with that default value makes sense.
>
> I have two suggestions:
>
> - Mention the switch explicitly in the error message (--{enable,disable}-unit-tests)
>   so the user knows exactly what you are talking about.
> - Share the code between gdb's and gdbserver's configure script
>
>> +`--enable-unit-tests[=yes|no]'
>> +     Enable (i.e., include) support for unit tests when compiling GDB
>> +     and GDBServer.  Note that if this option is not passed, GDB will
>> +     have selftests if it is a development build, and will *not* have
>> +     selftests if it a non-development build.
>
> Missing a word: "if it is a".
>
> Here's a patch that goes on top of yours that does what I suggest (I had to try it
> before suggesting it to make sure I don't ask for the impossible, so I might as well
> share it).  With this (especially the code-sharing part), the patch LGTM (the
> ChangeLog entry would need to be updated to account for the new changes).

Fair enough.  I applied your patch, updated the ChangeLog, and pushed
it.

8ecfd7bd4acd69213c06fac6de9af38299123547

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-10-10 20:31 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-14  5:42 [RFC/PATCH] Don't disable selftests in a non-development build Sergio Durigan Junior
2018-08-14 16:28 ` Sergio Durigan Junior
2018-08-14 18:08 ` Pedro Alves
2018-08-14 18:42   ` Philippe Waroquiers
2018-08-14 20:17     ` Sergio Durigan Junior
2018-08-14 20:12   ` Sergio Durigan Junior
2018-09-17 20:22 ` [PATCH] Add parameter to allow enabling/disabling selftests via configure Sergio Durigan Junior
2018-09-23  3:56   ` Sergio Durigan Junior
2018-10-05 21:42     ` Sergio Durigan Junior
2018-10-06  1:19   ` Simon Marchi
2018-10-10 20:31     ` Sergio Durigan Junior

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).