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

login
register
mail settings
Submitter Sandra Loosemore
Date Sept. 27, 2018, 2:20 a.m.
Message ID <bbbef432-bab0-d928-1c36-7b6aa3e5ac21@codesourcery.com>
Download mbox | patch
Permalink /patch/29558/
State New
Headers show

Comments

Sandra Loosemore - Sept. 27, 2018, 2:20 a.m.
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
Tom Tromey - Sept. 27, 2018, 8:30 p.m.
>>>>> "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

Patch

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.