[PATCH 1/1] sframe: Use UNRESOLVED instead of FAIL
Indu Bhagat
indu.bhagat@oracle.com
Tue Jan 20 00:40:22 GMT 2026
On 1/16/26 6:57 AM, claudiu.zissulescu-ianculescu@oracle.com wrote:
> From: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@oracle.com>
>
> Fix for PR ld/33784.
>
> In the case of PR ld/33401, we need to link twice. First time, the
> linker process generates R_*_NONE relocs for .sframe section, while
> the second link process should pass without any issue. I have reworked
> check_pr33401 by removing the call to run_cc_link_tests, by handling
> the compilation, assembling and linking processes locally.
>
Thanks Claudiu for taking care of this.
May be some more in the commit log. Something like:
The SFrame test for PR ld/33401 is a two-step test where the second link
depends on the output of the first. This patch reworks the test to use
manual compilation and linking. It introduces the UNRESOLVED status if
the setup cannot be completed and UNTESTED if the first stage doesn't
produce any R_*_NONE to test the original bug.
> ld/
>
> PR ld/33784
> * testsuite/ld-sframe/sframe.exp (check_pr33401): Rework the
> procedure for UNRESOLVED and UNTESTED cases.
> (check_dump): New procedure.
> * testsuite/ld-sframe/pr33401.rd: Add extra line at the end of the
> file.
>
> Signed-off-by: Claudiu Zissulescu <claudiu.zissulescu-ianculescu@oracle.com>
> ---
> ld/testsuite/ld-sframe/pr33401.rd | 3 +-
> ld/testsuite/ld-sframe/sframe.exp | 112 +++++++++++++++++++++++++-----
> 2 files changed, 95 insertions(+), 20 deletions(-)
>
> diff --git a/ld/testsuite/ld-sframe/pr33401.rd b/ld/testsuite/ld-sframe/pr33401.rd
> index 00142818b34..455a9d6efcf 100644
> --- a/ld/testsuite/ld-sframe/pr33401.rd
> +++ b/ld/testsuite/ld-sframe/pr33401.rd
> @@ -1,3 +1,4 @@
> #...
> [0-9a-f]+ +0+ +R_.*_NONE +.*
> -#pass
> \ No newline at end of file
> +#pass
> +
> diff --git a/ld/testsuite/ld-sframe/sframe.exp b/ld/testsuite/ld-sframe/sframe.exp
> index ab386d7ca65..32714016991 100644
> --- a/ld/testsuite/ld-sframe/sframe.exp
> +++ b/ld/testsuite/ld-sframe/sframe.exp
> @@ -23,43 +23,117 @@
> # Sframe section contains R_*_NONE relocations intermingled with other
> # relas in the output relocatable object.
> #
> +proc check_dump { binfile dumpfile } {
> + global srcdir
> + global READELF
> + global subdir
> + global env
> + global runtests
> +
> + set binary $READELF
> + set progopts "-j .rela.sframe"
> + set failed 0
> +
Why this if the var failed is not used here ?
> + # Ensure consistent sorting of symbols
> + if {[info exists env(LC_ALL)]} {
> + set old_lc_all $env(LC_ALL)
> + }
> + set env(LC_ALL) "C"
> + set cmd "$binary $progopts $binfile > dump.out"
> + send_log "$cmd\n"
> + catch "exec $cmd" comp_output
> + if {[info exists old_lc_all]} {
> + set env(LC_ALL) $old_lc_all
> + } else {
> + unset env(LC_ALL)
> + }
> + set comp_output [prune_warnings $comp_output]
> + if ![string match "" $comp_output] then {
> + send_log "$comp_output\n"
> + return 1
> + }
> +
> + if { [regexp_diff "dump.out" "$srcdir/$subdir/$dumpfile"] } then {
> + verbose -log "output is [file_contents "dump.out"]" 2
> + return 1
> + }
> + return 0
> +}
> +
> proc check_pr33401 { } {
> global CXX_FOR_TARGET
> global ld
> global srcdir
> global subdir
> + global as
> + global LDFLAGS
> + global env
>
> - # Check to see if C++ compiler work
> - if { [which $CXX_FOR_TARGET] == 0 } {
> - unsupported "PR ld/33401"
> - return 0
> + set objfiles {}
> + set testname "PR ld/33401"
> + set failed 0
> + set linkfile "tmpdir/pr33401.o"
> +
> + set src_files [list "StateClient.cpp" "StatePlaying.cpp"]
> + set failed 0
> +
Redundant set failed 0 ?
> + # Check to see if the C and C++ compilers work
> + if { ![check_compiler_available] || [which $CXX_FOR_TARGET] == 0 } {
> + unsupported $testname
> + return
> }
>
> # 1. First step: Create a relocatable object (pr33401.o) from sources.
> # Some R_*_NONE are expected for this input.
> - set test_list {
> - { "PR ld/33401 (Step 1: Create relocatable object and check R_*_NONE)"
> - "-r"
> - "-g -O2 -finline -w -Wa,--gsframe"
> - {StateClient.cpp StatePlaying.cpp}
> - {{readelf "-r -W" pr33401.rd}}
> - "pr33401.o"
> - "c++" }
I think a small comment regarding why run_cc_link_tests cannot be used
directly and a mention of PR ld/33784 in the comment will be good to keep.
> + 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"
> + set cmd "$CXX_FOR_TARGET -S -g -O2 -w"
You dropped -inline from cflags. Is that intentional?
It was added by the following commit to address the case when user
cflags include -fno-inline, affected the coverage of this test.
commit ba038bba603d18b431ad1b69505553881b8a39c7
Author: Alan Modra <amodra@gmail.com>
Date: Sat Dec 13 09:30:46 2025 +1030
Re: ld: testsuite: Add sframe test for PR 33401
If user CXXFLAGS include -fno-inline the test fails with
FAIL: PR ld/33401 (Step 1: Create relocatable object and check
R_*_NONE)
This should not be a FAIL. If the compiler does not generate R_*_NONE
(a good thing!) then it should be UNTESTED, which means the readelf
check can't be done by run_cc_link_tests. I haven't made that change
in this patch, just worked around a user -fno-inline.
> + if ![ld_compile $cmd $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 previous action errors.
> + if { $failed } {
> + verbose -log "Error during compiling/assembling one of the input files."
> + unresolved $testname
> + return
> + }
> +
> + # 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
> }
>
> - # Run the test
> - if { [run_cc_link_tests $test_list] == 0 } {
> + # Check the output of the first ld invocation
> + if { [check_dump $linkfile pr33401.rd] } {
> verbose -log "No R_*_NONE in .rela.sframe"
> + untested $testname
> return
> }
>
> + # Clear error and warning counts.
> + reset_vars
> +
> # 2. Final step: Run ld -r again on the output of step 1.
> # The assertion failure in PR 33401 occurred when processing input BFDs
> # containing R_*_NONE relocations in the .sframe section.
> - set testname "PR ld/33401 (Step 2: Final link)"
> -
> - # Attempt to link the object created in step 1
> - if { ![ld_link $ld /dev/null "-L$srcdir/$subdir -r tmpdir/pr33401.o"] } {
> + if { ![ld_link $ld /dev/null "-L$srcdir/$subdir -r $linkfile"] } {
> fail $testname
> } else {
> pass $testname
> @@ -95,7 +169,7 @@ if { [check_compiler_available] } {
> run_cc_link_tests [list \
> [list \
> "Build pr32789-1a.o pr32789-1b.o" \
> - "-r" \
> + "-r -nostdlib" \
This change is unrelated to 33784. IIUC, this is because it is
safer/sensible to do. Right ?
https://sourceware.org/pipermail/binutils/2026-January/147439.html
Also, did you intend to use -nostdlib for the check_pr33401 too then ?
May be do this hunk for 32789 in a separate commit with ref to the
mailing list
https://sourceware.org/pipermail/binutils/2026-January/147439.html
Up to you though, if you prefer to commit as one commit, please do leave
a note in the commit log about this hunk being unrelated to 33784.
> "-Wa,--gsframe" \
> { pr32789-1a.c pr32789-1b.c } \
> {{readelf --sframe pr32789-1.sd}
More information about the Binutils
mailing list