[PATCH 6/7] [gdb/testsuite] use -Ttext-segment for jit-elf tests

[PATCH 6/7] [gdb/testsuite] use -Ttext-segment for jit-elf tests

[Mihails Strasuns]

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

Change-Id: Ifdd70d2838d9235e5d4fb49b7cafd03cb4865751
Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
---
 gdb/testsuite/gdb.base/jit-elf-main.c | 25 ++++++++++++++-----------
 gdb/testsuite/lib/jit-elf-helpers.exp | 19 +++++++++++++++++++
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index 0e4b2e9a40..09b9a90ada 100644
--- a/gdb/testsuite/gdb.base/jit-elf-main.c
+++ b/gdb/testsuite/gdb.base/jit-elf-main.c
@@ -51,20 +51,16 @@ usage ()
   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 while (wait_for_gdb)
@@ -137,7 +139,8 @@ MAIN (int argc, char *argv[])
 	  exit (1);
 	}
 
-      const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE,
+      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, fd, 0);
       struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
 
@@ -147,7 +150,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 f63694b8f5..cff47ec73c 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -13,15 +13,25 @@
 # 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
 
 proc compile_jit_main {binsuffix options} {
     global srcdir subdir testfile srcfile binfile
+    global load_address load_increment
     set testfile jit-elf-main
     set srcfile ${testfile}.c
     set binfile [standard_output_file $testfile$binsuffix]
     set options [concat \
 	$options \
+	additional_flags=-DLOAD_ADDRESS=$load_address \
+	additional_flags=-DLOAD_INCREMENT=$load_increment \
 	debug]
     if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
 	executable $options] != "" } {
@@ -33,12 +43,15 @@ proc compile_jit_main {binsuffix options} {
 
 proc compile_jit_main_as_so {binsuffix options} {
     global srcdir subdir testfile srcfile binfile
+    global load_address load_increment
     set testfile jit-elf-main
     set srcfile ${testfile}.c
     set binfile [standard_output_file $testfile$binsuffix]
     set options [concat \
 	$options \
 	additional_flags="-DMAIN=jit_dl_main" \
+	additional_flags=-DLOAD_ADDRESS=$load_address \
+	additional_flags=-DLOAD_INCREMENT=$load_increment \
 	debug]
     if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
 	 $options] != "" } {
@@ -51,6 +64,7 @@ proc compile_jit_main_as_so {binsuffix options} {
 
 proc compile_n_jit_so {count binsuffix options} {
     global srcdir subdir solib_binfile_targets
+    global load_address load_increment
     set solib_binfile_targets {}
     set solib_testfile jit-elf-solib
 
@@ -63,6 +77,11 @@ proc compile_n_jit_so {count binsuffix options} {
 	# 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.
+	set addr [format 0x%x [expr $load_address + $load_increment * [expr $i-1]]]
+	set options [concat \
+	    $options \
+	    additional_flags=-Xlinker \
+	    additional_flags=-Ttext-segment=$addr]
 	if { [gdb_compile_shlib $solib_srcfile $solib_binfile $options] != "" } {
 	    untested "Failure to compile $solib_binfile_test_msg"
 	}
-- 
2.25.0

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

[PATCH 5/7] [gdb/testsuite] add lib/jit-elf-helpers.exp

[Mihails Strasuns]

New utility library to be used by jit-elf tests responsible for
compiling binary artifacts. In the next commit the compilation process
will become more complicated because of extra mandatory flag - keeping
it in one place will make tests less fragile.

gdb/testsuite/ChangeLog:

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

	* lib/jit-elf-helpers.exp: new file
	* gdb.base/jit-elf.exp: updated to use jit-elf-helpers.exp
	* gdb.base/jit-elf-so.exp: updated to use jit-elf-helpers.exp

Change-Id: I2fda518406aeca55e82df45edd67cef149497bbe
Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
---
 gdb/testsuite/gdb.base/jit-elf-so.exp | 69 +++++++------------------
 gdb/testsuite/gdb.base/jit-elf.exp    | 39 +-------------
 gdb/testsuite/lib/jit-elf-helpers.exp | 73 +++++++++++++++++++++++++++
 3 files changed, 92 insertions(+), 89 deletions(-)
 create mode 100644 gdb/testsuite/lib/jit-elf-helpers.exp

diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
index eee20e16c2..31a422a5d2 100644
--- a/gdb/testsuite/gdb.base/jit-elf-so.exp
+++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
@@ -26,67 +26,32 @@ if {[get_compiler_info]} {
     return 1
 }
 
-proc compile_jit_main {options} {
-    global srcdir subdir testfile2 srcfile2 binfile2
-    set testfile2 jit-elf-main
-    set srcfile2 ${testfile2}.c
-    set binfile2 [standard_output_file $testfile2.so]
-    set options [concat \
-	$options \
-	debug]
-    if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" \
-	$options] != "" } {
-	untested "Failure to compile jit-elf-main"
-    }
-}
+load_lib jit-elf-helpers.exp
 
 proc compile_jit_dlmain {options} {
-    global srcdir subdir testfile srcfile binfile
-    set testfile jit-elf-dlmain
-    set srcfile ${testfile}.c
-    set binfile [standard_output_file $testfile]
+    global srcdir subdir testfile_dlmain srcfile_dlmain
+    set testfile_dlmain jit-elf-dlmain
+    set srcfile_dlmain ${testfile_dlmain}.c
+    set binfile [standard_output_file $testfile_dlmain]
     set options [concat \
 	$options \
 	debug]
-    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile_dlmain}" "${binfile}" \
 	executable $options] != "" } {
 	untested "Failure to compile jit-elf-main"
     }
 }
 
-proc compile_n_jit_so {count options} {
-    global srcdir subdir solib_binfile_targets
-    set solib_binfile_targets {}
-    set solib_testfile jit-elf-solib
-
-    for {set i 1} {$i <= $count} {incr i} { 
-	set solib_binfile [standard_output_file ${solib_testfile}.so.$i]
-	set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
-	set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so.$i"
-
-	# Note: compiling without debug info by default: some test
-	# 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 ${solib_srcfile} ${solib_binfile} $options] != "" } {
-	    untested "Failure to compile ${solib_binfile_test_msg}"
-	}
-
-	set path [gdb_remote_download target ${solib_binfile}]
-	set solib_binfile_targets [concat $solib_binfile_targets $path]
-    }
-}
-
-compile_jit_main {additional_flags="-DMAIN=jit_dl_main"}
+compile_jit_main_as_so "" {}
 compile_jit_dlmain {shlib_load}
-compile_n_jit_so 2 {}
+compile_n_jit_so 2 "" {}
 
 proc one_jit_test {count match_str} {
     with_test_prefix "one_jit_test-$count" {
-	global verbose testfile srcfile2 binfile2 binfile2_dlopen solib_binfile_targets
+	global verbose testfile_dlmain srcfile_dlmain srcfile binfile solib_binfile_targets
 
-	clean_restart $testfile
-	gdb_load_shlib $binfile2
+	clean_restart $testfile_dlmain
+	gdb_load_shlib $binfile
 
 	# This is just to help debugging when things fail
 	if {$verbose > 0} {
@@ -98,12 +63,12 @@ proc one_jit_test {count match_str} {
 	    return
 	}
 
-	gdb_breakpoint [gdb_get_line_number "break here before-dlopen" ]
+	gdb_breakpoint [gdb_get_line_number "break here before-dlopen" $srcfile_dlmain]
 	gdb_continue_to_breakpoint "break here before-dlopen"
 
 	# Poke desired values directly into inferior instead of using "set args"
 	# because "set args" does not work under gdbserver.
-	gdb_test_no_output "set var jit_libname = \"$binfile2\""
+	gdb_test_no_output "set var jit_libname = \"$binfile\""
 	incr count
 	gdb_test "set var argc=$count"
 	gdb_test "set var argv=malloc(sizeof(char*)*$count)"
@@ -111,13 +76,13 @@ proc one_jit_test {count match_str} {
 	    gdb_test "set var argv\[$i\]=\"[lindex $solib_binfile_targets [expr $i-1]]\""
 	}		
 
-	gdb_breakpoint [gdb_get_line_number "break here after-dlopen" ]
+	gdb_breakpoint [gdb_get_line_number "break here after-dlopen" $srcfile_dlmain]
 	gdb_continue_to_breakpoint "break here after-dlopen"
 
-	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 0} $srcfile2]"
+	gdb_breakpoint "$srcfile:[gdb_get_line_number {break here 0} $srcfile]"
 	gdb_continue_to_breakpoint "break here 0"
 
-	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 1} $srcfile2]"
+	gdb_breakpoint "$srcfile:[gdb_get_line_number {break here 1} $srcfile]"
 	gdb_continue_to_breakpoint "break here 1"
 
 	gdb_test "info function jit_function" "$match_str"
@@ -128,7 +93,7 @@ proc one_jit_test {count match_str} {
 	    gdb_test "maintenance info break"
 	}
 
-	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 2} $srcfile2]"
+	gdb_breakpoint "$srcfile:[gdb_get_line_number {break here 2} $srcfile]"
 	gdb_continue_to_breakpoint "break here 2"
 	# All jit librares must have been unregistered
 	gdb_test "info function jit_function" \
diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
index ba79efaf1c..71affae698 100644
--- a/gdb/testsuite/gdb.base/jit-elf.exp
+++ b/gdb/testsuite/gdb.base/jit-elf.exp
@@ -23,42 +23,7 @@ if {[get_compiler_info]} {
     return 1
 }
 
-proc compile_jit_main {binsuffix options} {
-    global srcdir subdir testfile srcfile binfile
-    set testfile jit-elf-main
-    set srcfile ${testfile}.c
-    set binfile [standard_output_file $testfile$binsuffix]
-    set options [concat \
-	$options \
-	debug]
-    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
-	executable $options] != "" } {
-	untested "Failure to compile jit-elf-main"
-    }
-}
-
-proc compile_n_jit_so {count options} {
-    global srcdir subdir solib_binfile_targets
-    set solib_binfile_targets {}
-    set solib_testfile jit-elf-solib
-
-    for {set i 1} {$i <= $count} {incr i} { 
-	set solib_binfile [standard_output_file ${solib_testfile}.so.$i]
-	set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
-	set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so.$i"
-
-	# Note: compiling without debug info by default: some test
-	# 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 ${solib_srcfile} ${solib_binfile} $options] != "" } {
-	    untested "Failure to compile ${solib_binfile_test_msg}"
-	}
-
-	set path [gdb_remote_download target ${solib_binfile}]
-	set solib_binfile_targets [concat $solib_binfile_targets $path]
-    }
-}
+load_lib jit-elf-helpers.exp
 
 # Detach, restart GDB, and re-attach to the program.
 
@@ -150,7 +115,7 @@ proc one_jit_test {count match_str reattach} {
 }
 
 compile_jit_main "" {}
-compile_n_jit_so 2 {}
+compile_n_jit_so 2 "" {}
 
 one_jit_test 1 "${hex}  jit_function_0001" 0
 one_jit_test 2 "${hex}  jit_function_0001\[\r\n\]+${hex}  jit_function_0002" 0
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
new file mode 100644
index 0000000000..f63694b8f5
--- /dev/null
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -0,0 +1,73 @@
+# Copyright 2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Compiles jit-elf-main.c as a regular executable
+
+proc compile_jit_main {binsuffix options} {
+    global srcdir subdir testfile srcfile binfile
+    set testfile jit-elf-main
+    set srcfile ${testfile}.c
+    set binfile [standard_output_file $testfile$binsuffix]
+    set options [concat \
+	$options \
+	debug]
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	executable $options] != "" } {
+	untested "Failure to compile jit-elf-main"
+    }
+}
+
+# Compiles jit-elf-main.c as a shared library
+
+proc compile_jit_main_as_so {binsuffix options} {
+    global srcdir subdir testfile srcfile binfile
+    set testfile jit-elf-main
+    set srcfile ${testfile}.c
+    set binfile [standard_output_file $testfile$binsuffix]
+    set options [concat \
+	$options \
+	additional_flags="-DMAIN=jit_dl_main" \
+	debug]
+    if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	 $options] != "" } {
+	untested "Failure to compile jit-elf-main"
+    }
+}
+
+# Compiles jit-elf-solib.c as multiple shared libraries
+# distinguished by a numerical suffix
+
+proc compile_n_jit_so {count binsuffix options} {
+    global srcdir subdir solib_binfile_targets
+    set solib_binfile_targets {}
+    set solib_testfile jit-elf-solib
+
+    for {set i 1} {$i <= $count} {incr i} { 
+	set solib_binfile [standard_output_file $solib_testfile.so.$i]
+	set solib_srcfile "${srcdir}/${subdir}/$solib_testfile.c"
+	set solib_binfile_test_msg "SHLIBDIR/$solib_testfile$binsuffix.so.$i"
+
+	# Note: compiling without debug info by default: some test
+	# 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 $solib_srcfile $solib_binfile $options] != "" } {
+	    untested "Failure to compile $solib_binfile_test_msg"
+	}
+
+	set path [gdb_remote_download target $solib_binfile]
+	set solib_binfile_targets [concat $solib_binfile_targets $path]
+    }
+}
\ No newline at end of file
-- 
2.25.0

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

[PATCH 1/7] [gdb/testsuite] allow more registers in reader test

[Mihails Strasuns]

Fixes jit-reader test failures on systems that have more registers than
expected by the current condition.

gdb/testsuite/ChangeLog:

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

	* gdb.base/jit-reader.exp: relax register output check

Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
Change-Id: I227ab8691b2363d626f7100216477ab637f619fa
---
 gdb/testsuite/gdb.base/jit-reader.exp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
index 7852a5b550..5140e3d930 100644
--- a/gdb/testsuite/gdb.base/jit-reader.exp
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -92,6 +92,7 @@ proc info_registers_current_frame {sp} {
 	     "es             $hex +$neg_decimal" \
 	     "fs             $hex +$neg_decimal" \
 	     "gs             $hex +$neg_decimal" \
+	     ".*" \
 	    ]
 }
 
@@ -196,6 +197,7 @@ proc jit_reader_test {} {
 			 "es             <not saved>" \
 			 "fs             <not saved>" \
 			 "gs             <not saved>" \
+			 ".*" \
 			]
 
 		# Make sure that "info frame" doesn't crash.
-- 
2.25.0

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

[PATCH 4/7] [gdb/testsuite] use args as lib list for jit-elf tests

[Mihails Strasuns]

Old usage: jit-elf-main lib.so 2
New usage: jit-elf-main lib.so.1 lib.so.2

Refactoring necessary to support running tests over multiple jit
binaries rather than mapping the same binary muultiple times.

gdb/testsuite/ChangeLog:

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

	* gdb.base/jit-elf-main.c: read lib list from argc/argv
	* gdb.base/jit-elf.exp: compile N jit libraries and use the list
	* gdb.base/jit-elf-so.exp: ditto

Change-Id: Ie8f85ec6358604c14557b0417d6621b2f8942033
Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
---
 gdb/testsuite/gdb.base/jit-elf-main.c |  52 +++++--------
 gdb/testsuite/gdb.base/jit-elf-so.exp | 108 ++++++++++++++++----------
 gdb/testsuite/gdb.base/jit-elf.exp    |  99 +++++++++++------------
 3 files changed, 133 insertions(+), 126 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index fe0f540d6f..0e4b2e9a40 100644
--- a/gdb/testsuite/gdb.base/jit-elf-main.c
+++ b/gdb/testsuite/gdb.base/jit-elf-main.c
@@ -45,9 +45,9 @@
 #endif /* !ElfW  */
 
 static void
-usage (const char *const argv0)
+usage ()
 {
-  fprintf (stderr, "Usage: %s library [count]\n", argv0);
+  fprintf (stderr, "Usage: jit-elf-main libraries...\n");
   exit (1);
 }
 
@@ -106,16 +106,11 @@ int mypid;
 int
 MAIN (int argc, char *argv[])
 {
-  /* These variables are here so they can easily be set from jit.exp.  */
-  const char *libname = NULL;
-  int count = 0, i, fd;
-  struct stat st;
-
+  int i;
   alarm (300);
 
   mypid = getpid ();
-
-  count = count;  /* gdb break here 0  */
+  /* gdb break here 0  */
 
   if (argc < 2)
     {
@@ -123,32 +118,25 @@ MAIN (int argc, char *argv[])
       exit (1);
     }
 
-  if (libname == NULL)
-    /* Only set if not already set from GDB.  */
-    libname = argv[1];
-
-  if (argc > 2 && count == 0)
-    /* Only set if not already set from GDB.  */
-    count = atoi (argv[2]);
-
-  printf ("%s:%d: libname = %s, count = %d\n", __FILE__, __LINE__,
-	  libname, count);
-
-  if ((fd = open (libname, O_RDONLY)) == -1)
+  for (i = 1; i < argc; ++i)
     {
-      fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", libname,
-	       strerror (errno));
-      exit (1);
-    }
+      struct stat st;
+      int fd;
 
-  if (fstat (fd, &st) != 0)
-    {
-      fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
-      exit (1);
-    }
+      printf ("%s:%d: libname = %s, i = %d\n", __FILE__, __LINE__, argv[i], i);
+      if ((fd = open (argv[i], O_RDONLY)) == -1)
+	{
+	  fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", argv[i],
+		   strerror (errno));
+	  exit (1);
+	}
+
+      if (fstat (fd, &st) != 0)
+	{
+	  fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
+	  exit (1);
+	}
 
-  for (i = 0; i < count; ++i)
-    {
       const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE,
 				     MAP_PRIVATE, fd, 0);
       struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
index 526414f43c..eee20e16c2 100644
--- a/gdb/testsuite/gdb.base/jit-elf-so.exp
+++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
@@ -26,45 +26,64 @@ if {[get_compiler_info]} {
     return 1
 }
 
-#
-# test running programs
-#
-
-set testfile jit-elf-dlmain
-set srcfile ${testfile}.c
-set binfile [standard_output_file ${testfile}]
-if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug shlib_load}] != "" } {
-    untested "failed to compile"
-    return -1
+proc compile_jit_main {options} {
+    global srcdir subdir testfile2 srcfile2 binfile2
+    set testfile2 jit-elf-main
+    set srcfile2 ${testfile2}.c
+    set binfile2 [standard_output_file $testfile2.so]
+    set options [concat \
+	$options \
+	debug]
+    if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" \
+	$options] != "" } {
+	untested "Failure to compile jit-elf-main"
+    }
 }
 
-set testfile2 jit-elf-main
-set srcfile2 ${testfile2}.c
-set binfile2 [standard_output_file ${testfile2}.so]
-set binfile2_dlopen [shlib_target_file ${testfile2}.so]
-if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" ${binfile2} {debug additional_flags="-DMAIN=jit_dl_main"}] != "" } {
-    untested "failed to compile main shared library"
-    return -1
+proc compile_jit_dlmain {options} {
+    global srcdir subdir testfile srcfile binfile
+    set testfile jit-elf-dlmain
+    set srcfile ${testfile}.c
+    set binfile [standard_output_file $testfile]
+    set options [concat \
+	$options \
+	debug]
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+	executable $options] != "" } {
+	untested "Failure to compile jit-elf-main"
+    }
 }
 
-set solib_testfile "jit-elf-solib"
-set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
-set solib_binfile [standard_output_file ${solib_testfile}.so]
-set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so"
+proc compile_n_jit_so {count options} {
+    global srcdir subdir solib_binfile_targets
+    set solib_binfile_targets {}
+    set solib_testfile jit-elf-solib
+
+    for {set i 1} {$i <= $count} {incr i} { 
+	set solib_binfile [standard_output_file ${solib_testfile}.so.$i]
+	set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
+	set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so.$i"
+
+	# Note: compiling without debug info by default: some test
+	# 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 ${solib_srcfile} ${solib_binfile} $options] != "" } {
+	    untested "Failure to compile ${solib_binfile_test_msg}"
+	}
 
-# Note: compiling without debug info: the library goes through symbol
-# renaming by munging on its symbol table, and that wouldn't work for .debug
-# sections.  Also, output for "info function" changes when debug info is resent.
-if { [gdb_compile_shlib ${solib_srcfile} ${solib_binfile} {}] != "" } {
-    untested "failed to compile jit shared library"
-    return -1
+	set path [gdb_remote_download target ${solib_binfile}]
+	set solib_binfile_targets [concat $solib_binfile_targets $path]
+    }
 }
 
-set solib_binfile_target [gdb_remote_download target ${solib_binfile}]
+compile_jit_main {additional_flags="-DMAIN=jit_dl_main"}
+compile_jit_dlmain {shlib_load}
+compile_n_jit_so 2 {}
 
 proc one_jit_test {count match_str} {
     with_test_prefix "one_jit_test-$count" {
-	global verbose testfile srcfile2 binfile2 binfile2_dlopen solib_binfile_target solib_binfile_test_msg
+	global verbose testfile srcfile2 binfile2 binfile2_dlopen solib_binfile_targets
 
 	clean_restart $testfile
 	gdb_load_shlib $binfile2
@@ -81,9 +100,16 @@ proc one_jit_test {count match_str} {
 
 	gdb_breakpoint [gdb_get_line_number "break here before-dlopen" ]
 	gdb_continue_to_breakpoint "break here before-dlopen"
+
 	# Poke desired values directly into inferior instead of using "set args"
 	# because "set args" does not work under gdbserver.
-	gdb_test_no_output "set var jit_libname = \"$binfile2_dlopen\""
+	gdb_test_no_output "set var jit_libname = \"$binfile2\""
+	incr count
+	gdb_test "set var argc=$count"
+	gdb_test "set var argv=malloc(sizeof(char*)*$count)"
+	for {set i 1} {$i < $count} {incr i} {
+	    gdb_test "set var argv\[$i\]=\"[lindex $solib_binfile_targets [expr $i-1]]\""
+	}		
 
 	gdb_breakpoint [gdb_get_line_number "break here after-dlopen" ]
 	gdb_continue_to_breakpoint "break here after-dlopen"
@@ -91,10 +117,6 @@ proc one_jit_test {count match_str} {
 	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 0} $srcfile2]"
 	gdb_continue_to_breakpoint "break here 0"
 
-	gdb_test_no_output "set var argc = 2"
-	gdb_test_no_output "set var libname = \"$solib_binfile_target\"" "set var libname = \"$solib_binfile_test_msg\""
-	gdb_test_no_output "set var count = $count"
-
 	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 1} $srcfile2]"
 	gdb_continue_to_breakpoint "break here 1"
 
@@ -114,12 +136,14 @@ proc one_jit_test {count match_str} {
     }
 }
 
-one_jit_test 1 "${hex}  jit_function_0000"
-one_jit_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001"
+one_jit_test 1 "${hex}  jit_function_0001"
+one_jit_test 2 "${hex}  jit_function_0001\[\r\n\]+${hex}  jit_function_0002"
 
-# We don't intend to load the .so as a JIT debuginfo reader, but we
-# need some handy file name for a completion test.
-gdb_test \
-    "complete jit-reader-load [standard_output_file ${solib_testfile}.s]" \
-    "jit-reader-load $solib_binfile" \
-    "test jit-reader-load filename completion"
+foreach solib $solib_binfile_targets {
+    # We don't intend to load the .so as a JIT debuginfo reader, but we
+    # need some handy file name for a completion test.
+    gdb_test \
+	"complete jit-reader-load [standard_output_file $solib]" \
+	"jit-reader-load $solib" \
+	"test jit-reader-load filename completion"
+}
diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
index 71d3e37dfb..ba79efaf1c 100644
--- a/gdb/testsuite/gdb.base/jit-elf.exp
+++ b/gdb/testsuite/gdb.base/jit-elf.exp
@@ -23,42 +23,41 @@ if {[get_compiler_info]} {
     return 1
 }
 
-# Compile the testcase program and library.  BINSUFFIX is the suffix
-# to append to the program and library filenames, to make them unique
-# between invocations.  OPTIONS is passed to gdb_compile when
-# compiling the program.
-
-proc compile_jit_test {testname binsuffix options} {
-    global testfile srcfile binfile srcdir subdir
-    global solib_testfile solib_srcfile solib_binfile solib_binfile_test_msg
-    global solib_binfile_target
-
+proc compile_jit_main {binsuffix options} {
+    global srcdir subdir testfile srcfile binfile
     set testfile jit-elf-main
     set srcfile ${testfile}.c
     set binfile [standard_output_file $testfile$binsuffix]
+    set options [concat \
+	$options \
+	debug]
     if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
-	      executable [concat debug $options]] != "" } {
-	untested $testname
-	return -1
-    }
-
-    set solib_testfile "jit-elf-solib"
-    set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
-    set solib_binfile [standard_output_file ${solib_testfile}$binsuffix.so]
-    set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}$binsuffix.so"
-
-    # Note: compiling without debug info: the library goes through
-    # symbol renaming by munging on its 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 ${solib_srcfile} ${solib_binfile} {-fPIC}] != "" } {
-	untested $testname
-	return -1
+	executable $options] != "" } {
+	untested "Failure to compile jit-elf-main"
     }
+}
 
-    set solib_binfile_target [gdb_remote_download target ${solib_binfile}]
+proc compile_n_jit_so {count options} {
+    global srcdir subdir solib_binfile_targets
+    set solib_binfile_targets {}
+    set solib_testfile jit-elf-solib
+
+    for {set i 1} {$i <= $count} {incr i} { 
+	set solib_binfile [standard_output_file ${solib_testfile}.so.$i]
+	set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
+	set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so.$i"
+
+	# Note: compiling without debug info by default: some test
+	# 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 ${solib_srcfile} ${solib_binfile} $options] != "" } {
+	    untested "Failure to compile ${solib_binfile_test_msg}"
+	}
 
-    return 0
+	set path [gdb_remote_download target ${solib_binfile}]
+	set solib_binfile_targets [concat $solib_binfile_targets $path]
+    }
 }
 
 # Detach, restart GDB, and re-attach to the program.
@@ -105,7 +104,7 @@ proc continue_to_test_location {location reattach} {
 
 proc one_jit_test {count match_str reattach} {
     with_test_prefix "one_jit_test-$count" {
-	global verbose testfile solib_binfile_target solib_binfile_test_msg
+	global verbose testfile solib_binfile_targets
 
 	clean_restart $testfile
 
@@ -119,14 +118,18 @@ proc one_jit_test {count match_str reattach} {
 	    return
 	}
 
+	# Poke desired values directly into inferior instead of using "set args"
+	# because "set args" does not work under gdbserver.
+	incr count
+	gdb_test "set var argc=$count"
+	gdb_test "set var argv=malloc(sizeof(char*)*$count)"
+	for {set i 1} {$i < $count} {incr i} {
+	    gdb_test "set var argv\[$i\]=\"[lindex $solib_binfile_targets [expr $i-1]]\""
+	}
+
 	gdb_breakpoint [gdb_get_line_number "break here 0"]
 	gdb_continue_to_breakpoint "break here 0"
 
-	# Poke desired values directly into inferior instead of using "set args"
-	# because "set args" does not work under gdbserver.
-	gdb_test_no_output "set var argc = 2"
-	gdb_test_no_output "set var libname = \"$solib_binfile_target\"" "set var libname = \"$solib_binfile_test_msg\""
-	gdb_test_no_output "set var count = $count"
 
 	continue_to_test_location "break here 1" $reattach
 
@@ -146,31 +149,23 @@ proc one_jit_test {count match_str reattach} {
     }
 }
 
-if {[compile_jit_test jit.exp "" {}] < 0} {
-    return
-}
-one_jit_test 1 "${hex}  jit_function_0000" 0
-one_jit_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001" 0
+compile_jit_main "" {}
+compile_n_jit_so 2 {}
+
+one_jit_test 1 "${hex}  jit_function_0001" 0
+one_jit_test 2 "${hex}  jit_function_0001\[\r\n\]+${hex}  jit_function_0002" 0
 
 # Test attaching to an inferior with some JIT libraries already
 # registered.  We reuse the normal test, and detach/reattach at
 # specific interesting points.
 if {[can_spawn_for_attach]} {
-    if {[compile_jit_test "jit.exp attach tests" \
-	     "-attach" {additional_flags=-DATTACH=1}] < 0} {
-	return
-    }
-
+    compile_jit_main "-attach" {additional_flags=-DATTACH=1}
     with_test_prefix attach {
-	one_jit_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001" 1
+	one_jit_test 2 "${hex}  jit_function_0001\[\r\n\]+${hex}  jit_function_0002" 1
     }
 }
 
 with_test_prefix PIE {
-    if {[compile_jit_test "jit.exp PIE tests" \
-	     "-pie" {additional_flags=-fPIE ldflags=-pie}] < 0} {
-	return
-    }
-
-    one_jit_test 1 "${hex}  jit_function_0000" 0
+    compile_jit_main "-pie" {additional_flags=-fPIE ldflags=-pie}
+    one_jit_test 1 "${hex}  jit_function_0001" 0
 }
-- 
2.25.0

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

[PATCH 2/7] [gdb/testsuite] structured rename of jit test files

[Mihails Strasuns]

Reorganizes how JIT related test files to be more clear what are related
to JIT reader system tests and what use JIT from ELF objfiles. Those two
approaches are quite different in GDB implementation and require very
different test setup. Keeping distinction clear at the file name level
makes it easier to maintain the testsuite.

gdb/testsuite/ChangeLog:

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

	* gdb.base: rename all jit related test and source files

Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
Change-Id: I51524c2791744ebad4ea7cb45185ee4e2cd97993
---
 gdb/testsuite/gdb.base/{jit-dlmain.c => jit-elf-dlmain.c}   | 0
 gdb/testsuite/gdb.base/{jit-main.c => jit-elf-main.c}       | 0
 gdb/testsuite/gdb.base/{jit-so.exp => jit-elf-so.exp}       | 6 +++---
 gdb/testsuite/gdb.base/{jit-solib.c => jit-elf-solib.c}     | 0
 gdb/testsuite/gdb.base/{jit.exp => jit-elf.exp}             | 4 ++--
 gdb/testsuite/gdb.base/{jit-exec.c => jit-reader-exec.c}    | 2 +-
 .../gdb.base/{jit-exec.exp => jit-reader-exec.exp}          | 4 ++--
 gdb/testsuite/gdb.base/{jit-execd.c => jit-reader-execd.c}  | 0
 gdb/testsuite/gdb.base/{jithost.c => jit-reader-host.c}     | 2 +-
 gdb/testsuite/gdb.base/{jithost.h => jit-reader-host.h}     | 0
 .../gdb.base/{jit-simple-dl.c => jit-reader-simple-dl.c}    | 0
 .../gdb.base/{jit-simple-jit.c => jit-reader-simple-jit.c}  | 0
 .../gdb.base/{jit-simple.c => jit-reader-simple.c}          | 2 +-
 .../gdb.base/{jit-simple.exp => jit-reader-simple.exp}      | 0
 gdb/testsuite/gdb.base/{jitreader.c => jit-reader.c}        | 2 +-
 gdb/testsuite/gdb.base/jit-reader.exp                       | 4 ++--
 16 files changed, 13 insertions(+), 13 deletions(-)
 rename gdb/testsuite/gdb.base/{jit-dlmain.c => jit-elf-dlmain.c} (100%)
 rename gdb/testsuite/gdb.base/{jit-main.c => jit-elf-main.c} (100%)
 rename gdb/testsuite/gdb.base/{jit-so.exp => jit-elf-so.exp} (97%)
 rename gdb/testsuite/gdb.base/{jit-solib.c => jit-elf-solib.c} (100%)
 rename gdb/testsuite/gdb.base/{jit.exp => jit-elf.exp} (98%)
 rename gdb/testsuite/gdb.base/{jit-exec.c => jit-reader-exec.c} (96%)
 rename gdb/testsuite/gdb.base/{jit-exec.exp => jit-reader-exec.exp} (95%)
 rename gdb/testsuite/gdb.base/{jit-execd.c => jit-reader-execd.c} (100%)
 rename gdb/testsuite/gdb.base/{jithost.c => jit-reader-host.c} (99%)
 rename gdb/testsuite/gdb.base/{jithost.h => jit-reader-host.h} (100%)
 rename gdb/testsuite/gdb.base/{jit-simple-dl.c => jit-reader-simple-dl.c} (100%)
 rename gdb/testsuite/gdb.base/{jit-simple-jit.c => jit-reader-simple-jit.c} (100%)
 rename gdb/testsuite/gdb.base/{jit-simple.c => jit-reader-simple.c} (96%)
 rename gdb/testsuite/gdb.base/{jit-simple.exp => jit-reader-simple.exp} (100%)
 rename gdb/testsuite/gdb.base/{jitreader.c => jit-reader.c} (99%)

diff --git a/gdb/testsuite/gdb.base/jit-dlmain.c b/gdb/testsuite/gdb.base/jit-elf-dlmain.c
similarity index 100%
rename from gdb/testsuite/gdb.base/jit-dlmain.c
rename to gdb/testsuite/gdb.base/jit-elf-dlmain.c
diff --git a/gdb/testsuite/gdb.base/jit-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
similarity index 100%
rename from gdb/testsuite/gdb.base/jit-main.c
rename to gdb/testsuite/gdb.base/jit-elf-main.c
diff --git a/gdb/testsuite/gdb.base/jit-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
similarity index 97%
rename from gdb/testsuite/gdb.base/jit-so.exp
rename to gdb/testsuite/gdb.base/jit-elf-so.exp
index 27dcdfa58e..526414f43c 100644
--- a/gdb/testsuite/gdb.base/jit-so.exp
+++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
@@ -30,7 +30,7 @@ if {[get_compiler_info]} {
 # test running programs
 #
 
-set testfile jit-dlmain
+set testfile jit-elf-dlmain
 set srcfile ${testfile}.c
 set binfile [standard_output_file ${testfile}]
 if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug shlib_load}] != "" } {
@@ -38,7 +38,7 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
     return -1
 }
 
-set testfile2 jit-main
+set testfile2 jit-elf-main
 set srcfile2 ${testfile2}.c
 set binfile2 [standard_output_file ${testfile2}.so]
 set binfile2_dlopen [shlib_target_file ${testfile2}.so]
@@ -47,7 +47,7 @@ if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" ${binfile2} {debug add
     return -1
 }
 
-set solib_testfile "jit-solib"
+set solib_testfile "jit-elf-solib"
 set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
 set solib_binfile [standard_output_file ${solib_testfile}.so]
 set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so"
diff --git a/gdb/testsuite/gdb.base/jit-solib.c b/gdb/testsuite/gdb.base/jit-elf-solib.c
similarity index 100%
rename from gdb/testsuite/gdb.base/jit-solib.c
rename to gdb/testsuite/gdb.base/jit-elf-solib.c
diff --git a/gdb/testsuite/gdb.base/jit.exp b/gdb/testsuite/gdb.base/jit-elf.exp
similarity index 98%
rename from gdb/testsuite/gdb.base/jit.exp
rename to gdb/testsuite/gdb.base/jit-elf.exp
index 094c37fa3d..71d3e37dfb 100644
--- a/gdb/testsuite/gdb.base/jit.exp
+++ b/gdb/testsuite/gdb.base/jit-elf.exp
@@ -33,7 +33,7 @@ proc compile_jit_test {testname binsuffix options} {
     global solib_testfile solib_srcfile solib_binfile solib_binfile_test_msg
     global solib_binfile_target
 
-    set testfile jit-main
+    set testfile jit-elf-main
     set srcfile ${testfile}.c
     set binfile [standard_output_file $testfile$binsuffix]
     if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
@@ -42,7 +42,7 @@ proc compile_jit_test {testname binsuffix options} {
 	return -1
     }
 
-    set solib_testfile "jit-solib"
+    set solib_testfile "jit-elf-solib"
     set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
     set solib_binfile [standard_output_file ${solib_testfile}$binsuffix.so]
     set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}$binsuffix.so"
diff --git a/gdb/testsuite/gdb.base/jit-exec.c b/gdb/testsuite/gdb.base/jit-reader-exec.c
similarity index 96%
rename from gdb/testsuite/gdb.base/jit-exec.c
rename to gdb/testsuite/gdb.base/jit-reader-exec.c
index 809308fabe..f92aa5d9da 100644
--- a/gdb/testsuite/gdb.base/jit-exec.c
+++ b/gdb/testsuite/gdb.base/jit-reader-exec.c
@@ -17,7 +17,7 @@
 
 /* Simple standalone program using the JIT API.  */
 
-#include "jit-simple-jit.c"
+#include "jit-reader-simple-jit.c"
 #include <unistd.h>
 
 int
diff --git a/gdb/testsuite/gdb.base/jit-exec.exp b/gdb/testsuite/gdb.base/jit-reader-exec.exp
similarity index 95%
rename from gdb/testsuite/gdb.base/jit-exec.exp
rename to gdb/testsuite/gdb.base/jit-reader-exec.exp
index 327646bb65..4235309f77 100644
--- a/gdb/testsuite/gdb.base/jit-exec.exp
+++ b/gdb/testsuite/gdb.base/jit-reader-exec.exp
@@ -21,9 +21,9 @@ if { ![istarget "*-linux*"] } then {
     return
 }
 
-standard_testfile jit-exec.c
+standard_testfile jit-reader-exec.c
 
-set testfile2 "jit-execd"
+set testfile2 "jit-reader-execd"
 set srcfile2 ${testfile2}.c
 set binfile2 [standard_output_file ${testfile2}]
 
diff --git a/gdb/testsuite/gdb.base/jit-execd.c b/gdb/testsuite/gdb.base/jit-reader-execd.c
similarity index 100%
rename from gdb/testsuite/gdb.base/jit-execd.c
rename to gdb/testsuite/gdb.base/jit-reader-execd.c
diff --git a/gdb/testsuite/gdb.base/jithost.c b/gdb/testsuite/gdb.base/jit-reader-host.c
similarity index 99%
rename from gdb/testsuite/gdb.base/jithost.c
rename to gdb/testsuite/gdb.base/jit-reader-host.c
index 19cc3e16c0..d07acd54bb 100644
--- a/gdb/testsuite/gdb.base/jithost.c
+++ b/gdb/testsuite/gdb.base/jit-reader-host.c
@@ -23,7 +23,7 @@
 #include <sys/mman.h>
 
 #include JIT_READER_H  /* Please see jit-reader.exp for an explanation.  */
-#include "jithost.h"
+#include "jit-reader-host.h"
 #include "jit-protocol.h"
 
 void __attribute__((noinline)) __jit_debug_register_code () { }
diff --git a/gdb/testsuite/gdb.base/jithost.h b/gdb/testsuite/gdb.base/jit-reader-host.h
similarity index 100%
rename from gdb/testsuite/gdb.base/jithost.h
rename to gdb/testsuite/gdb.base/jit-reader-host.h
diff --git a/gdb/testsuite/gdb.base/jit-simple-dl.c b/gdb/testsuite/gdb.base/jit-reader-simple-dl.c
similarity index 100%
rename from gdb/testsuite/gdb.base/jit-simple-dl.c
rename to gdb/testsuite/gdb.base/jit-reader-simple-dl.c
diff --git a/gdb/testsuite/gdb.base/jit-simple-jit.c b/gdb/testsuite/gdb.base/jit-reader-simple-jit.c
similarity index 100%
rename from gdb/testsuite/gdb.base/jit-simple-jit.c
rename to gdb/testsuite/gdb.base/jit-reader-simple-jit.c
diff --git a/gdb/testsuite/gdb.base/jit-simple.c b/gdb/testsuite/gdb.base/jit-reader-simple.c
similarity index 96%
rename from gdb/testsuite/gdb.base/jit-simple.c
rename to gdb/testsuite/gdb.base/jit-reader-simple.c
index 8ea6aec1f1..bcb83f09bf 100644
--- a/gdb/testsuite/gdb.base/jit-simple.c
+++ b/gdb/testsuite/gdb.base/jit-reader-simple.c
@@ -17,7 +17,7 @@
 
 /* Simple standalone program using the JIT API.  */
 
-#include "jit-simple-jit.c"
+#include "jit-reader-simple-jit.c"
 
 int
 main (void)
diff --git a/gdb/testsuite/gdb.base/jit-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp
similarity index 100%
rename from gdb/testsuite/gdb.base/jit-simple.exp
rename to gdb/testsuite/gdb.base/jit-reader-simple.exp
diff --git a/gdb/testsuite/gdb.base/jitreader.c b/gdb/testsuite/gdb.base/jit-reader.c
similarity index 99%
rename from gdb/testsuite/gdb.base/jitreader.c
rename to gdb/testsuite/gdb.base/jit-reader.c
index d0dc488fec..c5fd7a99be 100644
--- a/gdb/testsuite/gdb.base/jitreader.c
+++ b/gdb/testsuite/gdb.base/jit-reader.c
@@ -21,7 +21,7 @@
 #include <string.h>
 
 #include JIT_READER_H  /* Please see jit-reader.exp for an explanation.  */
-#include "jithost.h"
+#include "jit-reader-host.h"
 
 GDB_DECLARE_GPL_COMPATIBLE_READER;
 
diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
index 5140e3d930..6f90d6ebdf 100644
--- a/gdb/testsuite/gdb.base/jit-reader.exp
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -13,7 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-standard_testfile jithost.c
+standard_testfile jit-reader-host.c
 
 if { (![istarget x86_64-*-*] && ![istarget i?86-*-*]) || ![is_lp64_target] } {
     return -1;
@@ -47,7 +47,7 @@ if  { [gdb_compile "${srcdir}/${subdir}/${jit_host_src}" "${jit_host_bin}" \
     return -1
 }
 
-set jit_reader jitreader
+set jit_reader jit-reader
 set jit_reader_src ${jit_reader}.c
 set jit_reader_bin [standard_output_file ${jit_reader}.so]
 
-- 
2.25.0

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

[PATCH 7/7] [gdb/testsuite] add jit-elf-util.h and run jit function

[Mihails Strasuns]

Splits ELF related symbols into a separate jit-elf-util.h header and
enhances it with a few more.

Intention is to make adding new JIT tests possible without repeating
most of the common boilerplate.

As a test enhancement, jit-elf-main.c now calls the renamed function
after registering the jit object and ensures it returns an expected
result.

gdb/testsuite/ChangeLog:

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

	* gdb.base/jit-elf-util.h: new header file
	* gdb.base/jit-elf-main.c: use jit-elf-util.h, add a call to
	  the renamed JIT function to verify its result

Change-Id: I9ac35ff2d610a57536f8afd33794c2feb6f40e25
Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
---
 gdb/testsuite/gdb.base/jit-elf-main.c |  88 +++-------------
 gdb/testsuite/gdb.base/jit-elf-util.h | 145 ++++++++++++++++++++++++++
 2 files changed, 162 insertions(+), 71 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-elf-util.h

diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index 09b9a90ada..b11a3e1410 100644
--- a/gdb/testsuite/gdb.base/jit-elf-main.c
+++ b/gdb/testsuite/gdb.base/jit-elf-main.c
@@ -30,19 +30,7 @@
 #include <unistd.h>
 
 #include "jit-protocol.h"
-
-/* ElfW is coming from linux. On other platforms it does not exist.
-   Let us define it here. */
-#ifndef ElfW
-# if (defined  (_LP64) || defined (__LP64__)) 
-#   define WORDSIZE 64
-# else
-#   define WORDSIZE 32
-# endif /* _LP64 || __LP64__  */
-#define ElfW(type)      _ElfW (Elf, WORDSIZE, type)
-#define _ElfW(e,w,t)    _ElfW_1 (e, w, _##t)
-#define _ElfW_1(e,w,t)  e##w##t
-#endif /* !ElfW  */
+#include "jit-elf-util.h"
 
 static void
 usage ()
@@ -51,35 +39,6 @@ usage ()
   exit (1);
 }
 
-/* Rename jit_function_XXXX to match idx  */
-
-static void
-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_shnum; ++i)
-    {
-      if (shdr[i].sh_type == SHT_STRTAB)
-        {
-          /* Note: we update both .strtab and .dynstr.  The latter would
-             not be correct if this were a regular shared library (.hash
-             would be wrong), but this is a simulation -- the library is
-             never exposed to the dynamic loader, so it all ends up ok.  */
-          char *const strtab = (char *)((ElfW (Addr))addr + shdr[i].sh_offset);
-          char *const strtab_end = strtab + shdr[i].sh_size;
-          char *p;
-
-          for (p = strtab; p < strtab_end; p += strlen (p) + 1)
-            if (strcmp (p, "jit_function_XXXX") == 0)
-              sprintf (p, "jit_function_%04d", idx);
-        }
-    }
-}
-
 /* Defined by the .exp file if testing attach.  */
 #ifndef ATTACH
 #define ATTACH 0
@@ -122,39 +81,20 @@ MAIN (int argc, char *argv[])
 
   for (i = 1; i < argc; ++i)
     {
-      struct stat st;
-      int fd;
-
-      printf ("%s:%d: libname = %s, i = %d\n", __FILE__, __LINE__, argv[i], i);
-      if ((fd = open (argv[i], O_RDONLY)) == -1)
-	{
-	  fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", argv[i],
-		   strerror (errno));
-	  exit (1);
-	}
-
-      if (fstat (fd, &st) != 0)
-	{
-	  fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
-	  exit (1);
-	}
-
-      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, fd, 0);
-      struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
-
-      if (addr == MAP_FAILED)
-	{
-	  fprintf (stderr, "mmap: %s\n", strerror (errno));
-	  exit (1);
-	}
-
+      size_t obj_size;
+      ElfW (Addr) load_addr = LOAD_ADDRESS + (i-1) * LOAD_INCREMENT;
+      printf("Loading %s as JIT at %p\n", argv[i], (void*) load_addr);
+      ElfW (Addr) addr = load_elf (argv[i], &obj_size, load_addr);
       update_name (addr, i);
 
+      char name[32];
+      sprintf (name, "jit_function_%04d", i);
+      int (*jit_function) () = (int (*) ()) load_symbol (addr, name);
+
       /* Link entry at the end of the list.  */
+      struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
       entry->symfile_addr = (const char *)addr;
-      entry->symfile_size = st.st_size;
+      entry->symfile_size = obj_size;
       entry->prev_entry = __jit_debug_descriptor.relevant_entry;
       __jit_debug_descriptor.relevant_entry = entry;
 
@@ -166,6 +106,12 @@ MAIN (int argc, char *argv[])
       /* Notify GDB.  */
       __jit_debug_descriptor.action_flag = JIT_REGISTER;
       __jit_debug_register_code ();
+
+      if (jit_function () != 42)
+	{
+	  fprintf (stderr, "unexpected return value\n");
+	  exit (1);
+	}
     }
 
   WAIT_FOR_GDB; i = 0;  /* gdb break here 1 */
diff --git a/gdb/testsuite/gdb.base/jit-elf-util.h b/gdb/testsuite/gdb.base/jit-elf-util.h
new file mode 100644
index 0000000000..ad5041fa74
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-util.h
@@ -0,0 +1,145 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2020 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Simulates loading of JIT code by memory mapping a compiled
+   shared library binary and doing minimal post-processing.  */
+
+#include <elf.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <link.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+/* ElfW is coming from linux. On other platforms it does not exist.
+   Let us define it here. */
+#ifndef ElfW
+#if (defined(_LP64) || defined(__LP64__))
+#define WORDSIZE 64
+#else
+#define WORDSIZE 32
+#endif /* _LP64 || __LP64__  */
+#define ElfW(type) _ElfW (Elf, WORDSIZE, type)
+#define _ElfW(e, w, t) _ElfW_1 (e, w, _##t)
+#define _ElfW_1(e, w, t) e##w##t
+#endif /* !ElfW  */
+
+/* Looks into string tables for entry
+   "jit_function_XXXX" and updates it to use `rename_num`.  */
+static void
+update_name (ElfW (Addr) addr, int rename_num)
+{
+  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);
+
+  for (int i = 0; i < ehdr->e_shnum; ++i)
+    {
+      if (shdr[i].sh_type == SHT_STRTAB)
+	{
+	  /* Note: we update both .strtab and .dynstr.  The latter would
+	     not be correct if this were a regular shared library (.hash
+	     would be wrong), but this is a simulation -- the library is
+	     never exposed to the dynamic loader, so it all ends up ok.  */
+	  char *const strtab = (char *) (addr + shdr[i].sh_offset);
+	  char *const strtab_end = strtab + shdr[i].sh_size;
+	  char *p;
+
+	  for (p = strtab; p < strtab_end; p += strlen (p) + 1)
+	    if (strcmp (p, "jit_function_XXXX") == 0)
+	      sprintf (p, "jit_function_%04d", rename_num);
+	}
+    }
+}
+
+/* Find symbol with the name `sym_name`.  */
+static ElfW (Addr)
+load_symbol (ElfW (Addr) addr, const char *sym_name)
+{
+  const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *) addr;
+  ElfW (Shdr) *const shdr = (ElfW (Shdr) *) ((char *) addr + ehdr->e_shoff);
+
+  ElfW (Addr) sym_old_addr = 0;
+  ElfW (Addr) sym_new_addr = 0;
+
+  /* Find `func_name` in symbol_table and adjust it from the addr.  */
+
+  for (int i = 0; i < ehdr->e_shnum; ++i)
+    {
+      if (shdr[i].sh_type == SHT_SYMTAB)
+	{
+	  ElfW (Sym) *symtab = (ElfW (Sym) *) (addr + shdr[i].sh_offset);
+	  ElfW (Sym) *symtab_end
+	      = (ElfW (Sym) *) (addr + shdr[i].sh_offset + shdr[i].sh_size);
+	  char *const strtab
+	      = (char *) (addr + shdr[shdr[i].sh_link].sh_offset);
+
+	  for (ElfW (Sym) *p = symtab; p < symtab_end; ++p)
+	    {
+	      const char *s = strtab + p->st_name;
+	      if (strcmp (s, sym_name) == 0)
+	        return p->st_value;
+	    }
+	}
+    }
+
+  fprintf (stderr, "symbol '%s' not found\n", sym_name);
+  exit (1);
+  return 0;
+}
+
+/* Open an elf binary file and memory map it with execution flag enabled.  */
+ElfW (Addr)
+load_elf (const char *libname, size_t *size, ElfW (Addr) load_addr)
+{
+  int fd;
+  struct stat st;
+
+  if ((fd = open (libname, O_RDONLY)) == -1)
+    {
+      fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", libname,
+	       strerror (errno));
+      exit (1);
+    }
+
+  if (fstat (fd, &st) != 0)
+    {
+      fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
+      exit (1);
+    }
+
+  void *addr = mmap ((void *) load_addr, st.st_size,
+		     PROT_READ | PROT_WRITE | PROT_EXEC,
+		     load_addr ? MAP_PRIVATE | MAP_FIXED : MAP_PRIVATE, fd, 0);
+  close (fd);
+
+  if (addr == MAP_FAILED)
+    {
+      fprintf (stderr, "mmap: %s\n", strerror (errno));
+      exit (1);
+    }
+
+  if (size != NULL)
+    *size = st.st_size;
+
+  return (ElfW (Addr)) addr;
+}
\ No newline at end of file
-- 
2.25.0

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

[PATCH 0/7] refactor and enhance jit testing

[Mihails Strasuns]

Replaces https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/757 with a
different approach proposed by Simon.

After these patches it becomes possible to add new jit-elf tests which rely on
DWARF debug info present. There is a lot of refactoring being done - I tried to
split it into smallest individually compiling steps that still make sense.

3ad6f47703 [gdb/testsuite] add jit-elf-util.h and run jit function
99bee9a000 [gdb/testsuite] use -Ttext-segment for jit-elf tests
af8f23b301 [gdb/testsuite] add lib/jit-elf-helpers.exp
a0b17bef28 [gdb/testsuite] use args as lib list for jit-elf tests
513be63401 [gdb/testsuite] share jit-protocol.h by all jit tests
757c4f7100 [gdb/testsuite] structured rename of jit test files
d37a2fb49b [gdb/testsuite] allow more registers in reader test

gdb/testsuite/gdb.base/jit-attach-pie.c                            |  24 +--
gdb/testsuite/gdb.base/{jit-dlmain.c => jit-elf-dlmain.c}          |   0
gdb/testsuite/gdb.base/jit-elf-main.c                              | 143 ++++++++++++++++++
gdb/testsuite/gdb.base/jit-elf-so.exp                              | 114 +++++++++++++++
gdb/testsuite/gdb.base/{jit-solib.c => jit-elf-solib.c}            |   0
gdb/testsuite/gdb.base/jit-elf-util.h                              | 145 ++++++++++++++++++
gdb/testsuite/gdb.base/{jit.exp => jit-elf.exp}                    |  80 +++-------
gdb/testsuite/gdb.base/jit-main.c                                  | 236 ------------------------------
gdb/testsuite/gdb.base/jit-protocol.h                              |   8 +-
gdb/testsuite/gdb.base/{jit-exec.c => jit-reader-exec.c}           |   2 +-
gdb/testsuite/gdb.base/{jit-exec.exp => jit-reader-exec.exp}       |   4 +-
gdb/testsuite/gdb.base/{jit-execd.c => jit-reader-execd.c}         |   0
gdb/testsuite/gdb.base/{jithost.c => jit-reader-host.c}            |   5 +-
gdb/testsuite/gdb.base/{jithost.h => jit-reader-host.h}            |   0
gdb/testsuite/gdb.base/{jit-simple-dl.c => jit-reader-simple-dl.c} |   0
gdb/testsuite/gdb.base/jit-reader-simple-jit.c                     |  27 ++++
gdb/testsuite/gdb.base/{jit-simple.c => jit-reader-simple.c}       |   2 +-
gdb/testsuite/gdb.base/{jit-simple.exp => jit-reader-simple.exp}   |   0
gdb/testsuite/gdb.base/{jitreader.c => jit-reader.c}               |   2 +-
gdb/testsuite/gdb.base/jit-reader.exp                              |   6 +-
gdb/testsuite/gdb.base/jit-simple-jit.c                            |  50 -------
gdb/testsuite/gdb.base/jit-so.exp                                  | 125 ----------------
gdb/testsuite/lib/jit-elf-helpers.exp                              |  92 ++++++++++++
23 files changed, 559 insertions(+), 506 deletions(-)


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

[PATCH 3/7] [gdb/testsuite] share jit-protocol.h by all jit tests

[Mihails Strasuns]

There was an existing jit-protocol.h defining common symbols needed for
JIT-supporting application, however, it was only used by few tests.
Others redeclared the same symbols.

This unifies all tests to use jit-protocol.h

gdb/testsuite/ChangeLog:

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

	* gdb.base/jit-attach-pie.c: use jit-protocol.h
	* gdb.base/jit-elf-main.c: use jit-protocol.h
	* gdb.base/jit-reader-host.c: use jit-protocol.h
	* gdb.base/jit-reader-simple-jit.c: use jit-protocol.h
	* gdb.base/jit-protocol.h: update definitions to match all usage
	  contexts

Change-Id: I0dbfa7b7dcda0ba7b5f09a2d7530dd6dce88fa1e
Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
---
 gdb/testsuite/gdb.base/jit-attach-pie.c       | 24 +-----------
 gdb/testsuite/gdb.base/jit-elf-main.c         | 38 ++-----------------
 gdb/testsuite/gdb.base/jit-protocol.h         |  8 +++-
 gdb/testsuite/gdb.base/jit-reader-host.c      |  3 --
 .../gdb.base/jit-reader-simple-jit.c          | 25 +-----------
 5 files changed, 13 insertions(+), 85 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-attach-pie.c b/gdb/testsuite/gdb.base/jit-attach-pie.c
index 55a03f73ae..fd08233521 100644
--- a/gdb/testsuite/gdb.base/jit-attach-pie.c
+++ b/gdb/testsuite/gdb.base/jit-attach-pie.c
@@ -19,29 +19,7 @@
 #include <stdint.h>
 #include <pthread.h>
 
-struct jit_code_entry
-{
-  struct jit_code_entry *next_entry;
-  struct jit_code_entry *prev_entry;
-  const char *symfile_addr;
-  uint64_t symfile_size;
-};
-
-struct jit_descriptor
-{
-  uint32_t version;
-  /* This type should be jit_actions_t, but we use uint32_t
-     to be explicit about the bitwidth.  */
-  uint32_t action_flag;
-  struct jit_code_entry *relevant_entry;
-  struct jit_code_entry *first_entry;
-};
-
-struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
-
-void __jit_debug_register_code()
-{
-}
+#include "jit-protocol.h"
 
 static void *
 thread_proc (void *arg)
diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index 40958ef5b5..fe0f540d6f 100644
--- a/gdb/testsuite/gdb.base/jit-elf-main.c
+++ b/gdb/testsuite/gdb.base/jit-elf-main.c
@@ -29,6 +29,8 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+#include "jit-protocol.h"
+
 /* ElfW is coming from linux. On other platforms it does not exist.
    Let us define it here. */
 #ifndef ElfW
@@ -42,38 +44,6 @@
 #define _ElfW_1(e,w,t)  e##w##t
 #endif /* !ElfW  */
 
-typedef enum
-{
-  JIT_NOACTION = 0,
-  JIT_REGISTER_FN,
-  JIT_UNREGISTER_FN
-} jit_actions_t;
-
-struct jit_code_entry
-{
-  struct jit_code_entry *next_entry;
-  struct jit_code_entry *prev_entry;
-  const char *symfile_addr;
-  uint64_t symfile_size;
-};
-
-struct jit_descriptor
-{
-  uint32_t version;
-  /* This type should be jit_actions_t, but we use uint32_t
-     to be explicit about the bitwidth.  */
-  uint32_t action_flag;
-  struct jit_code_entry *relevant_entry;
-  struct jit_code_entry *first_entry;
-};
-
-/* GDB puts a breakpoint in this function.  */
-void __attribute__((noinline)) __jit_debug_register_code () { }
-
-/* Make sure to specify the version statically, because the
-   debugger may check the version before we can set it.  */
-struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
-
 static void
 usage (const char *const argv0)
 {
@@ -203,7 +173,7 @@ MAIN (int argc, char *argv[])
 	__jit_debug_descriptor.first_entry = entry;
 
       /* Notify GDB.  */
-      __jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
+      __jit_debug_descriptor.action_flag = JIT_REGISTER;
       __jit_debug_register_code ();
     }
 
@@ -225,7 +195,7 @@ MAIN (int argc, char *argv[])
 	__jit_debug_descriptor.first_entry = NULL;
 
       /* Notify GDB.  */
-      __jit_debug_descriptor.action_flag = JIT_UNREGISTER_FN;
+      __jit_debug_descriptor.action_flag = JIT_UNREGISTER;
       __jit_debug_register_code ();
 
       __jit_debug_descriptor.relevant_entry = prev_entry;
diff --git a/gdb/testsuite/gdb.base/jit-protocol.h b/gdb/testsuite/gdb.base/jit-protocol.h
index 458523e5ff..a3f1a13320 100644
--- a/gdb/testsuite/gdb.base/jit-protocol.h
+++ b/gdb/testsuite/gdb.base/jit-protocol.h
@@ -38,7 +38,7 @@ struct jit_code_entry
 {
   struct jit_code_entry *next_entry;
   struct jit_code_entry *prev_entry;
-  void *symfile_addr;
+  const void *symfile_addr;
   uint64_t symfile_size;
 };
 
@@ -51,4 +51,10 @@ struct jit_descriptor
   struct jit_code_entry *first_entry;
 };
 
+struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
+
+void __jit_debug_register_code()
+{
+}
+
 #endif /* JIT_PROTOCOL_H */
diff --git a/gdb/testsuite/gdb.base/jit-reader-host.c b/gdb/testsuite/gdb.base/jit-reader-host.c
index d07acd54bb..f9c4833083 100644
--- a/gdb/testsuite/gdb.base/jit-reader-host.c
+++ b/gdb/testsuite/gdb.base/jit-reader-host.c
@@ -26,9 +26,6 @@
 #include "jit-reader-host.h"
 #include "jit-protocol.h"
 
-void __attribute__((noinline)) __jit_debug_register_code () { }
-
-struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
 struct jit_code_entry only_entry;
 
 typedef void (jit_function_stack_mangle_t) (void);
diff --git a/gdb/testsuite/gdb.base/jit-reader-simple-jit.c b/gdb/testsuite/gdb.base/jit-reader-simple-jit.c
index 407666b98b..f446bf2d96 100644
--- a/gdb/testsuite/gdb.base/jit-reader-simple-jit.c
+++ b/gdb/testsuite/gdb.base/jit-reader-simple-jit.c
@@ -19,32 +19,9 @@
 
 #include <stdint.h>
 
-struct jit_code_entry
-{
-  struct jit_code_entry *next_entry;
-  struct jit_code_entry *prev_entry;
-  const char *symfile_addr;
-  uint64_t symfile_size;
-};
-
-struct jit_descriptor
-{
-  uint32_t version;
-  /* This type should be jit_actions_t, but we use uint32_t
-     to be explicit about the bitwidth.  */
-  uint32_t action_flag;
-  struct jit_code_entry *relevant_entry;
-  struct jit_code_entry *first_entry;
-};
-
 #ifdef SPACER
 /* This exists to change the address of __jit_debug_descriptor.  */
 int spacer = 4;
 #endif
 
-struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
-
-void
-__jit_debug_register_code (void)
-{
-}
+#include "jit-protocol.h"
\ No newline at end of file
-- 
2.25.0

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

Re: [PATCH 1/7] [gdb/testsuite] allow more registers in reader test

[Tom Tromey]

>>>>> "Mihails" == Mihails Strasuns <mihails.strasuns@intel.com> writes:

Mihails> Fixes jit-reader test failures on systems that have more registers than
Mihails> expected by the current condition.

Mihails> gdb/testsuite/ChangeLog:

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

Mihails> 	* gdb.base/jit-reader.exp: relax register output check

Ok.
Thank you.

Tom

Re: [PATCH 3/7] [gdb/testsuite] share jit-protocol.h by all jit tests

[Tom Tromey]

>>>>> "Mihails" == Mihails Strasuns <mihails.strasuns@intel.com> writes:

Mihails> There was an existing jit-protocol.h defining common symbols needed for
Mihails> JIT-supporting application, however, it was only used by few tests.
Mihails> Others redeclared the same symbols.

Mihails> This unifies all tests to use jit-protocol.h

Mihails> gdb/testsuite/ChangeLog:

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

Mihails> 	* gdb.base/jit-attach-pie.c: use jit-protocol.h
Mihails> 	* gdb.base/jit-elf-main.c: use jit-protocol.h
Mihails> 	* gdb.base/jit-reader-host.c: use jit-protocol.h
Mihails> 	* gdb.base/jit-reader-simple-jit.c: use jit-protocol.h
Mihails> 	* gdb.base/jit-protocol.h: update definitions to match all usage
Mihails> 	  contexts

Thank you for doing this.  This is ok.

Tom

Re: [PATCH 2/7] [gdb/testsuite] structured rename of jit test files

[Tom Tromey]

>>>>> "Mihails" == Mihails Strasuns <mihails.strasuns@intel.com> writes:

Mihails> Reorganizes how JIT related test files to be more clear what are related
Mihails> to JIT reader system tests and what use JIT from ELF objfiles. Those two
Mihails> approaches are quite different in GDB implementation and require very
Mihails> different test setup. Keeping distinction clear at the file name level
Mihails> makes it easier to maintain the testsuite.

Mihails> gdb/testsuite/ChangeLog:

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

Mihails> 	* gdb.base: rename all jit related test and source files

Ok.  Thanks.

Tom

RE: [PATCH 0/7] refactor and enhance jit testing

[Strasuns, Mihails]

Kind ping regarding last 3 patches ;)

Best regards,
Mihails

-----Original Message-----
From: gdb-patches-owner@sourceware.org <gdb-patches-owner@sourceware.org> On Behalf Of Mihails Strasuns
Sent: Tuesday, February 18, 2020 1:44 PM
To: gdb-patches@sourceware.org
Subject: [PATCH 0/7] refactor and enhance jit testing

Replaces https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/757 with a different approach proposed by Simon.

After these patches it becomes possible to add new jit-elf tests which rely on DWARF debug info present. There is a lot of refactoring being done - I tried to split it into smallest individually compiling steps that still make sense.

3ad6f47703 [gdb/testsuite] add jit-elf-util.h and run jit function
99bee9a000 [gdb/testsuite] use -Ttext-segment for jit-elf tests
af8f23b301 [gdb/testsuite] add lib/jit-elf-helpers.exp
a0b17bef28 [gdb/testsuite] use args as lib list for jit-elf tests
513be63401 [gdb/testsuite] share jit-protocol.h by all jit tests
757c4f7100 [gdb/testsuite] structured rename of jit test files d37a2fb49b [gdb/testsuite] allow more registers in reader test

gdb/testsuite/gdb.base/jit-attach-pie.c                            |  24 +--
gdb/testsuite/gdb.base/{jit-dlmain.c => jit-elf-dlmain.c}          |   0
gdb/testsuite/gdb.base/jit-elf-main.c                              | 143 ++++++++++++++++++
gdb/testsuite/gdb.base/jit-elf-so.exp                              | 114 +++++++++++++++
gdb/testsuite/gdb.base/{jit-solib.c => jit-elf-solib.c}            |   0
gdb/testsuite/gdb.base/jit-elf-util.h                              | 145 ++++++++++++++++++
gdb/testsuite/gdb.base/{jit.exp => jit-elf.exp}                    |  80 +++-------
gdb/testsuite/gdb.base/jit-main.c                                  | 236 ------------------------------
gdb/testsuite/gdb.base/jit-protocol.h                              |   8 +-
gdb/testsuite/gdb.base/{jit-exec.c => jit-reader-exec.c}           |   2 +-
gdb/testsuite/gdb.base/{jit-exec.exp => jit-reader-exec.exp}       |   4 +-
gdb/testsuite/gdb.base/{jit-execd.c => jit-reader-execd.c}         |   0
gdb/testsuite/gdb.base/{jithost.c => jit-reader-host.c}            |   5 +-
gdb/testsuite/gdb.base/{jithost.h => jit-reader-host.h}            |   0
gdb/testsuite/gdb.base/{jit-simple-dl.c => jit-reader-simple-dl.c} |   0
gdb/testsuite/gdb.base/jit-reader-simple-jit.c                     |  27 ++++
gdb/testsuite/gdb.base/{jit-simple.c => jit-reader-simple.c}       |   2 +-
gdb/testsuite/gdb.base/{jit-simple.exp => jit-reader-simple.exp}   |   0
gdb/testsuite/gdb.base/{jitreader.c => jit-reader.c}               |   2 +-
gdb/testsuite/gdb.base/jit-reader.exp                              |   6 +-
gdb/testsuite/gdb.base/jit-simple-jit.c                            |  50 -------
gdb/testsuite/gdb.base/jit-so.exp                                  | 125 ----------------
gdb/testsuite/lib/jit-elf-helpers.exp                              |  92 ++++++++++++
23 files changed, 559 insertions(+), 506 deletions(-)


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

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

Re: [PATCH 0/7] refactor and enhance jit testing

[Simon Marchi]

Pinging myself to keep this near the top and re-assure you I haven't forgot :)

On 2020-02-26 8:56 a.m., Strasuns, Mihails wrote:
> Kind ping regarding last 3 patches ;)
> 
> Best regards,
> Mihails
> 
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org <gdb-patches-owner@sourceware.org> On Behalf Of Mihails Strasuns
> Sent: Tuesday, February 18, 2020 1:44 PM
> To: gdb-patches@sourceware.org
> Subject: [PATCH 0/7] refactor and enhance jit testing
> 
> Replaces https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/757 with a different approach proposed by Simon.
> 
> After these patches it becomes possible to add new jit-elf tests which rely on DWARF debug info present. There is a lot of refactoring being done - I tried to split it into smallest individually compiling steps that still make sense.
> 
> 3ad6f47703 [gdb/testsuite] add jit-elf-util.h and run jit function
> 99bee9a000 [gdb/testsuite] use -Ttext-segment for jit-elf tests
> af8f23b301 [gdb/testsuite] add lib/jit-elf-helpers.exp
> a0b17bef28 [gdb/testsuite] use args as lib list for jit-elf tests
> 513be63401 [gdb/testsuite] share jit-protocol.h by all jit tests
> 757c4f7100 [gdb/testsuite] structured rename of jit test files d37a2fb49b [gdb/testsuite] allow more registers in reader test
> 
> gdb/testsuite/gdb.base/jit-attach-pie.c                            |  24 +--
> gdb/testsuite/gdb.base/{jit-dlmain.c => jit-elf-dlmain.c}          |   0
> gdb/testsuite/gdb.base/jit-elf-main.c                              | 143 ++++++++++++++++++
> gdb/testsuite/gdb.base/jit-elf-so.exp                              | 114 +++++++++++++++
> gdb/testsuite/gdb.base/{jit-solib.c => jit-elf-solib.c}            |   0
> gdb/testsuite/gdb.base/jit-elf-util.h                              | 145 ++++++++++++++++++
> gdb/testsuite/gdb.base/{jit.exp => jit-elf.exp}                    |  80 +++-------
> gdb/testsuite/gdb.base/jit-main.c                                  | 236 ------------------------------
> gdb/testsuite/gdb.base/jit-protocol.h                              |   8 +-
> gdb/testsuite/gdb.base/{jit-exec.c => jit-reader-exec.c}           |   2 +-
> gdb/testsuite/gdb.base/{jit-exec.exp => jit-reader-exec.exp}       |   4 +-
> gdb/testsuite/gdb.base/{jit-execd.c => jit-reader-execd.c}         |   0
> gdb/testsuite/gdb.base/{jithost.c => jit-reader-host.c}            |   5 +-
> gdb/testsuite/gdb.base/{jithost.h => jit-reader-host.h}            |   0
> gdb/testsuite/gdb.base/{jit-simple-dl.c => jit-reader-simple-dl.c} |   0
> gdb/testsuite/gdb.base/jit-reader-simple-jit.c                     |  27 ++++
> gdb/testsuite/gdb.base/{jit-simple.c => jit-reader-simple.c}       |   2 +-
> gdb/testsuite/gdb.base/{jit-simple.exp => jit-reader-simple.exp}   |   0
> gdb/testsuite/gdb.base/{jitreader.c => jit-reader.c}               |   2 +-
> gdb/testsuite/gdb.base/jit-reader.exp                              |   6 +-
> gdb/testsuite/gdb.base/jit-simple-jit.c                            |  50 -------
> gdb/testsuite/gdb.base/jit-so.exp                                  | 125 ----------------
> gdb/testsuite/lib/jit-elf-helpers.exp                              |  92 ++++++++++++
> 23 files changed, 559 insertions(+), 506 deletions(-)
> 
> 
> 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
> 
> 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
> 

Re: [PATCH 1/7] [gdb/testsuite] allow more registers in reader test

[Simon Marchi]

On 2020-02-18 7:43 a.m., Mihails Strasuns wrote:
> Fixes jit-reader test failures on systems that have more registers than
> expected by the current condition.
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-02-18  Mihails Strasuns  <mihails.strasuns@intel.com>
> 
> 	* gdb.base/jit-reader.exp: relax register output check
> 
> Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
> Change-Id: I227ab8691b2363d626f7100216477ab637f619fa

This is fine with me too, but could you please just quote in the commit message what
are the extra registers you see?  And maybe what hardware/cpu you run on, to help
anybody who would like to replicate what you are seeing.

Thanks,

Simon

Re: [PATCH 1/7] [gdb/testsuite] allow more registers in reader test

[Simon Marchi]

On 2020-03-21 12:03 p.m., Simon Marchi wrote:
> On 2020-02-18 7:43 a.m., Mihails Strasuns wrote:
>> Fixes jit-reader test failures on systems that have more registers than
>> expected by the current condition.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 2020-02-18  Mihails Strasuns  <mihails.strasuns@intel.com>
>>
>> 	* gdb.base/jit-reader.exp: relax register output check
>>
>> Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
>> Change-Id: I227ab8691b2363d626f7100216477ab637f619fa
> 
> This is fine with me too, but could you please just quote in the commit message what
> are the extra registers you see?  And maybe what hardware/cpu you run on, to help
> anybody who would like to replicate what you are seeing.
> 
> Thanks,
> 
> Simon
> 

Hmm, in fact, I now get some failures, which I don't get before:

Running /home/simark/src/binutils-gdb/gdb/testsuite/gdb.base/jit-reader.exp ...
FAIL: gdb.base/jit-reader.exp: with jit-reader: after mangling: current frame: info registers
FAIL: gdb.base/jit-reader.exp: with jit-reader: after mangling: caller frame: info registers
FAIL: gdb.base/jit-reader.exp: without jit-reader: info registers

The reason is that the regexp generated by multi_line mandates another line after the `gs` line.
It ends something like this:

    ...gs             $hex +$neg_decimal\r\n.*\r\n

If you don't have any line after the `gs` line, the regexp won't match.

multi_line just returns regexp as a string, so you can just append .* after it.  I would suggest
integrating the change below in your patch.


From 9a32960dfa05b322fd873aedc9768a8dd721acd6 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@polymtl.ca>
Date: Sat, 21 Mar 2020 22:06:36 -0400
Subject: [PATCH] fixup

---
 gdb/testsuite/gdb.base/jit-reader.exp | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-reader.exp b/gdb/testsuite/gdb.base/jit-reader.exp
index 5140e3d9306b..8663f0021def 100644
--- a/gdb/testsuite/gdb.base/jit-reader.exp
+++ b/gdb/testsuite/gdb.base/jit-reader.exp
@@ -66,7 +66,8 @@ proc info_registers_current_frame {sp} {
     set any "\[^\r\n\]*"

     set neg_decimal "-?$decimal"
-    gdb_test "info registers" \
+
+    set expected \
 	[multi_line \
 	     "rax            $hex +$neg_decimal" \
 	     "rbx            $hex +$neg_decimal" \
@@ -92,8 +93,12 @@ proc info_registers_current_frame {sp} {
 	     "es             $hex +$neg_decimal" \
 	     "fs             $hex +$neg_decimal" \
 	     "gs             $hex +$neg_decimal" \
-	     ".*" \
 	    ]
+
+    # There may be more registers.
+    append expected ".*"
+
+    gdb_test "info registers" $expected
 }

 proc jit_reader_test {} {
@@ -171,7 +176,8 @@ proc jit_reader_test {} {

 		# Since the JIT unwinder only provides RIP/RSP/RBP,
 		# all other registers should show as "<not saved>".
-		gdb_test "info registers" \
+
+		set expected \
 		    [multi_line \
 			 "rax            <not saved>" \
 			 "rbx            <not saved>" \
@@ -197,9 +203,13 @@ proc jit_reader_test {} {
 			 "es             <not saved>" \
 			 "fs             <not saved>" \
 			 "gs             <not saved>" \
-			 ".*" \
 			]

+		# There may be more registers.
+		append expected ".*"
+
+		gdb_test "info registers" $expected
+
 		# Make sure that "info frame" doesn't crash.
 		gdb_test "info frame" "Stack level 1, .*in main.*"

-- 
2.25.2

Re: [PATCH 2/7] [gdb/testsuite] structured rename of jit test files

[Simon Marchi]

On 2020-02-19 4:23 p.m., Tom Tromey wrote:
>>>>>> "Mihails" == Mihails Strasuns <mihails.strasuns@intel.com> writes:
> 
> Mihails> Reorganizes how JIT related test files to be more clear what are related
> Mihails> to JIT reader system tests and what use JIT from ELF objfiles. Those two
> Mihails> approaches are quite different in GDB implementation and require very
> Mihails> different test setup. Keeping distinction clear at the file name level
> Mihails> makes it easier to maintain the testsuite.
> 
> Mihails> gdb/testsuite/ChangeLog:
> 
> Mihails> 2020-02-18  Mihails Strasuns  <mihails.strasuns@intel.com>
> 
> Mihails> 	* gdb.base: rename all jit related test and source files
> 
> Ok.  Thanks.
> 
> Tom
> 

LGTM from me too, that's a good idea, thanks.

Simon

Re: [PATCH 3/7] [gdb/testsuite] share jit-protocol.h by all jit tests

[Simon Marchi]

On 2020-02-18 7:43 a.m., Mihails Strasuns wrote:
> @@ -51,4 +51,10 @@ struct jit_descriptor
>    struct jit_code_entry *first_entry;
>  };
>  
> +struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };

Some instances used  __attribute__((noinline)), I would suggest putting it here.

But otherwise this LGTM.

Simon

Re: [PATCH 4/7] [gdb/testsuite] use args as lib list for jit-elf tests

[Simon Marchi]

Hi Mihails,

Just a few nits, in general that looks good.

On 2020-02-18 7:43 a.m., Mihails Strasuns wrote:
> Old usage: jit-elf-main lib.so 2
> New usage: jit-elf-main lib.so.1 lib.so.2
> 
> Refactoring necessary to support running tests over multiple jit
> binaries rather than mapping the same binary muultiple times.
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-02-18  Mihails Strasuns  <mihails.strasuns@intel.com>
> 
> 	* gdb.base/jit-elf-main.c: read lib list from argc/argv
> 	* gdb.base/jit-elf.exp: compile N jit libraries and use the list
> 	* gdb.base/jit-elf-so.exp: ditto
> 
> Change-Id: Ie8f85ec6358604c14557b0417d6621b2f8942033
> Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
> ---
>  gdb/testsuite/gdb.base/jit-elf-main.c |  52 +++++--------
>  gdb/testsuite/gdb.base/jit-elf-so.exp | 108 ++++++++++++++++----------
>  gdb/testsuite/gdb.base/jit-elf.exp    |  99 +++++++++++------------
>  3 files changed, 133 insertions(+), 126 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
> index fe0f540d6f..0e4b2e9a40 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-main.c
> +++ b/gdb/testsuite/gdb.base/jit-elf-main.c
> @@ -45,9 +45,9 @@
>  #endif /* !ElfW  */
>  
>  static void
> -usage (const char *const argv0)
> +usage ()

usage (void)

> diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
> index 526414f43c..eee20e16c2 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-so.exp
> +++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
> @@ -26,45 +26,64 @@ if {[get_compiler_info]} {
>      return 1
>  }
>  
> -#
> -# test running programs
> -#
> -
> -set testfile jit-elf-dlmain
> -set srcfile ${testfile}.c
> -set binfile [standard_output_file ${testfile}]
> -if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug shlib_load}] != "" } {
> -    untested "failed to compile"
> -    return -1
> +proc compile_jit_main {options} {
> +    global srcdir subdir testfile2 srcfile2 binfile2
> +    set testfile2 jit-elf-main
> +    set srcfile2 ${testfile2}.c
> +    set binfile2 [standard_output_file $testfile2.so]

These variables don't need to have the suffix `2` anymore, let's remove it.

> +    set options [concat \
> +	$options \
> +	debug]
> +    if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" \
> +	$options] != "" } {
> +	untested "Failure to compile jit-elf-main"

If we fail to compile something, do we want to return some failure return status
and exit the test?  This is how it works currently.

> +    }
>  }
>  
> -set testfile2 jit-elf-main
> -set srcfile2 ${testfile2}.c
> -set binfile2 [standard_output_file ${testfile2}.so]
> -set binfile2_dlopen [shlib_target_file ${testfile2}.so]
> -if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" ${binfile2} {debug additional_flags="-DMAIN=jit_dl_main"}] != "" } {
> -    untested "failed to compile main shared library"
> -    return -1
> +proc compile_jit_dlmain {options} {
> +    global srcdir subdir testfile srcfile binfile
> +    set testfile jit-elf-dlmain
> +    set srcfile ${testfile}.c
> +    set binfile [standard_output_file $testfile]
> +    set options [concat \
> +	$options \
> +	debug]
> +    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +	executable $options] != "" } {

Indent this a bit more (typically 4 columns more than the `[`), otherwise it's
easy to confuse it with a statement in the `if`.  There are multiple instances
in the patch.

> +	untested "Failure to compile jit-elf-main"
> +    }
>  }
>  
> -set solib_testfile "jit-elf-solib"
> -set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
> -set solib_binfile [standard_output_file ${solib_testfile}.so]
> -set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so"
> +proc compile_n_jit_so {count options} {

`options` is unnecessary, callers always pass an empty string, you could remove it.

If it's needed in a subsequent patch, then this other patch can add the parameter
back when needed.

> +    global srcdir subdir solib_binfile_targets
> +    set solib_binfile_targets {}
> +    set solib_testfile jit-elf-solib
> +
> +    for {set i 1} {$i <= $count} {incr i} { 
> +	set solib_binfile [standard_output_file ${solib_testfile}.so.$i]
> +	set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
> +	set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so.$i"
> +
> +	# Note: compiling without debug info by default: some test
> +	# 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 ${solib_srcfile} ${solib_binfile} $options] != "" } {
> +	    untested "Failure to compile ${solib_binfile_test_msg}"
> +	}
>  
> -# Note: compiling without debug info: the library goes through symbol
> -# renaming by munging on its symbol table, and that wouldn't work for .debug
> -# sections.  Also, output for "info function" changes when debug info is resent.
> -if { [gdb_compile_shlib ${solib_srcfile} ${solib_binfile} {}] != "" } {
> -    untested "failed to compile jit shared library"
> -    return -1
> +	set path [gdb_remote_download target ${solib_binfile}]
> +	set solib_binfile_targets [concat $solib_binfile_targets $path]

You might want to use `lappend` here.

https://www.tcl.tk/man/tcl8.6/TclCmd/lappend.htm

> +    }
>  }
>  
> -set solib_binfile_target [gdb_remote_download target ${solib_binfile}]
> +compile_jit_main {additional_flags="-DMAIN=jit_dl_main"}
> +compile_jit_dlmain {shlib_load}
> +compile_n_jit_so 2 {}
>  
>  proc one_jit_test {count match_str} {
>      with_test_prefix "one_jit_test-$count" {
> -	global verbose testfile srcfile2 binfile2 binfile2_dlopen solib_binfile_target solib_binfile_test_msg
> +	global verbose testfile srcfile2 binfile2 binfile2_dlopen solib_binfile_targets
>  
>  	clean_restart $testfile
>  	gdb_load_shlib $binfile2
> @@ -81,9 +100,16 @@ proc one_jit_test {count match_str} {
>  
>  	gdb_breakpoint [gdb_get_line_number "break here before-dlopen" ]
>  	gdb_continue_to_breakpoint "break here before-dlopen"
> +
>  	# Poke desired values directly into inferior instead of using "set args"
>  	# because "set args" does not work under gdbserver.
> -	gdb_test_no_output "set var jit_libname = \"$binfile2_dlopen\""
> +	gdb_test_no_output "set var jit_libname = \"$binfile2\""
> +	incr count
> +	gdb_test "set var argc=$count"
> +	gdb_test "set var argv=malloc(sizeof(char*)*$count)"
> +	for {set i 1} {$i < $count} {incr i} {
> +	    gdb_test "set var argv\[$i\]=\"[lindex $solib_binfile_targets [expr $i-1]]\""
> +	}		


Instead of using malloc (which makes GDB call a function in the inferior, could
we instead just allocate a sufficiently large static array in the test program?
We want to avoid interfering with the inferior as much as possible.

>  
>  	gdb_breakpoint [gdb_get_line_number "break here after-dlopen" ]
>  	gdb_continue_to_breakpoint "break here after-dlopen"
> @@ -91,10 +117,6 @@ proc one_jit_test {count match_str} {
>  	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 0} $srcfile2]"
>  	gdb_continue_to_breakpoint "break here 0"
>  
> -	gdb_test_no_output "set var argc = 2"
> -	gdb_test_no_output "set var libname = \"$solib_binfile_target\"" "set var libname = \"$solib_binfile_test_msg\""
> -	gdb_test_no_output "set var count = $count"
> -
>  	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 1} $srcfile2]"
>  	gdb_continue_to_breakpoint "break here 1"
>  
> @@ -114,12 +136,14 @@ proc one_jit_test {count match_str} {
>      }
>  }
>  
> -one_jit_test 1 "${hex}  jit_function_0000"
> -one_jit_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001"
> +one_jit_test 1 "${hex}  jit_function_0001"
> +one_jit_test 2 "${hex}  jit_function_0001\[\r\n\]+${hex}  jit_function_0002"
>  
> -# We don't intend to load the .so as a JIT debuginfo reader, but we
> -# need some handy file name for a completion test.
> -gdb_test \
> -    "complete jit-reader-load [standard_output_file ${solib_testfile}.s]" \
> -    "jit-reader-load $solib_binfile" \
> -    "test jit-reader-load filename completion"
> +foreach solib $solib_binfile_targets {
> +    # We don't intend to load the .so as a JIT debuginfo reader, but we
> +    # need some handy file name for a completion test.
> +    gdb_test \
> +	"complete jit-reader-load [standard_output_file $solib]" \
> +	"jit-reader-load $solib" \
> +	"test jit-reader-load filename completion"
> +}

Hmm, there is not much point doing this test multiple times, and this
actually changes what is tested.  Before it tests that

    jit-reader-load .../something.s

gets completed to

    jit-reader-load .../something.so

With the patch, it tests that

    jit-reader-load .../something.so

gets completed to

    jit-reader-load .../something.so

It's a bit strange in the first place to test that in this particular file,
but anyhow, let's try to preserve the original intent of the test.  You could
just test with the first element of solib_binfile_targets and shorten it by one
character with:

string range $name 0 [expr { [string length $name] - 2 }]

> diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
> index 71d3e37dfb..ba79efaf1c 100644
> --- a/gdb/testsuite/gdb.base/jit-elf.exp
> +++ b/gdb/testsuite/gdb.base/jit-elf.exp
> @@ -23,42 +23,41 @@ if {[get_compiler_info]} {
>      return 1
>  }
>  
> -# Compile the testcase program and library.  BINSUFFIX is the suffix
> -# to append to the program and library filenames, to make them unique
> -# between invocations.  OPTIONS is passed to gdb_compile when
> -# compiling the program.
> -
> -proc compile_jit_test {testname binsuffix options} {
> -    global testfile srcfile binfile srcdir subdir
> -    global solib_testfile solib_srcfile solib_binfile solib_binfile_test_msg
> -    global solib_binfile_target
> -
> +proc compile_jit_main {binsuffix options} {

Please add a comment to describe what this proc does, in the style of the comment
that was above `compile_jit_test`.

> +    global srcdir subdir testfile srcfile binfile
>      set testfile jit-elf-main
>      set srcfile ${testfile}.c
>      set binfile [standard_output_file $testfile$binsuffix]
> +    set options [concat \
> +	$options \
> +	debug]
>      if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> -	      executable [concat debug $options]] != "" } {
> -	untested $testname
> -	return -1
> -    }
> -
> -    set solib_testfile "jit-elf-solib"
> -    set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
> -    set solib_binfile [standard_output_file ${solib_testfile}$binsuffix.so]
> -    set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}$binsuffix.so"
> -
> -    # Note: compiling without debug info: the library goes through
> -    # symbol renaming by munging on its 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 ${solib_srcfile} ${solib_binfile} {-fPIC}] != "" } {
> -	untested $testname
> -	return -1
> +	executable $options] != "" } {
> +	untested "Failure to compile jit-elf-main"
>      }
> +}
>  
> -    set solib_binfile_target [gdb_remote_download target ${solib_binfile}]
> +proc compile_n_jit_so {count options} {

Comment here too.

> +    global srcdir subdir solib_binfile_targets
> +    set solib_binfile_targets {}
> +    set solib_testfile jit-elf-solib
> +
> +    for {set i 1} {$i <= $count} {incr i} { 
> +	set solib_binfile [standard_output_file ${solib_testfile}.so.$i]
> +	set solib_srcfile "${srcdir}/${subdir}/${solib_testfile}.c"
> +	set solib_binfile_test_msg "SHLIBDIR/${solib_testfile}.so.$i"
> +
> +	# Note: compiling without debug info by default: some test
> +	# 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 ${solib_srcfile} ${solib_binfile} $options] != "" } {
> +	    untested "Failure to compile ${solib_binfile_test_msg}"
> +	}
>  
> -    return 0
> +	set path [gdb_remote_download target ${solib_binfile}]
> +	set solib_binfile_targets [concat $solib_binfile_targets $path]

lappend here too.

Simon

Re: [PATCH 4/7] [gdb/testsuite] use args as lib list for jit-elf tests

[Simon Marchi]

On 2020-03-22 8:04 p.m., Simon Marchi wrote:
>> +proc compile_jit_main {options} {
>> +    global srcdir subdir testfile2 srcfile2 binfile2
>> +    set testfile2 jit-elf-main
>> +    set srcfile2 ${testfile2}.c
>> +    set binfile2 [standard_output_file $testfile2.so]
> 
> These variables don't need to have the suffix `2` anymore, let's remove it.

In fact, correct me if I'm wrong, but I think testfile2, srcfile2 and binfile2 don't need to be global.

Simon

Re: [PATCH 5/7] [gdb/testsuite] add lib/jit-elf-helpers.exp

[Simon Marchi]

On 2020-02-18 7:43 a.m., Mihails Strasuns wrote:
> New utility library to be used by jit-elf tests responsible for
> compiling binary artifacts. In the next commit the compilation process
> will become more complicated because of extra mandatory flag - keeping
> it in one place will make tests less fragile.
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-02-18  Mihails Strasuns  <mihails.strasuns@intel.com>
> 
> 	* lib/jit-elf-helpers.exp: new file
> 	* gdb.base/jit-elf.exp: updated to use jit-elf-helpers.exp
> 	* gdb.base/jit-elf-so.exp: updated to use jit-elf-helpers.exp
> 
> Change-Id: I2fda518406aeca55e82df45edd67cef149497bbe
> Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
> ---
>  gdb/testsuite/gdb.base/jit-elf-so.exp | 69 +++++++------------------
>  gdb/testsuite/gdb.base/jit-elf.exp    | 39 +-------------
>  gdb/testsuite/lib/jit-elf-helpers.exp | 73 +++++++++++++++++++++++++++
>  3 files changed, 92 insertions(+), 89 deletions(-)
>  create mode 100644 gdb/testsuite/lib/jit-elf-helpers.exp
> 
> diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
> index eee20e16c2..31a422a5d2 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-so.exp
> +++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
> @@ -26,67 +26,32 @@ if {[get_compiler_info]} {
>      return 1
>  }
>  
> -proc compile_jit_main {options} {
> -    global srcdir subdir testfile2 srcfile2 binfile2
> -    set testfile2 jit-elf-main
> -    set srcfile2 ${testfile2}.c
> -    set binfile2 [standard_output_file $testfile2.so]
> -    set options [concat \
> -	$options \
> -	debug]
> -    if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile2}" "${binfile2}" \
> -	$options] != "" } {
> -	untested "Failure to compile jit-elf-main"
> -    }
> -}
> +load_lib jit-elf-helpers.exp
>  
>  proc compile_jit_dlmain {options} {
> -    global srcdir subdir testfile srcfile binfile
> -    set testfile jit-elf-dlmain
> -    set srcfile ${testfile}.c
> -    set binfile [standard_output_file $testfile]
> +    global srcdir subdir testfile_dlmain srcfile_dlmain
> +    set testfile_dlmain jit-elf-dlmain
> +    set srcfile_dlmain ${testfile_dlmain}.c
> +    set binfile [standard_output_file $testfile_dlmain]
>      set options [concat \
>  	$options \
>  	debug]
> -    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +    if { [gdb_compile "${srcdir}/${subdir}/${srcfile_dlmain}" "${binfile}" \
>  	executable $options] != "" } {
>  	untested "Failure to compile jit-elf-main"
>      }
>  }

It looks like we're returning values through global variables, which are then used
in the top-level code.  Let's use return values if possible.  The local variable
names can remain unchanged, to minimize the diff.

One convenient way to return multiple values is using a list and lassign:

proc foo { } {
	return [list a b c]
}
lassign [foo] x y z
puts "$x $y $z"

> diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
> new file mode 100644
> index 0000000000..f63694b8f5
> --- /dev/null
> +++ b/gdb/testsuite/lib/jit-elf-helpers.exp
> @@ -0,0 +1,73 @@
> +# Copyright 2020 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Compiles jit-elf-main.c as a regular executable
> +
> +proc compile_jit_main {binsuffix options} {
> +    global srcdir subdir testfile srcfile binfile
> +    set testfile jit-elf-main
> +    set srcfile ${testfile}.c
> +    set binfile [standard_output_file $testfile$binsuffix]
> +    set options [concat \
> +	$options \
> +	debug]
> +    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +	executable $options] != "" } {
> +	untested "Failure to compile jit-elf-main"
> +    }
> +}
> +
> +# Compiles jit-elf-main.c as a shared library
> +
> +proc compile_jit_main_as_so {binsuffix options} {
> +    global srcdir subdir testfile srcfile binfile
> +    set testfile jit-elf-main
> +    set srcfile ${testfile}.c
> +    set binfile [standard_output_file $testfile$binsuffix]
> +    set options [concat \
> +	$options \
> +	additional_flags="-DMAIN=jit_dl_main" \
> +	debug]
> +    if { [gdb_compile_shlib "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
> +	 $options] != "" } {
> +	untested "Failure to compile jit-elf-main"
> +    }
> +}
> +
> +# Compiles jit-elf-solib.c as multiple shared libraries
> +# distinguished by a numerical suffix
> +
> +proc compile_n_jit_so {count binsuffix options} {
> +    global srcdir subdir solib_binfile_targets

I almost mentioned it in the previous patch, but especially now since this is
split in a separate library, I would suggest make this function return
solib_binfile_targets, instead of setting a global variable, that will make it
much easier to follow.

Simon

Re: [PATCH 6/7] [gdb/testsuite] use -Ttext-segment for jit-elf tests

[Simon Marchi]

Just some formatting nits:

> @@ -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 while (wait_for_gdb)
> @@ -137,7 +139,8 @@ MAIN (int argc, char *argv[])
>  	  exit (1);
>  	}
>  
> -      const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE,
> +      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, fd, 0);

We try to use the same style in code for tests as code for GDB itself.  So

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

should be

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

I think we should be using MAP_FIXED in this mmap call, to make sure the mapping ends
up exactly at `load_addr`, and check the result to make sure it's not MAP_FAILED.

>        struct jit_code_entry *const entry = calloc (1, sizeof (*entry));
>  
> @@ -147,7 +150,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 f63694b8f5..cff47ec73c 100644
> --- a/gdb/testsuite/lib/jit-elf-helpers.exp
> +++ b/gdb/testsuite/lib/jit-elf-helpers.exp
> @@ -13,15 +13,25 @@
>  # 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

Our standard is to use two spaces after periods in comments and other "prose".

Simon

Re: [PATCH 7/7] [gdb/testsuite] add jit-elf-util.h and run jit function

[Simon Marchi]

On 2020-02-18 7:43 a.m., Mihails Strasuns wrote:
> +/* Looks into string tables for entry
> +   "jit_function_XXXX" and updates it to use `rename_num`.  */
> +static void
> +update_name (ElfW (Addr) addr, int rename_num)
> +{
> +  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);
> +
> +  for (int i = 0; i < ehdr->e_shnum; ++i)
> +    {
> +      if (shdr[i].sh_type == SHT_STRTAB)
> +	{
> +	  /* Note: we update both .strtab and .dynstr.  The latter would
> +	     not be correct if this were a regular shared library (.hash
> +	     would be wrong), but this is a simulation -- the library is
> +	     never exposed to the dynamic loader, so it all ends up ok.  */
> +	  char *const strtab = (char *) (addr + shdr[i].sh_offset);
> +	  char *const strtab_end = strtab + shdr[i].sh_size;
> +	  char *p;
> +
> +	  for (p = strtab; p < strtab_end; p += strlen (p) + 1)
> +	    if (strcmp (p, "jit_function_XXXX") == 0)
> +	      sprintf (p, "jit_function_%04d", rename_num);
> +	}
> +    }
> +}

I was wondering about this function.  It updates the function name in the ELF string
table, but not in the DWARF debug info, so what is GDB going to display if the debug
info says that the function is called `jit_function_XXXX`?

In fact, since we generate multiple shared objects, could we just generate them with
the right function names directly?  The .exp would compile the first one with

  -DJIT_FUNCTION_NAME=jit_function_0001

the second one with

  -DJIT_FUNCTION_NAME=jit_function_0002

and so forth.  We wouldn't need any of this name munging.

> +
> +/* Find symbol with the name `sym_name`.  */
> +static ElfW (Addr)
> +load_symbol (ElfW (Addr) addr, const char *sym_name)
> +{
> +  const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *) addr;
> +  ElfW (Shdr) *const shdr = (ElfW (Shdr) *) ((char *) addr + ehdr->e_shoff);
> +
> +  ElfW (Addr) sym_old_addr = 0;
> +  ElfW (Addr) sym_new_addr = 0;
> +
> +  /* Find `func_name` in symbol_table and adjust it from the addr.  */
> +
> +  for (int i = 0; i < ehdr->e_shnum; ++i)
> +    {
> +      if (shdr[i].sh_type == SHT_SYMTAB)
> +	{
> +	  ElfW (Sym) *symtab = (ElfW (Sym) *) (addr + shdr[i].sh_offset);
> +	  ElfW (Sym) *symtab_end
> +	      = (ElfW (Sym) *) (addr + shdr[i].sh_offset + shdr[i].sh_size);
> +	  char *const strtab
> +	      = (char *) (addr + shdr[shdr[i].sh_link].sh_offset);
> +
> +	  for (ElfW (Sym) *p = symtab; p < symtab_end; ++p)
> +	    {
> +	      const char *s = strtab + p->st_name;
> +	      if (strcmp (s, sym_name) == 0)
> +	        return p->st_value;
> +	    }
> +	}
> +    }
> +
> +  fprintf (stderr, "symbol '%s' not found\n", sym_name);
> +  exit (1);
> +  return 0;
> +}
> +
> +/* Open an elf binary file and memory map it with execution flag enabled.  */
> +ElfW (Addr)
> +load_elf (const char *libname, size_t *size, ElfW (Addr) load_addr)

I may not matter here, but since this is in a header, please make this function static.

> +{
> +  int fd;
> +  struct stat st;
> +
> +  if ((fd = open (libname, O_RDONLY)) == -1)
> +    {
> +      fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", libname,
> +	       strerror (errno));
> +      exit (1);
> +    }
> +
> +  if (fstat (fd, &st) != 0)
> +    {
> +      fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
> +      exit (1);
> +    }
> +
> +  void *addr = mmap ((void *) load_addr, st.st_size,
> +		     PROT_READ | PROT_WRITE | PROT_EXEC,
> +		     load_addr ? MAP_PRIVATE | MAP_FIXED : MAP_PRIVATE, fd, 0);

I see that you've used MAP_FIXED here, which means you changed the code in addition to
moving the function.  This makes it difficult to review, since I can't really look at
the diff for this function.

When moving code around, we try to avoid changing it at the same time (other than the
necessary adjustments).  Either make those changes part of the previous patch (it seems
to me like they are related enough), or make a separate patch for modifying the code vs
moving it.

Simon

RE: [PATCH 7/7] [gdb/testsuite] add jit-elf-util.h and run jit function

[Strasuns, Mihails]

> -----Original Message-----
> From: Simon Marchi <simark@simark.ca>
> Sent: Monday, March 23, 2020 4:13 AM
> To: Strasuns, Mihails <mihails.strasuns@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH 7/7] [gdb/testsuite] add jit-elf-util.h and run jit function
> 
> On 2020-02-18 7:43 a.m., Mihails Strasuns wrote:
> > +/* Looks into string tables for entry
> > +   "jit_function_XXXX" and updates it to use `rename_num`.  */ static
> > +void update_name (ElfW (Addr) addr, int rename_num) {
> > +  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);
> > +
> > +  for (int i = 0; i < ehdr->e_shnum; ++i)
> > +    {
> > +      if (shdr[i].sh_type == SHT_STRTAB)
> > +	{
> > +	  /* Note: we update both .strtab and .dynstr.  The latter would
> > +	     not be correct if this were a regular shared library (.hash
> > +	     would be wrong), but this is a simulation -- the library is
> > +	     never exposed to the dynamic loader, so it all ends up ok.  */
> > +	  char *const strtab = (char *) (addr + shdr[i].sh_offset);
> > +	  char *const strtab_end = strtab + shdr[i].sh_size;
> > +	  char *p;
> > +
> > +	  for (p = strtab; p < strtab_end; p += strlen (p) + 1)
> > +	    if (strcmp (p, "jit_function_XXXX") == 0)
> > +	      sprintf (p, "jit_function_%04d", rename_num);
> > +	}
> > +    }
> > +}
> 
> I was wondering about this function.  It updates the function name in the ELF
> string table, but not in the DWARF debug info, so what is GDB going to display
> if the debug info says that the function is called `jit_function_XXXX`?
> 
> In fact, since we generate multiple shared objects, could we just generate
> them with the right function names directly?  The .exp would compile the
> first one with
> 
>   -DJIT_FUNCTION_NAME=jit_function_0001
> 
> the second one with
> 
>   -DJIT_FUNCTION_NAME=jit_function_0002
> 
> and so forth.  We wouldn't need any of this name munging.

Hello,

Here I was trying to preserve the old test flow as much as possible - I did seem a bit strange to me though.
Original version relies on _not_ having DWARF debug info present (there is a comment about it in .exp file), the
same applies for the refactored one. If you think simply emitting different function names in the first place is
an equivalent change, I am more than happy to get rid of this :)

Will address rest of comments and submit a new patchset soon, thanks for review!

BR,
Mihails
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

Re: [PATCH 7/7] [gdb/testsuite] add jit-elf-util.h and run jit function

[Simon Marchi]

On 2020-03-23 5:23 a.m., Strasuns, Mihails wrote:
> Hello,
> 
> Here I was trying to preserve the old test flow as much as possible - I did seem a bit strange to me though.
> Original version relies on _not_ having DWARF debug info present (there is a comment about it in .exp file), the
> same applies for the refactored one. If you think simply emitting different function names in the first place is
> an equivalent change, I am more than happy to get rid of this :)
> 
> Will address rest of comments and submit a new patchset soon, thanks for review!

Yes, it's a good idea to keep the tests testing binaries without debug info.  But
I think it should be possible to generate binaries without debug info, but with
the right function names in the first place, isn't it?  These are not inter-dependent?

Simon