Re: [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225]
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
Re: [PATCH v2] testsuite: Fix test failures from outputs.exp [PR98225]
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
> wrote:
>>
>> On 1/8/21 3:23 PM, David Edelsohn wrote:
>>> On Thu, Jan 7, 2021 at 5:18 PM Bernd Edlinger
>>> 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]
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 wrote:
>
> On 1/8/21 3:23 PM, David Edelsohn wrote:
> > On Thu, Jan 7, 2021 at 5:18 PM Bernd Edlinger
> > 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.
