Alex Coplan <alex.cop...@arm.com> writes:
> I noticed while working on a test that uses LTO and requests a dump
> file, that we are failing to cleanup ltrans dump files in the testsuite.
>
> E.g. the test I was working on compiles with -flto
> -fdump-rtl-loop2_unroll, and we end up with the following file:
>
> ./gcc/testsuite/g++/pr116140.ltrans0.ltrans.287r.loop2_unroll
>
> being left behind by the testsuite. This is problematic not just from a
> "missing cleanup" POV, but also because it can cause the test to pass
> spuriously when the test is re-run wtih an unpatched compiler (without
> the bug fix). In the broken case, loop2_unroll isn't run at all, so we
> end up scanning the old dumpfile (from the previous test run) and making
> the dumpfile scan pass.
>
> Running with `-v -v` in RUNTESTFLAGS we can see the following cleanup
> attempt is made:
>
> remove-build-file
> `pr116140.{C,exe}.{ltrans[0-9]*.,}[0-9][0-9][0-9]{l,i,r,t}.*'
>
> looking again at the ltrans dump file above we can see this will fail for two
> reasons:
>
> - The actual dump file has no {C,exe} extension between the basename and
> ltrans0.
> - The actual dump file has an additional `.ltrans` component after
> `.ltrans0`.
>
> This patch therefore relaxes the pattern constructed for cleaning up such
> dumpfiles to also match dumpfiles with the above form.
>
> Running the testsuite before/after this patch shows the number of files in
> gcc/testsuite (in the build dir) with "ltrans" in the name goes from 1416 to
> 62
> on aarch64.
>
> No regressions on aarch64-linux-gnu, OK for trunk?
>
> Thanks,
> Alex
>
> gcc/testsuite/ChangeLog:
>
> PR libstdc++/116140
> * lib/gcc-dg.exp (schedule-cleanups): Relax ltrans dumpfile
> cleanup pattern to handle missing cases.
> ---
> gcc/testsuite/lib/gcc-dg.exp | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
> index 992062103c1..cdb677d7873 100644
> --- a/gcc/testsuite/lib/gcc-dg.exp
> +++ b/gcc/testsuite/lib/gcc-dg.exp
> @@ -190,7 +190,7 @@ proc schedule-cleanups { opts } {
> # Handle ltrans files around -flto
> if [regexp -- {(^|\s+)-flto(\s+|$)} $opts] {
> verbose "Cleanup -flto seen" 4
> - set ltrans "{ltrans\[0-9\]*.,}"
> + set ltrans "{ltrans\[0-9\]*{.ltrans,}.,}"
> } else {
> set ltrans ""
> }
> @@ -206,7 +206,7 @@ proc schedule-cleanups { opts } {
> if {$basename_ext != ""} {
> regsub -- {^.*\.} $basename_ext {} basename_ext
> }
> - lappend tfiles "$stem.{$basename_ext,exe}"
> + lappend tfiles "$stem{.$basename_ext,.exe,}"
> unset basename_ext
> } else {
> lappend tfiles $basename
Hmm, hadn't realised that we rely on shell expansion of braces for the
cleanup (if I've understood correctly). That seems like a bashism and
wouldn't work for testing via dash, for instance. But that's obviously
entirely pre-existing.
The patch LGTM. OK if no-one objects by the time the other patches
are approved.
Thanks for noticing and for cleaning this up.
Richard
