[PATCH v2] gas: Move gas_sframe_check to binutils-common.exp

[Indu Bhagat]

On 7/13/25 4:34 PM, Alan Modra wrote:
> I'll apply the following in a little while to give you and Indu time
> to comment.
> 
> Correct TCL errors trying to access error output file in commit
> ef7a634dc01d.  In fact, get rid of the output file test entirely since
> gas exit status is sufficient.
> 
> Also there is no need to firstly check for ELF support.
> 
> Set check_as_sframe_result, and remove ld-lib.exp check_as_sframe.
> 

Looks good to me.

Meanwhile, one question below.

> diff --git a/binutils/testsuite/lib/binutils-common.exp b/binutils/testsuite/lib/binutils-common.exp
> index d024bc55150..2da7138e211 100644
> --- a/binutils/testsuite/lib/binutils-common.exp
> +++ b/binutils/testsuite/lib/binutils-common.exp
> @@ -482,10 +482,6 @@ proc supports_dt_relr {} {
>   
>   # Whether a target assembler supports --gsframe.
>   proc gas_sframe_check {} {
> -    if { ![is_elf_format] } then {
> -	return 0;
> -    }
> -
>       global check_as_sframe_result
>       global AS
>       global ASFLAGS
> @@ -502,23 +498,16 @@ proc gas_sframe_check {} {
>       remote_download host $as_file
>       verbose -log "Checking SFrame support in AS:"
>   
> -    set old_ASFLAGS "$ASFLAGS"
> -    set ASFLAGS "$ASFLAGS --gsframe"
> -
> -    global comp_output
> +    set status [remote_exec host "$AS $ASFLAGS --gsframe $as_file"]
>   
> -    set output_file "tmpdir/check_as_sframe.out"
> -    set status [remote_exec host "$AS $ASFLAGS $as_file" "2> $output_file"]
> -    set comp_output [file_contents "$output_file"]
> -    set ASFLAGS "$old_ASFLAGS"
> -
> -    if { ![string match "" $comp_output] } then {
> +    if { [lindex $status 0] != 0 } then {
>   	verbose -log "SFrame not supported in AS"
> -	return 0
> +	set check_as_sframe_result 0
>       } else {
>   	verbose -log "SFrame supported in AS"
> -	return 1
> +	set check_as_sframe_result 1
>       }
> +    return $check_as_sframe_result
>   }
>   
>   # get_relative_path FROM TO
> diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
> index 119410bc523..22d2f987b87 100644
> --- a/ld/testsuite/lib/ld-lib.exp
> +++ b/ld/testsuite/lib/ld-lib.exp
> @@ -1668,36 +1668,6 @@ proc skip_ctf_tests { } {
>       return 1
>   }
>   
> -# Check if the assembler supports SFrame.
> -
> -proc check_as_sframe { } {
> -    global check_as_sframe_result
> -    global as
> -    if [info exists check_as_sframe_result] {
> -	return $check_as_sframe_result
> -    }
> -
> -    # SFrame generation needs CFI support
> -    if { ![check_as_cfi] } {
> -	set check_as_sframe_result 0;
> -	return 0
> -    }
> -

Now I am not sure why this safety check was added to begin with (can 
check_as_cfi even fail for gas ?

> -    set as_file "tmpdir/check_as_sframe.s"
> -    set as_fh [open $as_file w 0666]
> -    puts $as_fh "# Generated file. DO NOT EDIT"
> -    puts $as_fh "\t.cfi_sections \".sframe\""
> -    puts $as_fh "\t.cfi_startproc"
> -    puts $as_fh "\t.cfi_endproc"
> -    close $as_fh
> -    remote_download host $as_file
> -    verbose -log "Checking SFrame:"
> -    set success [ld_assemble $as $as_file "/dev/null"]
> -    #remote_file host delete $as_file
> -    set check_as_sframe_result $success
> -    return $success
> -}
> -
>   proc skip_sframe_tests { } {
>   # FIXME TODO
>   #    global enable_libsframe
> @@ -1706,7 +1676,7 @@ proc skip_sframe_tests { } {
>   #	return 1
>   #    }
>   
> -    if [check_as_sframe] {
> +    if [gas_sframe_check] {
>   	return 0
>       }
>   
>