[v2,4/8,gdb/testsuite] use args as lib list for jit-elf tests

Message ID 20200327103932.17765-4-mihails.strasuns@intel.com
State Superseded
Headers show
Series
  • [v2,1/8,gdb/testsuite] allow more registers in reader test
Related show

Commit Message

H.J. Lu via Gdb-patches March 27, 2020, 10:39 a.m.
Old usage: jit-elf-main lib.so 2
New usage: jit-elf-main lib.so.1 lib.so.2

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

gdb/testsuite/ChangeLog:

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

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

Change-Id: Ie8f85ec6358604c14557b0417d6621b2f8942033
Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>

---
 gdb/testsuite/gdb.base/jit-elf-main.c |  52 +++++--------
 gdb/testsuite/gdb.base/jit-elf-so.exp | 108 ++++++++++++++++----------
 gdb/testsuite/gdb.base/jit-elf.exp    |  99 +++++++++++------------
 3 files changed, 133 insertions(+), 126 deletions(-)

-- 
2.25.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

Comments

Simon Marchi March 30, 2020, 3 a.m. | #1
On 2020-03-27 6:39 a.m., Mihails Strasuns via Gdb-patches wrote:
> Old usage: jit-elf-main lib.so 2

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

> 

> Refactoring necessary to support running tests over multiple jit

> binaries rather than mapping the same binary muultiple times.

> 

> gdb/testsuite/ChangeLog:

> 

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

> 

> 	* gdb.base/jit-elf-main.c: read lib list from argc/argv

> 	* gdb.base/jit-elf.exp: compile N jit libraries and use the list

> 	* gdb.base/jit-elf-so.exp: ditto

> 

> Change-Id: Ie8f85ec6358604c14557b0417d6621b2f8942033

> Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>


Hmm, it seems like many comments from these messages were not addressed:

https://sourceware.org/pipermail/gdb-patches/2020-March/166923.html
https://sourceware.org/pipermail/gdb-patches/2020-March/166924.html

Simon
H.J. Lu via Gdb-patches March 31, 2020, 3:13 p.m. | #2
> -----Original Message-----

> From: Simon Marchi <simark@simark.ca>

> Sent: Monday, March 30, 2020 5:00 AM

> To: Strasuns, Mihails <mihails.strasuns@intel.com>; gdb-

> patches@sourceware.org

> Subject: Re: [PATCH v2 4/8] [gdb/testsuite] use args as lib list for jit-elf tests

> 

> On 2020-03-27 6:39 a.m., Mihails Strasuns via Gdb-patches wrote:

> > Old usage: jit-elf-main lib.so 2

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

> >

> > Refactoring necessary to support running tests over multiple jit

> > binaries rather than mapping the same binary muultiple times.

> >

> > gdb/testsuite/ChangeLog:

> >

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

> >

> > 	* gdb.base/jit-elf-main.c: read lib list from argc/argv

> > 	* gdb.base/jit-elf.exp: compile N jit libraries and use the list

> > 	* gdb.base/jit-elf-so.exp: ditto

> >

> > Change-Id: Ie8f85ec6358604c14557b0417d6621b2f8942033

> > Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>

> 

> Hmm, it seems like many comments from these messages were not

> addressed:


Hello,

Sorry, I have indeed missed these review mails. Will upload v3 in a moment, only one question:

> If we fail to compile something, do we want to return some failure return status

and exit the test?  This is how it works currently.

What do you mean here? That you would prefer to switch from untested to fail? It used to result in untested in the original test.

BR,
Mihails
 
> https://sourceware.org/pipermail/gdb-patches/2020-March/166923.html

> https://sourceware.org/pipermail/gdb-patches/2020-March/166924.html

> 

> 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
Simon Marchi March 31, 2020, 4:08 p.m. | #3
On 2020-03-31 11:13 a.m., Strasuns, Mihails via Gdb-patches wrote:
> What do you mean here? That you would prefer to switch from untested to fail? It used to result in untested in the original test.


In the current test case, if we fail to compile the test, we call "return -1", which
skips the execution of the rest of the test.  We don't want to keep executing the test if
the compilation failed.  I'd suggest that your new compile functions also returns something
to indicate to the caller that the compilation failed, so the caller can also return early.

Does that make sense?

Simon

Patch

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