Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Mihails Strasuns (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>
To: gdb-patches@sourceware.org
Subject: [review] jit: enhance test suite
Date: Thu, 19 Dec 2019 10:28:00 -0000	[thread overview]
Message-ID: <gerrit.1576751303000.I5f9d56e8eaaa7cf88881eda6a92a72962dd3f066@gnutoolchain-gerrit.osci.io> (raw)
In-Reply-To: <gerrit.1576751303000.I5f9d56e8eaaa7cf88881eda6a92a72962dd3f066@gnutoolchain-gerrit.osci.io>

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/757
......................................................................

jit: enhance test suite

Refactoring and enhancement of jit-main.c related facilities which:

1) Enables calling a specified function from the pseudo-jit elf
   binary.
2) Does simple adjustment of debug info by replacing all mentions of
   called function original address with relocated address. It does not
   fully fix the debug information (most notably line info remains
   unusable) but it makes possible to put a breakpoint on such function.
3) Splits functions for loading and adjusting an elf binary into a new
   header file, "jit-elf.h", so that it can be reused later in other
   test cases. Splis jit-related definitions into "jit-defs.h" for the
   same purpose.

This commit does not modify existing testing scenarios and is intended
to serve as a base for further jit test additions.

gdb/testsuite/ChangeLog:
2019-12-18  Mihails Strasuns  <mihails.strasuns@intel.com>
	* jit-elf.h, jit-defs.h: new file
	* jit-main.c: refactoring to use "jit-elf.h" and "jit-defs.h",
	  now will also execute the registered jit function

Signed-off-by: Mihails Strasuns <mihails.strasuns@intel.com>
Change-Id: I5f9d56e8eaaa7cf88881eda6a92a72962dd3f066
---
A gdb/testsuite/gdb.base/jit-defs.h
A gdb/testsuite/gdb.base/jit-elf.h
M gdb/testsuite/gdb.base/jit-main.c
3 files changed, 253 insertions(+), 118 deletions(-)



diff --git a/gdb/testsuite/gdb.base/jit-defs.h b/gdb/testsuite/gdb.base/jit-defs.h
new file mode 100644
index 0000000..d349d28
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-defs.h
@@ -0,0 +1,50 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2019 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/>.  */
+
+#include <stdint.h>
+
+typedef enum
+{
+  JIT_NOACTION = 0,
+  JIT_REGISTER_FN,
+  JIT_UNREGISTER_FN
+} jit_actions_t;
+
+struct jit_code_entry
+{
+  struct jit_code_entry *next_entry;
+  struct jit_code_entry *prev_entry;
+  const char *symfile_addr;
+  uint64_t symfile_size;
+};
+
+struct jit_descriptor
+{
+  uint32_t version;
+  /* This type should be jit_actions_t, but we use uint32_t
+     to be explicit about the bitwidth.  */
+  uint32_t action_flag;
+  struct jit_code_entry *relevant_entry;
+  struct jit_code_entry *first_entry;
+};
+
+/* GDB puts a breakpoint in this function.  */
+void __attribute__((noinline)) __jit_debug_register_code () { }
+
+/* Make sure to specify the version statically, because the
+   debugger may check the version before we can set it.  */
+struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
\ No newline at end of file
diff --git a/gdb/testsuite/gdb.base/jit-elf.h b/gdb/testsuite/gdb.base/jit-elf.h
new file mode 100644
index 0000000..61de3b4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-elf.h
@@ -0,0 +1,184 @@
+/* This test program is part of GDB, the GNU debugger.
+
+   Copyright 2019 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  */
+
+/* Update section addresses to use `addr` as a base.
+   If `rename_num` is >= 0, look into string tables for entry
+   "jit_function_XXXX" and update it to use the supplied number. */
+static void
+update_locations (ElfW (Addr) addr, int rename_num)
+{
+  const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *) addr;
+  ElfW (Shdr) *const shdr = (ElfW (Shdr) *) ((char *) addr + ehdr->e_shoff);
+  ElfW (Phdr) *const phdr = (ElfW (Phdr) *) ((char *) addr + ehdr->e_phoff);
+
+  /* Update .p_vaddr and .sh_addr as if the code was JITted to ADDR.  */
+
+  for (int i = 0; i < ehdr->e_phnum; ++i)
+    if (phdr[i].p_type == PT_LOAD)
+      phdr[i].p_vaddr += addr;
+
+  for (int i = 0; i < ehdr->e_shnum; ++i)
+    {
+      if (rename_num >= 0 && shdr[i].sh_type == SHT_STRTAB)
+	{
+	  /* Note: we update both .strtab and .dynstr.  The latter would
+	     not be correct if this were a regular shared library (.hash
+	     would be wrong), but this is a simulation -- the library is
+	     never exposed to the dynamic loader, so it all ends up ok.  */
+	  char *const strtab = (char *) (addr + shdr[i].sh_offset);
+	  char *const strtab_end = strtab + shdr[i].sh_size;
+	  char *p;
+
+	  for (p = strtab; p < strtab_end; p += strlen (p) + 1)
+	    if (strcmp (p, "jit_function_XXXX") == 0)
+	      sprintf (p, "jit_function_%04d", rename_num);
+	}
+
+      if (shdr[i].sh_flags & SHF_ALLOC)
+	shdr[i].sh_addr += addr;
+    }
+}
+
+/* Find symbol with the name `sym_name`, relocate it to
+   use `addr` as a base and update all mentions to use the
+   new address. */
+static ElfW (Addr) load_symbol (ElfW (Addr) addr, const char *sym_name)
+{
+  const ElfW (Ehdr) *const ehdr = (ElfW (Ehdr) *) addr;
+  ElfW (Shdr) *const shdr = (ElfW (Shdr) *) ((char *) addr + ehdr->e_shoff);
+
+  ElfW (Addr) sym_old_addr = 0;
+  ElfW (Addr) sym_new_addr = 0;
+
+  /* Find `func_name` in symbol_table and adjust it from the addr */
+
+  for (int i = 0; i < ehdr->e_shnum; ++i)
+    {
+      if (shdr[i].sh_type == SHT_SYMTAB)
+	{
+	  ElfW (Sym) *symtab = (ElfW (Sym) *) (addr + shdr[i].sh_offset);
+	  ElfW (Sym) *symtab_end
+	      = (ElfW (Sym) *) (addr + shdr[i].sh_offset + shdr[i].sh_size);
+	  char *const strtab
+	      = (char *) (addr + shdr[shdr[i].sh_link].sh_offset);
+
+	  for (ElfW (Sym) *p = symtab; p < symtab_end; p += 1)
+	    {
+	      const char *s = strtab + p->st_name;
+	      if (strcmp (s, sym_name) == 0)
+		{
+		  sym_old_addr = p->st_value;
+		  p->st_value += addr;
+		  sym_new_addr = p->st_value;
+		  break;
+		}
+	    }
+
+	  if (sym_new_addr != 0)
+	    break;
+	}
+    }
+
+  if (sym_new_addr == 0)
+    {
+      fprintf (stderr, "symbol '%s' not found\n", sym_name);
+      exit (1);
+    }
+
+  /* Find all mentions of `func_name` old address in debug sections and adjust
+   * values */
+
+  for (int i = 0; i < ehdr->e_shnum; ++i)
+    {
+      if (shdr[i].sh_type == SHT_PROGBITS)
+	{
+	  size_t *start = (size_t *) (addr + shdr[i].sh_offset);
+	  size_t *end
+	      = (size_t *) (addr + shdr[i].sh_offset + shdr[i].sh_size);
+	  for (size_t *p = start; p < end; ++p)
+	    if (*p == (size_t) sym_old_addr)
+	      *p = (size_t) sym_new_addr;
+	}
+    }
+
+  return sym_new_addr;
+}
+
+/* Open an elf binary file and memory map it
+   with execution flag enabled. */
+ElfW (Addr) load_elf (const char *libname, size_t *size, ElfW (Addr) old_addr)
+{
+  int fd;
+  struct stat st;
+
+  if ((fd = open (libname, O_RDONLY)) == -1)
+    {
+      fprintf (stderr, "open (\"%s\", O_RDONLY): %s\n", libname,
+	       strerror (errno));
+      exit (1);
+    }
+
+  if (fstat (fd, &st) != 0)
+    {
+      fprintf (stderr, "fstat (\"%d\"): %s\n", fd, strerror (errno));
+      exit (1);
+    }
+
+  void *addr = mmap ((void *) old_addr, st.st_size,
+		     PROT_READ | PROT_WRITE | PROT_EXEC,
+		     old_addr ? MAP_PRIVATE | MAP_FIXED : MAP_PRIVATE, fd, 0);
+  close (fd);
+
+  if (addr == MAP_FAILED)
+    {
+      fprintf (stderr, "mmap: %s\n", strerror (errno));
+      exit (1);
+    }
+
+  if (size != NULL)
+    *size = st.st_size;
+
+  return (ElfW (Addr)) addr;
+}
\ No newline at end of file
diff --git a/gdb/testsuite/gdb.base/jit-main.c b/gdb/testsuite/gdb.base/jit-main.c
index c8e177c..4a82793 100644
--- a/gdb/testsuite/gdb.base/jit-main.c
+++ b/gdb/testsuite/gdb.base/jit-main.c
@@ -17,62 +17,8 @@
 
 /* Simulate loading of JIT code.  */
 
-#include <elf.h>
-#include <link.h>
-#include <errno.h>
-#include <fcntl.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  */
-
-typedef enum
-{
-  JIT_NOACTION = 0,
-  JIT_REGISTER_FN,
-  JIT_UNREGISTER_FN
-} jit_actions_t;
-
-struct jit_code_entry
-{
-  struct jit_code_entry *next_entry;
-  struct jit_code_entry *prev_entry;
-  const char *symfile_addr;
-  uint64_t symfile_size;
-};
-
-struct jit_descriptor
-{
-  uint32_t version;
-  /* This type should be jit_actions_t, but we use uint32_t
-     to be explicit about the bitwidth.  */
-  uint32_t action_flag;
-  struct jit_code_entry *relevant_entry;
-  struct jit_code_entry *first_entry;
-};
-
-/* GDB puts a breakpoint in this function.  */
-void __attribute__((noinline)) __jit_debug_register_code () { }
-
-/* Make sure to specify the version statically, because the
-   debugger may check the version before we can set it.  */
-struct jit_descriptor __jit_debug_descriptor = { 1, 0, 0, 0 };
+#include "jit-elf.h"
+#include "jit-defs.h"
 
 static void
 usage (const char *const argv0)
@@ -81,42 +27,6 @@
   exit (1);
 }
 
-/* Update .p_vaddr and .sh_addr as if the code was JITted to ADDR.  */
-
-static void
-update_locations (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)
-        {
-          /* 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);
-        }
-
-      if (shdr[i].sh_flags & SHF_ALLOC)
-        shdr[i].sh_addr += (ElfW (Addr))addr;
-    }
-}
-
 /* Defined by the .exp file if testing attach.  */
 #ifndef ATTACH
 #define ATTACH 0
@@ -138,8 +48,7 @@
 {
   /* 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 count = 0, i;
 
   alarm (300);
 
@@ -164,36 +73,22 @@
   printf ("%s:%d: libname = %s, count = %d\n", __FILE__, __LINE__,
 	  libname, count);
 
-  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);
-    }
-
   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));
-
-      if (addr == MAP_FAILED)
-	{
-	  fprintf (stderr, "mmap: %s\n", strerror (errno));
-	  exit (1);
-	}
-
+      size_t obj_size;
+      ElfW (Addr) addr = load_elf (libname, &obj_size);
       update_locations (addr, i);
 
+      char name[32];
+      sprintf (name, "jit_function_%04d", i);
+      int (*jit_function) () = (int (*) ()) load_symbol (addr, name);
+
+      struct jit_code_entry *const entry
+	  = (struct jit_code_entry *) calloc (1, sizeof (*entry));
+
       /* Link entry at the end of the list.  */
       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;
 
@@ -205,6 +100,12 @@
       /* Notify GDB.  */
       __jit_debug_descriptor.action_flag = JIT_REGISTER_FN;
       __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 */

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I5f9d56e8eaaa7cf88881eda6a92a72962dd3f066
Gerrit-Change-Number: 757
Gerrit-PatchSet: 1
Gerrit-Owner: Mihails Strasuns <mihails.strasuns@intel.com>
Gerrit-MessageType: newchange


       reply	other threads:[~2019-12-19 10:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-19 10:28 Mihails Strasuns (Code Review) [this message]
2020-01-29 15:50 ` Mihails Strasuns (Code Review)
2020-01-29 16:02 ` Simon Marchi (Code Review)
2020-02-05  3:27 ` Simon Marchi (Code Review)
2020-02-06 14:07 ` Mihails Strasuns (Code Review)
2020-02-06 14:22 ` Simon Marchi (Code Review)
2020-02-18 12:46 ` Mihails Strasuns (Code Review)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=gerrit.1576751303000.I5f9d56e8eaaa7cf88881eda6a92a72962dd3f066@gnutoolchain-gerrit.osci.io \
    --to=gerrit@gnutoolchain-gerrit.osci.io \
    --cc=gdb-patches@sourceware.org \
    --cc=mihails.strasuns@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox