public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests
       [not found] <167782>
@ 2020-05-11 10:28 ` Mihails Strasuns
  2020-05-11 10:28   ` [PATCH v5 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
                     ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Mihails Strasuns @ 2020-05-11 10:28 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.2

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] 13+ messages in thread

* [PATCH v5 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp
  2020-05-11 10:28 ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
@ 2020-05-11 10:28   ` Mihails Strasuns
  2020-05-12  0:23     ` Simon Marchi
  2020-05-11 10:28   ` [PATCH v5 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests Mihails Strasuns
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Mihails Strasuns @ 2020-05-11 10:28 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 | 56 ++-----------------
 gdb/testsuite/gdb.base/jit-elf.exp    | 65 +++-------------------
 gdb/testsuite/lib/jit-elf-helpers.exp | 80 +++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 106 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..23e5a451b4 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
@@ -181,7 +135,8 @@ proc one_jit_test {solib_binfiles_target match_str} {
 }
 
 # 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 } {
+if { [compile_jit_elf_main_as_so $main_solib_srcfile $main_solib_binfile \
+	{additional_flags="-DMAIN=jit_dl_main"}] < 0 } {
     return
 }
 
@@ -191,7 +146,8 @@ if { [compile_jit_dlmain {shlib_load}] < 0 } {
 }
 
 # Compile two shared libraries to use as JIT objects.
-set jit_solibs_target [compile_and_download_n_jit_so 2]
+set jit_solibs_target [compile_and_download_n_jit_so \
+    $jit_solib_basename $jit_solib_srcfile 2]
 if { $jit_solibs_target == -1 } {
     return
 }
diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
index 29638bd2c0..642653ac6a 100644
--- a/gdb/testsuite/gdb.base/jit-elf.exp
+++ b/gdb/testsuite/gdb.base/jit-elf.exp
@@ -23,8 +23,10 @@ if {[get_compiler_info]} {
     return 1
 }
 
+load_lib jit-elf-helpers.exp
+
 # The main code that loads and registers JIT objects.
-set main_basename jit-elf-main
+set main_basename "jit-elf-main"
 set main_srcfile ${srcdir}/${subdir}/${main_basename}.c
 set main_binfile [standard_output_file ${main_basename}]
 
@@ -32,58 +34,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
@@ -179,13 +130,14 @@ proc one_jit_test {jit_solibs_target match_str reattach} {
 }
 
 # Compile two shared libraries to use as JIT objects.
-set jit_solibs_target [compile_and_download_n_jit_so 2]
+set jit_solibs_target [compile_and_download_n_jit_so \
+    $jit_solib_basename $jit_solib_srcfile 2]
 if { $jit_solibs_target == -1 } {
     return
 }
 
 # Compile the main code (which loads the JIT objects).
-if { [compile_jit_main "" {}] == 0 } {
+if { [compile_jit_main ${main_srcfile} ${main_binfile} {}] == 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
 }
@@ -194,14 +146,15 @@ if { [compile_jit_main "" {}] == 0 } {
 # registered.  We reuse the normal test, and detach/reattach at
 # specific interesting points.
 if {[can_spawn_for_attach]} {
-    if { [compile_jit_main "-attach" {additional_flags=-DATTACH=1}] == 0 } {
+    if { [compile_jit_main ${main_srcfile} ${main_binfile}"-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
 	}
     }
 }
 
-if { [compile_jit_main "-pie" {additional_flags=-fPIE ldflags=-pie}] == 0 } {
+if { [compile_jit_main ${main_srcfile} ${main_binfile}"-pie" {additional_flags=-fPIE ldflags=-pie}] == 0 } {
     with_test_prefix PIE {
 	one_jit_test [lindex $jit_solibs_target 0] "${hex}  jit_function_0001" 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..de6ac46f05
--- /dev/null
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -0,0 +1,80 @@
+# 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 {main_srcfile main_binfile options} {
+    set options [concat $options debug]
+
+    if { [gdb_compile ${main_srcfile} ${main_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 {main_solib_srcfile main_solib_binfile options} {
+    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 {jit_solib_basename jit_solib_srcfile count} {
+    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.2

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] 13+ messages in thread

* [PATCH v5 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests
  2020-05-11 10:28 ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
  2020-05-11 10:28   ` [PATCH v5 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
@ 2020-05-11 10:28   ` Mihails Strasuns
  2020-05-12  0:28     ` Simon Marchi
  2020-05-11 10:28   ` [PATCH v5 4/5] [gdb/testsuite] define jit function name via macro Mihails Strasuns
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Mihails Strasuns @ 2020-05-11 10:28 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 | 30 ++++++++++++++++++++++++---
 2 files changed, 42 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..be7185ef1a 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 de6ac46f05..246d0dfcea 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.
@@ -23,7 +30,13 @@
 # On success, return 0.
 # On failure, return -1.
 proc compile_jit_main {main_srcfile main_binfile options} {
-    set options [concat $options debug]
+    global load_address load_increment
+
+    set options [concat \
+	$options \
+	additional_flags=-DLOAD_ADDRESS=$load_address \
+	additional_flags=-DLOAD_INCREMENT=$load_increment \
+	debug]
 
     if { [gdb_compile ${main_srcfile} ${main_binfile} \
 	  executable $options] != "" } {
@@ -41,7 +54,13 @@ proc compile_jit_main {main_srcfile main_binfile options} {
 # On success, return 0.
 # On failure, return -1.
 proc compile_jit_elf_main_as_so {main_solib_srcfile main_solib_binfile options} {
-    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] != "" } {
@@ -58,6 +77,7 @@ proc compile_jit_elf_main_as_so {main_solib_srcfile main_solib_binfile options}
 # On success, return a list of target path to the shared libraries.
 # On failure, return -1.
 proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile count} {
+    global load_address load_increment
     set binfiles_target {}
 
     for {set i 1} {$i <= $count} {incr i} {
@@ -67,7 +87,11 @@ proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile 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.2

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] 13+ messages in thread

* [PATCH v5 4/5] [gdb/testsuite] define jit function name via macro
  2020-05-11 10:28 ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
  2020-05-11 10:28   ` [PATCH v5 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
  2020-05-11 10:28   ` [PATCH v5 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests Mihails Strasuns
@ 2020-05-11 10:28   ` Mihails Strasuns
  2020-05-12  0:31     ` Simon Marchi
  2020-05-11 10:28   ` [PATCH v5 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function Mihails Strasuns
  2020-05-12  0:17   ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Simon Marchi
  4 siblings, 1 reply; 13+ messages in thread
From: Mihails Strasuns @ 2020-05-11 10:28 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 | 10 ++++++---
 gdb/testsuite/lib/jit-elf-helpers.exp  |  1 +
 3 files changed, 8 insertions(+), 34 deletions(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf-main.c b/gdb/testsuite/gdb.base/jit-elf-main.c
index be7185ef1a..2d654891d9 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..66d6a2be6d 100644
--- a/gdb/testsuite/gdb.base/jit-elf-solib.c
+++ b/gdb/testsuite/gdb.base/jit-elf-solib.c
@@ -15,7 +15,11 @@
    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; }
+#ifndef FUNCTION_NAME
+#error "Must define the FUNCTION_NAME macro to set a jited function name"
+#endif
+
+int FUNCTION_NAME() { return 42; }
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
index 246d0dfcea..24ce94d767 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -89,6 +89,7 @@ proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile 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.2

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] 13+ messages in thread

* [PATCH v5 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function
  2020-05-11 10:28 ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
                     ` (2 preceding siblings ...)
  2020-05-11 10:28   ` [PATCH v5 4/5] [gdb/testsuite] define jit function name via macro Mihails Strasuns
@ 2020-05-11 10:28   ` Mihails Strasuns
  2020-05-12  0:37     ` Simon Marchi
  2020-05-12  0:17   ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Simon Marchi
  4 siblings, 1 reply; 13+ messages in thread
From: Mihails Strasuns @ 2020-05-11 10:28 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 |  54 ++++--------
 gdb/testsuite/gdb.base/jit-elf-util.h | 118 ++++++++++++++++++++++++++
 2 files changed, 133 insertions(+), 39 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 2d654891d9..07361e6a6b 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);
-	}
-
-      if (fstat (fd, &st) != 0)
-	{
-	  fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
-	  exit (1);
-	}
-
+      size_t obj_size;
       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));
+      printf ("Loading %s as JIT at %p\n", argv[i], load_addr);
+      void *addr = load_elf (argv[i], &obj_size, load_addr);
 
-      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) (void) = (int (*) (void)) 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..843cf30618
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf-util.h
@@ -0,0 +1,118 @@
+/* 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 void *
+load_symbol (void *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 return its address.  */
+
+  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 (void *) 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 void *
+load_elf (const char *libname, size_t *size, void *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 (load_addr, st.st_size,
+  		     PROT_READ | PROT_WRITE | PROT_EXEC,
+		     load_addr != NULL ? 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 addr;
+}
-- 
2.26.2

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] 13+ messages in thread

* Re: [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests
  2020-05-11 10:28 ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
                     ` (3 preceding siblings ...)
  2020-05-11 10:28   ` [PATCH v5 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function Mihails Strasuns
@ 2020-05-12  0:17   ` Simon Marchi
  2020-05-12  7:51     ` Strasuns, Mihails
  4 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2020-05-12  0:17 UTC (permalink / raw)
  To: Mihails Strasuns, gdb-patches

On 2020-05-11 6:28 a.m., Mihails Strasuns via Gdb-patches wrote:
> 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.

Thanks to a patch by Andrew that was merged today, the testsuite now points out duplicate
test names as well as paths in test names.  We want to avoid paths in test names to make
it possible to compare test runs that are done in different build directories.  If you rebase
and run the test, you'll see:

Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/jit-elf-so.exp ...
PATH: gdb.base/jit-elf-so.exp: one_jit_test-1: set var jit_libname = "/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf-so/jit-elf-main.so"
PATH: gdb.base/jit-elf-so.exp: one_jit_test-1: set var argv[1]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf-so/jit-elf-solib.1.so"
PATH: gdb.base/jit-elf-so.exp: one_jit_test-2: set var jit_libname = "/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf-so/jit-elf-main.so"
PATH: gdb.base/jit-elf-so.exp: one_jit_test-2: set var argv[1]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf-so/jit-elf-solib.1.so"
PATH: gdb.base/jit-elf-so.exp: one_jit_test-2: set var argv[2]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf-so/jit-elf-solib.2.so"
DUPLICATE: gdb.base/jit-elf-so.exp: test jit-reader-load filename completion
Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/jit-elf.exp ...
PATH: gdb.base/jit-elf.exp: one_jit_test-1: set var argv[1]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.1.so"
PATH: gdb.base/jit-elf.exp: one_jit_test-2: set var argv[1]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.1.so"
PATH: gdb.base/jit-elf.exp: one_jit_test-2: set var argv[2]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.2.so"
PATH: gdb.base/jit-elf.exp: attach: one_jit_test-2: set var argv[1]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.1.so"
PATH: gdb.base/jit-elf.exp: attach: one_jit_test-2: set var argv[2]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.2.so"
PATH: gdb.base/jit-elf.exp: PIE: one_jit_test-1: set var argv[1]="/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.1.so"

Could you fix this?  Otherwise the patch looks good to me.

Simon

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

* Re: [PATCH v5 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp
  2020-05-11 10:28   ` [PATCH v5 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
@ 2020-05-12  0:23     ` Simon Marchi
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2020-05-12  0:23 UTC (permalink / raw)
  To: Mihails Strasuns, gdb-patches

Just some small nits, otherwise the patch LGTM.

On 2020-05-11 6:28 a.m., Mihails Strasuns via Gdb-patches wrote:
> diff --git a/gdb/testsuite/gdb.base/jit-elf-so.exp b/gdb/testsuite/gdb.base/jit-elf-so.exp
> index 59ca1ac8cf..23e5a451b4 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
> @@ -181,7 +135,8 @@ proc one_jit_test {solib_binfiles_target match_str} {
>  }
>  
>  # 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 } {
> +if { [compile_jit_elf_main_as_so $main_solib_srcfile $main_solib_binfile \
> +	{additional_flags="-DMAIN=jit_dl_main"}] < 0 } {
>      return
>  }
>  
> @@ -191,7 +146,8 @@ if { [compile_jit_dlmain {shlib_load}] < 0 } {
>  }
>  
>  # Compile two shared libraries to use as JIT objects.
> -set jit_solibs_target [compile_and_download_n_jit_so 2]
> +set jit_solibs_target [compile_and_download_n_jit_so \
> +    $jit_solib_basename $jit_solib_srcfile 2]

Align the second line on the square bracket like this, a bit like we do in the C code:

set jit_solibs_target [compile_and_download_n_jit_so \
		       $jit_solib_basename $jit_solib_srcfile 2]

>  if { $jit_solibs_target == -1 } {
>      return
>  }
> diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
> index 29638bd2c0..642653ac6a 100644
> --- a/gdb/testsuite/gdb.base/jit-elf.exp
> +++ b/gdb/testsuite/gdb.base/jit-elf.exp
> @@ -23,8 +23,10 @@ if {[get_compiler_info]} {
>      return 1
>  }
>  
> +load_lib jit-elf-helpers.exp
> +
>  # The main code that loads and registers JIT objects.
> -set main_basename jit-elf-main
> +set main_basename "jit-elf-main"
>  set main_srcfile ${srcdir}/${subdir}/${main_basename}.c
>  set main_binfile [standard_output_file ${main_basename}]
>  
> @@ -32,58 +34,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
> @@ -179,13 +130,14 @@ proc one_jit_test {jit_solibs_target match_str reattach} {
>  }
>  
>  # Compile two shared libraries to use as JIT objects.
> -set jit_solibs_target [compile_and_download_n_jit_so 2]
> +set jit_solibs_target [compile_and_download_n_jit_so \
> +    $jit_solib_basename $jit_solib_srcfile 2]

Same comment about alignment.

> diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
> new file mode 100644
> index 0000000000..de6ac46f05
> --- /dev/null
> +++ b/gdb/testsuite/lib/jit-elf-helpers.exp
> @@ -0,0 +1,80 @@
> +# 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.

The first of these two comments could be removed, I think.

> +#
> +# 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 {main_srcfile main_binfile options} {

Update the documentation to match the parameters.

Simon

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

* Re: [PATCH v5 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests
  2020-05-11 10:28   ` [PATCH v5 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests Mihails Strasuns
@ 2020-05-12  0:28     ` Simon Marchi
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2020-05-12  0:28 UTC (permalink / raw)
  To: Mihails Strasuns, gdb-patches

Just some nits again, otherwise LGTM.

On 2020-05-11 6:28 a.m., Mihails Strasuns via Gdb-patches wrote:
> @@ -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

Prefix these with "jit_".

> @@ -67,7 +87,11 @@ proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile 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]

A comment about what we are doing here and why would be nice, it's probably not clear
for someone missing the context.  So maybe add a bit of details just above this, or
in the proc comment.

Simon

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

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

On 2020-05-11 6:28 a.m., Mihails Strasuns via Gdb-patches wrote:
> 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.

This LGTM as is.  It really removes a layer of hackiness, many thanks!

Simon


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

* Re: [PATCH v5 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function
  2020-05-11 10:28   ` [PATCH v5 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function Mihails Strasuns
@ 2020-05-12  0:37     ` Simon Marchi
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2020-05-12  0:37 UTC (permalink / raw)
  To: Mihails Strasuns, gdb-patches

On 2020-05-11 6:28 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.

This patch LGTM.

Simon


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

* RE: [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests
  2020-05-12  0:17   ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Simon Marchi
@ 2020-05-12  7:51     ` Strasuns, Mihails
  2020-05-12 14:13       ` Simon Marchi
  0 siblings, 1 reply; 13+ messages in thread
From: Strasuns, Mihails @ 2020-05-12  7:51 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches

Fixed nits for the series and pushed. Thank you for your patience!

BR,
Mihails

> -----Original Message-----
> From: Simon Marchi <simark@simark.ca>
> Sent: Tuesday, May 12, 2020 2:17 AM
> To: Strasuns, Mihails <mihails.strasuns@intel.com>; gdb-
> patches@sourceware.org
> Subject: Re: [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests
> 
> On 2020-05-11 6:28 a.m., Mihails Strasuns via Gdb-patches wrote:
> > 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.
> 
> Thanks to a patch by Andrew that was merged today, the testsuite now
> points out duplicate test names as well as paths in test names.  We want to
> avoid paths in test names to make it possible to compare test runs that are
> done in different build directories.  If you rebase and run the test, you'll see:
> 
> Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/jit-elf-
> so.exp ...
> PATH: gdb.base/jit-elf-so.exp: one_jit_test-1: set var jit_libname =
> "/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf-
> so/jit-elf-main.so"
> PATH: gdb.base/jit-elf-so.exp: one_jit_test-1: set var
> argv[1]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf-so/jit-elf-solib.1.so"
> PATH: gdb.base/jit-elf-so.exp: one_jit_test-2: set var jit_libname =
> "/home/smarchi/build/binutils-gdb/gdb/testsuite/outputs/gdb.base/jit-elf-
> so/jit-elf-main.so"
> PATH: gdb.base/jit-elf-so.exp: one_jit_test-2: set var
> argv[1]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf-so/jit-elf-solib.1.so"
> PATH: gdb.base/jit-elf-so.exp: one_jit_test-2: set var
> argv[2]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf-so/jit-elf-solib.2.so"
> DUPLICATE: gdb.base/jit-elf-so.exp: test jit-reader-load filename completion
> Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.base/jit-elf.exp
> ...
> PATH: gdb.base/jit-elf.exp: one_jit_test-1: set var
> argv[1]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.1.so"
> PATH: gdb.base/jit-elf.exp: one_jit_test-2: set var
> argv[1]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.1.so"
> PATH: gdb.base/jit-elf.exp: one_jit_test-2: set var
> argv[2]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.2.so"
> PATH: gdb.base/jit-elf.exp: attach: one_jit_test-2: set var
> argv[1]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.1.so"
> PATH: gdb.base/jit-elf.exp: attach: one_jit_test-2: set var
> argv[2]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.2.so"
> PATH: gdb.base/jit-elf.exp: PIE: one_jit_test-1: set var
> argv[1]="/home/smarchi/build/binutils-
> gdb/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-solib.1.so"
> 
> Could you fix this?  Otherwise the patch looks good to me.
> 
> 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] 13+ messages in thread

* Re: [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests
  2020-05-12  7:51     ` Strasuns, Mihails
@ 2020-05-12 14:13       ` Simon Marchi
  2020-05-12 14:15         ` Simon Marchi
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Marchi @ 2020-05-12 14:13 UTC (permalink / raw)
  To: Strasuns, Mihails, gdb-patches

On 2020-05-12 3:51 a.m., Strasuns, Mihails wrote:
> Fixed nits for the series and pushed. Thank you for your patience!

Thanks for _your_ patience, this has taken a while because of me, thanks
for pushing through.

If I recall, the initial goal of this was to fix some JIT bug in GDB itself,
and these test refactorings were to make it possible to write a test for that?

Simon


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

* Re: [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests
  2020-05-12 14:13       ` Simon Marchi
@ 2020-05-12 14:15         ` Simon Marchi
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Marchi @ 2020-05-12 14:15 UTC (permalink / raw)
  To: Strasuns, Mihails, gdb-patches

On 2020-05-12 10:13 a.m., Simon Marchi wrote:
> On 2020-05-12 3:51 a.m., Strasuns, Mihails wrote:
>> Fixed nits for the series and pushed. Thank you for your patience!
> 
> Thanks for _your_ patience, this has taken a while because of me, thanks
> for pushing through.
> 
> If I recall, the initial goal of this was to fix some JIT bug in GDB itself,
> and these test refactorings were to make it possible to write a test for that?

Ah, I see you already sent it :).

Simon

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

end of thread, other threads:[~2020-05-12 14:15 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <167782>
2020-05-11 10:28 ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Mihails Strasuns
2020-05-11 10:28   ` [PATCH v5 2/5] [gdb/testsuite] add lib/jit-elf-helpers.exp Mihails Strasuns
2020-05-12  0:23     ` Simon Marchi
2020-05-11 10:28   ` [PATCH v5 3/5] [gdb/testsuite] use -Ttext-segment for jit-elf tests Mihails Strasuns
2020-05-12  0:28     ` Simon Marchi
2020-05-11 10:28   ` [PATCH v5 4/5] [gdb/testsuite] define jit function name via macro Mihails Strasuns
2020-05-12  0:31     ` Simon Marchi
2020-05-11 10:28   ` [PATCH v5 5/5] [gdb/testsuite] add jit-elf-util.h and run jit function Mihails Strasuns
2020-05-12  0:37     ` Simon Marchi
2020-05-12  0:17   ` [PATCH v5 1/5] [gdb/testsuite] use args as lib list for jit-elf tests Simon Marchi
2020-05-12  7:51     ` Strasuns, Mihails
2020-05-12 14:13       ` Simon Marchi
2020-05-12 14:15         ` 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).