[PATCH] Fix test failures from outputs.exp (PR testsuite/98225)

[PATCH] Fix test failures from outputs.exp (PR testsuite/98225)

[Bernd Edlinger]

[-- 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

Re: [PATCH] Fix test failures from outputs.exp (PR testsuite/98225)

[Rainer Orth]

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

[PATCH] testsuite: Fix test failures from outputs.exp [PR98225]

[Bernd Edlinger]

[-- 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

Re: [PATCH] testsuite: Fix test failures from outputs.exp [PR98225]

[Alexandre Oliva]

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

Re: [PATCH] testsuite: Fix test failures from outputs.exp [PR98225]

[David Edelsohn]

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

[PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225]

[Bernd Edlinger]

[-- 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

Re: [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225]

[David Edelsohn]

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.

Re: [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225]

[Bernd Edlinger]

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.

Re: [PATCH] testsuite: Fix test failures from outputs.exp [PR98225]

[Rainer Orth]

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

Re: [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225]

[Rainer Orth]

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

Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target"

[Hans-Peter Nilsson]

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

Re: Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target"

[Bernd Edlinger]

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
> 

Re: Committed: gcc.misc-tests/outputs.exp (outest): Fix typo "is_target"

[Hans-Peter Nilsson]

> 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