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

["Balasubrmanian, Vignesh" <Vignesh.Balasubrmanian@amd.com>]

[-- 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