[PATCH] ld: testsuite: Add sframe test for PR 33401

[Claudiu Zissulescu]

Hello Jens,

On 12/11/25 11:31 AM, Jens Remus wrote:
> Hello Claudiu!
> 
> On 12/10/2025 8:06 PM, claudiu.zissulescu-ianculescu--- via Binutils wrote:
>> From: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@oracle.com>
>>
>> When linking for a relocable output file (-r), one or more R_*_NONE
>> relocations may be generated for .sframe section. Two new tcl
>> procedures are added to sframe.exp file. 'check-dump' is checking if
>> an input bin file has the same relocation as specified in the second
>> input argument. 'check_pr33401' is the main checking function for
>> PR33401 which calls twice the ld tool to produce an relocable output
>> file.
> 
>> diff --git a/ld/testsuite/ld-sframe/sframe.exp b/ld/testsuite/ld-sframe/sframe.exp
> 
>> +# Test infrastructure for bug 33401
>> +# https://sourceware.org/bugzilla/show_bug.cgi?id=33401
>> +# Sframe section contains R_*_NONE relocations intermingled with other
>> +# relas in the output relocatable object.
>> +#
>> +# src_files filenames of assembler files
>> +proc check_pr33401 { src_files } {
>> +    global ld
>> +    global CXX_FOR_TARGET
>> +    global as
>> +    global LDFLAGS
>> +    global srcdir
>> +    global subdir
>> +    global env
>> +    global runtests
>> +
>> +    set objfiles {}
>> +    set testname "PR33401"
>> +    set failed 0
>> +    set linkfile "tmpdir/automa_module.o"
>> +
>> +    if ![runtest_file_p $runtests $testname] then {

This if-statement is used to skip this test on demand.

> 
> 	unresolved $testname
> 
> To report that the test could not be run?
The above construction is used in other places as well, and I haven't 
seen that it returns an error (unresolved is an error).

> 
>> +	return 0
>> +    }
>> +
>> +    # Check to see if C++ compiler work
>> +    if { [which $CXX_FOR_TARGET] == 0 } {
> 
> 	unsupported $testname
> 
> To report that that test requirements were not met?

It can be added.

> 
>> +	return 0
>> +    }
>> +
>> +    # Assemble each file in the test.
>> +    foreach src_file $src_files {
>> +	set fileroot "[file rootname [file tail $src_file]]"
>> +	set objfile "tmpdir/$fileroot.o"
>> +
>> +	if { [file extension $src_file] == ".cpp" } {
>> +	    set as_file "tmpdir/$fileroot.s"
>> +	    if ![ld_compile "$CXX_FOR_TARGET -S -O2 -w" $srcdir/$subdir/$src_file $as_file] {
>> +		set failed 1
>> +		break
>> +	    }
>> +	} else {
>> +	    set as_file "$srcdir/$subdir/$src_file"
>> +	}
>> +	if { ![ld_assemble $as "--gsframe $as_file" $objfile] } {
>> +	    set failed 1
>> +	    break
>> +	}
>> +	lappend objfiles $objfile
>> +    }
>> +
>> +    # Catch assembler errors.
>> +    if { $failed } {
>> +	verbose -log "Error during assembling one if the input files."
>> +	unresolved $testname
>> +	return 0
>> +    }
>> +
>> +    # Do the first linking. If this fails, we cannot resolve the test.
>> +    if { ![ld_link $ld $linkfile "-L$srcdir/$subdir -r $objfiles"] } {
>> +	verbose -log "Error during linking assembled objects."
>> +	unresolved $testname
>> +	return 0
>> +    }
>> +
>> +    # Check the output of the first ld invocation
>> +    if { [check_dump $linkfile pr33401.rd] } {
>> +	verbose -log "No R_*_NONE in .rela.sframe"
>> +	unresolved $testname
>> +	return 0
>> +    }
>> +
>> +    # Check if the second ld -r invocation is ok
>> +    if { ![ld_link $ld /dev/null "-L$srcdir/$subdir -r $linkfile"] } {
>> +	fail $testname
>> +    } else {
>> +	pass $testname
>> +    }
>> +}
>> +
>>   if [skip_sframe_tests] {
>>       unsupported "no SFrame format support in the assembler, or SFrame disabled"
> 
> How can SFrame be disabled?  skip_sframe_tests currently tests for
> whether the assembler supports --gsframe.  There are comments that
> read as if the skip_sframe_tests should be extended to test for
> whether libsframe is available.
> 
> Shouldn't unsupported be invoked with the test name as argument?
> 
>       unsupported $testname
> 

It is not part of my patch. Probably, we can address the above concern 
in a different patch :)

>>       return 0
>> @@ -40,6 +157,8 @@ foreach sframe_test $sframe_test_list {
>>       run_dump_test [file rootname $sframe_test]
>>   }
>>   
>> +check_pr33401 {StateClient.cpp  StatePlaying.cpp}
>> +
>>   if {[info exists old_lc_all]} {
>>       set env(LC_ALL) $old_lc_all
>>   } else {
> 
Best,
Claudiu