[Bug testsuite/95720] New: New dump output filename strategy invalidates tests

[Bug testsuite/95720] New: New dump output filename strategy invalidates tests

[akrl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

            Bug ID: 95720
           Summary: New dump output filename strategy invalidates tests
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: testsuite
          Assignee: unassigned at gcc dot gnu.org
          Reporter: akrl at gcc dot gnu.org
  Target Milestone: ---

GCC invoked as 'gcc foo.c --save-temps -o foo.out' produces an assembly
file named 'foo.out-foo.s'.

AFAIU this is consequence of "1dedc12d186 * revamp dump and aux output
names".

This behavior is causing tests in the testsuite relying on
--save-temps to scan the assembly to be marked as unresolved because
dejagnu failing to find the output assembly file.

One example from a gcc.log:

gcc.target/arm/memset-inline-1.c: output file does not exist
UNRESOLVED: gcc.target/arm/memset-inline-1.c scan-assembler-not bl?[ \t]*memset

Wasn't sure if the right component to be categorized in is testsuite or driver.

Bests

  Andrea

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |11.0
   Last reconfirmed|                            |2020-06-17
                 CC|                            |aoliva at gcc dot gnu.org
     Ever confirmed|0                           |1
            Summary|New dump output filename    |[11 Regression] New dump
                   |strategy invalidates tests  |output filename strategy
                   |                            |invalidates tests

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Indeed that part of the testsuite needs to be adapted but the change was
deliberate (and good).

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Liska <marxin@gcc.gnu.org>:

https://gcc.gnu.org/g:9efbb03616f556dcceabe364f734b5b11fb13587

commit r11-1446-g9efbb03616f556dcceabe364f734b5b11fb13587
Author: Martin Liska <mliska@suse.cz>
Date:   Wed Jun 17 13:42:20 2020 +0200

    testsuite: clean up .gcov files in test-suite.

    gcc/testsuite/ChangeLog:

            PR testsuite/95720
            * gcc.misc-tests/gcov-pr94029.c: Remove not needed remove-gcda.
            * lib/gcov.exp: Delete properly .gcov files.

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #3 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
akrl, any clue as to where this .out is coming from in your runs?  I get .exe
in my arm test runs; I don't see anything that selects .out, only .exe; and the
driver disregards the .exe suffix in executable output, regardless of platform,
when recognizing that the single source has the same basename as the linker
output.

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[andrea.corallo at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #4 from Andrea Corallo <andrea.corallo at arm dot com> ---
"aoliva at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> writes:

> --- Comment #3 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
> akrl, any clue as to where this .out is coming from in your runs?  I get .exe
> in my arm test runs; I don't see anything that selects .out, only .exe; and the
> driver disregards the .exe suffix in executable output, regardless of platform,
> when recognizing that the single source has the same basename as the linker
> output.

Hi Alexandre,

Apologies I guess the provided example was not the best.

Here what I've specifically for the mentioned testcase, this is the
compiler invocation:

.../build/gcc/xgcc -B.../build/gcc/
.../src/gcc/gcc/testsuite/gcc.target/arm/memset-inline-1.c gcc_tg.o
-march=armv8.1-m.main -fno-diagnostics-show-caret
-fno-diagnostics-show-line-numbers -fdiagnostics-color=never
-fdiagnostics-urls=never -save-temps -O2 -fno-inline -ffat-lto-objects
-fno-ident -specs=rdimon.specs -Wa,-mno-warn-deprecated
-Wl,-Ttext-segment=0x00500000 -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main
-Wl,-wrap,abort -lm -o ./memset-inline-1.exe

The assembly file generated is called
'memset-inline-1-memset-inline-1.s' but IIUC the expected one by proc
'scan-assembler-not' (scanasm.exp:92) is 'memset-inline-1.s' (I'm just
printing $output_file).

Hope it helps.

  Andrea

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #5 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
that's because of the second input gcc_tg.o

can you tell where that comes from?

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #6 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
In case that's from some board config file, I suggest prefixing it with -Wl, so
that it doesn't count as an additional input.

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #7 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
now, if it is from the board config file, maybe it had better be moved to
ldflags or libs; both of them undergo some -Wl, treatment of object files and
libs already.

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[clyon at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clyon at gcc dot gnu.org

--- Comment #8 from Christophe Lyon <clyon at gcc dot gnu.org> ---
(In reply to Alexandre Oliva from comment #5)
> that's because of the second input gcc_tg.o
> 
> can you tell where that comes from?

I guess that's the "testglue" object file added by Dejagnu when
needs_status_wrapper is set in the .exp file.

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[andrea.corallo at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #9 from Andrea Corallo <andrea.corallo at arm dot com> ---
"clyon at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> writes:

> I guess that's the "testglue" object file added by Dejagnu when
> needs_status_wrapper is set in the .exp file.

Yep, is set in libs in proc 'gcc_target_compile'

If I print all $options in 'gcc_target_compile' I've:

{additional_flags=-fno-diagnostics-show-caret
-fno-diagnostics-show-line-numbers -fdiagnostics-color=never 
-fdiagnostics-urls=never } additional_flags=-mthumb libs=gcc_tg.o
{ldflags=-Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort}
timeout=300 {compiler=.../gcc/xgcc -B.../gcc/}

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #10 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Created attachment 48886
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48886&action=edit
patch that hopefully fixes the problem

Does this fix the problem in your testglue-requiring test runs?

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[andrea.corallo at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #11 from Andrea Corallo <andrea.corallo at arm dot com> ---
Hi Alexandre,

thanks for the patch, the version attached does the job here for the
mentioned testcase.

I touched a couple of lines, please have a look as my tcl is quite
limited.

  Andrea

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #48886|0                           |1
        is obsolete|                            |
  Attachment #48889|0                           |1
        is obsolete|                            |

--- Comment #12 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Created attachment 48891
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48891&action=edit
another try

Sorry about the bugs in the previous patch.

Besides integrating your s/_1/_list/ fix (thanks), I understood why you needed
the [set $opts], but the intended logic was to have a [set $i] in the previous
line.  I didn't know about this use for "set", though; I would have gone for
"eval set opts \$$i" instead.  I liked set opts [set $i] much better.  Thanks
for cluing me into it!

Would you be so kind as to confirm that this slightly modified version still
works for you, and that I didn't miss any of your changes?

Thanks,

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[andrea.corallo at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #13 from Andrea Corallo <andrea.corallo at arm dot com> ---
"aoliva at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> writes:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720
>
> Alexandre Oliva <aoliva at gcc dot gnu.org> changed:
>
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>   Attachment #48886|0                           |1
>         is obsolete|                            |
>   Attachment #48889|0                           |1
>         is obsolete|                            |
>
> --- Comment #12 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
> Created attachment 48891
>   --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48891&action=edit
> another try
>
> Sorry about the bugs in the previous patch.
>
> Besides integrating your s/_1/_list/ fix (thanks), I understood why you needed
> the [set $opts], but the intended logic was to have a [set $i] in the previous
> line.  I didn't know about this use for "set", though; I would have gone for
> "eval set opts \$$i" instead.  I liked set opts [set $i] much better.  Thanks
> for cluing me into it!
>
> Would you be so kind as to confirm that this slightly modified version still
> works for you, and that I didn't miss any of your changes?

Hi Alexandre,

I confirm the patch works here.

Just a small nit, whitespace-mode shows me trailing spaces at the end of
gcc_adjust_linker_flags_list.

> Thanks,

Thank you for the fix!

  Andrea

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #14 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Alexandre Oliva <aoliva@gcc.gnu.org>:

https://gcc.gnu.org/g:5fa1767aa45e1a927e832ccc09e959d4c8a9548c

commit r11-2311-g5fa1767aa45e1a927e832ccc09e959d4c8a9548c
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Fri Jul 24 15:38:35 2020 -0300

    [PR95720] protect gluefile and wrap_flags with -Wl too

    The testglue object file gets interpreted as another input file,
    changing the dump and aux output names in GCC unless it is protected
    by -Wl, like board file-named extra inputs.

    Refactor the code that modifies the board settings so that it can be
    used to modify regular variables as well, and do so.


    for  gcc/testsuite/ChangeLog

            PR testsuite/95720
            * lib/gcc-defs.exp (gcc_adjust_linker_flags_list): Split out of...
            (gcc_adjust_linker_flags): ... this.  Protect gluefile and
            wrap_flags.
            * gcc.misc-tests/outputs.exp: Use gcc_adjust_linker_flags_list.

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[aoliva at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

Alexandre Oliva <aoliva at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #15 from Alexandre Oliva <aoliva at gcc dot gnu.org> ---
Fixed

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[torbjorn.svensson at foss dot st.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

Torbjörn SVENSSON <torbjorn.svensson at foss dot st.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |torbjorn.svensson at foss dot st.c
                   |                            |om

--- Comment #16 from Torbjörn SVENSSON <torbjorn.svensson at foss dot st.com> ---
I think this ticket should be reopened as the change broke how linker scripts
are handled in dejagnu.
If your board file have the following statement:

set_board_info ldscript "-T /path/to/foo.ld"

Then, the commands will be issued like this:

spawn -ignore SIGHUP .../build-native/gcc-final/gcc/xgcc
-B.../build-native/gcc-final/gcc/ linker_plugin9341.c -mthumb -march=armv6s-m
-mcpu=cortex-m0 -mfloat-abi=soft -fdiagnostics-plain-output -dumpbase  -flto
-fuse-linker-plugin -Wl,--start-group -lc -lm -Wl,--end-group
--specs=nosys.specs -Wl,--allow-multiple-definition -Wl,-u,_isatty,-u,_fstat
-Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -Wl,gcc_tg.o -lm
-T -Wl,/path/to/foo.ld -o linker_plugin9341.exe

Note that the ldscript symbol has been replaced by "-T -Wl,/path/to/foo.ld".
According to the manual, it's correct to have a white-space between -T and the
path.

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[clyon at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #17 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I think Torbjorn is right, at the fix seems as simple as handling "-T" the same
as "-Xlinker" in gcc_adjust_linker_flags_list.

However, looking at the GCC documentation, under "Linker Options", I noticed:
-e entry
-u symbol
-z keyword
where although it is unlikely that 'entry', 'symbol' or 'keyword' actually
return true to [file isfile $opt], there's still a possibility that someone
uses e.g. -e file.o :-(

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #18 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Yvan Roux <yroux@gcc.gnu.org>:

https://gcc.gnu.org/g:9d503515ceebc778a5c2958f7459367c34f1fed0

commit r13-2655-g9d503515ceebc778a5c2958f7459367c34f1fed0
Author: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
Date:   Wed Sep 14 07:22:27 2022 +0200

    testsuite: gluefile file need to be prefixed

    When the status wrapper is used, the gluefile need to be prefixed with
    -Wl, in order for the test cases to have the dump files with the
    expected names.

    2022-09-14  Torbjörn SVENSSON  <torbjorn.svensson@foss.st.com>

    gcc/testsuite/
            PR target/95720
            * lib/g++.exp: Moved gluefile block to after flags have been
              prefixed for the target_compile call.
            * lib/gcc.exp: Likewise.
            * lib/wrapper.exp: Reset adjusted state flag.

    Co-Authored-By: Yvan ROUX  <yvan.roux@foss.st.com>
    Signed-off-by: Torbjörn SVENSSON  <torbjorn.svensson@foss.st.com>

[Bug testsuite/95720] [11 Regression] New dump output filename strategy invalidates tests

[torbjorn.svensson at foss dot st.com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95720

--- Comment #19 from Torbjörn SVENSSON <torbjorn.svensson at foss dot st.com> ---
Forgot to mention the PR number in the commit message, but this ticket is also
related to the change.

https://gcc.gnu.org/g:86291da0057d01efdaba71f28cad80b69dc703a4

commit r13-2989-g86291da0057
Author: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
Date:   Mon Sep 19 18:52:50 2022 +0200

    testsuite: Do not prefix linker script with "-Wl,"

    The linker script should not be prefixed with "-Wl," - it's not an
    input file and does not interfere with the new dump output filename
    strategy.

    gcc/testsuite/ChangeLog:

            * lib/gcc-defs.exp: Do not prefix linker script with "-Wl,".

    Signed-off-by: Torbjörn SVENSSON  <torbjorn.svensson@foss.st.com>