* [PATCH] Fix test failures from outputs.exp (PR testsuite/98225) @ 2021-01-07 8:48 Bernd Edlinger 2021-01-07 16:12 ` Rainer Orth 0 siblings, 1 reply; 13+ messages in thread From: Bernd Edlinger @ 2021-01-07 8:48 UTC (permalink / raw) To: gcc-patches, Rainer Orth, Mike Stump [-- Attachment #1: Type: text/plain, Size: 97 bytes --] Hi, this should fix the test failures in this test case. Is it OK for trunk? Thanks Bernd. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-test-failures-from-outputs.exp.patch --] [-- Type: text/x-patch; name="0001-Fix-test-failures-from-outputs.exp.patch", Size: 1613 bytes --] From a8008af3db94a9dff7ae243ebfb40f45c54b3a81 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Thu, 7 Jan 2021 09:37:32 +0100 Subject: [PATCH] Fix test failures from outputs.exp The .ld1_args file is not created when HAVE_GNU_LD is false. The ltrans0.ltrans_arg file is not created when the make jobserver is avaliable. 2021-01-07 Bernd Edlinger <bernd.edlinger@hotmail.de> PR testsuite/98225 * gcc.misc-tests/outputs.exp: Fix test case. --- gcc/testsuite/gcc.misc-tests/outputs.exp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/gcc/testsuite/gcc.misc-tests/outputs.exp b/gcc/testsuite/gcc.misc-tests/outputs.exp index 80d4b61..495dbcd 100644 --- a/gcc/testsuite/gcc.misc-tests/outputs.exp +++ b/gcc/testsuite/gcc.misc-tests/outputs.exp @@ -67,6 +67,10 @@ if {[board_info $dest exists output_format]} { append link_options " additional_flags=-Wl,-oformat,[board_info $dest output_format]" } +# Avoid possible influence from the make jobserver, +# otherwise ltrans0.ltrans_args files may be missing. +unsetenv MAKEFLAGS + # For the test named TEST, run the compiler with SOURCES and OPTS, and # look in DIRS for OUTPUTS. SOURCES is a list of suffixes for source # files starting with $b in $srcdir/$subdir, OPTS is a string with @@ -163,6 +167,9 @@ proc outest { test sources opts dirs outputs } { if { $ogl != {} } { pass "$test: $d$o" file delete $ogl + } elseif { [string match "*.ld1_args" $o] } { + # This file may be missing if !HAVE_GNU_LD + pass "$test: $d$o" } else { fail "$test: $d$o" } -- 1.9.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] Fix test failures from outputs.exp (PR testsuite/98225) 2021-01-07 8:48 [PATCH] Fix test failures from outputs.exp (PR testsuite/98225) Bernd Edlinger @ 2021-01-07 16:12 ` Rainer Orth 2021-01-07 22:18 ` [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] Bernd Edlinger 0 siblings, 1 reply; 13+ messages in thread From: Rainer Orth @ 2021-01-07 16:12 UTC (permalink / raw) To: Bernd Edlinger; +Cc: gcc-patches, Mike Stump, Alexandre Oliva Hi Bernd, > this should fix the test failures in this test case. > > Is it OK for trunk? unfortunately not: there are two bugs and a couple of nits: * When testing with runtest --tool gcc outputs.exp I get ERROR: tcl error sourcing /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.misc-tests/outputs.exp. ERROR: can't unset "env(MAKEFLAGS)": no such element in array while executing "unset env($var)" (procedure "unsetenv" line 3) invoked from within "unsetenv MAKEFLAGS" (file "/vol/gcc/src/hg/master/local/gcc/testsuite/gcc.misc-tests/outputs.exp" line 72) invoked from within "source /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.misc-tests/outputs.exp" ("uplevel" body line 1) invoked from within "uplevel #0 source /vol/gcc/src/hg/master/local/gcc/testsuite/gcc.misc-tests/outputs.exp" invoked from within "catch "uplevel #0 source $test_file_name"" The unsetenv needs to be wrapped in if [info exists env(MAKEFLAGS)] { to avoid this. * diff --git a/gcc/testsuite/gcc.misc-tests/outputs.exp b/gcc/testsuite/gcc.misc-tests/outputs.exp --- a/gcc/testsuite/gcc.misc-tests/outputs.exp +++ b/gcc/testsuite/gcc.misc-tests/outputs.exp @@ -67,6 +67,10 @@ if {[board_info $dest exists output_form append link_options " additional_flags=-Wl,-oformat,[board_info $dest output_format]" } +# Avoid possible influence from the make jobserver, +# otherwise ltrans0.ltrans_args files may be missing. +unsetenv MAKEFLAGS The comment is misleading: it's not just *.ltrans_args, but also *.ltrans.args.0. Maybe there's a collective term for those files in lto-wrapper instead? @@ -163,6 +167,9 @@ proc outest { test sources opts dirs out if { $ogl != {} } { pass "$test: $d$o" file delete $ogl + } elseif { [string match "*.ld1_args" $o] } { + # This file may be missing if !HAVE_GNU_LD + pass "$test: $d$o" Always PASSing the test even if it isn't run is wrong. Either wrap the whole group of tests with response files in if [check_effective_target_gld] { or make the test for the *.ld1_args file conditional on that (e.g. along the lines of $ltop used elsewhere). I'd welcome input from Alexandre which is preferred. A few nits on the patch submission: * Please review https://gcc.gnu.org/contribute.html for the syntax of subject lines: in the present case this should be something like [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] * Both the mail and the patch description should contain a self-contained description of the bug and the fix so potential reviewers don't have to re-read a (potentially excessively long) bugzilla report. * Your ChangeLog entry isn't particularly helpful: 2021-01-07 Bernd Edlinger <bernd.edlinger@hotmail.de> PR testsuite/98225 * gcc.misc-tests/outputs.exp: Fix test case. This tells the reader almost nothing. Instead, it should list *what changed* in the patch; for the current patch something like * gcc.misc-tests/outputs.exp: Unset MAKEFLAGS. Always pass *.ld1_args tests. There's more than you ever wanted to know on ChangeLogs in the GNU Coding Standards ;-) Thanks for working on this. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] 2021-01-07 16:12 ` Rainer Orth @ 2021-01-07 22:18 ` Bernd Edlinger 2021-01-08 8:52 ` Alexandre Oliva 2021-01-08 14:23 ` David Edelsohn 0 siblings, 2 replies; 13+ messages in thread From: Bernd Edlinger @ 2021-01-07 22:18 UTC (permalink / raw) To: Rainer Orth; +Cc: gcc-patches, Mike Stump, Alexandre Oliva, David Edelsohn [-- Attachment #1: Type: text/plain, Size: 1287 bytes --] Hi, On 1/7/21 5:12 PM, Rainer Orth wrote: > The unsetenv needs to be wrapped in > > if [info exists env(MAKEFLAGS)] { > Done. > @@ -163,6 +167,9 @@ proc outest { test sources opts dirs out > if { $ogl != {} } { > pass "$test: $d$o" > file delete $ogl > + } elseif { [string match "*.ld1_args" $o] } { > + # This file may be missing if !HAVE_GNU_LD > + pass "$test: $d$o" > > Always PASSing the test even if it isn't run is wrong. Either wrap > the whole group of tests with response files in > > if [check_effective_target_gld] { > > or make the test for the *.ld1_args file conditional on that > (e.g. along the lines of $ltop used elsewhere). I'd welcome input > from Alexandre which is preferred. > Ah, yes that is a good idea. Thanks. I think the .cdtor.* handling, is probably a bad example that I followed here. I don't know why that is there in the first place, as there are no C++ test cases, these files should not be created at all. If they are ever created we would have a couple of other files created as well IMHO. If there are still misssing files in some cases, I'd prefer to track these per test case, instead of globally. Therefore I propose to remove that exception for now. Is it OK for trunk? Thanks Bernd. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-testsuite-Fix-test-failures-from-outputs.exp-PR98225.patch --] [-- Type: text/x-patch; name="0001-testsuite-Fix-test-failures-from-outputs.exp-PR98225.patch", Size: 4184 bytes --] From 9e0fc10b1c655320ccb63c1798141f4a572410f8 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Thu, 7 Jan 2021 09:37:32 +0100 Subject: [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] The .ld1_args file is not created when HAVE_GNU_LD is false. The ltrans0.ltrans_arg file is not created when the make jobserver is available, so remove the MAKEFLAGS variable. There are no .cdtor.* files ever created with any of the tests, so remove the exception for those files. 2021-01-07 Bernd Edlinger <bernd.edlinger@hotmail.de> PR testsuite/98225 * gcc.misc-tests/outputs.exp: Unset MAKEFLAGS. Expect .ld1_args only when GNU LD is used. Remove exception for .cdtor.* files. --- gcc/testsuite/gcc.misc-tests/outputs.exp | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/gcc/testsuite/gcc.misc-tests/outputs.exp b/gcc/testsuite/gcc.misc-tests/outputs.exp index 80d4b61..05e5b1b2 100644 --- a/gcc/testsuite/gcc.misc-tests/outputs.exp +++ b/gcc/testsuite/gcc.misc-tests/outputs.exp @@ -50,6 +50,9 @@ if !$skip_lto { set ltop [check_linker_plugin_available] } +# Check for GNU LD. Some files like .ld1_args depend on this. +set gld [check_effective_target_gld] + # Prepare additional options to be used for linking. # We do not compile to an executable, because that requires naming an output. set link_options "" @@ -67,6 +70,12 @@ if {[board_info $dest exists output_format]} { append link_options " additional_flags=-Wl,-oformat,[board_info $dest output_format]" } +# Avoid possible influence from the make jobserver, +# otherwise ltrans0.ltrans_args files may be missing. +if [info exists env(MAKEFLAGS)] { + unsetenv MAKEFLAGS +} + # For the test named TEST, run the compiler with SOURCES and OPTS, and # look in DIRS for OUTPUTS. SOURCES is a list of suffixes for source # files starting with $b in $srcdir/$subdir, OPTS is a string with @@ -130,6 +139,7 @@ proc outest { test sources opts dirs outputs } { foreach og $olist { if { [string index $og 0] == "!" } { global gspd ltop + global gld set cond [expr $og] continue } @@ -179,11 +189,7 @@ proc outest { test sources opts dirs outputs } { set outb {} foreach f $outs { file delete $f - # collect2 may create <execname>.cdtor* files in -save-temps link tests, - # ??? without regard to aux output naming conventions. - if ![string match "*.cdtor.*" $f] then { - lappend outb $f - } + lappend outb $f } foreach d $dirs { file delete -force $d @@ -285,10 +291,10 @@ outest "$b exe savetmp namedb" $sing "-o $b.exe -save-temps" {} {{--0.i --0.s -- outest "$b exe savetmp named2" $mult "-o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o .exe}} # Additional files are created when an @file is used -outest "$b exe savetmp namedb" $sing "@/dev/null -o $b.exe -save-temps" {} {{--0.i --0.s --0.o .args.0 .ld1_args .exe}} -outest "$b exe savetmp named2" $mult "@/dev/null -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o .args.0 .ld1_args .exe}} -outest "$b exe savetmp named2" $mult "@/dev/null -I dummy -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o -args.0 -args.1 .args.2 .ld1_args .exe}} -outest "$b exe savetmp named2" $mult "@/dev/null -I dummy -L dummy -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o -args.0 -args.1 .args.2 .args.3 .ld1_args .exe}} +outest "$b exe savetmp namedb" $sing "@/dev/null -o $b.exe -save-temps" {} {{--0.i --0.s --0.o .args.0 !!$gld .ld1_args !0 .exe}} +outest "$b exe savetmp named2" $mult "@/dev/null -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o .args.0 !!$gld .ld1_args !0 .exe}} +outest "$b exe savetmp named2" $mult "@/dev/null -I dummy -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o -args.0 -args.1 .args.2 !!$gld .ld1_args !0 .exe}} +outest "$b exe savetmp named2" $mult "@/dev/null -I dummy -L dummy -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o -args.0 -args.1 .args.2 .args.3 !!$gld .ld1_args !0 .exe}} # Setting the main output to a dir selects it as the default aux&dump # location. -- 1.9.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] 2021-01-07 22:18 ` [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] Bernd Edlinger @ 2021-01-08 8:52 ` Alexandre Oliva 2021-01-11 12:11 ` Rainer Orth 2021-01-08 14:23 ` David Edelsohn 1 sibling, 1 reply; 13+ messages in thread From: Alexandre Oliva @ 2021-01-08 8:52 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Rainer Orth, gcc-patches, Mike Stump, David Edelsohn On Jan 7, 2021, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > I don't know why that is there in the first place, as there > are no C++ test cases, these files should not be created at all. collect2, on platforms that use it, create .cdtor files even for C. David Edelsohn told me so back then; the problem was on AIX IIRC. That was why I added code to tolerate such outputs. Removing it would likely bring that failure back. > Is it OK for trunk? It looks good to me, aside from the removal of the .cdtor handler. I don't think I have authority to approve it with that change, but I would if I did ;-) Thanks! -- Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ Free Software Activist GNU Toolchain Engineer Vim, Vi, Voltei pro Emacs -- GNUlius Caesar ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] 2021-01-08 8:52 ` Alexandre Oliva @ 2021-01-11 12:11 ` Rainer Orth 0 siblings, 0 replies; 13+ messages in thread From: Rainer Orth @ 2021-01-11 12:11 UTC (permalink / raw) To: Alexandre Oliva; +Cc: Bernd Edlinger, gcc-patches, Mike Stump, David Edelsohn Hi Alexandre, > On Jan 7, 2021, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > >> I don't know why that is there in the first place, as there >> are no C++ test cases, these files should not be created at all. > > collect2, on platforms that use it, create .cdtor files even for C. > David Edelsohn told me so back then; the problem was on AIX IIRC. That > was why I added code to tolerate such outputs. Removing it would likely > bring that failure back. > > >> Is it OK for trunk? > > It looks good to me, aside from the removal of the .cdtor handler. > > I don't think I have authority to approve it with that change, > but I would if I did ;-) Thanks! that's exactly the kind of feedback I've been hoping for ;-) Thanks. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] 2021-01-07 22:18 ` [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] Bernd Edlinger 2021-01-08 8:52 ` Alexandre Oliva @ 2021-01-08 14:23 ` David Edelsohn 2021-01-08 18:59 ` [PATCH v2] " Bernd Edlinger 1 sibling, 1 reply; 13+ messages in thread From: David Edelsohn @ 2021-01-08 14:23 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Rainer Orth, gcc-patches, Mike Stump, Alexandre Oliva On Thu, Jan 7, 2021 at 5:18 PM Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > > Hi, > > On 1/7/21 5:12 PM, Rainer Orth wrote: > > The unsetenv needs to be wrapped in > > > > if [info exists env(MAKEFLAGS)] { > > > > Done. > > > @@ -163,6 +167,9 @@ proc outest { test sources opts dirs out > > if { $ogl != {} } { > > pass "$test: $d$o" > > file delete $ogl > > + } elseif { [string match "*.ld1_args" $o] } { > > + # This file may be missing if !HAVE_GNU_LD > > + pass "$test: $d$o" > > > > Always PASSing the test even if it isn't run is wrong. Either wrap > > the whole group of tests with response files in > > > > if [check_effective_target_gld] { > > > > or make the test for the *.ld1_args file conditional on that > > (e.g. along the lines of $ltop used elsewhere). I'd welcome input > > from Alexandre which is preferred. > > > > Ah, yes that is a good idea. Thanks. > > > I think the .cdtor.* handling, is probably a bad example that I followed here. > I don't know why that is there in the first place, as there > are no C++ test cases, these files should not be created at all. > If they are ever created we would have a couple of other files created > as well IMHO. > If there are still missing files in some cases, > I'd prefer to track these per test case, instead of globally. > > Therefore I propose to remove that exception for now. > > Is it OK for trunk? As Alex said, please don't just remove features and functionality if you don't know why they were added. The history is online in the mailing list and the repo history. AIX uses constructors to register EH frames and libgcc has an EH frame. ctors and dtors can be found in non-C++ code. Thanks, David ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225] 2021-01-08 14:23 ` David Edelsohn @ 2021-01-08 18:59 ` Bernd Edlinger 2021-01-08 19:27 ` David Edelsohn 2021-02-16 1:33 ` Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target" Hans-Peter Nilsson 0 siblings, 2 replies; 13+ messages in thread From: Bernd Edlinger @ 2021-01-08 18:59 UTC (permalink / raw) To: David Edelsohn; +Cc: Rainer Orth, gcc-patches, Mike Stump, Alexandre Oliva [-- Attachment #1: Type: text/plain, Size: 2410 bytes --] On 1/8/21 3:23 PM, David Edelsohn wrote: > On Thu, Jan 7, 2021 at 5:18 PM Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: >> >> Hi, >> >> On 1/7/21 5:12 PM, Rainer Orth wrote: >>> The unsetenv needs to be wrapped in >>> >>> if [info exists env(MAKEFLAGS)] { >>> >> >> Done. >> >>> @@ -163,6 +167,9 @@ proc outest { test sources opts dirs out >>> if { $ogl != {} } { >>> pass "$test: $d$o" >>> file delete $ogl >>> + } elseif { [string match "*.ld1_args" $o] } { >>> + # This file may be missing if !HAVE_GNU_LD >>> + pass "$test: $d$o" >>> >>> Always PASSing the test even if it isn't run is wrong. Either wrap >>> the whole group of tests with response files in >>> >>> if [check_effective_target_gld] { >>> >>> or make the test for the *.ld1_args file conditional on that >>> (e.g. along the lines of $ltop used elsewhere). I'd welcome input >>> from Alexandre which is preferred. >>> >> >> Ah, yes that is a good idea. Thanks. >> >> >> I think the .cdtor.* handling, is probably a bad example that I followed here. >> I don't know why that is there in the first place, as there >> are no C++ test cases, these files should not be created at all. >> If they are ever created we would have a couple of other files created >> as well IMHO. >> If there are still missing files in some cases, >> I'd prefer to track these per test case, instead of globally. >> >> Therefore I propose to remove that exception for now. >> >> Is it OK for trunk? > > As Alex said, please don't just remove features and functionality if > you don't know why they were added. The history is online in the > mailing list and the repo history. > > AIX uses constructors to register EH frames and libgcc has an EH > frame. ctors and dtors can be found in non-C++ code. > Okydoky. I think I understand now better what the issue is here. Although the name cdtor suggests that it has something to do with C++ it is also needed to collect EH frame info, in certain targets. Those are mainly AIX but also hppa*-*-hpux*. I believe those exceptions are only necessary for targets that define EH_FRAME_THROUGH_COLLECT2. I have tested this new version of my patch but only on not-affected x86_64-pc-linux-gnu. @David, @Rainer: I would very much appreciate if you could give this patch a test on your systems. Thanks Berns. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-testsuite-Fix-test-failures-from-outputs.exp-PR98225.patch --] [-- Type: text/x-patch; name="0001-testsuite-Fix-test-failures-from-outputs.exp-PR98225.patch", Size: 4477 bytes --] From 861f6631c34bdcbc0d6f61247cc231c1f1b36708 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger <bernd.edlinger@hotmail.de> Date: Thu, 7 Jan 2021 09:37:32 +0100 Subject: [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] The .ld1_args file is not created when HAVE_GNU_LD is false. The ltrans0.ltrans_arg file is not created when the make jobserver is available, so remove the MAKEFLAGS variable. Add an exception for *.gcc_args files similar to the exception for *.cdtor.* files. Limit both exceptions to targets that define EH_FRAME_THROUGH_COLLECT2. That means although the test case does not use C++ constructors or destructors it is still using dwarf2 frame info. 2021-01-07 Bernd Edlinger <bernd.edlinger@hotmail.de> PR testsuite/98225 * gcc.misc-tests/outputs.exp: Unset MAKEFLAGS. Expect .ld1_args only when GNU LD is used. Add an exception for *.gcc_args files. --- gcc/testsuite/gcc.misc-tests/outputs.exp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/gcc/testsuite/gcc.misc-tests/outputs.exp b/gcc/testsuite/gcc.misc-tests/outputs.exp index 80d4b61..d5a9709 100644 --- a/gcc/testsuite/gcc.misc-tests/outputs.exp +++ b/gcc/testsuite/gcc.misc-tests/outputs.exp @@ -50,6 +50,9 @@ if !$skip_lto { set ltop [check_linker_plugin_available] } +# Check for GNU LD. Some files like .ld1_args depend on this. +set gld [check_effective_target_gld] + # Prepare additional options to be used for linking. # We do not compile to an executable, because that requires naming an output. set link_options "" @@ -67,6 +70,12 @@ if {[board_info $dest exists output_format]} { append link_options " additional_flags=-Wl,-oformat,[board_info $dest output_format]" } +# Avoid possible influence from the make jobserver, +# otherwise ltrans0.ltrans_args files may be missing. +if [info exists env(MAKEFLAGS)] { + unsetenv MAKEFLAGS +} + # For the test named TEST, run the compiler with SOURCES and OPTS, and # look in DIRS for OUTPUTS. SOURCES is a list of suffixes for source # files starting with $b in $srcdir/$subdir, OPTS is a string with @@ -130,6 +139,7 @@ proc outest { test sources opts dirs outputs } { foreach og $olist { if { [string index $og 0] == "!" } { global gspd ltop + global gld set cond [expr $og] continue } @@ -181,7 +191,10 @@ proc outest { test sources opts dirs outputs } { file delete $f # collect2 may create <execname>.cdtor* files in -save-temps link tests, # ??? without regard to aux output naming conventions. - if ![string match "*.cdtor.*" $f] then { + # Limit this exception to targets that define EH_FRAME_THROUGH_COLLECT2. + if { !(([istarget powerpc*-*-aix*] || [is_target hppa*-*-hpux*]) + && ([string match "*.cdtor.*" $f] + || [string match "*.gcc_args" $f])) } { lappend outb $f } } @@ -285,10 +298,10 @@ outest "$b exe savetmp namedb" $sing "-o $b.exe -save-temps" {} {{--0.i --0.s -- outest "$b exe savetmp named2" $mult "-o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o .exe}} # Additional files are created when an @file is used -outest "$b exe savetmp namedb" $sing "@/dev/null -o $b.exe -save-temps" {} {{--0.i --0.s --0.o .args.0 .ld1_args .exe}} -outest "$b exe savetmp named2" $mult "@/dev/null -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o .args.0 .ld1_args .exe}} -outest "$b exe savetmp named2" $mult "@/dev/null -I dummy -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o -args.0 -args.1 .args.2 .ld1_args .exe}} -outest "$b exe savetmp named2" $mult "@/dev/null -I dummy -L dummy -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o -args.0 -args.1 .args.2 .args.3 .ld1_args .exe}} +outest "$b exe savetmp namedb" $sing "@/dev/null -o $b.exe -save-temps" {} {{--0.i --0.s --0.o .args.0 !!$gld .ld1_args !0 .exe}} +outest "$b exe savetmp named2" $mult "@/dev/null -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o .args.0 !!$gld .ld1_args !0 .exe}} +outest "$b exe savetmp named2" $mult "@/dev/null -I dummy -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o -args.0 -args.1 .args.2 !!$gld .ld1_args !0 .exe}} +outest "$b exe savetmp named2" $mult "@/dev/null -I dummy -L dummy -o $b.exe -save-temps" {} {{--1.i --1.s --1.o --2.i --2.s --2.o -args.0 -args.1 .args.2 .args.3 !!$gld .ld1_args !0 .exe}} # Setting the main output to a dir selects it as the default aux&dump # location. -- 1.9.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225] 2021-01-08 18:59 ` [PATCH v2] " Bernd Edlinger @ 2021-01-08 19:27 ` David Edelsohn 2021-01-11 11:13 ` Bernd Edlinger 2021-02-16 1:33 ` Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target" Hans-Peter Nilsson 1 sibling, 1 reply; 13+ messages in thread From: David Edelsohn @ 2021-01-08 19:27 UTC (permalink / raw) To: Bernd Edlinger; +Cc: Rainer Orth, gcc-patches, Mike Stump, Alexandre Oliva Hi, Bernd Thanks for investigating this and creating a revised version of the patch. With the second patch, the gcc.misc-test/outputs.exp results are clean on AIX. Thanks, David On Fri, Jan 8, 2021 at 1:59 PM Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > > On 1/8/21 3:23 PM, David Edelsohn wrote: > > On Thu, Jan 7, 2021 at 5:18 PM Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: > >> > >> Hi, > >> > >> On 1/7/21 5:12 PM, Rainer Orth wrote: > >>> The unsetenv needs to be wrapped in > >>> > >>> if [info exists env(MAKEFLAGS)] { > >>> > >> > >> Done. > >> > >>> @@ -163,6 +167,9 @@ proc outest { test sources opts dirs out > >>> if { $ogl != {} } { > >>> pass "$test: $d$o" > >>> file delete $ogl > >>> + } elseif { [string match "*.ld1_args" $o] } { > >>> + # This file may be missing if !HAVE_GNU_LD > >>> + pass "$test: $d$o" > >>> > >>> Always PASSing the test even if it isn't run is wrong. Either wrap > >>> the whole group of tests with response files in > >>> > >>> if [check_effective_target_gld] { > >>> > >>> or make the test for the *.ld1_args file conditional on that > >>> (e.g. along the lines of $ltop used elsewhere). I'd welcome input > >>> from Alexandre which is preferred. > >>> > >> > >> Ah, yes that is a good idea. Thanks. > >> > >> > >> I think the .cdtor.* handling, is probably a bad example that I followed here. > >> I don't know why that is there in the first place, as there > >> are no C++ test cases, these files should not be created at all. > >> If they are ever created we would have a couple of other files created > >> as well IMHO. > >> If there are still missing files in some cases, > >> I'd prefer to track these per test case, instead of globally. > >> > >> Therefore I propose to remove that exception for now. > >> > >> Is it OK for trunk? > > > > As Alex said, please don't just remove features and functionality if > > you don't know why they were added. The history is online in the > > mailing list and the repo history. > > > > AIX uses constructors to register EH frames and libgcc has an EH > > frame. ctors and dtors can be found in non-C++ code. > > > > Okydoky. > > I think I understand now better what the issue is here. > Although the name cdtor suggests that it has something to do with > C++ it is also needed to collect EH frame info, in certain targets. > Those are mainly AIX but also hppa*-*-hpux*. > I believe those exceptions are only necessary for targets that > define EH_FRAME_THROUGH_COLLECT2. > > I have tested this new version of my patch but only on not-affected > x86_64-pc-linux-gnu. > > @David, @Rainer: I would very much appreciate if you could give this patch > a test on your systems. > > > Thanks > Berns. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225] 2021-01-08 19:27 ` David Edelsohn @ 2021-01-11 11:13 ` Bernd Edlinger 2021-01-11 12:13 ` Rainer Orth 0 siblings, 1 reply; 13+ messages in thread From: Bernd Edlinger @ 2021-01-11 11:13 UTC (permalink / raw) To: David Edelsohn; +Cc: Rainer Orth, gcc-patches, Mike Stump, Alexandre Oliva On 1/8/21 8:27 PM, David Edelsohn wrote: > Hi, Bernd > > Thanks for investigating this and creating a revised version of the > patch. With the second patch, the gcc.misc-test/outputs.exp results > are clean on AIX. > Many thanks for confirming that the patch works. Is it OK to push? Thanks Bernd. > Thanks, David > > On Fri, Jan 8, 2021 at 1:59 PM Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: >> >> On 1/8/21 3:23 PM, David Edelsohn wrote: >>> On Thu, Jan 7, 2021 at 5:18 PM Bernd Edlinger <bernd.edlinger@hotmail.de> wrote: >>>> >>>> Hi, >>>> >>>> On 1/7/21 5:12 PM, Rainer Orth wrote: >>>>> The unsetenv needs to be wrapped in >>>>> >>>>> if [info exists env(MAKEFLAGS)] { >>>>> >>>> >>>> Done. >>>> >>>>> @@ -163,6 +167,9 @@ proc outest { test sources opts dirs out >>>>> if { $ogl != {} } { >>>>> pass "$test: $d$o" >>>>> file delete $ogl >>>>> + } elseif { [string match "*.ld1_args" $o] } { >>>>> + # This file may be missing if !HAVE_GNU_LD >>>>> + pass "$test: $d$o" >>>>> >>>>> Always PASSing the test even if it isn't run is wrong. Either wrap >>>>> the whole group of tests with response files in >>>>> >>>>> if [check_effective_target_gld] { >>>>> >>>>> or make the test for the *.ld1_args file conditional on that >>>>> (e.g. along the lines of $ltop used elsewhere). I'd welcome input >>>>> from Alexandre which is preferred. >>>>> >>>> >>>> Ah, yes that is a good idea. Thanks. >>>> >>>> >>>> I think the .cdtor.* handling, is probably a bad example that I followed here. >>>> I don't know why that is there in the first place, as there >>>> are no C++ test cases, these files should not be created at all. >>>> If they are ever created we would have a couple of other files created >>>> as well IMHO. >>>> If there are still missing files in some cases, >>>> I'd prefer to track these per test case, instead of globally. >>>> >>>> Therefore I propose to remove that exception for now. >>>> >>>> Is it OK for trunk? >>> >>> As Alex said, please don't just remove features and functionality if >>> you don't know why they were added. The history is online in the >>> mailing list and the repo history. >>> >>> AIX uses constructors to register EH frames and libgcc has an EH >>> frame. ctors and dtors can be found in non-C++ code. >>> >> >> Okydoky. >> >> I think I understand now better what the issue is here. >> Although the name cdtor suggests that it has something to do with >> C++ it is also needed to collect EH frame info, in certain targets. >> Those are mainly AIX but also hppa*-*-hpux*. >> I believe those exceptions are only necessary for targets that >> define EH_FRAME_THROUGH_COLLECT2. >> >> I have tested this new version of my patch but only on not-affected >> x86_64-pc-linux-gnu. >> >> @David, @Rainer: I would very much appreciate if you could give this patch >> a test on your systems. >> >> >> Thanks >> Berns. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225] 2021-01-11 11:13 ` Bernd Edlinger @ 2021-01-11 12:13 ` Rainer Orth 0 siblings, 0 replies; 13+ messages in thread From: Rainer Orth @ 2021-01-11 12:13 UTC (permalink / raw) To: Bernd Edlinger; +Cc: David Edelsohn, gcc-patches, Mike Stump, Alexandre Oliva Hi Bernd, > On 1/8/21 8:27 PM, David Edelsohn wrote: >> Hi, Bernd >> >> Thanks for investigating this and creating a revised version of the >> patch. With the second patch, the gcc.misc-test/outputs.exp results >> are clean on AIX. >> > > Many thanks for confirming that the patch works. > > Is it OK to push? I've now tested the v2 patch on Solaris without and with GNU ld. Based on Alexandre's and David's feedback, it is ok. Thanks. Rainer -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University ^ permalink raw reply [flat|nested] 13+ messages in thread
* Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target" 2021-01-08 18:59 ` [PATCH v2] " Bernd Edlinger 2021-01-08 19:27 ` David Edelsohn @ 2021-02-16 1:33 ` Hans-Peter Nilsson 2021-02-16 7:35 ` Bernd Edlinger 1 sibling, 1 reply; 13+ messages in thread From: Hans-Peter Nilsson @ 2021-02-16 1:33 UTC (permalink / raw) To: gcc-patches; +Cc: Bernd Edlinger Committed as obvious. Please be more careful; this typo should have been obvious in "make check" output as below. Commit-log: ------------------------------- Fix typo for istarget in "is_target hppa*-*-hpux*", yielding an error running the test-suite for any target not matching powerpc*-*-aix* (presumably, by code inspection), aborting the check-gcc (check-gcc-c) regression test run some 3000 tests before the last one, missing e.g. all gcc.target tests like so: ----- ... Running /x/gcc/gcc/testsuite/gcc.misc-tests/outputs.exp ... ERROR: (DejaGnu) proc "is_target hppa*-*-hpux*" does not exist. The error code is TCL LOOKUP COMMAND is_target The info on the error is: invalid command name "is_target" while executing "::tcl_unknown is_target hppa*-*-hpux*" ("uplevel" body line 1) invoked from within "uplevel 1 ::tcl_unknown $args" === gcc Summary === ... ----- gcc/testsuite: * gcc.misc-tests/outputs.exp (outest): Fix typo "is_target". --- gcc/testsuite/gcc.misc-tests/outputs.exp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gcc.misc-tests/outputs.exp b/gcc/testsuite/gcc.misc-tests/outputs.exp index d5a9709910c2..4d904bde31d5 100644 --- a/gcc/testsuite/gcc.misc-tests/outputs.exp +++ b/gcc/testsuite/gcc.misc-tests/outputs.exp @@ -192,7 +192,7 @@ proc outest { test sources opts dirs outputs } { # collect2 may create <execname>.cdtor* files in -save-temps link tests, # ??? without regard to aux output naming conventions. # Limit this exception to targets that define EH_FRAME_THROUGH_COLLECT2. - if { !(([istarget powerpc*-*-aix*] || [is_target hppa*-*-hpux*]) + if { !(([istarget powerpc*-*-aix*] || [istarget hppa*-*-hpux*]) && ([string match "*.cdtor.*" $f] || [string match "*.gcc_args" $f])) } { lappend outb $f -- 2.11.0 brgds, H-P ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target" 2021-02-16 1:33 ` Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target" Hans-Peter Nilsson @ 2021-02-16 7:35 ` Bernd Edlinger 2021-02-16 10:48 ` Hans-Peter Nilsson 0 siblings, 1 reply; 13+ messages in thread From: Bernd Edlinger @ 2021-02-16 7:35 UTC (permalink / raw) To: Hans-Peter Nilsson, gcc-patches Oops, thanks for fixing this problem. To my excuse I would like to note, that the script error does not happen on x86_64-pc-linux-gnu, probably it would only happen when a file is left over. Since usually this is never executed because $outs is empty: foreach f $outs { file delete $f # collect2 may create <execname>.cdtor* files in -save-temps link tests, # ??? without regard to aux output naming conventions. # Limit this exception to targets that define EH_FRAME_THROUGH_COLLECT2. if { !(([istarget powerpc*-*-aix*] || [is_target hppa*-*-hpux*]) && ([string match "*.cdtor.*" $f] || [string match "*.gcc_args" $f])) } { lappend outb $f } } Can you say which target was this, where you found this bug? Which file was in $outs? I am also a bit surprised that a script error in one test aborts the whole gcc.target tests. How can that be? Thanks Bernd. On 2/16/21 2:33 AM, Hans-Peter Nilsson wrote: > Committed as obvious. Please be more careful; this typo > should have been obvious in "make check" output as below. > > Commit-log: > ------------------------------- > Fix typo for istarget in "is_target hppa*-*-hpux*", yielding > an error running the test-suite for any target not matching > powerpc*-*-aix* (presumably, by code inspection), aborting > the check-gcc (check-gcc-c) regression test run some 3000 > tests before the last one, missing e.g. all gcc.target > tests like so: > > ----- > ... > Running /x/gcc/gcc/testsuite/gcc.misc-tests/outputs.exp ... > ERROR: (DejaGnu) proc "is_target hppa*-*-hpux*" does not exist. > The error code is TCL LOOKUP COMMAND is_target > The info on the error is: > invalid command name "is_target" > while executing > "::tcl_unknown is_target hppa*-*-hpux*" > ("uplevel" body line 1) > invoked from within > "uplevel 1 ::tcl_unknown $args" > > === gcc Summary === > ... > ----- > > gcc/testsuite: > * gcc.misc-tests/outputs.exp (outest): Fix typo "is_target". > --- > gcc/testsuite/gcc.misc-tests/outputs.exp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/testsuite/gcc.misc-tests/outputs.exp b/gcc/testsuite/gcc.misc-tests/outputs.exp > index d5a9709910c2..4d904bde31d5 100644 > --- a/gcc/testsuite/gcc.misc-tests/outputs.exp > +++ b/gcc/testsuite/gcc.misc-tests/outputs.exp > @@ -192,7 +192,7 @@ proc outest { test sources opts dirs outputs } { > # collect2 may create <execname>.cdtor* files in -save-temps link tests, > # ??? without regard to aux output naming conventions. > # Limit this exception to targets that define EH_FRAME_THROUGH_COLLECT2. > - if { !(([istarget powerpc*-*-aix*] || [is_target hppa*-*-hpux*]) > + if { !(([istarget powerpc*-*-aix*] || [istarget hppa*-*-hpux*]) > && ([string match "*.cdtor.*" $f] > || [string match "*.gcc_args" $f])) } { > lappend outb $f > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target" 2021-02-16 7:35 ` Bernd Edlinger @ 2021-02-16 10:48 ` Hans-Peter Nilsson 0 siblings, 0 replies; 13+ messages in thread From: Hans-Peter Nilsson @ 2021-02-16 10:48 UTC (permalink / raw) To: Bernd Edlinger; +Cc: gcc-patches > From: Bernd Edlinger <bernd.edlinger@hotmail.de> > Date: Tue, 16 Feb 2021 08:35:04 +0100 > Oops, > > thanks for fixing this problem. > > To my excuse I would like to note, > that the script error does not happen on x86_64-pc-linux-gnu, > probably it would only happen when a file is left over. Sorry but that just sounds to me like bad coverage when testing this patch. AFAIU unfortunately errors including tcl syntax errors do not cause any difference in error return codes for "make check" (compared to the usual unclean run; with some errors). > Since usually this is never executed because $outs is empty: > ... > Can you say which target was this, where you found this bug? A simulator target: cris-elf: 'make check RUNTESTFLAGS=--target_board=cris-sim' > Which file was in $outs? I added: --- outputs.exp~ 2021-01-11 17:44:41.000000000 +0100 +++ outputs.exp 2021-02-16 11:41:32.130512321 +0100 @@ -187,6 +187,7 @@ proc outest { test sources opts dirs out } set outb {} + send_log "for test <$test>, outs is <$outs>\n" foreach f $outs { file delete $f # collect2 may create <execname>.cdtor* files in -save-temps link tests, and ran "make check-gcc-c 'RUNTESTFLAGS=--target_board=cris-sim outputs.exp'" which resulted in this in gcc.log: ... PASS: outputs exe savetmp namedb: outputs-outputs-0.i PASS: outputs exe savetmp namedb: outputs-outputs-0.s PASS: outputs exe savetmp namedb: outputs-outputs-0.o PASS: outputs exe savetmp namedb: outputs.args.0 PASS: outputs exe savetmp namedb: outputs.ld1_args PASS: outputs exe savetmp namedb: outputs.exe for test <outputs exe savetmp namedb>, outs is <outputs.args.1> ERROR: (DejaGnu) proc "is_target hppa*-*-hpux*" does not exist. ... > I am also a bit surprised that a script error in one test aborts > the whole gcc.target tests. How can that be? TBH, I'm not really surprised by dejagnu exiting due to an undefined function/proc, but maybe for penance you can look into this and wrap key points (like each .exp file) into try-clauses? ;-] brgds, H-P ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-02-16 10:48 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-07 8:48 [PATCH] Fix test failures from outputs.exp (PR testsuite/98225) Bernd Edlinger 2021-01-07 16:12 ` Rainer Orth 2021-01-07 22:18 ` [PATCH] testsuite: Fix test failures from outputs.exp [PR98225] Bernd Edlinger 2021-01-08 8:52 ` Alexandre Oliva 2021-01-11 12:11 ` Rainer Orth 2021-01-08 14:23 ` David Edelsohn 2021-01-08 18:59 ` [PATCH v2] " Bernd Edlinger 2021-01-08 19:27 ` David Edelsohn 2021-01-11 11:13 ` Bernd Edlinger 2021-01-11 12:13 ` Rainer Orth 2021-02-16 1:33 ` Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target" Hans-Peter Nilsson 2021-02-16 7:35 ` Bernd Edlinger 2021-02-16 10:48 ` Hans-Peter Nilsson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).