[patch, testsuite] Skip gnu-ifunc tests if building the testcase fails

[patch, testsuite] Skip gnu-ifunc tests if building the testcase fails

[Sandra Loosemore]

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

gdb.base/gnu-ifunc.exp doesn't fail gracefully on targets that don't 
support this feature -- on nios2-linux-gnu I've seen TCL errors from 
trying to copy the nonexistent shared library that fails to build to the 
target.

I see that ld/testsuite/ld-ifunc/ifunc.exp explicitly lists all the 
targets where IFUNC is expected to work, but it seemed more maintainable 
to me to tweak these gdb tests to pay attention to the return status 
from trying to build the test cases.  Is this OK to commit?

-Sandra

[-- Attachment #2: ifunc.patch --]
[-- Type: text/x-patch, Size: 1516 bytes --]

commit 1e4269b8d83d6c8cdb5351efa2b346a714a250aa
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Tue Sep 25 21:54:52 2018 -0700

    Skip gnu-ifunc tests if building the testcase fails.
    
    2018-09-25  Sandra Loosemore  <sandra@codesourcery.com>
    
    	* gdb.base/gnu-ifunc.exp: Skip tests if building testcase fails.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index d15fcff..72bf6bd 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-25  Sandra Loosemore  <sandra@codesourcery.com>
+
+	* gdb.base/gnu-ifunc.exp: Skip tests if building testcase fails.
+
 2018-09-24  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
 
 	PR gdb/20948
diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
index d6ec698..ffaf254 100644
--- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
+++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
@@ -365,9 +365,10 @@ proc misc_tests {resolver_attr resolver_debug final_debug} {
 foreach_with_prefix resolver_attr {0 1} {
     foreach_with_prefix resolver_debug {0 1} {
 	foreach_with_prefix final_debug {0 1} {
-	    build $resolver_attr $resolver_debug $final_debug
-	    misc_tests $resolver_attr $resolver_debug $final_debug
-	    set-break $resolver_attr $resolver_debug $final_debug
+	    if { [build $resolver_attr $resolver_debug $final_debug] != 0 } {
+		misc_tests $resolver_attr $resolver_debug $final_debug
+		set-break $resolver_attr $resolver_debug $final_debug
+	    }
 	}
     }
 }

Re: [patch, testsuite] Skip gnu-ifunc tests if building the testcase fails

[Rainer Orth]

Hi Sandra,

> gdb.base/gnu-ifunc.exp doesn't fail gracefully on targets that don't
> support this feature -- on nios2-linux-gnu I've seen TCL errors from trying
> to copy the nonexistent shared library that fails to build to the target.

I just tried the patch on Solaris which also doesn't have (and never
will have) ifunc support: unfortunately, there's still lots of noise in
gdb.sum:

Running /vol/src/gnu/gdb/hg/master/local/gdb/testsuite/gdb.base/gnu-ifunc.exp ...
gdb compile failed, ld: warning: relocation error: 0xa: file /var/tmp//ccyG.via.o: symbol gnu_ifunc: unexpected symbol referenced from file /vol/obj/gnu/gdb/gdb/11.5-amd64-local/gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-lib-0-0-0.so
ld: warning: relocation error: 0xa: file /var/tmp//ccyG.via.o: symbol gnu_ifunc: unexpected symbol referenced from file /vol/obj/gnu/gdb/gdb/11.5-amd64-local/gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-lib-0-0-0.so
warning: Text relocation remains        	referenced
    against symbol		    offset	in file
gnu_ifunc                           0x55      	/var/tmp//ccyG.via.o
gnu_ifunc                           0x81      	/var/tmp//ccyG.via.o
UNTESTED: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0: final_debug=0: failed to compile testcase
gdb compile failed, ld: warning: relocation error: 0xa: file /var/tmp//ccD64Npa.o: symbol gnu_ifunc: unexpected symbol referenced from file /vol/obj/gnu/gdb/gdb/11.5-amd64-local/gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-lib-0-0-1.so
ld: warning: relocation error: 0xa: file /var/tmp//ccD64Npa.o: symbol gnu_ifunc: unexpected symbol referenced from file /vol/obj/gnu/gdb/gdb/11.5-amd64-local/gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-lib-0-0-1.so
warning: Text relocation remains        	referenced
    against symbol		    offset	in file
gnu_ifunc                           0x55      	/var/tmp//ccD64Npa.o
gnu_ifunc                           0x81      	/var/tmp//ccD64Npa.o
UNTESTED: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=0: final_debug=1: failed to compile testcase
gdb compile failed, ld: warning: relocation error: 0xa: file /var/tmp//ccLB0Wba.o: symbol gnu_ifunc: unexpected symbol referenced from file /vol/obj/gnu/gdb/gdb/11.5-amd64-local/gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-lib-0-1-0.so
ld: warning: relocation error: 0xa: file /var/tmp//ccLB0Wba.o: symbol gnu_ifunc: unexpected symbol referenced from file /vol/obj/gnu/gdb/gdb/11.5-amd64-local/gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-lib-0-1-0.so
warning: Text relocation remains        	referenced
    against symbol		    offset	in file
gnu_ifunc                           0x55      	/var/tmp//ccLB0Wba.o
gnu_ifunc                           0x81      	/var/tmp//ccLB0Wba.o
UNTESTED: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: final_debug=0: failed to compile testcase
gdb compile failed, ld: warning: relocation error: 0xa: file /var/tmp//ccxKrUka.o: symbol gnu_ifunc: unexpected symbol referenced from file /vol/obj/gnu/gdb/gdb/11.5-amd64-local/gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-lib-0-1-1.so
ld: warning: relocation error: 0xa: file /var/tmp//ccxKrUka.o: symbol gnu_ifunc: unexpected symbol referenced from file /vol/obj/gnu/gdb/gdb/11.5-amd64-local/gdb/testsuite/outputs/gdb.base/gnu-ifunc/gnu-ifunc-lib-0-1-1.so
warning: Text relocation remains        	referenced
    against symbol		    offset	in file
gnu_ifunc                           0x55      	/var/tmp//ccxKrUka.o
gnu_ifunc                           0x81      	/var/tmp//ccxKrUka.o
UNTESTED: gdb.base/gnu-ifunc.exp: resolver_attr=0: resolver_debug=1: final_debug=1: failed to compile testcase
gdb compile failed, /vol/src/gnu/gdb/hg/master/local/gdb/testsuite/gdb.base/gnu-ifunc-lib.c:44:22: error: ifunc is not supported on this target
 __typeof (gnu_ifunc) gnu_ifunc __attribute__ ((ifunc ("gnu_ifunc_resolver")));
                      ^~~~~~~~~
UNTESTED: gdb.base/gnu-ifunc.exp: resolver_attr=1: resolver_debug=0: final_debug=0: failed to compile testcase
gdb compile failed, /vol/src/gnu/gdb/hg/master/local/gdb/testsuite/gdb.base/gnu-ifunc-lib.c:44:22: error: ifunc is not supported on this target
 __typeof (gnu_ifunc) gnu_ifunc __attribute__ ((ifunc ("gnu_ifunc_resolver")));
                      ^~~~~~~~~
UNTESTED: gdb.base/gnu-ifunc.exp: resolver_attr=1: resolver_debug=0: final_debug=1: failed to compile testcase
gdb compile failed, /vol/src/gnu/gdb/hg/master/local/gdb/testsuite/gdb.base/gnu-ifunc-lib.c:44:22: error: ifunc is not supported on this target
 __typeof (gnu_ifunc) gnu_ifunc __attribute__ ((ifunc ("gnu_ifunc_resolver")));
                      ^~~~~~~~~
UNTESTED: gdb.base/gnu-ifunc.exp: resolver_attr=1: resolver_debug=1: final_debug=0: failed to compile testcase
gdb compile failed, /vol/src/gnu/gdb/hg/master/local/gdb/testsuite/gdb.base/gnu-ifunc-lib.c:44:22: error: ifunc is not supported on this target
 __typeof (gnu_ifunc) gnu_ifunc __attribute__ ((ifunc ("gnu_ifunc_resolver")));
                      ^~~~~~~~~
UNTESTED: gdb.base/gnu-ifunc.exp: resolver_attr=1: resolver_debug=1: final_debug=1: failed to compile testcase
PASS: gdb.base/gnu-ifunc.exp: static: static gnu_ifunc

		=== gdb Summary ===

# of expected passes		1
# of untested testcases		8

It would be good to further reduce this.

Btw., shouldn't gdb.compile/compile-ifunc.exp get similar treatment?

> I see that ld/testsuite/ld-ifunc/ifunc.exp explicitly lists all the targets
> where IFUNC is expected to work, but it seemed more maintainable to me to
> tweak these gdb tests to pay attention to the return status from trying to
> build the test cases.  Is this OK to commit?

gcc has gcc/testsuite/lib/target-supports.exp (check_ifunc_available)
instead, again a compile test instead of a hardcoded list.  I wonder,
though, if it wouldn't be better to run such a check early in the two
gdb ifunc tests and skip them if that fails, rather than producing lots
of noise and UNSUPPORTED tests?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [patch, testsuite] Skip gnu-ifunc tests if building the testcase fails

[Sandra Loosemore]

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

On 09/26/2018 05:50 AM, Rainer Orth wrote:
> Hi Sandra,
> 
>> gdb.base/gnu-ifunc.exp doesn't fail gracefully on targets that don't
>> support this feature -- on nios2-linux-gnu I've seen TCL errors from trying
>> to copy the nonexistent shared library that fails to build to the target.
> 
> I just tried the patch on Solaris which also doesn't have (and never
> will have) ifunc support: unfortunately, there's still lots of noise in
> gdb.sum:
> 
> [snip]
> 
> It would be good to further reduce this.
> 
> Btw., shouldn't gdb.compile/compile-ifunc.exp get similar treatment?

Yeah, although that test is broken for other reasons in my environment.

>> I see that ld/testsuite/ld-ifunc/ifunc.exp explicitly lists all the targets
>> where IFUNC is expected to work, but it seemed more maintainable to me to
>> tweak these gdb tests to pay attention to the return status from trying to
>> build the test cases.  Is this OK to commit?
> 
> gcc has gcc/testsuite/lib/target-supports.exp (check_ifunc_available)
> instead, again a compile test instead of a hardcoded list.  I wonder,
> though, if it wouldn't be better to run such a check early in the two
> gdb ifunc tests and skip them if that fails, rather than producing lots
> of noise and UNSUPPORTED tests?

Here's a revised patch that adapts the same compile test that gcc uses. 
Is this one OK?

-Sandra

[-- Attachment #2: ifunc.patch --]
[-- Type: text/x-patch, Size: 3779 bytes --]

commit 03ae8dd2f9d2812b8d2744e8d78a3f6c4c65ed9b
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Wed Sep 26 19:14:14 2018 -0700

    Skip ifunc tests if no target support.
    
    2018-09-26  Sandra Loosemore  <sandra@codesourcery.com>
    
    	* lib/gdb.exp (skip_ifunc_tests): New.
    	* gdb.base/gnu-ifunc.exp: Skip if no ifunc support.  Handle
    	other compile failures.
    	* gdb.compile/compile-ifunc.exp: Skip if no ifunc support.

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 2b4b097..c30db8f 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2018-09-26  Sandra Loosemore  <sandra@codesourcery.com>
+
+	* lib/gdb.exp (skip_ifunc_tests): New.
+	* gdb.base/gnu-ifunc.exp: Skip if no ifunc support.  Handle
+	other compile failures.
+	* gdb.compile/compile-ifunc.exp: Skip if no ifunc support.
+
 2018-09-26  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.base/large-frame-1.c: New file.
diff --git a/gdb/testsuite/gdb.base/gnu-ifunc.exp b/gdb/testsuite/gdb.base/gnu-ifunc.exp
index d6ec698..322de5a 100644
--- a/gdb/testsuite/gdb.base/gnu-ifunc.exp
+++ b/gdb/testsuite/gdb.base/gnu-ifunc.exp
@@ -17,6 +17,10 @@ if {[skip_shlib_tests]} {
     return 0
 }
 
+if {[skip_ifunc_tests]} {
+    return 0
+}
+
 standard_testfile .c
 set staticexecutable ${testfile}-static
 set staticbinfile [standard_output_file ${staticexecutable}]
@@ -365,9 +369,10 @@ proc misc_tests {resolver_attr resolver_debug final_debug} {
 foreach_with_prefix resolver_attr {0 1} {
     foreach_with_prefix resolver_debug {0 1} {
 	foreach_with_prefix final_debug {0 1} {
-	    build $resolver_attr $resolver_debug $final_debug
-	    misc_tests $resolver_attr $resolver_debug $final_debug
-	    set-break $resolver_attr $resolver_debug $final_debug
+	    if { [build $resolver_attr $resolver_debug $final_debug] != 0 } {
+		misc_tests $resolver_attr $resolver_debug $final_debug
+		set-break $resolver_attr $resolver_debug $final_debug
+	    }
 	}
     }
 }
diff --git a/gdb/testsuite/gdb.compile/compile-ifunc.exp b/gdb/testsuite/gdb.compile/compile-ifunc.exp
index 979e391..bb3af4d 100644
--- a/gdb/testsuite/gdb.compile/compile-ifunc.exp
+++ b/gdb/testsuite/gdb.compile/compile-ifunc.exp
@@ -13,6 +13,10 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+if {[skip_ifunc_tests]} {
+    return 0
+}
+
 standard_testfile
 
 with_test_prefix "nodebug" {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index f32abfe..2cdc80a 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2842,6 +2842,34 @@ gdb_caching_proc has_int128_cxx {
     return [gdb_int128_helper c++]
 }
 
+# Return true if the IFUNC feature is unsupported.
+gdb_caching_proc skip_ifunc_tests {
+    # Set up, compile, and execute a test program.
+    # Include the current process ID in the file names to prevent conflicts
+    # with invocations for multiple testsuites.
+    set src [standard_temp_file ifunc[pid].c]
+    set obj [standard_temp_file ifunc[pid].o]
+
+    verbose -log "checking for ifunc support"
+    gdb_produce_source $src {
+	extern void f_ ();
+	typedef void F (void);
+	F* g (void) { return &f_; }
+	void f () __attribute__ ((ifunc ("g")));
+    }
+
+    set lines [gdb_compile $src $obj object {quiet}]
+    file delete $src
+    file delete $obj
+
+    if ![string match "" $lines] then {
+        verbose -log "ifunc testfile compilation failed"
+        return 1
+    }
+    verbose -log "ifunc testfile compilation successful"
+    return 0
+}
+
 # Return whether we should skip tests for showing inlined functions in
 # backtraces.  Requires get_compiler_info and get_debug_format.
 

Re: [patch, testsuite] Skip gnu-ifunc tests if building the testcase fails

[Tom Tromey]

>>>>> "Sandra" == Sandra Loosemore <sandra@codesourcery.com> writes:

Sandra> Here's a revised patch that adapts the same compile test that gcc
Sandra> uses. Is this one OK?

Thanks for the patch.

Sandra> +# Return true if the IFUNC feature is unsupported.
Sandra> +gdb_caching_proc skip_ifunc_tests {
Sandra> +    # Set up, compile, and execute a test program.
Sandra> +    # Include the current process ID in the file names to prevent conflicts
Sandra> +    # with invocations for multiple testsuites.
Sandra> +    set src [standard_temp_file ifunc[pid].c]
Sandra> +    set obj [standard_temp_file ifunc[pid].o]
Sandra> +
Sandra> +    verbose -log "checking for ifunc support"
Sandra> +    gdb_produce_source $src {
Sandra> +	extern void f_ ();
Sandra> +	typedef void F (void);
Sandra> +	F* g (void) { return &f_; }
Sandra> +	void f () __attribute__ ((ifunc ("g")));
Sandra> +    }
Sandra> +
Sandra> +    set lines [gdb_compile $src $obj object {quiet}]
Sandra> +    file delete $src
Sandra> +    file delete $obj


I wonder whether this could use gdb_can_simple_compile.  A recent patch
changed a bunch of other caching procs to use this helper function
instead.

I didn't examine it in detail, though, so if there is some reason it
can't be used, that's fine.

But if it can be used, this is ok with that change.

Tom