[PATCH] Fix oopsie in ld-plugin tests.

[PATCH] Fix oopsie in ld-plugin tests.

[Dave Korn]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]


    Good morning list,


  H-P pointed out to me that I overlooked to avoid returning an "ERROR:" in
the testsuite results when there is no target compiler available.  Sorry for
cluttering up your overnight test results.  The attached patch copies a test
that I have in fact seen before and should have thought of, and uses it to
mark the tests "UNSUPPORTED:" if no target compiler is available.

ld/testsuite/ChangeLog:

	* ld-plugin/plugin.exp: Don't error out if there is no target compiler
	available, make tests UNSUPPORTED instead.

  I don't know if it counts as obvious, though.  OK to commit?

    cheers,
      DaveK

[-- Attachment #2: tests-fix.diff --]
[-- Type: text/x-c, Size: 1598 bytes --]

diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp
index 796cb0e..416159a 100644
--- a/ld/testsuite/ld-plugin/plugin.exp
+++ b/ld/testsuite/ld-plugin/plugin.exp
@@ -24,6 +24,13 @@ if ![check_plugin_api_available] {
     return
 }
 
+# And a compiler to be available.
+set can_compile 1
+if { [which $CC] == 0 } {
+  # Don't fail immediately, 
+  set can_compile 0
+}
+
 pass "plugin API enabled"
 
 global base_dir
@@ -62,12 +69,15 @@ set regcln "-plugin-opt registercleanup"
 set failed_compile 0
 set _ ""
 set plugin_nm_output ""
-if { ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/main.c tmpdir/main.o]
-	|| ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/func.c tmpdir/func.o]
-	|| ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/text.c tmpdir/text.o] } {
+if { $can_compile && \
+	(![ld_compile "$CC $CFLAGS" $srcdir/$subdir/main.c tmpdir/main.o] \
+	|| ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/func.c tmpdir/func.o] \
+	|| ![ld_compile "$CC $CFLAGS" $srcdir/$subdir/text.c tmpdir/text.o]) } {
     # Defer fail until we have list of tests set.
     set failed_compile 1
-} else {
+}
+
+if { $can_compile && !$failed_compile } {
     # Find out if symbols have prefix on this platform before setting tests.
     catch "exec $NM tmpdir/func.o" plugin_nm_output
     if { [regexp "_func" "$plugin_nm_output"] } {
@@ -142,7 +152,7 @@ set plugin_extra_elf_tests [list \
 				{readelf -s plugin-vis-1.d}} "main.x" ] \
 ]
 
-if { $failed_compile != 0 } {
+if { !$can_compile || $failed_compile } {
     foreach testitem $plugin_tests {
 	unresolved [lindex $testitem 0]
     }

Re: [PATCH] Fix oopsie in ld-plugin tests.

[Dave Korn]

On 14/10/2010 14:38, Tristan Gingold wrote:
> Dave,
> 
> I haven't closely followed your commits.  Are the ld-plugin patches commit
> completed ?
> 
> Tristan.
> 

  I hope you don't mind, I Cc'd the list in so that everyone can be aware of
the plan.

  There are two matters outstanding (the fix for the testsuite problems aside):

1- Libltdl-ization of the makefile and configury parts so that mingw can run.
 I'm starting on this next and should be able to post it tomorrow.

2- I may need to add support for rescanning libraries after the plugin adds
object files.  This is something I've just only just run into while porting
the gcc lto-plugin, and it may be something that ends up being addressed in
the plugin rather than in ld, but I need to get some advice from the gcc list
to clarify the semantics of how the plugin and linker are supposed to interact
before I can be sure.  If it does need fixing in ld, it's probably no more
than a day or two to get it working.

  So, best case before the weekend, worst case early next week it should be
complete.  How is that with you?

    cheers,
      DaveK

Re: [PATCH] Fix oopsie in ld-plugin tests.

[Tristan Gingold]

On Oct 14, 2010, at 4:14 PM, Dave Korn wrote:

> On 14/10/2010 14:38, Tristan Gingold wrote:
>> Dave,
>> 
>> I haven't closely followed your commits.  Are the ld-plugin patches commit
>> completed ?
>> 
>> Tristan.
>> 
> 
>  I hope you don't mind, I Cc'd the list in so that everyone can be aware of
> the plan.
> 
>  There are two matters outstanding (the fix for the testsuite problems aside):
> 
> 1- Libltdl-ization of the makefile and configury parts so that mingw can run.
> I'm starting on this next and should be able to post it tomorrow.
> 
> 2- I may need to add support for rescanning libraries after the plugin adds
> object files.  This is something I've just only just run into while porting
> the gcc lto-plugin, and it may be something that ends up being addressed in
> the plugin rather than in ld, but I need to get some advice from the gcc list
> to clarify the semantics of how the plugin and linker are supposed to interact
> before I can be sure.  If it does need fixing in ld, it's probably no more
> than a day or two to get it working.
> 
>  So, best case before the weekend, worst case early next week it should be
> complete.  How is that with you?

Thank you for explaining the plan.

I am fine with that.  As far as I know nobody asked very loudly for a release immediately :-)

Tristan.

Re: [PATCH] Fix oopsie in ld-plugin tests.

[Hans-Peter Nilsson]

> Date: Thu, 14 Oct 2010 06:24:04 +0100
> From: Dave Korn <dave.korn.cygwin@gmail.com>

> ld/testsuite/ChangeLog:
> 
> 	* ld-plugin/plugin.exp: Don't error out if there is no target compiler
> 	available, make tests UNSUPPORTED instead.

> +if { !$can_compile || $failed_compile } {
>      foreach testitem $plugin_tests {
>  	unresolved [lindex $testitem 0]

Incorrect, or you sent and committed the wrong patch.  You made
them UNRESOLVED, which causes make to (still) return an error.

brgds, H-P

Re: [PATCH] Fix oopsie in ld-plugin tests.

[Dave Korn]

On 15/10/2010 19:10, Hans-Peter Nilsson wrote:
>> Date: Thu, 14 Oct 2010 06:24:04 +0100
>> From: Dave Korn <dave.korn.cygwin@gmail.com>
> 
>> ld/testsuite/ChangeLog:
>>
>> 	* ld-plugin/plugin.exp: Don't error out if there is no target compiler
>> 	available, make tests UNSUPPORTED instead.
> 
>> +if { !$can_compile || $failed_compile } {
>>      foreach testitem $plugin_tests {
>>  	unresolved [lindex $testitem 0]
> 
> Incorrect, or you sent and committed the wrong patch.  You made
> them UNRESOLVED, which causes make to (still) return an error.

  Sorry, that was what you suggested in your off-list email.  Did you mean to
says UNSUPPORTED perhaps?

    cheers,
      DaveK

Re: [PATCH] Fix oopsie in ld-plugin tests.

[Hans-Peter Nilsson]

> Date: Fri, 15 Oct 2010 20:18:21 +0100
> From: Dave Korn <dave.korn.cygwin@gmail.com>

> On 15/10/2010 19:10, Hans-Peter Nilsson wrote:
> >> Date: Thu, 14 Oct 2010 06:24:04 +0100
> >> From: Dave Korn <dave.korn.cygwin@gmail.com>
> > 
> >> ld/testsuite/ChangeLog:
> >>
> >> 	* ld-plugin/plugin.exp: Don't error out if there is no target compiler
> >> 	available, make tests UNSUPPORTED instead.
> > 
> >> +if { !$can_compile || $failed_compile } {
> >>      foreach testitem $plugin_tests {
> >>  	unresolved [lindex $testitem 0]
> > 
> > Incorrect, or you sent and committed the wrong patch.  You made
> > them UNRESOLVED, which causes make to (still) return an error.
> 
>   Sorry, that was what you suggested in your off-list email.  Did you mean to
> says UNSUPPORTED perhaps?

Yes, as I also mentioned off-list, presumably after you sent this. :)
(and the patch didn't match the ChangeLog; UNSUPPORTED != UNRESOLVED)
Right, too many choices!

brgds, H-P

[PATCH, take 2] Fix ld-plugin testsuite noise properly [was Re: [PATCH] Fix oopsie in ld-plugin tests.]

[Dave Korn]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

On 15/10/2010 20:02, Hans-Peter Nilsson wrote:
>> Date: Fri, 15 Oct 2010 20:18:21 +0100
>> From: Dave Korn <dave.korn.cygwin@gmail.com>
> 
>> On 15/10/2010 19:10, Hans-Peter Nilsson wrote:
>>>> Date: Thu, 14 Oct 2010 06:24:04 +0100
>>>> From: Dave Korn <dave.korn.cygwin@gmail.com>
>>>> ld/testsuite/ChangeLog:
>>>>
>>>> 	* ld-plugin/plugin.exp: Don't error out if there is no target compiler
>>>> 	available, make tests UNSUPPORTED instead.
>>>> +if { !$can_compile || $failed_compile } {
>>>>      foreach testitem $plugin_tests {
>>>>  	unresolved [lindex $testitem 0]
>>> Incorrect, or you sent and committed the wrong patch.  You made
>>> them UNRESOLVED, which causes make to (still) return an error.
>>   Sorry, that was what you suggested in your off-list email.  Did you mean to
>> says UNSUPPORTED perhaps?
> 
> Yes, as I also mentioned off-list, presumably after you sent this. :)

  Heh, just a bit!

> (and the patch didn't match the ChangeLog; UNSUPPORTED != UNRESOLVED)
> Right, too many choices!

  The attached patch makes it UNSUPPORTED if no compiler is found, and only
UNRESOLVED if there's a compiler error of some sort.

ld/testsuite/ChangeLog:

	* ld-plugin/plugin.exp: Mark tests UNSUPPORTED, not UNRESOLVED, if
	no suitable target compiler is available.

  OK for trunk?

    cheers,
      DaveK


[-- Attachment #2: ld-plugin-fixes-4-tests-refix.diff --]
[-- Type: text/x-c, Size: 1609 bytes --]

From 408a48b67f43adb6ddabc8cfd67eed9d24e39158 Mon Sep 17 00:00:00 2001
From: Dave Korn <dave.korn.cygwin@gmail.com>
Date: Fri, 15 Oct 2010 20:43:28 +0100
Subject: [PATCH] Clean up ld-plugin testsuite noise.

ld/testsuite/ChangeLog:

	* ld-plugin/plugin.exp: Mark tests UNSUPPORTED, not UNRESOLVED, if
	no suitable target compiler is available.
---
 ld/testsuite/ld-plugin/plugin.exp |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/ld/testsuite/ld-plugin/plugin.exp b/ld/testsuite/ld-plugin/plugin.exp
index 416159a..0a556e2 100644
--- a/ld/testsuite/ld-plugin/plugin.exp
+++ b/ld/testsuite/ld-plugin/plugin.exp
@@ -26,9 +26,11 @@ if ![check_plugin_api_available] {
 
 # And a compiler to be available.
 set can_compile 1
+set failure_kind "unresolved"
 if { [which $CC] == 0 } {
   # Don't fail immediately, 
   set can_compile 0
+  set failure_kind "unsupported"
 }
 
 pass "plugin API enabled"
@@ -154,11 +156,11 @@ set plugin_extra_elf_tests [list \
 
 if { !$can_compile || $failed_compile } {
     foreach testitem $plugin_tests {
-	unresolved [lindex $testitem 0]
+	$failure_kind [lindex $testitem 0]
     }
     if { [is_elf_format] } {
 	foreach testitem $plugin_extra_elf_tests {
-	    unresolved [lindex $testitem 0]
+	    $failure_kind [lindex $testitem 0]
 	}
     }
     return
@@ -172,7 +174,7 @@ if { [is_elf_format] } {
 
 if ![ar_simple_create $ar "" "tmpdir/libtext.a" "tmpdir/text.o"] {
     foreach testitem $plugin_lib_tests {
-	unresolved [lindex $testitem 0]
+	$failure_kind [lindex $testitem 0]
     }
 } else {
     run_ld_link_tests $plugin_lib_tests

Re: [PATCH, take 2] Fix ld-plugin testsuite noise properly [was Re: [PATCH] Fix oopsie in ld-plugin tests.]

[Hans-Peter Nilsson]

> Date: Fri, 15 Oct 2010 20:47:31 +0100
> From: Dave Korn <dave.korn.cygwin@gmail.com>

>   The attached patch makes it UNSUPPORTED if no compiler is found, and only
> UNRESOLVED if there's a compiler error of some sort.
> 
> ld/testsuite/ChangeLog:
> 
> 	* ld-plugin/plugin.exp: Mark tests UNSUPPORTED, not UNRESOLVED, if
> 	no suitable target compiler is available.
> 
>   OK for trunk?

Looks obvious to me, FWIW (though I'm not an approver).  It
implements what your previous ChangeLog entry said, so there's a
case for calling it so.  IWBN to have this resolved (no pun
intended).

brgds, H-P

Re: [PATCH, take 2] Fix ld-plugin testsuite noise properly [was Re: [PATCH] Fix oopsie in ld-plugin tests.]

[Alan Modra]

On Tue, Oct 19, 2010 at 03:12:49PM +0200, Hans-Peter Nilsson wrote:
> > Date: Fri, 15 Oct 2010 20:47:31 +0100
> > From: Dave Korn <dave.korn.cygwin@gmail.com>
> 
> >   The attached patch makes it UNSUPPORTED if no compiler is found, and only
> > UNRESOLVED if there's a compiler error of some sort.
> > 
> > ld/testsuite/ChangeLog:
> > 
> > 	* ld-plugin/plugin.exp: Mark tests UNSUPPORTED, not UNRESOLVED, if
> > 	no suitable target compiler is available.
> > 
> >   OK for trunk?
> 
> Looks obvious to me

Yes, go ahead and commit.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH, take 2] Fix ld-plugin testsuite noise properly [was Re: [PATCH] Fix oopsie in ld-plugin tests.]

[Dave Korn]

On 20/10/2010 00:44, Alan Modra wrote:
> On Tue, Oct 19, 2010 at 03:12:49PM +0200, Hans-Peter Nilsson wrote:
>>> Date: Fri, 15 Oct 2010 20:47:31 +0100
>>> From: Dave Korn <dave.korn.cygwin@gmail.com>
>>>   The attached patch makes it UNSUPPORTED if no compiler is found, and only
>>> UNRESOLVED if there's a compiler error of some sort.
>>>
>>> ld/testsuite/ChangeLog:
>>>
>>> 	* ld-plugin/plugin.exp: Mark tests UNSUPPORTED, not UNRESOLVED, if
>>> 	no suitable target compiler is available.
>>>
>>>   OK for trunk?
>> Looks obvious to me

  :) In TCL, appearances can be deceiving!

> Yes, go ahead and commit.

  Thanks, checked in.

  OK also for http://sourceware.org/ml/binutils/2010-10/msg00261.html, which
fixes the only actual real bug (at least as far as I could tell from
lto-bootstrapping gcc, which dies at the end of stage2 for unrelated reasons)?

    cheers,
      DaveK

Re: [PATCH, take 2] Fix ld-plugin testsuite noise properly [was Re: [PATCH] Fix oopsie in ld-plugin tests.]

[Alan Modra]

On Wed, Oct 20, 2010 at 04:04:10PM +0100, Dave Korn wrote:
>   OK also for http://sourceware.org/ml/binutils/2010-10/msg00261.html, which

Sure.

-- 
Alan Modra
Australia Development Lab, IBM