Ping: https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02459.html
On Tue, 2017-11-28 at 14:31 -0500, David Malcolm wrote:
> This patch adds selftest coverage for the fix for PR c/82050
> (r255214).
>
> The selftest iterates over various "interesting" column and line-
> width
> values to try to shake out bugs in the fix-it printing routines, a
> kind
> of "torture" selftest.
>
> Unfortunately this selftest is noticably slower than the other
> selftests;
> adding it to diagnostic-show-locus.c led to:
> -fself-test: 40218 pass(es) in 0.172000 seconds
> slowing down to:
> -fself-test: 97315 pass(es) in 6.109000 seconds
> for an unoptimized build (e.g. when hacking with --disable-
> bootstrap).
>
> Given that this affects the compile-edit-test cycle of the "gcc"
> subdirectory, this felt like an unacceptable amount of overhead to
> add.
>
> I attempted to optimize the test by reducing the amount of coverage,
> but
> the test seems useful, and there seems to be a valid role for
> "torture"
> selftests.
>
> Hence this patch adds a:
> gcc.dg/plugin/expensive_selftests_plugin.c
> with the responsibility for running "expensive" selftests, and adds
> the
> expensive test there. The patch moves a small amount of code from
> selftest::run_tests into a helper class so that the plugin can print
> a useful summary line (to reassure us that the tests are actually
> being
> run).
>
> With that, the compile-edit-test cycle of the "gcc" subdir is
> unaffected;
> the plugin takes:
> expensive_selftests_plugin: 26641 pass(es) in 3.127000 seconds
> which seems reasonable within the much longer time taken by "make
> check"
> (I optimized some of the overhead away, hence the reduction from 6
> seconds
> above down to 3 seconds).
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/ChangeLog:
> PR c/82050
> * selftest-run-tests.c (selftest::run_tests): Move start/finish
> code
> to...
> * selftest.c (selftest::test_runner::test_runner): New ctor.
> (selftest::test_runner::~test_runner): New dtor.
> * selftest.h (class selftest::test_runner): New class.
>
> gcc/testsuite/ChangeLog:
> PR c/82050
> * gcc.dg/plugin/expensive-selftests-1.c: New file.
> * gcc.dg/plugin/expensive_selftests_plugin.c: New file.
> * gcc.dg/plugin/plugin.exp (plugin_test_list): Add the above.
> ---
> gcc/selftest-run-tests.c | 11 +-
> gcc/selftest.c | 22 +++
> gcc/selftest.h | 14 ++
> .../gcc.dg/plugin/expensive-selftests-1.c | 3 +
> .../gcc.dg/plugin/expensive_selftests_plugin.c | 175
> +++++++++++++++++++++
> gcc/testsuite/gcc.dg/plugin/plugin.exp | 2 +
> 6 files changed, 218 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/plugin/expensive-selftests-
> 1.c
> create mode 100644
> gcc/testsuite/gcc.dg/plugin/expensive_selftests_plugin.c
>
> diff --git a/gcc/selftest-run-tests.c b/gcc/selftest-run-tests.c
> index 6030d3b..f539c66 100644
> --- a/gcc/selftest-run-tests.c
> +++ b/gcc/selftest-run-tests.c
> @@ -46,7 +46,7 @@ selftest::run_tests ()
> option-handling. */
> path_to_selftest_files = flag_self_test;
>
> - long start_time = get_run_time ();
> + test_runner r ("-fself-test");
>
> /* Run all the tests, in hand-coded order of (approximate)
> dependencies:
> run the tests for lowest-level code first. */
> @@ -106,14 +106,7 @@ selftest::run_tests ()
> failed to be finalized can be detected by valgrind. */
> forcibly_ggc_collect ();
>
> - /* Finished running tests. */
> - long finish_time = get_run_time ();
> - long elapsed_time = finish_time - start_time;
> -
> - fprintf (stderr,
> - "-fself-test: %i pass(es) in %ld.%06ld seconds\n",
> - num_passes,
> - elapsed_time / 1000000, elapsed_time % 1000000);
> + /* Finished running tests; the test_runner dtor will print a
> summary. */
> }
>
> #endif /* #if CHECKING_P */
> diff --git a/gcc/selftest.c b/gcc/selftest.c
> index b41b9f5..ca84bfa 100644
> --- a/gcc/selftest.c
> +++ b/gcc/selftest.c
> @@ -213,6 +213,28 @@ locate_file (const char *name)
> return concat (path_to_selftest_files, "/", name, NULL);
> }
>
> +/* selftest::test_runner's ctor. */
> +
> +test_runner::test_runner (const char *name)
> +: m_name (name),
> + m_start_time (get_run_time ())
> +{
> +}
> +
> +/* selftest::test_runner's dtor. Print a summary line to
> stderr. */
> +
> +test_runner::~test_runner ()
> +{
> + /* Finished running tests. */
> + long finish_time = get_run_time ();
> + long elapsed_time = finish_time - m_start_time;
> +
> + fprintf (stderr,
> + "%s: %i pass(es) in %ld.%06ld seconds\n",
> + m_name, num_passes,
> + elapsed_time / 1000000, elapsed_time % 1000000);
> +}
> +
> /* Selftests for libiberty. */
>
> /* Verify that xstrndup generates EXPECTED when called on SRC and
> N. */
> diff --git a/gcc/selftest.h b/gcc/selftest.h
> index cdad939..f282055 100644
> --- a/gcc/selftest.h
> +++ b/gcc/selftest.h
> @@ -168,6 +168,20 @@ extern char *locate_file (const char *path);
>
> extern const char *path_to_selftest_files;
>
> +/* selftest::test_runner is an implementation detail of
> selftest::run_tests,
> + exposed here to allow plugins to run their own suites of
> tests. */
> +
> +class test_runner
> +{
> + public:
> + test_runner (const char *name);
> + ~test_runner ();
> +
> + private:
> + const char *m_name;
> + long m_start_time;
> +};
> +
> /* Declarations for specific families of tests (by source file), in
> alphabetical order. */
> extern void bitmap_c_tests ();
> diff --git a/gcc/testsuite/gcc.dg/plugin/expensive-selftests-1.c
> b/gcc/testsuite/gcc.dg/plugin/expensive-selftests-1.c
> new file mode 100644
> index 0000000..e464117
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/plugin/expensive-selftests-1.c
> @@ -0,0 +1,3 @@
> +int not_empty;
> +
> +/* { dg-regexp "expensive_selftests_plugin: .* pass\\(es\\) in .*
> seconds" } */
> diff --git a/gcc/testsuite/gcc.dg/plugin/expensive_selftests_plugin.c
> b/gcc/testsuite/gcc.dg/plugin/expensive_selftests_plugin.c
> new file mode 100644
> index 0000000..9470764
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/plugin/expensive_selftests_plugin.c
> @@ -0,0 +1,175 @@
> +/* Run expensive selftests. */
> +/* { dg-options "-O" } */
> +
> +#include "gcc-plugin.h"
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "diagnostic.h"
> +#include "edit-context.h"
> +#include "selftest.h"
> +#include "selftest-diagnostic.h"
> +
> +int plugin_is_GPL_compatible;
> +
> +#if CHECKING_P
> +
> +namespace selftest {
> +
> +/* Subroutine of test_fixit_on_very_long_line.
> + Verify that LOC has the EXPECTED_COLUMN, apart from the various
> + cases where it can't. */
> +
> +static void
> +verify_column (location_t loc,
> + const line_map_ordinary *ord_map,
> + int line_width,
> + int expected_column)
> +{
> + ASSERT_TRUE (/* Normal case. */
> + LOCATION_COLUMN (loc) == expected_column
> + /* ord_map can't store columns e.g. due to
> + max_column_hint being too high. */
> + || ord_map->m_column_and_range_bits == 0
> + /* Running out of location_t values. */
> + || loc > LINE_MAP_MAX_LOCATION_WITH_COLS
> + /* column exceeds LINE_MAP_MAX_COLUMN_NUMBER. */
> + || expected_column > (int)LINE_MAP_MAX_COLUMN_NUMBER
> + /* column exceeds max_column_hint for ord_map. */
> + || expected_column > line_width);
> +}
> +
> +/* Subroutine of test_fixit_on_very_long_line.
> + Run various things for RICHLOC, but don't check; we just want
> them
> + to survive. */
> +
> +static void
> +test_richloc (rich_location *richloc)
> +{
> + /* Run the diagnostic and fix-it printing code. */
> + test_diagnostic_context dc;
> + diagnostic_show_locus (&dc, richloc, DK_ERROR);
> +
> + /* Generate a diff. */
> + edit_context ec;
> + ec.add_fixits (richloc);
> + char *diff = ec.generate_diff (true);
> + free (diff);
> +}
> +
> +/* Verify that the fix-it-printing code can cope with very long
> lines
> + (PR c/82050). */
> +
> +static void
> +test_fixit_on_very_long_line (const line_table_case &case_)
> +{
> + /* Various interesting column/line-width values, to try to tickle
> + out bugs. */
> + const int VERY_LONG_LINE = 8192;
> + const int columns[] = {0,
> + 1,
> + 80,
> + LINE_MAP_MAX_COLUMN_NUMBER - 2,
> + LINE_MAP_MAX_COLUMN_NUMBER - 1,
> + LINE_MAP_MAX_COLUMN_NUMBER,
> + LINE_MAP_MAX_COLUMN_NUMBER + 1,
> + LINE_MAP_MAX_COLUMN_NUMBER + 2,
> + VERY_LONG_LINE,
> + VERY_LONG_LINE + 5};
> + for (unsigned int width_idx = 0; width_idx < ARRAY_SIZE (columns);
> + width_idx++)
> + {
> + int line_width = columns[width_idx];
> +
> + /* Create a source file with a very long line. */
> + named_temp_file tmp (".c");
> + FILE *f = fopen (tmp.get_filename (), "w");
> + for (int i = 0; i < line_width; i++)
> + fputc (' ', f);
> + fputc ('\n', f);
> + fclose (f);
> +
> + line_table_test ltt (case_);
> + const line_map_ordinary *ord_map = linemap_check_ordinary
> + (linemap_add (line_table, LC_ENTER, false, tmp.get_filename
> (), 0));
> + linemap_line_start (line_table, 1, line_width);
> +
> + for (unsigned int start_idx = 0; start_idx < ARRAY_SIZE
> (columns);
> + start_idx++)
> + {
> + int start_col = columns[start_idx];
> + location_t start_loc
> + = linemap_position_for_line_and_column (line_table,
> ord_map, 1,
> + start_col);
> + verify_column (start_loc, ord_map, line_width, start_col);
> + for (unsigned int finish_idx = 0; finish_idx < ARRAY_SIZE
> (columns);
> + finish_idx++)
> + {
> + int finish_col = columns[finish_idx];
> + location_t finish_loc
> + = linemap_position_for_line_and_column (line_table,
> ord_map, 1,
> + finish_col);
> + verify_column (finish_loc, ord_map, line_width,
> finish_col);
> +
> + /* Now use start-finish to exercise the fix-it code.
> + In each case, run the printing code, but don't
> check;
> + we just want it to survive. */
> +
> + /* Insertion. */
> + {
> + rich_location richloc (line_table, start_loc);
> + richloc.add_fixit_insert_after (start_loc,
> "insertion");
> + test_richloc (&richloc);
> + }
> +
> + /* Replacement. */
> + {
> + rich_location richloc (line_table, start_loc);
> + source_range range
> + = source_range::from_locations (start_loc,
> finish_loc);
> + richloc.add_fixit_replace (range, "replacement");
> + test_richloc (&richloc);
> + }
> +
> + /* Deletion. */
> + {
> + rich_location richloc (line_table, start_loc);
> + source_range range
> + = source_range::from_locations (start_loc,
> finish_loc);
> + richloc.add_fixit_remove (range);
> + test_richloc (&richloc);
> + }
> + }
> + }
> + }
> +}
> +
> +/* Callback handler for the PLUGIN_FINISH event.
> + At this point, all GCC subsystems should be initialized and
> + "warmed up"; this is where we run our unit tests. */
> +
> +static void
> +expensive_tests (void */*gcc_data*/, void */*user_data*/)
> +{
> + test_runner r ("expensive_selftests_plugin");
> +
> + for_each_line_table_case (test_fixit_on_very_long_line);
> +}
> +
> +} // namespace selftest
> +
> +#endif /* #if CHECKING_P */
> +
> +int
> +plugin_init (struct plugin_name_args *plugin_info,
> + struct plugin_gcc_version *version)
> +{
> +#if CHECKING_P
> + const char *plugin_name = plugin_info->base_name;
> + register_callback (plugin_info->base_name,
> + PLUGIN_FINISH,
> + selftest::expensive_tests,
> + NULL); /* void *user_data */
> + return 0;
> +#endif /* #if CHECKING_P */
> +}
> diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp
> b/gcc/testsuite/gcc.dg/plugin/plugin.exp
> index c7a3b4d..ff3c976 100644
> --- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
> +++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
> @@ -82,6 +82,8 @@ set plugin_test_list [list \
> { must_tail_call_plugin.c \
> must-tail-call-1.c \
> must-tail-call-2.c } \
> + { expensive_selftests_plugin.c \
> + expensive-selftests-1.c } \
> ]
>
> foreach plugin_test $plugin_test_list {
