[PATCH 1/2] Add lld(linker) specific option.

[PATCH 1/2] Add lld(linker) specific option.

[Balasubrmanian, Vignesh]

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


Please review the attached patch.

LLD doesn't have the option "-Ttext-segment" but "--image-base".
So first try using "-Ttext-segment". If it fails, try the lld option "--image-base" before failing.

Thanks,
vigneshbalu



[-- Attachment #2: 0001-Add-lld-linker-specific-option.patch --]
[-- Type: application/octet-stream, Size: 1997 bytes --]

From 2517f8889d7371602e38b64e4a5264eda1f6fae4 Mon Sep 17 00:00:00 2001
From: Vignesh Balasubramanian <Vignesh.Balasubrmanian@amd.com>
Date: Thu, 17 Mar 2022 14:56:52 +0530
Subject: [PATCH 1/2] Add lld(linker) specific option.

LLD doesn't have option "-Ttext-segment" but "--image-base".
So first try using "-Ttext-segment". If it fails, try the lld
option "--image-base" before failing.
---
 gdb/testsuite/lib/jit-elf-helpers.exp | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
index af70b11644c..1ec295c54bd 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -92,15 +92,26 @@ proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile count}
 	# in the compiled shared library against a fixed base address.  Combined
 	# with mapping the resulting binary to the same fixed base it allows
 	# to dynamically execute functions from it without any further adjustments.
-	set options [list \
+	set options_ld [list \
 	    additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
 	    additional_flags=-Xlinker \
 	    additional_flags=-Ttext-segment=$addr]
+
+	# Use "--image-base" instead of "-Ttext-segment" for LLD linker.
+	set options_lld [list \
+            additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
+            additional_flags=-Xlinker \
+            additional_flags=--image-base=$addr]
+
+
 	if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} \
-		  $options] != "" } {
-	    set f [file tail $binfile]
-	    untested "failed to compile shared library $f"
-	    return -1
+		  $options_ld] != "" } {
+	    if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} \
+		  $options_lld] != "" } {
+		set f [file tail $binfile]
+		untested "failed to compile shared library $f"
+		return -1
+	    }
 	}
 
 	set path [gdb_remote_download target ${binfile}]
-- 
2.17.1

Re: [PATCH 1/2] Add lld(linker) specific option.

[Simon Marchi]

On 2022-03-21 08:15, Balasubrmanian, Vignesh via Gdb-patches wrote:
>
> Please review the attached patch.
>
> LLD doesn't have the option "-Ttext-segment" but "--image-base".
> So first try using "-Ttext-segment". If it fails, try the lld option "--image-base" before failing.
>
> Thanks,
> vigneshbalu
>
>

Hi,

The approach looks ok to me.

Grepping for "text-segment", I see three other tests using
-Ttext-segment, are we going to need to fix those up too?

 - gdb.base/execl-update-breakpoints.exp
 - gdb.arch/arm-bl-branch-dest.exp
 - gdb.threads/step-over-exec.exp

If so, it would be interesting to make this an option to gdb_compile,
instead of specifying a linker-specific flag.  For example, you'd use:

       set options [list \
            additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
	    text_segment=$addr]

and text_segment would be recognized by gdb_compile and the complexity
would be hidden there.

Can you mention (here and in the commit message) the full "make check"
line you use to test this?

Simon

RE: [PATCH 1/2] Add lld(linker) specific option.

[Balasubrmanian, Vignesh]

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

Simon,

Thanks for reviewing.
I was trying to confine the fix to the test case for better readability.
Modified as you suggested and fixed other test cases as well (couldn’t verify the arm test case due to machine unavailability)

Failing make check command:
make check RUNTESTFLAGS="--all -v -v -v GDB='${GDB_INSTALL_DIR}/bin/gdb' CFLAGS_FOR_TARGET='-w -gdwarf-4' CXXFLAGS_FOR_TARGET='-w -gdwarf-4' CPPFLAGS_FOR_TARGET='-w -gdwarf-4' CC_FOR_TARGET='clang'  CXX_FOR_TARGET='clang++'" TESTS="gdb.base/jit-elf.exp"

LLD Error:
ld.lld: error: -Ttext-segment is not supported. Use --image-base if you intend to set the base address

thanks,
vigneshbalu.


-----Original Message-----
From: Simon Marchi <simark@simark.ca> 
Sent: Monday, March 21, 2022 7:15 PM
To: Balasubrmanian, Vignesh <Vignesh.Balasubrmanian@amd.com>; gdb-patches@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>; Kumar N, Bhuvanendra <Bhuvanendra.KumarN@amd.com>
Subject: Re: [PATCH 1/2] Add lld(linker) specific option.

[CAUTION: External Email]

On 2022-03-21 08:15, Balasubrmanian, Vignesh via Gdb-patches wrote:
>
> Please review the attached patch.
>
> LLD doesn't have the option "-Ttext-segment" but "--image-base".
> So first try using "-Ttext-segment". If it fails, try the lld option "--image-base" before failing.
>
> Thanks,
> vigneshbalu
>
>

Hi,

The approach looks ok to me.

Grepping for "text-segment", I see three other tests using -Ttext-segment, are we going to need to fix those up too?

 - gdb.base/execl-update-breakpoints.exp
 - gdb.arch/arm-bl-branch-dest.exp
 - gdb.threads/step-over-exec.exp

If so, it would be interesting to make this an option to gdb_compile, instead of specifying a linker-specific flag.  For example, you'd use:

       set options [list \
            additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
            text_segment=$addr]

and text_segment would be recognized by gdb_compile and the complexity would be hidden there.

Can you mention (here and in the commit message) the full "make check"
line you use to test this?

Simon

[-- Attachment #2: 0001-Add-lld-linker-specific-option.patch --]
[-- Type: application/octet-stream, Size: 6758 bytes --]

From b7825bd24fb55b6bd80c1eaa192399e9632a2735 Mon Sep 17 00:00:00 2001
From: Vignesh Balasubramanian <Vignesh.Balasubrmanian@amd.com>
Date: Tue, 22 Mar 2022 17:41:22 +0530
Subject: [PATCH 1/2] Add lld(linker) specific option.

LLD doesn't have option "-Ttext-segment" but "--image-base".
So, verify the available option by compiling simple test case
and use the same.
make check Command:
make check RUNTESTFLAGS="--all -v -v -v GDB='${GDB_INSTALL_DIR}/bin/gdb'
CFLAGS_FOR_TARGET='-w -gdwarf-4' CXXFLAGS_FOR_TARGET='-w -gdwarf-4'
CPPFLAGS_FOR_TARGET='-w -gdwarf-4' CC_FOR_TARGET='clang'
CXX_FOR_TARGET='clang'" TESTS="gdb.base/jit-elf.exp"
---
 gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp |  2 +-
 .../gdb.base/execl-update-breakpoints.exp     | 19 +++---------
 gdb/testsuite/gdb.threads/step-over-exec.exp  |  4 +--
 gdb/testsuite/lib/gdb.exp                     | 31 ++++++++++++++++++-
 gdb/testsuite/lib/jit-elf-helpers.exp         |  5 ++-
 5 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp b/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
index cebeb629a9f..34d98aec96e 100644
--- a/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
+++ b/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
@@ -26,7 +26,7 @@ standard_testfile
 # the "-Wl,-Ttext-segment" option compile the binary.
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
-    [list debug ldflags=-Wl,-Ttext-segment=0xb0000000]] } {
+    [list debug text_segment=0xb0000000]] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.base/execl-update-breakpoints.exp b/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
index 8a9bdf66216..bae48ee4c51 100644
--- a/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
+++ b/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
@@ -32,20 +32,11 @@ if { [gdb_compile [file join $srcdir $subdir $srcfile] $objfile \
     return -1
 }
 
-set opts1_ld [list debug ldflags=-Wl,-Ttext-segment=0x1000000]
-set opts1_gold [list debug ldflags=-Wl,-Ttext=0x1000000]
-set opts2_ld [list debug ldflags=-Wl,-Ttext-segment=0x2000000]
-set opts2_gold [list debug ldflags=-Wl,-Ttext=0x2000000]
-
-if { [gdb_compile $objfile $exec1 executable $opts1_ld] != "" } {
-    # Old gold linker versions don't support -Ttext-segment.  Fall
-    # back to -Ttext.
-    if { [gdb_compile $objfile $exec1 executable $opts1_gold] != ""
-	 || [gdb_compile $objfile $exec2 executable $opts2_gold] != ""} {
-	untested "link failed"
-	return -1
-    }
-} elseif { [gdb_compile $objfile $exec2 executable $opts2_ld] != "" } {
+set opts1_ld [list debug text_segment=0x1000000]
+set opts2_ld [list debug text_segment=0x2000000]
+
+if { [gdb_compile $objfile $exec1 executable $opts1_ld] != ""
+     || [gdb_compile $objfile $exec2 executable $opts2_ld] != ""} {
     untested "link failed"
     return -1
 }
diff --git a/gdb/testsuite/gdb.threads/step-over-exec.exp b/gdb/testsuite/gdb.threads/step-over-exec.exp
index 9e252e65f97..783f865585c 100644
--- a/gdb/testsuite/gdb.threads/step-over-exec.exp
+++ b/gdb/testsuite/gdb.threads/step-over-exec.exp
@@ -47,8 +47,8 @@ proc do_test { execr_thread different_text_segments displaced_stepping } {
     set execd_opts [list debug]
 
     if { $different_text_segments } {
-	lappend execr_opts "ldflags=-Wl,-Ttext-segment=0x600000"
-	lappend execd_opts "ldflags=-Wl,-Ttext-segment=0x800000"
+	lappend execr_opts "text_segment=0x600000"
+	lappend execd_opts "text_segment=0x800000"
     }
 
     if { $execr_thread == "leader" } {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 729bded2950..b015419c5fa 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4478,10 +4478,22 @@ proc gdb_compile {source dest type options} {
 	} elseif { $opt == "getting_compiler_info" } {
 	    # If this is set, calling test_compiler_info will cause recursion.
 	    set getting_compiler_info 1
+        } elseif {[regexp "^text_segment=" $opt]} {
+            regsub "^text_segment=" $opt "" addr
+            if { [have_text_segment] == 1} {
+                # for linker "LD"
+                lappend new_options "ldflags=-Wl,-Ttext-segment=$addr"
+            } elseif { [have_image_base] == 1 } {
+                # for linker "LLD"
+                lappend new_options "ldflags=-Wl,--image-base=$addr"
+            } else {
+                # old linker version
+                lappend new_options "ldflags=-Wl,-Ttext=$addr"
+            }
         } else {
             lappend new_options $opt
         }
-    }
+	}
 
     # Ensure stack protector is disabled for GCC, as this causes problems with
     # DWARF line numbering.
@@ -8240,6 +8252,23 @@ gdb_caching_proc have_fuse_ld_gold {
     return [gdb_simple_compile $me $src executable $flags]
 }
 
+# Return 1 if linker supports -Ttext-segment, otherwise return 0.
+gdb_caching_proc have_text_segment {
+    set me "have_text_segment"
+    set flags additional_flags="-Wl,-Ttext-segment=0x7000000"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
+# Return 1 if linker supports --image-base, otherwise 0.
+gdb_caching_proc have_image_base {
+    set me "have_image_base"
+    set flags additional_flags="-Wl,--image-base=0x7000000"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
+
 # Return 1 if compiler supports scalar_storage_order attribute, otherwise
 # return 0.
 gdb_caching_proc supports_scalar_storage_order_attribute {
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
index af70b11644c..4d6a516762d 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -88,14 +88,13 @@ proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile count}
 	# wouldn't work for .debug sections.  Also, output for "info
 	# function" changes when debug info is present.
 	set addr [format 0x%x [expr $jit_load_address + $jit_load_increment * [expr $i-1]]]
-	# Using -Ttext-segment flag to ask linked to relocate everything
+	# Using -Ttext-segment or --image-base flag to ask linked to relocate everything
 	# in the compiled shared library against a fixed base address.  Combined
 	# with mapping the resulting binary to the same fixed base it allows
 	# to dynamically execute functions from it without any further adjustments.
 	set options [list \
 	    additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
-	    additional_flags=-Xlinker \
-	    additional_flags=-Ttext-segment=$addr]
+	    text_segment=$addr]
 	if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} \
 		  $options] != "" } {
 	    set f [file tail $binfile]
-- 
2.17.1

Re: [PATCH 1/2] Add lld(linker) specific option.

[Luis Machado]

On 3/22/22 16:21, Balasubrmanian, Vignesh via Gdb-patches wrote:
> Simon,
> 
> Thanks for reviewing.
> I was trying to confine the fix to the test case for better readability.
> Modified as you suggested and fixed other test cases as well (couldn’t verify the arm test case due to machine unavailability)

I checked on arm and I didn't notice any changes to the results. The 
test seems to have a few fails when LLVM is used, but those were already 
there it seems.

Re: [PATCH 1/2] Add lld(linker) specific option.

[Simon Marchi]

On 2022-03-22 12:21, Balasubrmanian, Vignesh via Gdb-patches wrote:
> Simon,
> 
> Thanks for reviewing.
> I was trying to confine the fix to the test case for better readability.
> Modified as you suggested and fixed other test cases as well (couldn’t verify the arm test case due to machine unavailability)
> 
> Failing make check command:
> make check RUNTESTFLAGS="--all -v -v -v GDB='${GDB_INSTALL_DIR}/bin/gdb' CFLAGS_FOR_TARGET='-w -gdwarf-4' CXXFLAGS_FOR_TARGET='-w -gdwarf-4' CPPFLAGS_FOR_TARGET='-w -gdwarf-4' CC_FOR_TARGET='clang'  CXX_FOR_TARGET='clang++'" TESTS="gdb.base/jit-elf.exp"
> 
> LLD Error:
> ld.lld: error: -Ttext-segment is not supported. Use --image-base if you intend to set the base address
> 
> thanks,
> vigneshbalu.

> From b7825bd24fb55b6bd80c1eaa192399e9632a2735 Mon Sep 17 00:00:00 2001
> From: Vignesh Balasubramanian <Vignesh.Balasubrmanian@amd.com>
> Date: Tue, 22 Mar 2022 17:41:22 +0530
> Subject: [PATCH 1/2] Add lld(linker) specific option.
> 
> LLD doesn't have option "-Ttext-segment" but "--image-base".
> So, verify the available option by compiling simple test case
> and use the same.
> make check Command:
> make check RUNTESTFLAGS="--all -v -v -v GDB='${GDB_INSTALL_DIR}/bin/gdb'
> CFLAGS_FOR_TARGET='-w -gdwarf-4' CXXFLAGS_FOR_TARGET='-w -gdwarf-4'
> CPPFLAGS_FOR_TARGET='-w -gdwarf-4' CC_FOR_TARGET='clang'
> CXX_FOR_TARGET='clang'" TESTS="gdb.base/jit-elf.exp"

Does your clang use lld by default?  Mine (clang packages on Arch Linux
and Ubuntu) uses GNU ld (/usr/bin/ld) by default.  I can force it to use
lld using this though:

  $ make check TESTS="gdb.base/jit-elf.exp" RUNTESTFLAGS="CC_FOR_TARGET=clang LDFLAGS_FOR_TARGET=-fuse-ld=lld"

Like Luis said, I see some failures when running the test with the lld
linker, do you see those too?  If so, it would be good to mention it in
the commit message, so that others aren't surprised to see the test case
fail.  Getting the test case to compile is already an improvement.

> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4478,10 +4478,22 @@ proc gdb_compile {source dest type options} {
>  	} elseif { $opt == "getting_compiler_info" } {
>  	    # If this is set, calling test_compiler_info will cause recursion.
>  	    set getting_compiler_info 1
> +        } elseif {[regexp "^text_segment=" $opt]} {
> +            regsub "^text_segment=" $opt "" addr
> +            if { [have_text_segment] == 1} {
> +                # for linker "LD"
> +                lappend new_options "ldflags=-Wl,-Ttext-segment=$addr"
> +            } elseif { [have_image_base] == 1 } {
> +                # for linker "LLD"
> +                lappend new_options "ldflags=-Wl,--image-base=$addr"
> +            } else {
> +                # old linker version
> +                lappend new_options "ldflags=-Wl,-Ttext=$addr"
> +            }

Let's change this to:

	} elseif {[regexp "^text_segment=(.*)" $opt dummy_var addr]} {
	    if { [linker_supports_Ttext_segment_flag] } {
		# For GNU ld.
		lappend new_options "ldflags=-Wl,-Ttext-segment=$addr"
	    } elseif { [linker_supports_image_base_flag] } {
		# For LLVM's lld.
		lappend new_options "ldflags=-Wl,--image-base=$addr"
	    } elseif { [linker_supports_Ttext_flag] } {
		# For Old gold linker versions.
		lappend new_options "ldflags=-Wl,-Ttext=$addr"
	    } else {
		error "Don't know how to handle text_segment option."
	    }

The changes are:

 - extract address directly with the regexp proc, a bit like we do in
   the shlib handling a bit higher
 - add a check for the -Ttext flag, and an "else" where we error out if
   we can't find a working flag

Watch out for the whitespace formatting, use tabs for whole indents of 8
columns.

>          } else {
>              lappend new_options $opt
>          }
> -    }
> +	}

Spurious whitespace change.

>      # Ensure stack protector is disabled for GCC, as this causes problems with
>      # DWARF line numbering.
> @@ -8240,6 +8252,23 @@ gdb_caching_proc have_fuse_ld_gold {
>      return [gdb_simple_compile $me $src executable $flags]
>  }
>  
> +# Return 1 if linker supports -Ttext-segment, otherwise return 0.
> +gdb_caching_proc have_text_segment {
> +    set me "have_text_segment"
> +    set flags additional_flags="-Wl,-Ttext-segment=0x7000000"
> +    set src { int main() { return 0; } }
> +    return [gdb_simple_compile $me $src executable $flags]
> +}
> +
> +# Return 1 if linker supports --image-base, otherwise 0.
> +gdb_caching_proc have_image_base {
> +    set me "have_image_base"
> +    set flags additional_flags="-Wl,--image-base=0x7000000"
> +    set src { int main() { return 0; } }
> +    return [gdb_simple_compile $me $src executable $flags]
> +}

Rename have_text_segment to linker_supports_Ttext_segment_flag,
have_image_base to linker_supports_image_base_flag, and add
linker_supports_Ttext_flag.

Simon

Re: [PATCH 1/2] Add lld(linker) specific option.

[Simon Marchi]

> Like Luis said, I see some failures when running the test with the lld
> linker, do you see those too?  If so, it would be good to mention it in
> the commit message, so that others aren't surprised to see the test case
> fail.  Getting the test case to compile is already an improvement.

Oh, fixing the test is done in patch #2, right?  If so, you can mention
in the commit message that after this patch (patch #1), the test fails,
but it is fixed by the subsequent patch.

Simon

RE: [PATCH 1/2] Add lld(linker) specific option.

[Balasubrmanian, Vignesh]

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

Simon,

Thanks for the suggestion.
Yes, our clang is configured to use lld.
I modified it as you suggested and attached the same.

Thanks,
vigneshbalu

-----Original Message-----
From: Simon Marchi <simon.marchi@polymtl.ca> 
Sent: Tuesday, March 29, 2022 5:52 PM
To: Balasubrmanian, Vignesh <Vignesh.Balasubrmanian@amd.com>; Simon Marchi <simark@simark.ca>; gdb-patches@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>; Kumar N, Bhuvanendra <Bhuvanendra.KumarN@amd.com>
Subject: Re: [PATCH 1/2] Add lld(linker) specific option.

[CAUTION: External Email]

On 2022-03-22 12:21, Balasubrmanian, Vignesh via Gdb-patches wrote:
> Simon,
>
> Thanks for reviewing.
> I was trying to confine the fix to the test case for better readability.
> Modified as you suggested and fixed other test cases as well (couldn’t 
> verify the arm test case due to machine unavailability)
>
> Failing make check command:
> make check RUNTESTFLAGS="--all -v -v -v GDB='${GDB_INSTALL_DIR}/bin/gdb' CFLAGS_FOR_TARGET='-w -gdwarf-4' CXXFLAGS_FOR_TARGET='-w -gdwarf-4' CPPFLAGS_FOR_TARGET='-w -gdwarf-4' CC_FOR_TARGET='clang'  CXX_FOR_TARGET='clang++'" TESTS="gdb.base/jit-elf.exp"
>
> LLD Error:
> ld.lld: error: -Ttext-segment is not supported. Use --image-base if 
> you intend to set the base address
>
> thanks,
> vigneshbalu.

> From b7825bd24fb55b6bd80c1eaa192399e9632a2735 Mon Sep 17 00:00:00 2001
> From: Vignesh Balasubramanian <Vignesh.Balasubrmanian@amd.com>
> Date: Tue, 22 Mar 2022 17:41:22 +0530
> Subject: [PATCH 1/2] Add lld(linker) specific option.
>
> LLD doesn't have option "-Ttext-segment" but "--image-base".
> So, verify the available option by compiling simple test case and use 
> the same.
> make check Command:
> make check RUNTESTFLAGS="--all -v -v -v GDB='${GDB_INSTALL_DIR}/bin/gdb'
> CFLAGS_FOR_TARGET='-w -gdwarf-4' CXXFLAGS_FOR_TARGET='-w -gdwarf-4'
> CPPFLAGS_FOR_TARGET='-w -gdwarf-4' CC_FOR_TARGET='clang'
> CXX_FOR_TARGET='clang'" TESTS="gdb.base/jit-elf.exp"

Does your clang use lld by default?  Mine (clang packages on Arch Linux and Ubuntu) uses GNU ld (/usr/bin/ld) by default.  I can force it to use lld using this though:

  $ make check TESTS="gdb.base/jit-elf.exp" RUNTESTFLAGS="CC_FOR_TARGET=clang LDFLAGS_FOR_TARGET=-fuse-ld=lld"

Like Luis said, I see some failures when running the test with the lld linker, do you see those too?  If so, it would be good to mention it in the commit message, so that others aren't surprised to see the test case fail.  Getting the test case to compile is already an improvement.

> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -4478,10 +4478,22 @@ proc gdb_compile {source dest type options} {
>       } elseif { $opt == "getting_compiler_info" } {
>           # If this is set, calling test_compiler_info will cause recursion.
>           set getting_compiler_info 1
> +        } elseif {[regexp "^text_segment=" $opt]} {
> +            regsub "^text_segment=" $opt "" addr
> +            if { [have_text_segment] == 1} {
> +                # for linker "LD"
> +                lappend new_options "ldflags=-Wl,-Ttext-segment=$addr"
> +            } elseif { [have_image_base] == 1 } {
> +                # for linker "LLD"
> +                lappend new_options "ldflags=-Wl,--image-base=$addr"
> +            } else {
> +                # old linker version
> +                lappend new_options "ldflags=-Wl,-Ttext=$addr"
> +            }

Let's change this to:

        } elseif {[regexp "^text_segment=(.*)" $opt dummy_var addr]} {
            if { [linker_supports_Ttext_segment_flag] } {
                # For GNU ld.
                lappend new_options "ldflags=-Wl,-Ttext-segment=$addr"
            } elseif { [linker_supports_image_base_flag] } {
                # For LLVM's lld.
                lappend new_options "ldflags=-Wl,--image-base=$addr"
            } elseif { [linker_supports_Ttext_flag] } {
                # For Old gold linker versions.
                lappend new_options "ldflags=-Wl,-Ttext=$addr"
            } else {
                error "Don't know how to handle text_segment option."
            }

The changes are:

 - extract address directly with the regexp proc, a bit like we do in
   the shlib handling a bit higher
 - add a check for the -Ttext flag, and an "else" where we error out if
   we can't find a working flag

Watch out for the whitespace formatting, use tabs for whole indents of 8 columns.

>          } else {
>              lappend new_options $opt
>          }
> -    }
> +     }

Spurious whitespace change.

>      # Ensure stack protector is disabled for GCC, as this causes problems with
>      # DWARF line numbering.
> @@ -8240,6 +8252,23 @@ gdb_caching_proc have_fuse_ld_gold {
>      return [gdb_simple_compile $me $src executable $flags]  }
>
> +# Return 1 if linker supports -Ttext-segment, otherwise return 0.
> +gdb_caching_proc have_text_segment {
> +    set me "have_text_segment"
> +    set flags additional_flags="-Wl,-Ttext-segment=0x7000000"
> +    set src { int main() { return 0; } }
> +    return [gdb_simple_compile $me $src executable $flags] }
> +
> +# Return 1 if linker supports --image-base, otherwise 0.
> +gdb_caching_proc have_image_base {
> +    set me "have_image_base"
> +    set flags additional_flags="-Wl,--image-base=0x7000000"
> +    set src { int main() { return 0; } }
> +    return [gdb_simple_compile $me $src executable $flags] }

Rename have_text_segment to linker_supports_Ttext_segment_flag,
have_image_base to linker_supports_image_base_flag, and add linker_supports_Ttext_flag.

Simon

[-- Attachment #2: 0001-Add-lld-linker-specific-option.patch --]
[-- Type: application/octet-stream, Size: 7305 bytes --]

From 217af57629cc383495afb1ee7b5ed792a04949ba Mon Sep 17 00:00:00 2001
From: Vignesh Balasubramanian <Vignesh.Balasubrmanian@amd.com>
Date: Wed, 30 Mar 2022 16:29:43 +0530
Subject: [PATCH 1/2] Add lld(linker) specific option.

LLD doesn't have option "-Ttext-segment" but "--image-base".
So, verify the available option by compiling simple test case
and use the same.
make check Command:
make check RUNTESTFLAGS="--all -v -v -v GDB='${GDB_INSTALL_DIR}/bin/gdb'
CFLAGS_FOR_TARGET='-w -gdwarf-4' CXXFLAGS_FOR_TARGET='-w -gdwarf-4'
CPPFLAGS_FOR_TARGET='-w -gdwarf-4' CC_FOR_TARGET='clang'
CXX_FOR_TARGET='clang'" TESTS="gdb.base/jit-elf.exp"

This patch fixes only compilation error. With this, the test results are
 of expected passes        24
 of unexpected failures    13
 of unsupported tests      1
---
 gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp |  2 +-
 .../gdb.base/execl-update-breakpoints.exp     | 19 +++-------
 gdb/testsuite/gdb.threads/step-over-exec.exp  |  4 +-
 gdb/testsuite/lib/gdb.exp                     | 38 +++++++++++++++++++
 gdb/testsuite/lib/jit-elf-helpers.exp         |  5 +--
 5 files changed, 48 insertions(+), 20 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp b/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
index cebeb629a9f..34d98aec96e 100644
--- a/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
+++ b/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
@@ -26,7 +26,7 @@ standard_testfile
 # the "-Wl,-Ttext-segment" option compile the binary.
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
-    [list debug ldflags=-Wl,-Ttext-segment=0xb0000000]] } {
+    [list debug text_segment=0xb0000000]] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.base/execl-update-breakpoints.exp b/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
index 8a9bdf66216..bae48ee4c51 100644
--- a/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
+++ b/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
@@ -32,20 +32,11 @@ if { [gdb_compile [file join $srcdir $subdir $srcfile] $objfile \
     return -1
 }
 
-set opts1_ld [list debug ldflags=-Wl,-Ttext-segment=0x1000000]
-set opts1_gold [list debug ldflags=-Wl,-Ttext=0x1000000]
-set opts2_ld [list debug ldflags=-Wl,-Ttext-segment=0x2000000]
-set opts2_gold [list debug ldflags=-Wl,-Ttext=0x2000000]
-
-if { [gdb_compile $objfile $exec1 executable $opts1_ld] != "" } {
-    # Old gold linker versions don't support -Ttext-segment.  Fall
-    # back to -Ttext.
-    if { [gdb_compile $objfile $exec1 executable $opts1_gold] != ""
-	 || [gdb_compile $objfile $exec2 executable $opts2_gold] != ""} {
-	untested "link failed"
-	return -1
-    }
-} elseif { [gdb_compile $objfile $exec2 executable $opts2_ld] != "" } {
+set opts1_ld [list debug text_segment=0x1000000]
+set opts2_ld [list debug text_segment=0x2000000]
+
+if { [gdb_compile $objfile $exec1 executable $opts1_ld] != ""
+     || [gdb_compile $objfile $exec2 executable $opts2_ld] != ""} {
     untested "link failed"
     return -1
 }
diff --git a/gdb/testsuite/gdb.threads/step-over-exec.exp b/gdb/testsuite/gdb.threads/step-over-exec.exp
index 9e252e65f97..783f865585c 100644
--- a/gdb/testsuite/gdb.threads/step-over-exec.exp
+++ b/gdb/testsuite/gdb.threads/step-over-exec.exp
@@ -47,8 +47,8 @@ proc do_test { execr_thread different_text_segments displaced_stepping } {
     set execd_opts [list debug]
 
     if { $different_text_segments } {
-	lappend execr_opts "ldflags=-Wl,-Ttext-segment=0x600000"
-	lappend execd_opts "ldflags=-Wl,-Ttext-segment=0x800000"
+	lappend execr_opts "text_segment=0x600000"
+	lappend execd_opts "text_segment=0x800000"
     }
 
     if { $execr_thread == "leader" } {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0b242b64992..e77c51c8cb8 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4466,6 +4466,19 @@ proc gdb_compile {source dest type options} {
 	} elseif { $opt == "getting_compiler_info" } {
 	    # If this is set, calling test_compiler_info will cause recursion.
 	    set getting_compiler_info 1
+        } elseif {[regexp "^text_segment=(.*)" $opt dummy_var addr]} {
+            if { [linker_supports_Ttext_segment_flag] } {
+                # For Gnu ld.
+                lappend new_options "ldflags=-Wl,-Ttext-segment=$addr"
+            } elseif { [linker_supports_image_base_flag] } {
+                # For LLVM's lld.
+                lappend new_options "ldflags=-Wl,--image-base=$addr"
+            } elseif { [linker_supports_Ttext_flag] } {
+                # For Old gold linker versions.
+                lappend new_options "ldflags=-Wl,-Ttext=$addr"
+            } else {
+                error "Don't know how to handle text_segment option."
+            }
         } else {
             lappend new_options $opt
         }
@@ -8220,6 +8233,31 @@ gdb_caching_proc have_fuse_ld_gold {
     return [gdb_simple_compile $me $src executable $flags]
 }
 
+# Return 1 if linker supports -Ttext-segment, otherwise return 0.
+gdb_caching_proc linker_supports_Ttext_segment_flag {
+    set me "linker_supports_Ttext_segment_flag"
+    set flags additional_flags="-Wl,-Ttext-segment=0x7000000"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
+# Return 1 if linker supports -Ttext-segment, otherwise return 0.
+gdb_caching_proc linker_supports_Ttext_flag {
+    set me "linker_supports_Ttext_flag"
+    set flags additional_flags="-Wl,-Ttext=0x7000000"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
+# Return 1 if linker supports --image-base, otherwise 0.
+gdb_caching_proc linker_supports_image_base_flag {
+    set me "linker_supports_image_base_flag"
+    set flags additional_flags="-Wl,--image-base=0x7000000"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
+
 # Return 1 if compiler supports scalar_storage_order attribute, otherwise
 # return 0.
 gdb_caching_proc supports_scalar_storage_order_attribute {
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
index af70b11644c..4d6a516762d 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -88,14 +88,13 @@ proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile count}
 	# wouldn't work for .debug sections.  Also, output for "info
 	# function" changes when debug info is present.
 	set addr [format 0x%x [expr $jit_load_address + $jit_load_increment * [expr $i-1]]]
-	# Using -Ttext-segment flag to ask linked to relocate everything
+	# Using -Ttext-segment or --image-base flag to ask linked to relocate everything
 	# in the compiled shared library against a fixed base address.  Combined
 	# with mapping the resulting binary to the same fixed base it allows
 	# to dynamically execute functions from it without any further adjustments.
 	set options [list \
 	    additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
-	    additional_flags=-Xlinker \
-	    additional_flags=-Ttext-segment=$addr]
+	    text_segment=$addr]
 	if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} \
 		  $options] != "" } {
 	    set f [file tail $binfile]
-- 
2.17.1

Re: [PATCH 1/2] Add lld(linker) specific option.

[Simon Marchi]

On 2022-03-30 07:21, Balasubrmanian, Vignesh wrote:
> Simon,
> 
> Thanks for the suggestion.
> Yes, our clang is configured to use lld.
> I modified it as you suggested and attached the same.
> 
> Thanks,
> vigneshbalu

Thanks, I did a few adjustements to the patch and commit message, and
pushed it as below.

Simon


From 2bb8c72b39f4dcce0be292b2c701edbe4dd4ca7c Mon Sep 17 00:00:00 2001
From: Vignesh Balasubramanian <Vignesh.Balasubrmanian@amd.com>
Date: Wed, 30 Mar 2022 16:29:43 +0530
Subject: [PATCH] gdb/testsuite: add text_segment option to gdb_compile

LLVM's lld linker doesn't have the "-Ttext-segment" option, but
"--image-base" can be used instead.

To centralize the logic of checking which option is supported, add the
text_segment option to gdb_compile.  Change tests that are currently
using -Ttext-segment to use that new option instead.

This patch fixes only compilation error, for example:

Before:

    $ make check TESTS="gdb.base/jit-elf.exp" RUNTESTFLAGS="CC_FOR_TARGET=clang LDFLAGS_FOR_TARGET=-fuse-ld=ld"
    Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-elf.exp ...
    gdb compile failed, clang-13: warning: -Xlinker -Ttext-segment=0x7000000: 'linker' input unused [-Wunused-command-line-argument]

After:

    $ make check TESTS="gdb.base/jit-elf.exp" RUNTESTFLAGS="CC_FOR_TARGET=clang LDFLAGS_FOR_TARGET=-fuse-ld=ld"
    Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-elf.exp ...
    FAIL: gdb.base/jit-elf.exp: one_jit_test-1: continue to breakpoint: break here 1
    FAIL: gdb.base/jit-elf.exp: one_jit_test-1: continue to breakpoint: break here 2
    FAIL: gdb.base/jit-elf.exp: one_jit_test-2: continue to breakpoint: break here 1
    FAIL: gdb.base/jit-elf.exp: one_jit_test-2: info function ^jit_function
    FAIL: gdb.base/jit-elf.exp: one_jit_test-2: continue to breakpoint: break here 2
    FAIL: gdb.base/jit-elf.exp: attach: one_jit_test-2: continue to breakpoint: break here 1
    FAIL: gdb.base/jit-elf.exp: attach: one_jit_test-2: break here 1: attach
    FAIL: gdb.base/jit-elf.exp: PIE: one_jit_test-1: continue to breakpoint: break here 1
    FAIL: gdb.base/jit-elf.exp: PIE: one_jit_test-1: continue to breakpoint: break here 2

                    === gdb Summary ===

    # of expected passes            26
    # of unexpected failures        9

Change-Id: I3678c5c9bbfc2f80671698e28a038e6b3d14e635
---
 gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp |  2 +-
 .../gdb.base/execl-update-breakpoints.exp     | 16 +-------
 gdb/testsuite/gdb.threads/step-over-exec.exp  |  4 +-
 gdb/testsuite/lib/gdb.exp                     | 39 +++++++++++++++++++
 gdb/testsuite/lib/jit-elf-helpers.exp         |  8 ++--
 5 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp b/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
index cebeb629a9f4..34d98aec96e3 100644
--- a/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
+++ b/gdb/testsuite/gdb.arch/arm-bl-branch-dest.exp
@@ -26,7 +26,7 @@ standard_testfile
 # the "-Wl,-Ttext-segment" option compile the binary.
 
 if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
-    [list debug ldflags=-Wl,-Ttext-segment=0xb0000000]] } {
+    [list debug text_segment=0xb0000000]] } {
     return -1
 }
 
diff --git a/gdb/testsuite/gdb.base/execl-update-breakpoints.exp b/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
index 8a9bdf662160..29647958e65c 100644
--- a/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
+++ b/gdb/testsuite/gdb.base/execl-update-breakpoints.exp
@@ -32,20 +32,8 @@ if { [gdb_compile [file join $srcdir $subdir $srcfile] $objfile \
     return -1
 }
 
-set opts1_ld [list debug ldflags=-Wl,-Ttext-segment=0x1000000]
-set opts1_gold [list debug ldflags=-Wl,-Ttext=0x1000000]
-set opts2_ld [list debug ldflags=-Wl,-Ttext-segment=0x2000000]
-set opts2_gold [list debug ldflags=-Wl,-Ttext=0x2000000]
-
-if { [gdb_compile $objfile $exec1 executable $opts1_ld] != "" } {
-    # Old gold linker versions don't support -Ttext-segment.  Fall
-    # back to -Ttext.
-    if { [gdb_compile $objfile $exec1 executable $opts1_gold] != ""
-	 || [gdb_compile $objfile $exec2 executable $opts2_gold] != ""} {
-	untested "link failed"
-	return -1
-    }
-} elseif { [gdb_compile $objfile $exec2 executable $opts2_ld] != "" } {
+if { [gdb_compile $objfile $exec1 executable {debug text_segment=0x1000000}] != ""
+     || [gdb_compile $objfile $exec2 executable {debug text_segment=0x2000000}] != ""} {
     untested "link failed"
     return -1
 }
diff --git a/gdb/testsuite/gdb.threads/step-over-exec.exp b/gdb/testsuite/gdb.threads/step-over-exec.exp
index 9e252e65f97f..783f865585c0 100644
--- a/gdb/testsuite/gdb.threads/step-over-exec.exp
+++ b/gdb/testsuite/gdb.threads/step-over-exec.exp
@@ -47,8 +47,8 @@ proc do_test { execr_thread different_text_segments displaced_stepping } {
     set execd_opts [list debug]
 
     if { $different_text_segments } {
-	lappend execr_opts "ldflags=-Wl,-Ttext-segment=0x600000"
-	lappend execd_opts "ldflags=-Wl,-Ttext-segment=0x800000"
+	lappend execr_opts "text_segment=0x600000"
+	lappend execd_opts "text_segment=0x800000"
     }
 
     if { $execr_thread == "leader" } {
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 7c35fbb18c4e..10f78736629e 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -4351,6 +4351,7 @@ set gdb_saved_set_unbuffered_mode_obj ""
 #   - nopie: Prevent creation of PIE executables.
 #   - macros: Add the required compiler flag to include macro information in
 #     debug information
+#   - text_segment=addr: Tell the linker to place the text segment at ADDR.
 #
 # And here are some of the not too obscure options understood by DejaGnu that
 # influence the compilation:
@@ -4470,6 +4471,19 @@ proc gdb_compile {source dest type options} {
 	} elseif { $opt == "getting_compiler_info" } {
 	    # If this is set, calling test_compiler_info will cause recursion.
 	    set getting_compiler_info 1
+        } elseif {[regexp "^text_segment=(.*)" $opt dummy_var addr]} {
+            if { [linker_supports_Ttext_segment_flag] } {
+                # For GNU ld.
+                lappend new_options "ldflags=-Wl,-Ttext-segment=$addr"
+            } elseif { [linker_supports_image_base_flag] } {
+                # For LLVM's lld.
+                lappend new_options "ldflags=-Wl,--image-base=$addr"
+            } elseif { [linker_supports_Ttext_flag] } {
+                # For old GNU gold versions.
+                lappend new_options "ldflags=-Wl,-Ttext=$addr"
+            } else {
+                error "Don't know how to handle text_segment option."
+            }
         } else {
             lappend new_options $opt
         }
@@ -8235,6 +8249,31 @@ gdb_caching_proc have_fuse_ld_gold {
     return [gdb_simple_compile $me $src executable $flags]
 }
 
+# Return 1 if linker supports -Ttext-segment, otherwise return 0.
+gdb_caching_proc linker_supports_Ttext_segment_flag {
+    set me "linker_supports_Ttext_segment_flag"
+    set flags additional_flags="-Wl,-Ttext-segment=0x7000000"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
+# Return 1 if linker supports -Ttext, otherwise return 0.
+gdb_caching_proc linker_supports_Ttext_flag {
+    set me "linker_supports_Ttext_flag"
+    set flags additional_flags="-Wl,-Ttext=0x7000000"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
+# Return 1 if linker supports --image-base, otherwise 0.
+gdb_caching_proc linker_supports_image_base_flag {
+    set me "linker_supports_image_base_flag"
+    set flags additional_flags="-Wl,--image-base=0x7000000"
+    set src { int main() { return 0; } }
+    return [gdb_simple_compile $me $src executable $flags]
+}
+
+
 # Return 1 if compiler supports scalar_storage_order attribute, otherwise
 # return 0.
 gdb_caching_proc supports_scalar_storage_order_attribute {
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
index af70b11644ce..b699917f2090 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -88,14 +88,14 @@ proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile count}
 	# wouldn't work for .debug sections.  Also, output for "info
 	# function" changes when debug info is present.
 	set addr [format 0x%x [expr $jit_load_address + $jit_load_increment * [expr $i-1]]]
-	# Using -Ttext-segment flag to ask linked to relocate everything
-	# in the compiled shared library against a fixed base address.  Combined
+
+	# Use "text_segment=..." to ask the linker to relocate everything in the
+	# compiled shared library against a fixed base address.  Combined
 	# with mapping the resulting binary to the same fixed base it allows
 	# to dynamically execute functions from it without any further adjustments.
 	set options [list \
 	    additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
-	    additional_flags=-Xlinker \
-	    additional_flags=-Ttext-segment=$addr]
+	    text_segment=$addr]
 	if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} \
 		  $options] != "" } {
 	    set f [file tail $binfile]

base-commit: a5c69b1e49bae4d0dcb20f324cebb310c63495c6
-- 
2.35.2