[v4,3/5,gdb/testsuite] use -Ttext-segment for jit-elf tests

Message ID 20200421133123.21284-4-mihails.strasuns@intel.com
State Superseded
Headers show
Series
  • jit testsuite refactoring
Related show

Commit Message

Rogerio Alves via Gdb-patches April 21, 2020, 1:31 p.m.
Removes the need to manually relocate loaded ELF binary by using a fixed
constant as both mmap base address and as a requested first segment
address supplied to the linker.

In future will enable JIT tests with a valid DWARF debug info.  Current
tests still need to compile without a debug info though, because they do
a function name modification.

gdb/testsuite/ChangeLog:

2020-02-18  Mihails Strasuns  <mihails.strasuns@intel.com>

	* lib/jit-elf-helpers.exp: Supply -Ttext-segment linker flag and
	  define LOAD_ADDRESS/LOAD_INCREMENT macros for the compiled binaries.
	* gdb.base/jit-elf-main.c: Use LOAD_ADDRESS/LOAD_INCREMENT to
	  calculate the mmap address.
---
 gdb/testsuite/gdb.base/jit-elf-main.c | 27 ++++++++++++++------------
 gdb/testsuite/lib/jit-elf-helpers.exp | 28 ++++++++++++++++++++++++---
 2 files changed, 40 insertions(+), 15 deletions(-)

-- 
2.26.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Comments

Simon Marchi April 22, 2020, 2:08 a.m. | #1
On 2020-04-21 9:31 a.m., Mihails Strasuns via Gdb-patches wrote:
> Removes the need to manually relocate loaded ELF binary by using a fixed

> constant as both mmap base address and as a requested first segment

> address supplied to the linker.

> 

> In future will enable JIT tests with a valid DWARF debug info.  Current

> tests still need to compile without a debug info though, because they do

> a function name modification.

> 

> gdb/testsuite/ChangeLog:

> 

> 2020-02-18  Mihails Strasuns  <mihails.strasuns@intel.com>

> 

> 	* lib/jit-elf-helpers.exp: Supply -Ttext-segment linker flag and

> 	  define LOAD_ADDRESS/LOAD_INCREMENT macros for the compiled binaries.

> 	* gdb.base/jit-elf-main.c: Use LOAD_ADDRESS/LOAD_INCREMENT to

> 	  calculate the mmap address.

> ---

>  gdb/testsuite/gdb.base/jit-elf-main.c | 27 ++++++++++++++------------

>  gdb/testsuite/lib/jit-elf-helpers.exp | 28 ++++++++++++++++++++++++---

>  2 files changed, 40 insertions(+), 15 deletions(-)

> 

> diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c

> index acfd17d417..ca150f3b1b 100644

> --- a/gdb/testsuite/gdb.base/jit-elf-main.c

> +++ b/gdb/testsuite/gdb.base/jit-elf-main.c

> @@ -51,20 +51,16 @@ usage (void)

>    exit (1);

>  }

>  

> -/* Update .p_vaddr and .sh_addr as if the code was JITted to ADDR.  */

> +/* Rename jit_function_XXXX to match idx  */

>  

>  static void

> -update_locations (const void *const addr, int idx)

> +update_name (const void *const addr, int idx)

>  {

>    const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *)addr;

>    ElfW (Shdr) *const shdr = (ElfW (Shdr) *)((char *)addr + ehdr->e_shoff);

>    ElfW (Phdr) *const phdr = (ElfW (Phdr) *)((char *)addr + ehdr->e_phoff);

>    int i;

>  

> -  for (i = 0; i < ehdr->e_phnum; ++i)

> -    if (phdr[i].p_type == PT_LOAD)

> -      phdr[i].p_vaddr += (ElfW (Addr))addr;

> -

>    for (i = 0; i < ehdr->e_shnum; ++i)

>      {

>        if (shdr[i].sh_type == SHT_STRTAB)

> @@ -81,9 +77,6 @@ update_locations (const void *const addr, int idx)

>              if (strcmp (p, "jit_function_XXXX") == 0)

>                sprintf (p, "jit_function_%04d", idx);

>          }

> -

> -      if (shdr[i].sh_flags & SHF_ALLOC)

> -        shdr[i].sh_addr += (ElfW (Addr))addr;

>      }

>  }

>  

> @@ -96,6 +89,15 @@ update_locations (const void *const addr, int idx)

>  #define MAIN main

>  #endif

>  

> +/* Must be defined by .exp file when compiling to know

> +   what address to map the ELF binary to.  */

> +#ifndef LOAD_ADDRESS

> +#error "Must define LOAD_ADDRESS"

> +#endif

> +#ifndef LOAD_INCREMENT

> +#error "Must define LOAD_INCREMENT"

> +#endif

> +

>  /* Used to spin waiting for GDB.  */

>  volatile int wait_for_gdb = ATTACH;

>  #define WAIT_FOR_GDB do {} while (wait_for_gdb)

> @@ -139,8 +141,9 @@ MAIN (int argc, char *argv[])

>  	  exit (1);

>  	}

>  

> -      const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE,

> -				     MAP_PRIVATE, fd, 0);

> +      void* load_addr = (void*) (size_t) (LOAD_ADDRESS + (i - 1) * LOAD_INCREMENT);


Space before asterisk.

Otherwise, this patch LGTM.

Simon

Patch

diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index acfd17d417..ca150f3b1b 100644
--- a/gdb/testsuite/gdb.base/jit-elf-main.c
+++ b/gdb/testsuite/gdb.base/jit-elf-main.c
@@ -51,20 +51,16 @@  usage (void)
   exit (1);
 }
 
-/* Update .p_vaddr and .sh_addr as if the code was JITted to ADDR.  */
+/* Rename jit_function_XXXX to match idx  */
 
 static void
-update_locations (const void *const addr, int idx)
+update_name (const void *const addr, int idx)
 {
   const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *)addr;
   ElfW (Shdr) *const shdr = (ElfW (Shdr) *)((char *)addr + ehdr->e_shoff);
   ElfW (Phdr) *const phdr = (ElfW (Phdr) *)((char *)addr + ehdr->e_phoff);
   int i;
 
-  for (i = 0; i < ehdr->e_phnum; ++i)
-    if (phdr[i].p_type == PT_LOAD)
-      phdr[i].p_vaddr += (ElfW (Addr))addr;
-
   for (i = 0; i < ehdr->e_shnum; ++i)
     {
       if (shdr[i].sh_type == SHT_STRTAB)
@@ -81,9 +77,6 @@  update_locations (const void *const addr, int idx)
             if (strcmp (p, "jit_function_XXXX") == 0)
               sprintf (p, "jit_function_%04d", idx);
         }
-
-      if (shdr[i].sh_flags & SHF_ALLOC)
-        shdr[i].sh_addr += (ElfW (Addr))addr;
     }
 }
 
@@ -96,6 +89,15 @@  update_locations (const void *const addr, int idx)
 #define MAIN main
 #endif
 
+/* Must be defined by .exp file when compiling to know
+   what address to map the ELF binary to.  */
+#ifndef LOAD_ADDRESS
+#error "Must define LOAD_ADDRESS"
+#endif
+#ifndef LOAD_INCREMENT
+#error "Must define LOAD_INCREMENT"
+#endif
+
 /* Used to spin waiting for GDB.  */
 volatile int wait_for_gdb = ATTACH;
 #define WAIT_FOR_GDB do {} while (wait_for_gdb)
@@ -139,8 +141,9 @@  MAIN (int argc, char *argv[])
 	  exit (1);
 	}
 
-      const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE,
-				     MAP_PRIVATE, fd, 0);
+      void* load_addr = (void*) (size_t) (LOAD_ADDRESS + (i - 1) * LOAD_INCREMENT);
+      const void *const addr = mmap (load_addr, st.st_size, PROT_READ|PROT_WRITE,
+				     MAP_PRIVATE | MAP_FIXED, fd, 0);
       struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
 
       if (addr == MAP_FAILED)
@@ -149,7 +152,7 @@  MAIN (int argc, char *argv[])
 	  exit (1);
 	}
 
-      update_locations (addr, i);
+      update_name (addr, i);
 
       /* Link entry at the end of the list.  */
       entry->symfile_addr = (const char *)addr;
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
index a40dd94cda..a46edf2051 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -13,6 +13,13 @@ 
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+# Magic constants used to calculate a starting address when linking
+# "jit" shared libraries.  When loaded, will be mapped by jit-elf-main
+# to the same address.
+
+set load_address   0x7000000
+set load_increment 0x1000000
+
 # Compiles jit-elf-main.c as a regular executable
 
 # Compile jit-elf-main.c as an executable.
@@ -24,9 +31,14 @@ 
 # On failure, return -1.
 proc compile_jit_main {binsuffix options} {
     global main_binfile main_srcfile main_basename
+    global load_address load_increment
 
     set binfile ${main_binfile}${binsuffix}
-    set options [concat $options debug]
+    set options [concat \
+	$options \
+	additional_flags=-DLOAD_ADDRESS=$load_address \
+	additional_flags=-DLOAD_INCREMENT=$load_increment \
+	debug]
 
     if { [gdb_compile ${main_srcfile} ${binfile} \
 	  executable $options] != "" } {
@@ -45,7 +57,12 @@  proc compile_jit_main {binsuffix options} {
 # On failure, return -1.
 proc compile_jit_elf_main_as_so {options} {
     global main_solib_srcfile main_solib_binfile
-    set options [concat $options debug]
+    global load_address load_increment
+    set options [list \
+	additional_flags="-DMAIN=jit_dl_main" \
+	additional_flags=-DLOAD_ADDRESS=$load_address \
+	additional_flags=-DLOAD_INCREMENT=$load_increment \
+	debug]
 
     if { [gdb_compile_shlib ${main_solib_srcfile} ${main_solib_binfile} \
 	    $options] != "" } {
@@ -63,6 +80,7 @@  proc compile_jit_elf_main_as_so {options} {
 # On failure, return -1.
 proc compile_and_download_n_jit_so {count} {
     global jit_solib_basename jit_solib_srcfile
+    global load_address load_increment
     set binfiles_target {}
 
     for {set i 1} {$i <= $count} {incr i} {
@@ -72,7 +90,11 @@  proc compile_and_download_n_jit_so {count} {
 	# do symbol renaming by munging on ELF symbol table, and that
 	# wouldn't work for .debug sections.  Also, output for "info
 	# function" changes when debug info is present.
-	if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} {}] != "" } {
+	set addr [format 0x%x [expr $load_address + $load_increment * [expr $i-1]]]
+	set options [list \
+	    additional_flags=-Xlinker \
+	    additional_flags=-Ttext-segment=$addr]
+	if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} $options] != "" } {
 	    untested "failed to compile ${jit_solib_basename}.c as a shared library"
 	    return -1
 	}