On Fri, Jul 8, 2016 at 12:46 PM, David Malcolm <dmalc...@redhat.com> wrote:
> This patch adds a new phony target to gcc/Makefile.in to make it easy
> to run the selftests under valgrind, via "make selftest-valgrind".
> This phony target isn't a dependency of anything; it's purely for
> convenience (it takes about 4-5 seconds on my box).
>
> Doing so uncovered a few leaks in the selftest suite, which the
> patch also fixes, so that it runs cleanly under valgrind (on
> x86_64-pc-linux-gnu, configured with --enable-valgrind-annotations,
> at least).
>
> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
> Manually verified that the valgrind output is "clean" on
> x86_64-pc-linux-gnu [1].
>
> OK for trunk?

I think this is a good idea.  I assume this not turned on by default.
valgrind is still not fully working on aarch64 :).

Thanks,
Andrew

>
> [1]:
>
>  HEAP SUMMARY:
>      in use at exit: 1,203,983 bytes in 2,114 blocks
>    total heap usage: 4,545 allocs, 2,431 frees, 3,212,841 bytes allocated
>
>  LEAK SUMMARY:
>     definitely lost: 0 bytes in 0 blocks
>     indirectly lost: 0 bytes in 0 blocks
>       possibly lost: 0 bytes in 0 blocks
>     still reachable: 1,203,983 bytes in 2,114 blocks
>          suppressed: 0 bytes in 0 blocks
>  Reachable blocks (those to which a pointer was found) are not shown.
>  To see them, rerun with: --leak-check=full --show-leak-kinds=all
>
>  For counts of detected and suppressed errors, rerun with: -v
>  ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 2 from 2)
>
> gcc/ChangeLog:
>         * Makefile.in (selftest-valgrind): New phony target.
>         * function-tests.c (selftest::build_cfg): Delete pass instances
>         created by the test.
>         (selftest::convert_to_ssa): Likewise.
>         (selftest::test_expansion_to_rtl): Likewise.
>         * tree-cfg.c (selftest::test_linear_chain): Release dominator
>         vectors.
>         (selftest::test_diamond): Likewise.
> ---
>  gcc/Makefile.in      | 6 ++++++
>  gcc/function-tests.c | 4 ++++
>  gcc/tree-cfg.c       | 6 ++++++
>  3 files changed, 16 insertions(+)
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 5e7422d..1a4b5d7 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1869,6 +1869,12 @@ s-selftest: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
>  selftest-gdb: $(GCC_PASSES) cc1$(exeext) stmp-int-hdrs
>         $(GCC_FOR_TARGET) -xc -S -c /dev/null -fself-test -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 \
> +         -wrapper valgrind,--leak-check=full
> +
>  # Recompile all the language-independent object files.
>  # This is used only if the user explicitly asks for it.
>  compilations: $(BACKEND)
> diff --git a/gcc/function-tests.c b/gcc/function-tests.c
> index c8188e7..edd355f 100644
> --- a/gcc/function-tests.c
> +++ b/gcc/function-tests.c
> @@ -296,6 +296,7 @@ build_cfg (tree fndecl)
>    push_cfun (fun);
>    lower_cf_pass->execute (fun);
>    pop_cfun ();
> +  delete lower_cf_pass;
>
>    /* We can now convert to CFG form; for our trivial test function this
>       gives us:
> @@ -310,6 +311,7 @@ build_cfg (tree fndecl)
>    push_cfun (fun);
>    build_cfg_pass->execute (fun);
>    pop_cfun ();
> +  delete build_cfg_pass;
>  }
>
>  /* Convert a gimple+CFG function to SSA form.  */
> @@ -325,6 +327,7 @@ convert_to_ssa (tree fndecl)
>    push_cfun (fun);
>    build_ssa_pass->execute (fun);
>    pop_cfun ();
> +  delete build_ssa_pass;
>  }
>
>  /* Assuming we have a simple 3-block CFG like this:
> @@ -594,6 +597,7 @@ test_expansion_to_rtl ()
>    init_function_start (fndecl);
>    expand_pass->execute (fun);
>    pop_cfun ();
> +  delete expand_pass;
>
>    /* On x86_64, I get this:
>         (note 3 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index 0fac49c..6d69435 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -9276,6 +9276,7 @@ test_linear_chain ()
>    ASSERT_EQ (1, dom_by_b.length ());
>    ASSERT_EQ (bb_c, dom_by_b[0]);
>    free_dominance_info (CDI_DOMINATORS);
> +  dom_by_b.release ();
>
>    /* Similarly for post-dominance: each BB in our chain is post-dominated
>       by the one after it.  */
> @@ -9286,6 +9287,7 @@ test_linear_chain ()
>    ASSERT_EQ (1, postdom_by_b.length ());
>    ASSERT_EQ (bb_a, postdom_by_b[0]);
>    free_dominance_info (CDI_POST_DOMINATORS);
> +  postdom_by_b.release ();
>
>    pop_cfun ();
>  }
> @@ -9346,8 +9348,10 @@ test_diamond ()
>    ASSERT_EQ (bb_a, get_immediate_dominator (CDI_DOMINATORS, bb_d));
>    vec<basic_block> dom_by_a = get_dominated_by (CDI_DOMINATORS, bb_a);
>    ASSERT_EQ (3, dom_by_a.length ()); /* B, C, D, in some order.  */
> +  dom_by_a.release ();
>    vec<basic_block> dom_by_b = get_dominated_by (CDI_DOMINATORS, bb_b);
>    ASSERT_EQ (0, dom_by_b.length ());
> +  dom_by_b.release ();
>    free_dominance_info (CDI_DOMINATORS);
>
>    /* Similarly for post-dominance.  */
> @@ -9357,8 +9361,10 @@ test_diamond ()
>    ASSERT_EQ (bb_d, get_immediate_dominator (CDI_POST_DOMINATORS, bb_c));
>    vec<basic_block> postdom_by_d = get_dominated_by (CDI_POST_DOMINATORS, 
> bb_d);
>    ASSERT_EQ (3, postdom_by_d.length ()); /* A, B, C in some order.  */
> +  postdom_by_d.release ();
>    vec<basic_block> postdom_by_b = get_dominated_by (CDI_POST_DOMINATORS, 
> bb_b);
>    ASSERT_EQ (0, postdom_by_b.length ());
> +  postdom_by_b.release ();
>    free_dominance_info (CDI_POST_DOMINATORS);
>
>    pop_cfun ();
> --
> 1.8.5.3
>

Reply via email to