[PATCH] Introduce selftest::locate_file

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2016-09-19 at 11:31 -0600, Jeff Law wrote:
> On 09/16/2016 03:19 PM, David Malcolm wrote:
> > 
> > > When possible I don't think we want the tests to be target
> > > specific.
> > > Hmm, I'm probably about to argue for Bernd's work...  The 71779
> > > testcase
> > > is a great example -- it uses high/lo_sum.  Not all targets
> > > support
> > > that
> > > -- as long as we don't try to recognize those insns we're likely
> > > OK,
> > > but
> > > that seems fragile long term.  Having an idealized target means
> > > we
> > > can
> > > ignore much of these issues.
> > 
> > An alternative would be to pick a specific target for each test.
> It's an alternative, but not one I particularly like since those
> tests
> won't be consistently run.  With an abstracted target like Bernd
> suggests we ought to be able to make most tests work with the
> abstracted
> target and minimize the number of truely target specific tests.
> 
> 
> > > So I'm real curious, what happens if you run this RTL selftest
> > > under
> > > valgrind?  I have the sneaking suspicion that we'll start doing
> > > some
> > > uninitialized memory reads.
> > 
> > I'm seeing various leaks (an htab within linemap_init for all of
> > the
> > line_table fixtures), but no uninitialized reads.
> Wow.  I must say I'm surprised.  Good news though.
> 
> 
> > >   +
> > > > +  /* Dump taken from comment 2 of PR 71779, of
> > > > +     "...the relevant memory access coming out of expand"
> > > > +     with basic block IDs added, and prev/next insns set to
> > > > +     0 at ends.  */
> > > > +  const char *input_dump
> > > > +    = (";; MEM[(struct isl_obj *)&obj1] =
> > > > &isl_obj_map_vtable;\n"
> > > > +       "(insn 1045 0 1046 2 (set (reg:SI 480)\n"
> > > > +       "        (high:SI (symbol_ref:SI
> > > > (\"isl_obj_map_vtable\")
> > > > [flags 0xc0] <var_decl 0x7fa0363ea240 isl_obj_map_vtable>)))
> > > > y.c:12702 -1\n"
> > > > +       "     (nil))\n"
> > > > +       "(insn 1046 1045 1047 2 (set (reg/f:SI 479)\n"
> > > > +       "        (lo_sum:SI (reg:SI 480)\n"
> > > > +       "            (symbol_ref:SI (\"isl_obj_map_vtable\")
> > > > [flags
> > > > 0xc0] <var_decl 0x7fa0363ea240 isl_obj_map_vtable>))) y.c:12702
> > > > -1\n"
> > > > +       "     (expr_list:REG_EQUAL (symbol_ref:SI
> > > > (\"isl_obj_map_vtable\") [flags 0xc0] <var_decl 0x7fa0363ea240
> > > > isl_obj_map_vtable>)\n"
> > > > +       "        (nil)))\n"
> > > > +       "(insn 1047 1046 1048 2 (set (reg:DI 481)\n"
> > > > +       "        (subreg:DI (reg/f:SI 479) 0)) y.c:12702 -1\n"
> > > > +       "     (nil))\n"
> > > > +       "(insn 1048 1047 1049 2 (set (zero_extract:DI (reg/v:DI
> > > > 191
> > > > [ obj1D.17368 ])\n"
> > > > +       "            (const_int 32 [0x20])\n"
> > > > +       "            (const_int 0 [0]))\n"
> > > > +       "        (reg:DI 481)) y.c:12702 -1\n"
> > > > +       "     (nil))\n"
> > > So looking at this just makes my head hurt.  Escaping, lots of
> > > quotes,
> > > unnecessary things in the dump, etc.  The question I would have
> > > is
> > > why
> > > not have a file with the dump and read the file?
> > 
> > (nods)
> > 
> > Seems like I need to add a mechanism for telling the selftests
> > which
> > directory to load the tests relative to.
> What about putting them inside the appropriate gcc.target
> directories?
> We could have one for the "generic" target assuming we build
> something
> like that, the others could live in their target specific directory.
> 
> 
> jeff

Having selftests that read RTL dumps load them from files rather than
embedding them as strings in the source requires a way to locate the
relevant files.

This patch adds a selftest::locate_file function for locating such
files, relative to "$(SRCDIR)/gcc/testsuite/selftests".  This is
done via a new argument to -fself-test, which supplies the current
value of "$(SRCDIR)/gcc" to cc1.

I chose "$(SRCDIR)/gcc/testsuite/selftests", so as to be below
gcc/testsuite, but not below any of the existing DejaGnu subdirectories,
to avoid selftest-specific files from being picked up by .exp globbing
patterns.  We could add target-specific directories below that dir if
necessary.

I've rewritten the rest of the patch kit to use this to load from .rtl
dump files within that directory, rather than embedding the dumps as
string literals in the C source.

The patch also exposes a selftests::path_to_src_gcc, which could be
used by a selftest to e.g. load a DejaGnu file, so that if need be
we could share .rtl input files between both -fself-test tests and
DejaGnu-based tests for the .rtl frontend.

(Depends on the approved-when-needed
  "[PATCH 2/9] Add selftest::read_file"
    https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00476.html ).

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk once the dependencies are in?

gcc/ChangeLog:
	* Makefile.in (s-selftest) Add $(srcdir) as an argument of
	-fself-test.
	(selftest-gdb): Likewise.
	(selftest-valgrind): Likewise.
	* common.opt (fself-test): Rename to...
	(fself-test=): ...this, documenting the meaning of the argument.
	* selftest-run-tests.c: Include "options.h".
	(selftest::run_tests): Initialize selftest::path_to_src_gcc from
	flag_self_test.
	* selftest.c (selftest::path_to_src_gcc): New global.
	(selftest::locate_file): New function.
	(selftest::test_locate_file): New function.
	(selftest::selftest_c_tests): Call test_locate_file.
	* selftest.h (selftest::locate_file): New decl.
	(selftest::path_to_src_gcc): New decl.

gcc/testsuite/ChangeLog:
	* gcc.dg/cpp/pr71591.c: Add a fake value for the argument of
	-fself-test.
	* selftests/example.txt: New file.
---
 gcc/Makefile.in                     |  7 ++++---
 gcc/common.opt                      |  6 +++---
 gcc/selftest-run-tests.c            |  7 +++++++
 gcc/selftest.c                      | 28 ++++++++++++++++++++++++++++
 gcc/selftest.h                      | 10 ++++++++++
 gcc/testsuite/gcc.dg/cpp/pr71591.c  |  2 +-
 gcc/testsuite/selftests/example.txt |  1 +
 7 files changed, 54 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/selftests/example.txt

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 332c85e..94146ae 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1876,18 +1876,19 @@ rest.cross: specs
 .PHONY: selftest
 selftest: s-selftest
 s-selftest: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
-	$(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test
+	$(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test=$(srcdir)
 	$(STAMP) $@
 
 # Convenience method for running selftests under gdb:
 .PHONY: selftest-gdb
 selftest-gdb: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
-	$(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test -wrapper gdb,--args
+	$(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test=$(srcdir) \
+	  -wrapper gdb,--args
 
 # Convenience method for running selftests under valgrind:
 .PHONY: selftest-valgrind
 selftest-valgrind: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
-	$(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test \
+	$(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test=$(srcdir) \
 	  -wrapper valgrind,--leak-check=full
 
 # Recompile all the language-independent object files.
diff --git a/gcc/common.opt b/gcc/common.opt
index fa1c036..603fade 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2098,9 +2098,9 @@ fselective-scheduling2
 Common Report Var(flag_selective_scheduling2) Optimization
 Run selective scheduling after reload.
 
-fself-test
-Common Undocumented Var(flag_self_test)
-Run self-tests.
+fself-test=
+Common Undocumented Joined Var(flag_self_test)
+Run self-tests, using the given path to locate test files.
 
 fsel-sched-pipelining
 Common Report Var(flag_sel_sched_pipelining) Init(0) Optimization
diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
index 54a9b0f..ecc3d71 100644
--- a/gcc/selftest-run-tests.c
+++ b/gcc/selftest-run-tests.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "selftest.h"
 #include "tree.h"
 #include "langhooks.h"
+#include "options.h"
 
 /* This function needed to be split out from selftest.c as it references
    tests from the whole source tree, and so is within
@@ -37,6 +38,12 @@ along with GCC; see the file COPYING3.  If not see
 void
 selftest::run_tests ()
 {
+  /* Makefile.in has -fself-test=$(srcdir), so that flag_self_test
+     contains the path to the "gcc" subdirectory of the source tree
+     (without a trailing slash).  Copy it up to path_to_src_gcc, to
+     avoid selftest.c depending on option-handling.  */
+  path_to_src_gcc = flag_self_test;
+
   long start_time = get_run_time ();
 
   /* Run all the tests, in hand-coded order of (approximate) dependencies:
diff --git a/gcc/selftest.c b/gcc/selftest.c
index cf7031f..2a481ad 100644
--- a/gcc/selftest.c
+++ b/gcc/selftest.c
@@ -198,6 +198,21 @@ read_file (const location &loc, const char *path)
   return result;
 }
 
+/* The path of SRCDIR/gcc.  */
+
+const char *path_to_src_gcc = NULL;
+
+/* Convert a path relative to SRCDIR/gcc/testsuite/selftests
+   to a real path (either absolute, or relative to pwd).
+   The result should be freed by the caller.  */
+
+char *
+locate_file (const char *name)
+{
+  ASSERT_NE (NULL, path_to_src_gcc);
+  return concat (path_to_src_gcc, "/testsuite/selftests/", name, NULL);
+}
+
 /* Selftests for the selftest system itself.  */
 
 /* Sanity-check the ASSERT_ macros with various passing cases.  */
@@ -240,6 +255,18 @@ test_read_file ()
   free (buf);
 }
 
+/* Verify locate_file (and read_file).  */
+
+static void
+test_locate_file ()
+{
+  char *path = locate_file ("example.txt");
+  char *buf = read_file (SELFTEST_LOCATION, path);
+  ASSERT_STREQ ("example of a selftest file\n", buf);
+  free (buf);
+  free (path);
+}
+
 /* Run all of the selftests within this file.  */
 
 void
@@ -248,6 +275,7 @@ selftest_c_tests ()
   test_assertions ();
   test_named_temp_file ();
   test_read_file ();
+  test_locate_file ();
 }
 
 } // namespace selftest
diff --git a/gcc/selftest.h b/gcc/selftest.h
index 86ad14c..8b9c007 100644
--- a/gcc/selftest.h
+++ b/gcc/selftest.h
@@ -187,6 +187,16 @@ class temp_override
 
 extern void forcibly_ggc_collect ();
 
+/* Convert a path relative to SRCDIR/gcc/testsuite/selftests
+   to a real path (either absolute, or relative to pwd).
+   The result should be freed by the caller.  */
+
+extern char *locate_file (const char *path);
+
+/* The path of SRCDIR/gcc.  */
+
+extern const char *path_to_src_gcc;
+
 /* Declarations for specific families of tests (by source file), in
    alphabetical order.  */
 extern void bitmap_c_tests ();
diff --git a/gcc/testsuite/gcc.dg/cpp/pr71591.c b/gcc/testsuite/gcc.dg/cpp/pr71591.c
index e92cb52..0e3d7b1 100644
--- a/gcc/testsuite/gcc.dg/cpp/pr71591.c
+++ b/gcc/testsuite/gcc.dg/cpp/pr71591.c
@@ -1,5 +1,5 @@
 /* PR rtl-optimization/71591 */
 /* { dg-do preprocess } */
-/* { dg-options "-fself-test" } */
+/* { dg-options "-fself-test=fake-value" } */
 
 /* { dg-message "self-tests incompatible with -E" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/selftests/example.txt b/gcc/testsuite/selftests/example.txt
new file mode 100644
index 0000000..fbfaa33
--- /dev/null
+++ b/gcc/testsuite/selftests/example.txt
@@ -0,0 +1 @@
+example of a selftest file
-- 
1.8.5.3