public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v4 0/5] jit testsuite refactoring
       [not found] <167748>
@ 2020-04-21 13:31 ` Mihails Strasuns
  2020-04-21 13:31   ` [PATCH v4 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
                     ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Mihails Strasuns @ 2020-04-21 13:31 UTC (permalink / raw)
  To: gdb-patches

Pushed first 3 patches in the series after addressing style comments:

946422b6a113063b9bbd72832ed13d4694134597
922a7c7c5d4040f9e4ab75a059b9ca33f45ab952
f49c464f933172bae5685c2fb51b9e220902146c

Remaining patches are rebased on the version suggested by Simon and submitted
here.

Mihails Strasuns (4):
  [gdb/testsuite] add lib/jit-elf-helpers.exp
  [gdb/testsuite] use -Ttext-segment for jit-elf tests
  [gdb/testsuite] define jit function name via macro
  [gdb/testsuite] add jit-elf-util.h and run jit function

Mihails Strasuns via Gdb-patches (1):
  [gdb/testsuite] use args as lib list for jit-elf tests

 gdb/testsuite/gdb.base/jit-elf-main.c  | 128 ++++++----------------
 gdb/testsuite/gdb.base/jit-elf-so.exp  | 143 ++++++++++++++++---------
 gdb/testsuite/gdb.base/jit-elf-solib.c |   6 +-
 gdb/testsuite/gdb.base/jit-elf-util.h  | 117 ++++++++++++++++++++
 gdb/testsuite/gdb.base/jit-elf.exp     | 113 ++++++++-----------
 gdb/testsuite/lib/jit-elf-helpers.exp  | 108 +++++++++++++++++++
 6 files changed, 399 insertions(+), 216 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-elf-util.h
 create mode 100644 gdb/testsuite/lib/jit-elf-helpers.exp

-- 
2.26.1

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 1/5] [gdb/testsuite] use args as lib list for jit-elf tests
  2020-04-21 13:31 ` [PATCH v4 0/5] jit testsuite refactoring Mihails Strasuns
@ 2020-04-21 13:31   ` Mihails Strasuns
  2020-04-21 13:31   ` [PATCH v4 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Mihails Strasuns @ 2020-04-21 13:31 UTC (permalink / raw)
  To: gdb-patches

From: Mihails Strasuns via Gdb-patches <gdb-patches@sourceware.org>

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.
---
 gdb/testsuite/gdb.base/jit-elf-main.c |  56 ++++----
 gdb/testsuite/gdb.base/jit-elf-so.exp | 179 +++++++++++++++++++-------
 gdb/testsuite/gdb.base/jit-elf.exp    | 154 +++++++++++++---------
 3 files changed, 248 insertions(+), 141 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index 4eaac514b5..acfd17d417 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 (void)
 {
-  fprintf (stderr, "Usage: %s library [count]\n", argv0);
+  fprintf (stderr, "Usage: jit-elf-main libraries...\n");
   exit (1);
 }
 
@@ -106,49 +106,39 @@ 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);
+  /* Used as backing storage for GDB to populate argv.  */
+  char *fake_argv[10];
 
   mypid = getpid ();
-
-  count = count;  /* gdb break here 0  */
+  /* gdb break here 0  */
 
   if (argc < 2)
     {
-      usage (argv[0]);
+      usage ();
       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..59ca1ac8cf 100644
--- a/gdb/testsuite/gdb.base/jit-elf-so.exp
+++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
@@ -26,48 +26,105 @@ if {[get_compiler_info]} {
     return 1
 }
 
+# The "real" main of this test, which loads jit-elf-main
+# as a shared library.
+set main_loader_basename jit-elf-dlmain
+set main_loader_srcfile ${srcdir}/${subdir}/${main_loader_basename}.c
+set main_loader_binfile [standard_output_file ${main_loader_basename}]
+
+# The main code that loads and registers JIT objects.
+set main_solib_basename jit-elf-main
+set main_solib_srcfile ${srcdir}/${subdir}/${main_solib_basename}.c
+set main_solib_binfile [standard_output_file ${main_solib_basename}.so]
+
+# The shared library that gets loaded as JIT objects.
+set jit_solib_basename jit-elf-solib
+set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
+
+# Compile jit-elf-main.c as a shared library.
 #
-# test running programs
+# OPTIONS is passed to gdb_compile when compiling the program.
 #
+# On success, return 0.
+# On failure, return -1.
+proc compile_jit_elf_main_as_so {options} {
+    global main_solib_srcfile main_solib_binfile
+    set options [concat $options debug]
+
+    if { [gdb_compile_shlib ${main_solib_srcfile} ${main_solib_binfile} \
+	    $options] != "" } {
+	untested "failed to compile ${main_solib_basename}.c as a shared library"
+	return -1
+    }
 
-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
+    return 0
 }
 
-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
+# Compile the testcase shared library loader.
+#
+# OPTIONS is passed to gdb_compile when compiling the binary.
+#
+# On success, return 0.
+# On failure, return -1.
+proc compile_jit_dlmain {options} {
+    global main_loader_srcfile main_loader_binfile
+    set options [concat $options debug]
+
+    if { [gdb_compile ${main_loader_srcfile} ${main_loader_binfile} \
+	    executable $options] != "" } {
+	untested "failed to compile ${main_loader_basename}.c as an executable"
+	return -1
+    }
+
+    return 0
 }
 
-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"
+# Compile jit-elf-solib.c as a shared library in multiple copies and
+# upload them to the target.
+#
+# On success, return a list of target path to the shared libraries.
+# On failure, return -1.
+proc compile_and_download_n_jit_so {count} {
+    global jit_solib_basename jit_solib_srcfile
+    set binfiles_target {}
+
+    for {set i 1} {$i <= $count} {incr i} {
+	set binfile [standard_output_file ${jit_solib_basename}.$i.so]
+
+	# 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 ${jit_solib_srcfile} ${binfile} {}] != "" } {
+	    untested "failed to compile ${jit_solib_basename}.c as a shared library"
+	    return -1
+	}
+
+	set path [gdb_remote_download target ${binfile}]
+	lappend binfiles_target $path
+    }
 
-# 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
+    return $binfiles_target
 }
 
-set solib_binfile_target [gdb_remote_download target ${solib_binfile}]
+# Run $main_loader_binfile and load $main_solib_binfile in
+# GDB.  Check jit-related debug output and matches `info function`
+# output for a jit loaded function using MATCH_STR.
+#
+# SOLIB_BINFILES_TARGETS is a list of shared libraries to pass
+# as arguments when running $main_loader_binfile.
+# MATCH_STR is a regular expression that output of `info function`
+# must match.
+proc one_jit_test {solib_binfiles_target match_str} {
+    set count [llength $solib_binfiles_target]
 
-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
+	global main_loader_binfile main_loader_srcfile
+	global main_solib_binfile main_solib_srcfile
 
-	clean_restart $testfile
-	gdb_load_shlib $binfile2
+	clean_restart $main_loader_binfile
+	gdb_load_shlib $main_solib_binfile
 
 	# This is just to help debugging when things fail
 	if {$verbose > 0} {
@@ -79,23 +136,30 @@ 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" \
+			    $main_loader_srcfile]
 	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 = \"$main_solib_binfile\""
+	gdb_test "set var argc=[expr $count + 1]"
+	gdb_test "set var argv=fake_argv"
+	for {set i 1} {$i <= $count} {incr i} {
+	    set binfile_target [lindex $solib_binfiles_target [expr $i-1]]
+	    gdb_test "set var argv\[$i\]=\"${binfile_target}\""
+	}
 
-	gdb_breakpoint [gdb_get_line_number "break here after-dlopen" ]
+	gdb_breakpoint [gdb_get_line_number "break here after-dlopen" \
+			$main_loader_srcfile]
 	gdb_continue_to_breakpoint "break here after-dlopen"
 
-	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 0} $srcfile2]"
+	set line [gdb_get_line_number {break here 0} $main_solib_srcfile]
+	gdb_breakpoint "$main_solib_srcfile:$line"
 	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]"
+	set line [gdb_get_line_number {break here 1} $main_solib_srcfile]
+	gdb_breakpoint "$main_solib_srcfile:$line"
 	gdb_continue_to_breakpoint "break here 1"
 
 	gdb_test "info function jit_function" "$match_str"
@@ -106,20 +170,41 @@ proc one_jit_test {count match_str} {
 	    gdb_test "maintenance info break"
 	}
 
-	gdb_breakpoint "$srcfile2:[gdb_get_line_number {break here 2} $srcfile2]"
+	set line [gdb_get_line_number {break here 2} $main_solib_srcfile]
+	gdb_breakpoint "$main_solib_srcfile:$line"
 	gdb_continue_to_breakpoint "break here 2"
+
 	# All jit librares must have been unregistered
 	gdb_test "info function jit_function" \
 	    "All functions matching regular expression \"jit_function\":"
     }
 }
 
-one_jit_test 1 "${hex}  jit_function_0000"
-one_jit_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001"
+# Compile the main code (which loads the JIT objects) as a shared library.
+if { [compile_jit_elf_main_as_so {additional_flags="-DMAIN=jit_dl_main"}] < 0 } {
+    return
+}
 
-# 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"
+# Compile the "real" main for this test.
+if { [compile_jit_dlmain {shlib_load}] < 0 } {
+    return
+}
+
+# Compile two shared libraries to use as JIT objects.
+set jit_solibs_target [compile_and_download_n_jit_so 2]
+if { $jit_solibs_target == -1 } {
+    return
+}
+
+one_jit_test [lindex $jit_solibs_target 0] "${hex}  jit_function_0001"
+one_jit_test $jit_solibs_target "${hex}  jit_function_0001\[\r\n\]+${hex}  jit_function_0002"
+
+foreach solib $jit_solibs_target {
+    # We don't intend to load the .so as a JIT debuginfo reader, but we
+    # need some handy file name for a completion test.
+    set input [string range $solib 0 [expr { [string length $solib] - 2 }]]
+    gdb_test \
+	"complete jit-reader-load [standard_output_file $input]" \
+	"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..29638bd2c0 100644
--- a/gdb/testsuite/gdb.base/jit-elf.exp
+++ b/gdb/testsuite/gdb.base/jit-elf.exp
@@ -23,48 +23,70 @@ 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
-
-    set testfile jit-elf-main
-    set srcfile ${testfile}.c
-    set binfile [standard_output_file $testfile$binsuffix]
-    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
-	      executable [concat debug $options]] != "" } {
-	untested $testname
-	return -1
-    }
+# The main code that loads and registers JIT objects.
+set main_basename jit-elf-main
+set main_srcfile ${srcdir}/${subdir}/${main_basename}.c
+set main_binfile [standard_output_file ${main_basename}]
 
-    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
-    }
+# The shared library that gets loaded as JIT objects.
+set jit_solib_basename jit-elf-solib
+set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
 
-    set solib_binfile_target [gdb_remote_download target ${solib_binfile}]
+# Compile jit-elf-main.c as an executable.
+#
+# BINSUFFIX is appended to the binary name.
+# OPTIONS is passed to gdb_compile when compiling the program.
+#
+# On success, return 0.
+# On failure, return -1.
+proc compile_jit_main {binsuffix options} {
+    global main_binfile main_srcfile main_basename
+
+    set binfile ${main_binfile}${binsuffix}
+    set options [concat $options debug]
+
+    if { [gdb_compile ${main_srcfile} ${binfile} \
+	  executable $options] != "" } {
+	      untested "failed to compile ${main_basename}.c"
+	      return -1
+    }
 
     return 0
 }
 
+# Compile jit-elf-solib.c as a shared library in multiple copies and
+# upload them to the target.
+#
+# On success, return a list of target paths to the shared libraries.
+# On failure, return -1.
+proc compile_and_download_n_jit_so {count} {
+    global jit_solib_basename jit_solib_srcfile
+    set binfiles_target {}
+
+    for {set i 1} {$i <= $count} {incr i} {
+	set binfile [standard_output_file ${jit_solib_basename}.$i.so]
+
+	# 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 ${jit_solib_srcfile} ${binfile} {}] != "" } {
+	    untested "failed to compile ${jit_solib_basename}.c as a shared library"
+	    return -1
+	}
+
+	set path [gdb_remote_download target ${binfile}]
+	lappend binfiles_target $path
+    }
+
+    return $binfiles_target
+}
+
 # Detach, restart GDB, and re-attach to the program.
 
 proc clean_reattach {} {
-    global decimal gdb_prompt srcfile testfile
+    global decimal gdb_prompt
+    global main_binfile main_srcfile
 
     # Get PID of test program.
     set testpid -1
@@ -79,11 +101,11 @@ proc clean_reattach {} {
     gdb_test_no_output "set var wait_for_gdb = 1"
     gdb_test "detach" "Detaching from .*"
 
-    clean_restart $testfile
+    clean_restart ${main_binfile}
 
     set test "attach"
     gdb_test_multiple "attach $testpid" "$test" {
-	-re "Attaching to program.*.*main.*at .*$srcfile:.*$gdb_prompt $" {
+	-re "Attaching to program.*.*main.*at .*$main_srcfile:.*$gdb_prompt $" {
 	    pass "$test"
 	}
     }
@@ -94,7 +116,9 @@ proc clean_reattach {} {
 # Continue to LOCATION in the program.  If REATTACH, detach and
 # re-attach to the program from scratch.
 proc continue_to_test_location {location reattach} {
-    gdb_breakpoint [gdb_get_line_number $location]
+    global main_srcfile
+
+    gdb_breakpoint [gdb_get_line_number $location $main_srcfile]
     gdb_continue_to_breakpoint $location
     if {$reattach} {
 	with_test_prefix "$location" {
@@ -103,11 +127,14 @@ proc continue_to_test_location {location reattach} {
     }
 }
 
-proc one_jit_test {count match_str reattach} {
+proc one_jit_test {jit_solibs_target match_str reattach} {
+    set count [llength $jit_solibs_target]
+
     with_test_prefix "one_jit_test-$count" {
-	global verbose testfile solib_binfile_target solib_binfile_test_msg
+	global verbose
+	global main_binfile main_srcfile
 
-	clean_restart $testfile
+	clean_restart ${main_binfile}
 
 	# This is just to help debugging when things fail
 	if {$verbose > 0} {
@@ -119,14 +146,19 @@ proc one_jit_test {count match_str reattach} {
 	    return
 	}
 
-	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"
+	incr count
+	gdb_test "set var argc=$count"
+	gdb_test "set var argv=fake_argv"
+	for {set i 1} {$i < $count} {incr i} {
+	    set jit_solib_target [lindex $jit_solibs_target [expr $i-1]]
+	    gdb_test "set var argv\[$i\]=\"${jit_solib_target}\""
+	}
+
+	gdb_breakpoint [gdb_get_line_number "break here 0" $main_srcfile]
+	gdb_continue_to_breakpoint "break here 0"
+
 
 	continue_to_test_location "break here 1" $reattach
 
@@ -146,31 +178,31 @@ proc one_jit_test {count match_str reattach} {
     }
 }
 
-if {[compile_jit_test jit.exp "" {}] < 0} {
+# Compile two shared libraries to use as JIT objects.
+set jit_solibs_target [compile_and_download_n_jit_so 2]
+if { $jit_solibs_target == -1 } {
     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 the main code (which loads the JIT objects).
+if { [compile_jit_main "" {}] == 0 } {
+    one_jit_test [lindex $jit_solibs_target 0] "${hex}  jit_function_0001" 0
+    one_jit_test $jit_solibs_target "${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
-    }
-
-    with_test_prefix attach {
-	one_jit_test 2 "${hex}  jit_function_0000\[\r\n\]+${hex}  jit_function_0001" 1
+    if { [compile_jit_main "-attach" {additional_flags=-DATTACH=1}] == 0 } {
+	with_test_prefix attach {
+	    one_jit_test $jit_solibs_target "${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
+if { [compile_jit_main "-pie" {additional_flags=-fPIE ldflags=-pie}] == 0 } {
+    with_test_prefix PIE {
+	one_jit_test [lindex $jit_solibs_target 0] "${hex}  jit_function_0001" 0
     }
-
-    one_jit_test 1 "${hex}  jit_function_0000" 0
 }
-- 
2.26.1

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp
  2020-04-21 13:31 ` [PATCH v4 0/5] jit testsuite refactoring Mihails Strasuns
  2020-04-21 13:31   ` [PATCH v4 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
@ 2020-04-21 13:31   ` Mihails Strasuns
  2020-04-21 15:55     ` Simon Marchi
  2020-04-21 13:31   ` [PATCH v4 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests Mihails Strasuns
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Mihails Strasuns @ 2020-04-21 13:31 UTC (permalink / raw)
  To: gdb-patches

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.
---
 gdb/testsuite/gdb.base/jit-elf-so.exp | 50 +---------------
 gdb/testsuite/gdb.base/jit-elf.exp    | 51 ----------------
 gdb/testsuite/lib/jit-elf-helpers.exp | 85 +++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 99 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 59ca1ac8cf..af80d95d92 100644
--- a/gdb/testsuite/gdb.base/jit-elf-so.exp
+++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
@@ -26,6 +26,8 @@ if {[get_compiler_info]} {
     return 1
 }
 
+load_lib jit-elf-helpers.exp
+
 # The "real" main of this test, which loads jit-elf-main
 # as a shared library.
 set main_loader_basename jit-elf-dlmain
@@ -41,25 +43,6 @@ set main_solib_binfile [standard_output_file ${main_solib_basename}.so]
 set jit_solib_basename jit-elf-solib
 set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
 
-# Compile jit-elf-main.c as a shared library.
-#
-# OPTIONS is passed to gdb_compile when compiling the program.
-#
-# On success, return 0.
-# On failure, return -1.
-proc compile_jit_elf_main_as_so {options} {
-    global main_solib_srcfile main_solib_binfile
-    set options [concat $options debug]
-
-    if { [gdb_compile_shlib ${main_solib_srcfile} ${main_solib_binfile} \
-	    $options] != "" } {
-	untested "failed to compile ${main_solib_basename}.c as a shared library"
-	return -1
-    }
-
-    return 0
-}
-
 # Compile the testcase shared library loader.
 #
 # OPTIONS is passed to gdb_compile when compiling the binary.
@@ -79,34 +62,6 @@ proc compile_jit_dlmain {options} {
     return 0
 }
 
-# Compile jit-elf-solib.c as a shared library in multiple copies and
-# upload them to the target.
-#
-# On success, return a list of target path to the shared libraries.
-# On failure, return -1.
-proc compile_and_download_n_jit_so {count} {
-    global jit_solib_basename jit_solib_srcfile
-    set binfiles_target {}
-
-    for {set i 1} {$i <= $count} {incr i} {
-	set binfile [standard_output_file ${jit_solib_basename}.$i.so]
-
-	# 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 ${jit_solib_srcfile} ${binfile} {}] != "" } {
-	    untested "failed to compile ${jit_solib_basename}.c as a shared library"
-	    return -1
-	}
-
-	set path [gdb_remote_download target ${binfile}]
-	lappend binfiles_target $path
-    }
-
-    return $binfiles_target
-}
-
 # Run $main_loader_binfile and load $main_solib_binfile in
 # GDB.  Check jit-related debug output and matches `info function`
 # output for a jit loaded function using MATCH_STR.
@@ -117,7 +72,6 @@ proc compile_and_download_n_jit_so {count} {
 # must match.
 proc one_jit_test {solib_binfiles_target match_str} {
     set count [llength $solib_binfiles_target]
-
     with_test_prefix "one_jit_test-$count" {
 	global verbose
 	global main_loader_binfile main_loader_srcfile
diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
index 29638bd2c0..df3199622c 100644
--- a/gdb/testsuite/gdb.base/jit-elf.exp
+++ b/gdb/testsuite/gdb.base/jit-elf.exp
@@ -32,58 +32,7 @@ set main_binfile [standard_output_file ${main_basename}]
 set jit_solib_basename jit-elf-solib
 set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
 
-# Compile jit-elf-main.c as an executable.
-#
-# BINSUFFIX is appended to the binary name.
-# OPTIONS is passed to gdb_compile when compiling the program.
-#
-# On success, return 0.
-# On failure, return -1.
-proc compile_jit_main {binsuffix options} {
-    global main_binfile main_srcfile main_basename
-
-    set binfile ${main_binfile}${binsuffix}
-    set options [concat $options debug]
-
-    if { [gdb_compile ${main_srcfile} ${binfile} \
-	  executable $options] != "" } {
-	      untested "failed to compile ${main_basename}.c"
-	      return -1
-    }
-
-    return 0
-}
-
-# Compile jit-elf-solib.c as a shared library in multiple copies and
-# upload them to the target.
-#
-# On success, return a list of target paths to the shared libraries.
-# On failure, return -1.
-proc compile_and_download_n_jit_so {count} {
-    global jit_solib_basename jit_solib_srcfile
-    set binfiles_target {}
-
-    for {set i 1} {$i <= $count} {incr i} {
-	set binfile [standard_output_file ${jit_solib_basename}.$i.so]
-
-	# 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 ${jit_solib_srcfile} ${binfile} {}] != "" } {
-	    untested "failed to compile ${jit_solib_basename}.c as a shared library"
-	    return -1
-	}
-
-	set path [gdb_remote_download target ${binfile}]
-	lappend binfiles_target $path
-    }
-
-    return $binfiles_target
-}
-
 # Detach, restart GDB, and re-attach to the program.
-
 proc clean_reattach {} {
     global decimal gdb_prompt
     global main_binfile main_srcfile
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
new file mode 100644
index 0000000000..a40dd94cda
--- /dev/null
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -0,0 +1,85 @@
+# 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
+
+# Compile jit-elf-main.c as an executable.
+#
+# BINSUFFIX is appended to the binary name.
+# OPTIONS is passed to gdb_compile when compiling the program.
+#
+# On success, return 0.
+# On failure, return -1.
+proc compile_jit_main {binsuffix options} {
+    global main_binfile main_srcfile main_basename
+
+    set binfile ${main_binfile}${binsuffix}
+    set options [concat $options debug]
+
+    if { [gdb_compile ${main_srcfile} ${binfile} \
+	  executable $options] != "" } {
+	      untested "failed to compile ${main_basename}.c"
+	      return -1
+    }
+
+    return 0
+}
+
+# Compile jit-elf-main.c as a shared library.
+#
+# OPTIONS is passed to gdb_compile when compiling the program.
+#
+# On success, return 0.
+# On failure, return -1.
+proc compile_jit_elf_main_as_so {options} {
+    global main_solib_srcfile main_solib_binfile
+    set options [concat $options debug]
+
+    if { [gdb_compile_shlib ${main_solib_srcfile} ${main_solib_binfile} \
+	    $options] != "" } {
+	untested "failed to compile ${main_solib_basename}.c as a shared library"
+	return -1
+    }
+
+    return 0
+}
+
+# Compile jit-elf-solib.c as a shared library in multiple copies and
+# upload them to the target.
+#
+# On success, return a list of target path to the shared libraries.
+# On failure, return -1.
+proc compile_and_download_n_jit_so {count} {
+    global jit_solib_basename jit_solib_srcfile
+    set binfiles_target {}
+
+    for {set i 1} {$i <= $count} {incr i} {
+	set binfile [standard_output_file ${jit_solib_basename}.$i.so]
+
+	# 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 ${jit_solib_srcfile} ${binfile} {}] != "" } {
+	    untested "failed to compile ${jit_solib_basename}.c as a shared library"
+	    return -1
+	}
+
+	set path [gdb_remote_download target ${binfile}]
+	lappend binfiles_target $path
+    }
+
+    return $binfiles_target
+}
\ No newline at end of file
-- 
2.26.1

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests
  2020-04-21 13:31 ` [PATCH v4 0/5] jit testsuite refactoring Mihails Strasuns
  2020-04-21 13:31   ` [PATCH v4 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
  2020-04-21 13:31   ` [PATCH v4 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
@ 2020-04-21 13:31   ` Mihails Strasuns
  2020-04-22  2:08     ` Simon Marchi
  2020-04-21 13:31   ` [PATCH v4 4/5] [gdb/testsuite] define jit function name via macro Mihails Strasuns
  2020-04-21 13:31   ` [PATCH v4 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function Mihails Strasuns
  4 siblings, 1 reply; 11+ messages in thread
From: Mihails Strasuns @ 2020-04-21 13:31 UTC (permalink / raw)
  To: gdb-patches

Removes the need to manually relocate loaded ELF binary by using a fixed
constant as both mmap base address and as a requested first segment
address supplied to the linker.

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

gdb/testsuite/ChangeLog:

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

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

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

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 4/5] [gdb/testsuite] define jit function name via macro
  2020-04-21 13:31 ` [PATCH v4 0/5] jit testsuite refactoring Mihails Strasuns
                     ` (2 preceding siblings ...)
  2020-04-21 13:31   ` [PATCH v4 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests Mihails Strasuns
@ 2020-04-21 13:31   ` Mihails Strasuns
  2020-04-22  2:11     ` Simon Marchi
  2020-04-21 13:31   ` [PATCH v4 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function Mihails Strasuns
  4 siblings, 1 reply; 11+ messages in thread
From: Mihails Strasuns @ 2020-04-21 13:31 UTC (permalink / raw)
  To: gdb-patches

Replaces previous approach with patching resulting ELF binary after
loading - now that each test iteration works on a separately compiled
binary it is not necessary anymore.

Tests are still being ran without debug info to preserve original test
functionality but this change opens up the possibility to enable debug
info if needed too.

gdb/testsuite/ChangeLog:

2020-03-27  Mihails Strasuns  <mihails.strasuns@intel.com>

	* lib/jit-elf-helpers.exp: Supply -DFUNCTION_NAME macro
	  definition when compiling jit-elf-solib.co.
	* gdb.base/jit-elf-main.c: Stop patching jit function name.
	* gdb.base/jit-elf-solib.c: Use FUNCTION_NAME macro value as a
	  function name.
---
 gdb/testsuite/gdb.base/jit-elf-main.c  | 31 --------------------------
 gdb/testsuite/gdb.base/jit-elf-solib.c |  6 ++---
 gdb/testsuite/lib/jit-elf-helpers.exp  |  1 +
 3 files changed, 4 insertions(+), 34 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index ca150f3b1b..b74212dbf5 100644
--- a/gdb/testsuite/gdb.base/jit-elf-main.c
+++ b/gdb/testsuite/gdb.base/jit-elf-main.c
@@ -51,35 +51,6 @@ usage (void)
   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
@@ -152,8 +123,6 @@ MAIN (int argc, char *argv[])
 	  exit (1);
 	}
 
-      update_name (addr, i);
-
       /* Link entry at the end of the list.  */
       entry->symfile_addr = (const char *)addr;
       entry->symfile_size = st.st_size;
diff --git a/gdb/testsuite/gdb.base/jit-elf-solib.c b/gdb/testsuite/gdb.base/jit-elf-solib.c
index 3bdebe9ed0..7901c58ac9 100644
--- a/gdb/testsuite/gdb.base/jit-elf-solib.c
+++ b/gdb/testsuite/gdb.base/jit-elf-solib.c
@@ -15,7 +15,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/>.  */
 
-/* This simulates a JIT library.  The function is "renamed" after being
-   loaded into memory.  */
+/* This simulates a JIT library.  The function name is supplied during
+   compilation as a macro.  */
 
-int jit_function_XXXX() { return 42; }
+int FUNCTION_NAME() { return 42; }
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
index a46edf2051..24118cf597 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -92,6 +92,7 @@ proc compile_and_download_n_jit_so {count} {
 	# function" changes when debug info is present.
 	set addr [format 0x%x [expr $load_address + $load_increment * [expr $i-1]]]
 	set options [list \
+	    additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
 	    additional_flags=-Xlinker \
 	    additional_flags=-Ttext-segment=$addr]
 	if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} $options] != "" } {
-- 
2.26.1

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v4 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function
  2020-04-21 13:31 ` [PATCH v4 0/5] jit testsuite refactoring Mihails Strasuns
                     ` (3 preceding siblings ...)
  2020-04-21 13:31   ` [PATCH v4 4/5] [gdb/testsuite] define jit function name via macro Mihails Strasuns
@ 2020-04-21 13:31   ` Mihails Strasuns
  2020-04-22  2:25     ` Simon Marchi
  4 siblings, 1 reply; 11+ messages in thread
From: Mihails Strasuns @ 2020-04-21 13:31 UTC (permalink / raw)
  To: gdb-patches

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.
---
 gdb/testsuite/gdb.base/jit-elf-main.c |  56 ++++--------
 gdb/testsuite/gdb.base/jit-elf-util.h | 117 ++++++++++++++++++++++++++
 2 files changed, 133 insertions(+), 40 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 b74212dbf5..b812cb91e5 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 (void)
@@ -95,37 +83,19 @@ 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);
-	}
+      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);
 
-      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 | MAP_FIXED, 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);
-	}
+      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;
 
@@ -137,6 +107,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..e48d99f098
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-util.h
@@ -0,0 +1,117 @@
+/* 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  */
+
+/* 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.  */
+static 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;
+}
-- 
2.26.1

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp
  2020-04-21 13:31   ` [PATCH v4 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
@ 2020-04-21 15:55     ` Simon Marchi
  2020-04-22  9:20       ` Strasuns, Mihails
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Marchi @ 2020-04-21 15:55 UTC (permalink / raw)
  To: Mihails Strasuns, gdb-patches

On 2020-04-21 9:31 a.m., Mihails Strasuns via Gdb-patches 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.
> ---
>  gdb/testsuite/gdb.base/jit-elf-so.exp | 50 +---------------
>  gdb/testsuite/gdb.base/jit-elf.exp    | 51 ----------------
>  gdb/testsuite/lib/jit-elf-helpers.exp | 85 +++++++++++++++++++++++++++
>  3 files changed, 87 insertions(+), 99 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 59ca1ac8cf..af80d95d92 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-so.exp
> +++ b/gdb/testsuite/gdb.base/jit-elf-so.exp
> @@ -26,6 +26,8 @@ if {[get_compiler_info]} {
>      return 1
>  }
>  
> +load_lib jit-elf-helpers.exp
> +
>  # The "real" main of this test, which loads jit-elf-main
>  # as a shared library.
>  set main_loader_basename jit-elf-dlmain
> @@ -41,25 +43,6 @@ set main_solib_binfile [standard_output_file ${main_solib_basename}.so]
>  set jit_solib_basename jit-elf-solib
>  set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c

Now it's a bit confusing, because jit-elf-helpers.exp uses some definitions
that must be provided by the including file.  It would be better for it to
be self-contained.  So either move the setting of these definitions to
jit-elf-helpers.exp (and work out a naming scheme that is not too confusing),
or modify the procedures to take parameters, and update the callers to pass
in the values as arguments.

>  
> -# Compile jit-elf-main.c as a shared library.
> -#
> -# OPTIONS is passed to gdb_compile when compiling the program.
> -#
> -# On success, return 0.
> -# On failure, return -1.
> -proc compile_jit_elf_main_as_so {options} {
> -    global main_solib_srcfile main_solib_binfile
> -    set options [concat $options debug]
> -
> -    if { [gdb_compile_shlib ${main_solib_srcfile} ${main_solib_binfile} \
> -	    $options] != "" } {
> -	untested "failed to compile ${main_solib_basename}.c as a shared library"
> -	return -1
> -    }
> -
> -    return 0
> -}
> -
>  # Compile the testcase shared library loader.
>  #
>  # OPTIONS is passed to gdb_compile when compiling the binary.
> @@ -79,34 +62,6 @@ proc compile_jit_dlmain {options} {
>      return 0
>  }
>  
> -# Compile jit-elf-solib.c as a shared library in multiple copies and
> -# upload them to the target.
> -#
> -# On success, return a list of target path to the shared libraries.
> -# On failure, return -1.
> -proc compile_and_download_n_jit_so {count} {
> -    global jit_solib_basename jit_solib_srcfile
> -    set binfiles_target {}
> -
> -    for {set i 1} {$i <= $count} {incr i} {
> -	set binfile [standard_output_file ${jit_solib_basename}.$i.so]
> -
> -	# 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 ${jit_solib_srcfile} ${binfile} {}] != "" } {
> -	    untested "failed to compile ${jit_solib_basename}.c as a shared library"
> -	    return -1
> -	}
> -
> -	set path [gdb_remote_download target ${binfile}]
> -	lappend binfiles_target $path
> -    }
> -
> -    return $binfiles_target
> -}
> -
>  # Run $main_loader_binfile and load $main_solib_binfile in
>  # GDB.  Check jit-related debug output and matches `info function`
>  # output for a jit loaded function using MATCH_STR.
> @@ -117,7 +72,6 @@ proc compile_and_download_n_jit_so {count} {
>  # must match.
>  proc one_jit_test {solib_binfiles_target match_str} {
>      set count [llength $solib_binfiles_target]
> -
>      with_test_prefix "one_jit_test-$count" {
>  	global verbose
>  	global main_loader_binfile main_loader_srcfile
> diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
> index 29638bd2c0..df3199622c 100644
> --- a/gdb/testsuite/gdb.base/jit-elf.exp
> +++ b/gdb/testsuite/gdb.base/jit-elf.exp
> @@ -32,58 +32,7 @@ set main_binfile [standard_output_file ${main_basename}]
>  set jit_solib_basename jit-elf-solib
>  set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
>  

We are missing a "load_lib" in this file.

Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests
  2020-04-21 13:31   ` [PATCH v4 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests Mihails Strasuns
@ 2020-04-22  2:08     ` Simon Marchi
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2020-04-22  2:08 UTC (permalink / raw)
  To: Mihails Strasuns, gdb-patches

On 2020-04-21 9:31 a.m., Mihails Strasuns via Gdb-patches wrote:
> Removes the need to manually relocate loaded ELF binary by using a fixed
> constant as both mmap base address and as a requested first segment
> address supplied to the linker.
> 
> In future will enable JIT tests with a valid DWARF debug info.  Current
> tests still need to compile without a debug info though, because they do
> a function name modification.
> 
> gdb/testsuite/ChangeLog:
> 
> 2020-02-18  Mihails Strasuns  <mihails.strasuns@intel.com>
> 
> 	* lib/jit-elf-helpers.exp: Supply -Ttext-segment linker flag and
> 	  define LOAD_ADDRESS/LOAD_INCREMENT macros for the compiled binaries.
> 	* gdb.base/jit-elf-main.c: Use LOAD_ADDRESS/LOAD_INCREMENT to
> 	  calculate the mmap address.
> ---
>  gdb/testsuite/gdb.base/jit-elf-main.c | 27 ++++++++++++++------------
>  gdb/testsuite/lib/jit-elf-helpers.exp | 28 ++++++++++++++++++++++++---
>  2 files changed, 40 insertions(+), 15 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
> index acfd17d417..ca150f3b1b 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-main.c
> +++ b/gdb/testsuite/gdb.base/jit-elf-main.c
> @@ -51,20 +51,16 @@ usage (void)
>    exit (1);
>  }
>  
> -/* Update .p_vaddr and .sh_addr as if the code was JITted to ADDR.  */
> +/* Rename jit_function_XXXX to match idx  */
>  
>  static void
> -update_locations (const void *const addr, int idx)
> +update_name (const void *const addr, int idx)
>  {
>    const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *)addr;
>    ElfW (Shdr) *const shdr = (ElfW (Shdr) *)((char *)addr + ehdr->e_shoff);
>    ElfW (Phdr) *const phdr = (ElfW (Phdr) *)((char *)addr + ehdr->e_phoff);
>    int i;
>  
> -  for (i = 0; i < ehdr->e_phnum; ++i)
> -    if (phdr[i].p_type == PT_LOAD)
> -      phdr[i].p_vaddr += (ElfW (Addr))addr;
> -
>    for (i = 0; i < ehdr->e_shnum; ++i)
>      {
>        if (shdr[i].sh_type == SHT_STRTAB)
> @@ -81,9 +77,6 @@ update_locations (const void *const addr, int idx)
>              if (strcmp (p, "jit_function_XXXX") == 0)
>                sprintf (p, "jit_function_%04d", idx);
>          }
> -
> -      if (shdr[i].sh_flags & SHF_ALLOC)
> -        shdr[i].sh_addr += (ElfW (Addr))addr;
>      }
>  }
>  
> @@ -96,6 +89,15 @@ update_locations (const void *const addr, int idx)
>  #define MAIN main
>  #endif
>  
> +/* Must be defined by .exp file when compiling to know
> +   what address to map the ELF binary to.  */
> +#ifndef LOAD_ADDRESS
> +#error "Must define LOAD_ADDRESS"
> +#endif
> +#ifndef LOAD_INCREMENT
> +#error "Must define LOAD_INCREMENT"
> +#endif
> +
>  /* Used to spin waiting for GDB.  */
>  volatile int wait_for_gdb = ATTACH;
>  #define WAIT_FOR_GDB do {} while (wait_for_gdb)
> @@ -139,8 +141,9 @@ MAIN (int argc, char *argv[])
>  	  exit (1);
>  	}
>  
> -      const void *const addr = mmap (0, st.st_size, PROT_READ|PROT_WRITE,
> -				     MAP_PRIVATE, fd, 0);
> +      void* load_addr = (void*) (size_t) (LOAD_ADDRESS + (i - 1) * LOAD_INCREMENT);

Space before asterisk.

Otherwise, this patch LGTM.

Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 4/5] [gdb/testsuite] define jit function name via macro
  2020-04-21 13:31   ` [PATCH v4 4/5] [gdb/testsuite] define jit function name via macro Mihails Strasuns
@ 2020-04-22  2:11     ` Simon Marchi
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2020-04-22  2:11 UTC (permalink / raw)
  To: Mihails Strasuns, gdb-patches

On 2020-04-21 9:31 a.m., Mihails Strasuns via Gdb-patches wrote:
> diff --git a/gdb/testsuite/gdb.base/jit-elf-solib.c b/gdb/testsuite/gdb.base/jit-elf-solib.c
> index 3bdebe9ed0..7901c58ac9 100644
> --- a/gdb/testsuite/gdb.base/jit-elf-solib.c
> +++ b/gdb/testsuite/gdb.base/jit-elf-solib.c
> @@ -15,7 +15,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/>.  */
>  
> -/* This simulates a JIT library.  The function is "renamed" after being
> -   loaded into memory.  */
> +/* This simulates a JIT library.  The function name is supplied during
> +   compilation as a macro.  */
>  
> -int jit_function_XXXX() { return 42; }
> +int FUNCTION_NAME() { return 42; }

Maybe put an

#ifndef FUNCTION_NAME
#error ...
#endif

?

Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function
  2020-04-21 13:31   ` [PATCH v4 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function Mihails Strasuns
@ 2020-04-22  2:25     ` Simon Marchi
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Marchi @ 2020-04-22  2:25 UTC (permalink / raw)
  To: Mihails Strasuns, gdb-patches

On 2020-04-21 9:31 a.m., Mihails Strasuns via Gdb-patches wrote:
> 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.
> ---
>  gdb/testsuite/gdb.base/jit-elf-main.c |  56 ++++--------
>  gdb/testsuite/gdb.base/jit-elf-util.h | 117 ++++++++++++++++++++++++++
>  2 files changed, 133 insertions(+), 40 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 b74212dbf5..b812cb91e5 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 (void)
> @@ -95,37 +83,19 @@ 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);
> -	}
> +      size_t obj_size;
> +      ElfW (Addr) load_addr = LOAD_ADDRESS + (i-1) * LOAD_INCREMENT;

(i - 1)

I think load_addr should just be a `void *`, and the `(void *)` casts should be removed.

> +      printf("Loading %s as JIT at %p\n", argv[i], (void*) load_addr);

(void *)

> +      ElfW (Addr) addr = load_elf (argv[i], &obj_size, load_addr);
>  
> -      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 | MAP_FIXED, 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);
> -	}
> +      char name[32];
> +      sprintf (name, "jit_function_%04d", i);
> +      int (*jit_function) () = (int (*) ()) load_symbol (addr, name);

Use (void) instead of ().

>  
>        /* 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;
>  
> @@ -137,6 +107,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..e48d99f098
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/jit-elf-util.h
> @@ -0,0 +1,117 @@
> +/* 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  */
> +
> +/* 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.  */

I'm wondering, is the "and adjust it from the addr" part of the comment still relevant?

> +
> +  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.  */
> +static ElfW (Addr)
> +load_elf (const char *libname, size_t *size, ElfW (Addr) load_addr)

I think load_addr should be a `void *` directly.

> +{
> +  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);

Assuming load_addr becomes a `void *`, the condition should be `load_addr != NULL`.

Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH v4 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp
  2020-04-21 15:55     ` Simon Marchi
@ 2020-04-22  9:20       ` Strasuns, Mihails
  0 siblings, 0 replies; 11+ messages in thread
From: Strasuns, Mihails @ 2020-04-22  9:20 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

> -----Original Message-----
> From: Simon Marchi <simark@simark.ca>
> Sent: Tuesday, April 21, 2020 5:55 PM
> To: Strasuns, Mihails <mihails.strasuns@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v4 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp
> 
> ...
>
> >  # The "real" main of this test, which loads jit-elf-main  # as a
> > shared library.
> >  set main_loader_basename jit-elf-dlmain @@ -41,25 +43,6 @@ set
> > main_solib_binfile [standard_output_file ${main_solib_basename}.so]
> > set jit_solib_basename jit-elf-solib  set jit_solib_srcfile
> > ${srcdir}/${subdir}/${jit_solib_basename}.c
> 
> Now it's a bit confusing, because jit-elf-helpers.exp uses some definitions
> that must be provided by the including file.  It would be better for it to be
> self-contained.  So either move the setting of these definitions to jit-elf-
> helpers.exp (and work out a naming scheme that is not too confusing), or
> modify the procedures to take parameters, and update the callers to pass in
> the values as arguments.

That was my initial guess (and the previous implementation) but I got confused by your proposed patch which used globals for input parameters - I have assumed you want to keep it that way :)
Will change it back.

> >
> > -# Compile jit-elf-main.c as a shared library.
> > -#
> > -# OPTIONS is passed to gdb_compile when compiling the program.
> > -#
> > -# On success, return 0.
> > -# On failure, return -1.
> > -proc compile_jit_elf_main_as_so {options} {
> > -    global main_solib_srcfile main_solib_binfile
> > -    set options [concat $options debug]
> > -
> > -    if { [gdb_compile_shlib ${main_solib_srcfile} ${main_solib_binfile} \
> > -	    $options] != "" } {
> > -	untested "failed to compile ${main_solib_basename}.c as a shared
> library"
> > -	return -1
> > -    }
> > -
> > -    return 0
> > -}
> > -
> >  # Compile the testcase shared library loader.
> >  #
> >  # OPTIONS is passed to gdb_compile when compiling the binary.
> > @@ -79,34 +62,6 @@ proc compile_jit_dlmain {options} {
> >      return 0
> >  }
> >
> > -# Compile jit-elf-solib.c as a shared library in multiple copies and
> > -# upload them to the target.
> > -#
> > -# On success, return a list of target path to the shared libraries.
> > -# On failure, return -1.
> > -proc compile_and_download_n_jit_so {count} {
> > -    global jit_solib_basename jit_solib_srcfile
> > -    set binfiles_target {}
> > -
> > -    for {set i 1} {$i <= $count} {incr i} {
> > -	set binfile [standard_output_file ${jit_solib_basename}.$i.so]
> > -
> > -	# 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 ${jit_solib_srcfile} ${binfile} {}] != "" } {
> > -	    untested "failed to compile ${jit_solib_basename}.c as a shared
> library"
> > -	    return -1
> > -	}
> > -
> > -	set path [gdb_remote_download target ${binfile}]
> > -	lappend binfiles_target $path
> > -    }
> > -
> > -    return $binfiles_target
> > -}
> > -
> >  # Run $main_loader_binfile and load $main_solib_binfile in  # GDB.
> > Check jit-related debug output and matches `info function`  # output
> > for a jit loaded function using MATCH_STR.
> > @@ -117,7 +72,6 @@ proc compile_and_download_n_jit_so {count} {  #
> > must match.
> >  proc one_jit_test {solib_binfiles_target match_str} {
> >      set count [llength $solib_binfiles_target]
> > -
> >      with_test_prefix "one_jit_test-$count" {
> >  	global verbose
> >  	global main_loader_binfile main_loader_srcfile diff --git
> > a/gdb/testsuite/gdb.base/jit-elf.exp
> > b/gdb/testsuite/gdb.base/jit-elf.exp
> > index 29638bd2c0..df3199622c 100644
> > --- a/gdb/testsuite/gdb.base/jit-elf.exp
> > +++ b/gdb/testsuite/gdb.base/jit-elf.exp
> > @@ -32,58 +32,7 @@ set main_binfile [standard_output_file
> > ${main_basename}]  set jit_solib_basename jit-elf-solib  set
> > jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
> >
> 
> We are missing a "load_lib" in this file.

Good catch, thanks!

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-04-22  9:20 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <167748>
2020-04-21 13:31 ` [PATCH v4 0/5] jit testsuite refactoring Mihails Strasuns
2020-04-21 13:31   ` [PATCH v4 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
2020-04-21 13:31   ` [PATCH v4 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
2020-04-21 15:55     ` Simon Marchi
2020-04-22  9:20       ` Strasuns, Mihails
2020-04-21 13:31   ` [PATCH v4 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests Mihails Strasuns
2020-04-22  2:08     ` Simon Marchi
2020-04-21 13:31   ` [PATCH v4 4/5] [gdb/testsuite] define jit function name via macro Mihails Strasuns
2020-04-22  2:11     ` Simon Marchi
2020-04-21 13:31   ` [PATCH v4 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function Mihails Strasuns
2020-04-22  2:25     ` Simon Marchi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).