Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Add support of shared lib for Darwin
@ 2009-01-08 14:09 Tristan Gingold
  2009-01-23 15:54 ` Ping: " Tristan Gingold
  0 siblings, 1 reply; 4+ messages in thread
From: Tristan Gingold @ 2009-01-08 14:09 UTC (permalink / raw)
  To: gdb-patches

Hi,

this patch is a slightly rewritten version of a previous one.

It adds support for shared libraries for Darwin.  It is based on a
patch from Ulrich (http://sourceware.org/ml/gdb-patches/2008-12/msg00317.html)
which made bfd file opening more flexible.

Tristan.

2009-01-08  Tristan Gingold  <gingold@adacore.com>

	* machoread.c (macho_symfile_read): Read minsymtab also from
	shared libraries.
	(macho_symfile_read): Try to read dwarf2 frame info from main
	object file, but not from OSO files.
	(macho_symfile_offsets): Update section names for latest BFD
	changes.
	* i386-darwin-tdep.c (i386_darwin_init_abi): Call set_solib_ops.
	(x86_darwin_init_abi_64): Ditto.
	* configure.tgt: Add solib.o solib-darwin.o for Darwin.


diff -u -r1.206 configure.tgt
--- configure.tgt	27 Nov 2008 09:23:01 -0000	1.206
+++ configure.tgt	8 Jan 2009 13:55:06 -0000
@@ -148,7 +148,7 @@
 i[34567]86-*-darwin*)
 	# Target: Darwin/i386
 	gdb_target_obs="amd64-tdep.o i386-tdep.o i387-tdep.o \
-			i386-darwin-tdep.o"
+			i386-darwin-tdep.o solib.o solib-darwin.o"
 	;;
 i[34567]86-*-dicos*)
 	# Target: DICOS/i386
diff -u -r1.2 i386-darwin-tdep.c
--- i386-darwin-tdep.c	3 Jan 2009 05:57:51 -0000	1.2
+++ i386-darwin-tdep.c	8 Jan 2009 13:55:07 -0000
@@ -39,6 +39,8 @@
 #include "frame.h"
 #include "gdb_assert.h"
 #include "i386-darwin-tdep.h"
+#include "solib.h"
+#include "solib-darwin.h"
 
 /* Offsets into the struct i386_thread_state where we'll find the saved regs.
    From <mach/i386/thread_status.h> and i386-tdep.h.  */
@@ -114,6 +116,8 @@
   tdep->sc_num_regs = 16;
 
   tdep->jb_pc_offset = 20;
+
+  set_solib_ops (gdbarch, &darwin_so_ops);
 }
 
 static void
@@ -131,6 +135,8 @@
   tdep->sc_num_regs = ARRAY_SIZE (amd64_darwin_thread_state_reg_offset);
 
   tdep->jb_pc_offset = 148;
+
+  set_solib_ops (gdbarch, &darwin_so_ops);
 }
 
 static enum gdb_osabi
diff -u -r1.2 machoread.c
--- machoread.c	3 Jan 2009 05:57:52 -0000	1.2
+++ machoread.c	8 Jan 2009 13:55:07 -0000
@@ -538,7 +538,7 @@
   /* Get symbols from the symbol table only if the file is an executable.
      The symbol table of object files is not relocated and is expected to
      be in the executable.  */
-  if (bfd_get_file_flags (abfd) & EXEC_P)
+  if (bfd_get_file_flags (abfd) & (EXEC_P | DYNAMIC))
     {
       /* Process the normal symbol table first.  */
       storage_needed = bfd_get_symtab_upper_bound (objfile->obfd);
@@ -566,6 +566,12 @@
       
       install_minimal_symbols (objfile);
 
+      /* Try to read .eh_frame / .debug_frame.  */
+      /* First, locate these sections.  We ignore the result status
+	 as it only checks for debug info.  */
+      dwarf2_has_info (objfile);
+      dwarf2_build_frame_info (objfile);
+      
       /* Check for DSYM file.  */
       dsym_bfd = macho_check_dsym (objfile);
       if (dsym_bfd != NULL)
@@ -588,7 +594,7 @@
 	  /* Now recurse: read dwarf from dsym.  */
 	  symbol_file_add_from_bfd (dsym_bfd, 0, NULL, 0, 0);
       
-	  /* Don't try to read dwarf2 from main file.  */
+	  /* Don't try to read dwarf2 from main file or shared libraries.  */
 	  return;
 	}
     }
@@ -599,9 +605,8 @@
       dwarf2_build_psymtabs (objfile, mainline);
     }
 
-  /* FIXME: kettenis/20030504: This still needs to be integrated with
-     dwarf2read.c in a better way.  */
-  dwarf2_build_frame_info (objfile);
+  /* Do not try to read .eh_frame/.debug_frame as they are not relocated
+     and dwarf2_build_frame_info cannot deal with unrelocated sections.  */
 
   /* Then the oso.  */
   if (oso_vector != NULL)
@@ -661,10 +666,11 @@
     {
       const char *bfd_sect_name = osect->the_bfd_section->name;
       int sect_index = osect->the_bfd_section->index;
-
-      if (strcmp (bfd_sect_name, "LC_SEGMENT.__TEXT") == 0)
-	objfile->sect_index_text = sect_index;
-      else if (strcmp (bfd_sect_name, "LC_SEGMENT.__TEXT.__text") == 0)
+      
+      if (strncmp (bfd_sect_name, "LC_SEGMENT.", 11) == 0)
+	bfd_sect_name += 11;
+      if (strcmp (bfd_sect_name, "__TEXT") == 0
+	  || strcmp (bfd_sect_name, "__TEXT.__text") == 0)
 	objfile->sect_index_text = sect_index;
     }
 }
--- /dev/null	2009-01-08 14:53:34.000000000 +0100
+++ solib-darwin.h	2009-01-08 15:01:55.000000000 +0100
@@ -0,0 +1,28 @@
+/* Handle shared libraries for GDB, the GNU Debugger.
+
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+#ifndef SOLIB_DARWIN_H
+#define SOLIB_DARWIN_H
+
+struct objfile;
+struct target_so_ops;
+
+extern struct target_so_ops darwin_so_ops;
+
+#endif /* solib-darwin.h */
--- /dev/null	2009-01-08 14:53:34.000000000 +0100
+++ solib-darwin.c	2009-01-08 15:02:07.000000000 +0100
@@ -0,0 +1,463 @@
+/* Handle Darwin shared libraries for GDB, the GNU Debugger.
+
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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 "defs.h"
+
+#include "symtab.h"
+#include "bfd.h"
+#include "symfile.h"
+#include "objfiles.h"
+#include "gdbcore.h"
+#include "target.h"
+#include "inferior.h"
+#include "gdbthread.h"
+
+#include "gdb_assert.h"
+
+#include "solist.h"
+#include "solib.h"
+#include "solib-svr4.h"
+
+#include "bfd-target.h"
+#include "elf-bfd.h"
+#include "exec.h"
+#include "auxv.h"
+#include "exceptions.h"
+#include "mach-o.h"
+
+struct gdb_dyld_image_info
+{
+  /* Base address (which corresponds to the Mach-O header).  */
+  CORE_ADDR mach_header;
+  /* Image file path.  */
+  CORE_ADDR file_path;
+  /* st.m_time of image file.  */
+  unsigned long mtime;
+};
+
+/* Content of inferior dyld_all_image_infos structure.  */
+struct gdb_dyld_all_image_infos
+{
+  /* Version (1).  */
+  unsigned long version;
+  /* Number of images.  */
+  unsigned long count;
+  /* Image description.  */
+  CORE_ADDR info;
+  /* Notifier (function called when a library is added or removed).  */
+  CORE_ADDR notifier;
+};
+
+/* Current all_image_infos version.  */
+#define DYLD_VERSION 1
+
+/* Address of structure dyld_all_image_infos in inferior.  */
+static CORE_ADDR dyld_all_image_addr;
+
+/* Gdb copy of dyld_all_info_infos.  */
+static struct gdb_dyld_all_image_infos dyld_all_image;
+
+/* Read dyld_all_image from inferior.  */
+static void
+darwin_load_image_infos (void)
+{
+  gdb_byte buf[24];
+  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
+  int len;
+
+  len = 4 + 4 + 2 * ptr_type->length;
+  memset (&dyld_all_image, 0, sizeof (dyld_all_image));
+
+  if (dyld_all_image_addr == 0)
+    return;
+
+  if (target_read_memory (dyld_all_image_addr, buf, len))
+    return;
+
+  dyld_all_image.version = extract_unsigned_integer (buf, 4);
+  if (dyld_all_image.version != DYLD_VERSION)
+    return;
+
+  dyld_all_image.count = extract_unsigned_integer (buf + 4, 4);
+  dyld_all_image.info = extract_typed_address (buf + 8, ptr_type);
+  dyld_all_image.notifier = extract_typed_address
+    (buf + 8 + ptr_type->length, ptr_type);
+}
+
+/* Link map info to include in an allocated so_list entry */
+
+struct lm_info
+{
+  /* The target location of lm.  */
+  CORE_ADDR lm_addr;
+};
+
+struct darwin_so_list
+{
+  struct so_list sl;
+  struct lm_info li;
+};
+
+/* Local function prototypes */
+
+static int match_main (char *);
+
+static CORE_ADDR bfd_lookup_symbol (bfd *, char *);
+
+/* Return non-zero if GDB_SO_NAME and INFERIOR_SO_NAME represent
+   the same shared library.  */
+
+static int
+darwin_same (struct so_list *gdb, struct so_list *inferior)
+{
+  return strcmp (gdb->so_original_name, inferior->so_original_name) == 0;
+}
+
+/* Lookup the value for a specific symbol.  */
+static CORE_ADDR
+bfd_lookup_symbol (bfd *abfd, char *symname)
+{
+  long storage_needed;
+  asymbol **symbol_table;
+  unsigned int number_of_symbols;
+  unsigned int i;
+  CORE_ADDR symaddr = 0;
+
+  storage_needed = bfd_get_symtab_upper_bound (abfd);
+
+  if (storage_needed > 0)
+    {
+      symbol_table = (asymbol **) xmalloc (storage_needed);
+      number_of_symbols = bfd_canonicalize_symtab (abfd, symbol_table);
+
+      for (i = 0; i < number_of_symbols; i++)
+	{
+	  asymbol *sym = symbol_table[i];
+	  if (strcmp (sym->name, symname) == 0
+              && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0)
+	    {
+	      /* BFD symbols are section relative.  */
+	      symaddr = sym->value + sym->section->vma;
+	      break;
+	    }
+	}
+      xfree (symbol_table);
+    }
+
+  return symaddr;
+}
+
+/* Return program interpreter string.  */
+static gdb_byte *
+find_program_interpreter (void)
+{
+  gdb_byte *buf = NULL;
+
+  /* If we have an exec_bfd, use its section table.  */
+  if (exec_bfd)
+    {
+      struct bfd_section *dylinker_sect;
+      
+      dylinker_sect = bfd_get_section_by_name (exec_bfd, "LC_LOAD_DYLINKER");
+      if (dylinker_sect != NULL)
+	{
+	  int sect_size = bfd_section_size (exec_bfd, dylinker_sect);
+
+	  buf = xmalloc (sect_size);
+	  if (bfd_get_section_contents (exec_bfd, dylinker_sect,
+					buf, 0, sect_size))
+	    return buf;
+	  xfree (buf);
+	}
+    }
+
+  /* If we didn't find it, read from memory.
+     FIXME: todo.  */
+  return buf;
+}
+
+/*  Not used.  I don't see how the main symbol file can be found: the
+    interpreter name is needed and it is known from the executable file.
+    Note that darwin-nat.c implements pid_to_exec_file.  */
+static int
+open_symbol_file_object (void *from_ttyp)
+{
+  return 0;
+}
+
+/* Build a list of currently loaded shared objects.  See solib-svr4.c  */
+static struct so_list *
+darwin_current_sos (void)
+{
+  struct type *ptr_type = builtin_type (target_gdbarch)->builtin_data_ptr;
+  int ptr_len = TYPE_LENGTH (ptr_type);
+  unsigned int image_info_size;
+  CORE_ADDR lm;
+  struct so_list *head = NULL;
+  struct so_list *tail = NULL;
+  int i;
+
+  /* Be sure image infos are loaded.  */
+  darwin_load_image_infos ();
+
+  if (dyld_all_image.version != DYLD_VERSION)
+    return NULL;
+
+  image_info_size = ptr_len * 3;
+
+  /* Read infos for each solib.  */
+  for (i = 0; i < dyld_all_image.count; i++)
+    {
+      CORE_ADDR info = dyld_all_image.info + i * image_info_size;
+      char buf[image_info_size];
+      CORE_ADDR load_addr;
+      CORE_ADDR path_addr;
+      char *file_path;
+      int errcode;
+      struct darwin_so_list *dnew;
+      struct so_list *new;
+      struct cleanup *old_chain;
+
+      /* Read image info from inferior.  */
+      if (target_read_memory (info, buf, image_info_size))
+	break;
+
+      load_addr = extract_typed_address (buf, ptr_type);
+      path_addr = extract_typed_address (buf + ptr_len, ptr_type);
+
+      target_read_string (path_addr, &file_path,
+			  SO_NAME_MAX_PATH_SIZE - 1, &errcode);
+      if (errcode)
+	break;
+
+      /* Ignore first entry as this is the executable itself.  */
+      if (i == 0)
+	continue;
+
+      /* Create and fill the new so_list element.  */
+      dnew = XZALLOC (struct darwin_so_list);
+      new = &dnew->sl;
+      old_chain = make_cleanup (xfree, dnew);
+
+      new->lm_info = &dnew->li;
+
+      strncpy (new->so_name, file_path, SO_NAME_MAX_PATH_SIZE - 1);
+      new->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
+      strcpy (new->so_original_name, new->so_name);
+      xfree (file_path);
+      new->lm_info->lm_addr = load_addr;
+
+      if (head == NULL)
+	head = new;
+      else
+	tail->next = new;
+      tail = new;
+
+      discard_cleanups (old_chain);
+    }
+
+  return head;
+}
+
+/* Return 1 if PC lies in the dynamic symbol resolution code of the
+   run time loader.  */
+int
+darwin_in_dynsym_resolve_code (CORE_ADDR pc)
+{
+  return 0;
+}
+
+
+/* No special symbol handling.  */
+static void
+darwin_special_symbol_handling (void)
+{
+}
+
+/* Shared library startup support.  See documentation in solib-svr4.c  */
+static void
+darwin_solib_create_inferior_hook (void)
+{
+  struct minimal_symbol *msymbol;
+  char **bkpt_namep;
+  asection *interp_sect;
+  gdb_byte *interp_name;
+  CORE_ADDR sym_addr;
+  CORE_ADDR load_addr = 0;
+  int load_addr_found = 0;
+  int loader_found_in_list = 0;
+  struct so_list *so;
+  bfd *dyld_bfd = NULL;
+  struct inferior *inf = current_inferior ();
+
+  /* First, remove all the solib event breakpoints.  Their addresses
+     may have changed since the last time we ran the program.  */
+  remove_solib_event_breakpoints ();
+
+  /* Find the program interpreter.  */
+  interp_name = find_program_interpreter ();
+  if (!interp_name)
+    return;
+
+  /* Create a bfd for the interpreter.  */
+  sym_addr = 0;
+  dyld_bfd = bfd_openr (interp_name, gnutarget);
+  if (dyld_bfd)
+    {
+      bfd *sub;
+      sub = bfd_mach_o_fat_extract (dyld_bfd, bfd_object,
+				    gdbarch_bfd_arch_info (current_gdbarch));
+      if (sub)
+	dyld_bfd = sub;
+      else
+	{
+	  bfd_close (dyld_bfd);
+	  dyld_bfd = NULL;
+	}
+    }
+  if (!dyld_bfd)
+    {
+      xfree (interp_name);
+      return;
+    }
+
+  if (!inf->attach_flag)
+    {
+      /* We find the dynamic linker's base address by examining
+	 the current pc (which should point at the entry point for the
+	 dynamic linker) and subtracting the offset of the entry point.  */
+      load_addr = (read_pc () - bfd_get_start_address (dyld_bfd));
+    }
+  else
+    {
+      /* FIXME: todo.
+	 Get address of __DATA.__dyld in exec_bfd, read address at offset 0
+      */
+      xfree (interp_name);
+      return;
+    }
+
+  /* Now try to set a breakpoint in the dynamic linker.  */
+  dyld_all_image_addr =
+    bfd_lookup_symbol (dyld_bfd, "_dyld_all_image_infos");
+  
+  bfd_close (dyld_bfd);
+  xfree (interp_name);
+
+  if (dyld_all_image_addr == 0)
+    return;
+
+  dyld_all_image_addr += load_addr;
+
+  darwin_load_image_infos ();
+
+  if (dyld_all_image.version == DYLD_VERSION)
+    create_solib_event_breakpoint (dyld_all_image.notifier);
+}
+
+static void
+darwin_clear_solib (void)
+{
+  dyld_all_image_addr = 0;
+  dyld_all_image.version = 0;
+}
+
+static void
+darwin_free_so (struct so_list *so)
+{
+}
+
+/* The section table is built from bfd sections using bfd VMAs.
+   Relocate these VMAs according to solib info.  */
+static void
+darwin_relocate_section_addresses (struct so_list *so,
+				   struct section_table *sec)
+{
+  sec->addr += so->lm_info->lm_addr;
+  sec->endaddr += so->lm_info->lm_addr;
+
+  /* Best effort to set addr_high/addr_low.  This is used only by
+     'info sharedlibary'.  */
+  if (so->addr_high == 0)
+    {
+      so->addr_low = sec->addr;
+      so->addr_high = sec->endaddr;
+    }
+  if (sec->endaddr > so->addr_high)
+    so->addr_high = sec->endaddr;
+  if (sec->addr < so->addr_low)
+    so->addr_low = sec->addr;
+}
+\f
+static struct symbol *
+darwin_lookup_lib_symbol (const struct objfile *objfile,
+			  const char *name,
+			  const char *linkage_name,
+			  const domain_enum domain)
+{
+  return NULL;
+}
+
+static bfd *
+darwin_bfd_open (char *pathname)
+{
+  char *found_pathname;
+  int found_file;
+  bfd *abfd;
+  bfd *res;
+
+  /* Search for shared library file.  */
+  found_pathname = solib_find (pathname, &found_file);
+  if (found_pathname == NULL)
+    perror_with_name (pathname);
+
+  /* Open bfd for shared library.  */
+  abfd = solib_bfd_fopen (found_pathname, found_file);
+
+  res = bfd_mach_o_fat_extract (abfd, bfd_object,
+				gdbarch_bfd_arch_info (current_gdbarch));
+  if (!res)
+    {
+      bfd_close (abfd);
+      make_cleanup (xfree, found_pathname);
+      error (_("`%s': not a shared-library: %s"),
+	     found_pathname, bfd_errmsg (bfd_get_error ()));
+    }
+  return res;
+}
+
+extern initialize_file_ftype _initialize_svr4_solib; /* -Wmissing-prototypes */
+
+struct target_so_ops darwin_so_ops;
+
+void
+_initialize_darwin_solib (void)
+{
+  darwin_so_ops.relocate_section_addresses = darwin_relocate_section_addresses;
+  darwin_so_ops.free_so = darwin_free_so;
+  darwin_so_ops.clear_solib = darwin_clear_solib;
+  darwin_so_ops.solib_create_inferior_hook = darwin_solib_create_inferior_hook;
+  darwin_so_ops.special_symbol_handling = darwin_special_symbol_handling;
+  darwin_so_ops.current_sos = darwin_current_sos;
+  darwin_so_ops.open_symbol_file_object = open_symbol_file_object;
+  darwin_so_ops.in_dynsym_resolve_code = darwin_in_dynsym_resolve_code;
+  darwin_so_ops.lookup_lib_global_symbol = darwin_lookup_lib_symbol;
+  darwin_so_ops.same = darwin_same;
+  darwin_so_ops.bfd_open = darwin_bfd_open;
+}


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

* Ping: [RFA] Add support of shared lib for Darwin
  2009-01-08 14:09 [RFA] Add support of shared lib for Darwin Tristan Gingold
@ 2009-01-23 15:54 ` Tristan Gingold
  2009-02-03  2:04   ` Joel Brobecker
  0 siblings, 1 reply; 4+ messages in thread
From: Tristan Gingold @ 2009-01-23 15:54 UTC (permalink / raw)
  To: gdb-patches

Just a ping for this patch.

(Ulrich has committed his patch).

Tristan.

On Jan 8, 2009, at 3:09 PM, Tristan Gingold wrote:

> Hi,
>
> this patch is a slightly rewritten version of a previous one.
>
> It adds support for shared libraries for Darwin.  It is based on a
> patch from Ulrich (http://sourceware.org/ml/gdb-patches/2008-12/msg00317.html 
> )
> which made bfd file opening more flexible.
>
> Tristan.
>
> 2009-01-08  Tristan Gingold  <gingold@adacore.com>
>
> 	* machoread.c (macho_symfile_read): Read minsymtab also from
> 	shared libraries.
> 	(macho_symfile_read): Try to read dwarf2 frame info from main
> 	object file, but not from OSO files.
> 	(macho_symfile_offsets): Update section names for latest BFD
> 	changes.
> 	* i386-darwin-tdep.c (i386_darwin_init_abi): Call set_solib_ops.
> 	(x86_darwin_init_abi_64): Ditto.
> 	* configure.tgt: Add solib.o solib-darwin.o for Darwin.
>
>
> diff -u -r1.206 configure.tgt
> --- configure.tgt	27 Nov 2008 09:23:01 -0000	1.206
> +++ configure.tgt	8 Jan 2009 13:55:06 -0000
> @@ -148,7 +148,7 @@
> i[34567]86-*-darwin*)
> 	# Target: Darwin/i386
> 	gdb_target_obs="amd64-tdep.o i386-tdep.o i387-tdep.o \
> -			i386-darwin-tdep.o"
> +			i386-darwin-tdep.o solib.o solib-darwin.o"
> 	;;
> i[34567]86-*-dicos*)
> 	# Target: DICOS/i386
> diff -u -r1.2 i386-darwin-tdep.c
> --- i386-darwin-tdep.c	3 Jan 2009 05:57:51 -0000	1.2
> +++ i386-darwin-tdep.c	8 Jan 2009 13:55:07 -0000
> @@ -39,6 +39,8 @@
> #include "frame.h"
> #include "gdb_assert.h"
> #include "i386-darwin-tdep.h"
> +#include "solib.h"
> +#include "solib-darwin.h"
>
> /* Offsets into the struct i386_thread_state where we'll find the  
> saved regs.
>    From <mach/i386/thread_status.h> and i386-tdep.h.  */
> @@ -114,6 +116,8 @@
>   tdep->sc_num_regs = 16;
>
>   tdep->jb_pc_offset = 20;
> +
> +  set_solib_ops (gdbarch, &darwin_so_ops);
> }
>
> static void
> @@ -131,6 +135,8 @@
>   tdep->sc_num_regs = ARRAY_SIZE  
> (amd64_darwin_thread_state_reg_offset);
>
>   tdep->jb_pc_offset = 148;
> +
> +  set_solib_ops (gdbarch, &darwin_so_ops);
> }
>
> static enum gdb_osabi
> diff -u -r1.2 machoread.c
> --- machoread.c	3 Jan 2009 05:57:52 -0000	1.2
> +++ machoread.c	8 Jan 2009 13:55:07 -0000
> @@ -538,7 +538,7 @@
>   /* Get symbols from the symbol table only if the file is an  
> executable.
>      The symbol table of object files is not relocated and is  
> expected to
>      be in the executable.  */
> -  if (bfd_get_file_flags (abfd) & EXEC_P)
> +  if (bfd_get_file_flags (abfd) & (EXEC_P | DYNAMIC))
>     {
>       /* Process the normal symbol table first.  */
>       storage_needed = bfd_get_symtab_upper_bound (objfile->obfd);
> @@ -566,6 +566,12 @@
>
>       install_minimal_symbols (objfile);
>
> +      /* Try to read .eh_frame / .debug_frame.  */
> +      /* First, locate these sections.  We ignore the result status
> +	 as it only checks for debug info.  */
> +      dwarf2_has_info (objfile);
> +      dwarf2_build_frame_info (objfile);
> +
>       /* Check for DSYM file.  */
>       dsym_bfd = macho_check_dsym (objfile);
>       if (dsym_bfd != NULL)
> @@ -588,7 +594,7 @@
> 	  /* Now recurse: read dwarf from dsym.  */
> 	  symbol_file_add_from_bfd (dsym_bfd, 0, NULL, 0, 0);
>
> -	  /* Don't try to read dwarf2 from main file.  */
> +	  /* Don't try to read dwarf2 from main file or shared libraries.   
> */
> 	  return;
> 	}
>     }
> @@ -599,9 +605,8 @@
>       dwarf2_build_psymtabs (objfile, mainline);
>     }
>
> -  /* FIXME: kettenis/20030504: This still needs to be integrated with
> -     dwarf2read.c in a better way.  */
> -  dwarf2_build_frame_info (objfile);
> +  /* Do not try to read .eh_frame/.debug_frame as they are not  
> relocated
> +     and dwarf2_build_frame_info cannot deal with unrelocated  
> sections.  */
>
>   /* Then the oso.  */
>   if (oso_vector != NULL)
> @@ -661,10 +666,11 @@
>     {
>       const char *bfd_sect_name = osect->the_bfd_section->name;
>       int sect_index = osect->the_bfd_section->index;
> -
> -      if (strcmp (bfd_sect_name, "LC_SEGMENT.__TEXT") == 0)
> -	objfile->sect_index_text = sect_index;
> -      else if (strcmp (bfd_sect_name, "LC_SEGMENT.__TEXT.__text")  
> == 0)
> +
> +      if (strncmp (bfd_sect_name, "LC_SEGMENT.", 11) == 0)
> +	bfd_sect_name += 11;
> +      if (strcmp (bfd_sect_name, "__TEXT") == 0
> +	  || strcmp (bfd_sect_name, "__TEXT.__text") == 0)
> 	objfile->sect_index_text = sect_index;
>     }
> }
> --- /dev/null	2009-01-08 14:53:34.000000000 +0100
> +++ solib-darwin.h	2009-01-08 15:01:55.000000000 +0100
> @@ -0,0 +1,28 @@
> +/* Handle shared libraries for GDB, the GNU Debugger.
> +
> +   Copyright (C) 2009 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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/ 
> >.  */
> +
> +#ifndef SOLIB_DARWIN_H
> +#define SOLIB_DARWIN_H
> +
> +struct objfile;
> +struct target_so_ops;
> +
> +extern struct target_so_ops darwin_so_ops;
> +
> +#endif /* solib-darwin.h */
> --- /dev/null	2009-01-08 14:53:34.000000000 +0100
> +++ solib-darwin.c	2009-01-08 15:02:07.000000000 +0100
> @@ -0,0 +1,463 @@
> +/* Handle Darwin shared libraries for GDB, the GNU Debugger.
> +
> +   Copyright (C) 2009 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   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 "defs.h"
> +
> +#include "symtab.h"
> +#include "bfd.h"
> +#include "symfile.h"
> +#include "objfiles.h"
> +#include "gdbcore.h"
> +#include "target.h"
> +#include "inferior.h"
> +#include "gdbthread.h"
> +
> +#include "gdb_assert.h"
> +
> +#include "solist.h"
> +#include "solib.h"
> +#include "solib-svr4.h"
> +
> +#include "bfd-target.h"
> +#include "elf-bfd.h"
> +#include "exec.h"
> +#include "auxv.h"
> +#include "exceptions.h"
> +#include "mach-o.h"
> +
> +struct gdb_dyld_image_info
> +{
> +  /* Base address (which corresponds to the Mach-O header).  */
> +  CORE_ADDR mach_header;
> +  /* Image file path.  */
> +  CORE_ADDR file_path;
> +  /* st.m_time of image file.  */
> +  unsigned long mtime;
> +};
> +
> +/* Content of inferior dyld_all_image_infos structure.  */
> +struct gdb_dyld_all_image_infos
> +{
> +  /* Version (1).  */
> +  unsigned long version;
> +  /* Number of images.  */
> +  unsigned long count;
> +  /* Image description.  */
> +  CORE_ADDR info;
> +  /* Notifier (function called when a library is added or  
> removed).  */
> +  CORE_ADDR notifier;
> +};
> +
> +/* Current all_image_infos version.  */
> +#define DYLD_VERSION 1
> +
> +/* Address of structure dyld_all_image_infos in inferior.  */
> +static CORE_ADDR dyld_all_image_addr;
> +
> +/* Gdb copy of dyld_all_info_infos.  */
> +static struct gdb_dyld_all_image_infos dyld_all_image;
> +
> +/* Read dyld_all_image from inferior.  */
> +static void
> +darwin_load_image_infos (void)
> +{
> +  gdb_byte buf[24];
> +  struct type *ptr_type = builtin_type (target_gdbarch)- 
> >builtin_data_ptr;
> +  int len;
> +
> +  len = 4 + 4 + 2 * ptr_type->length;
> +  memset (&dyld_all_image, 0, sizeof (dyld_all_image));
> +
> +  if (dyld_all_image_addr == 0)
> +    return;
> +
> +  if (target_read_memory (dyld_all_image_addr, buf, len))
> +    return;
> +
> +  dyld_all_image.version = extract_unsigned_integer (buf, 4);
> +  if (dyld_all_image.version != DYLD_VERSION)
> +    return;
> +
> +  dyld_all_image.count = extract_unsigned_integer (buf + 4, 4);
> +  dyld_all_image.info = extract_typed_address (buf + 8, ptr_type);
> +  dyld_all_image.notifier = extract_typed_address
> +    (buf + 8 + ptr_type->length, ptr_type);
> +}
> +
> +/* Link map info to include in an allocated so_list entry */
> +
> +struct lm_info
> +{
> +  /* The target location of lm.  */
> +  CORE_ADDR lm_addr;
> +};
> +
> +struct darwin_so_list
> +{
> +  struct so_list sl;
> +  struct lm_info li;
> +};
> +
> +/* Local function prototypes */
> +
> +static int match_main (char *);
> +
> +static CORE_ADDR bfd_lookup_symbol (bfd *, char *);
> +
> +/* Return non-zero if GDB_SO_NAME and INFERIOR_SO_NAME represent
> +   the same shared library.  */
> +
> +static int
> +darwin_same (struct so_list *gdb, struct so_list *inferior)
> +{
> +  return strcmp (gdb->so_original_name, inferior->so_original_name)  
> == 0;
> +}
> +
> +/* Lookup the value for a specific symbol.  */
> +static CORE_ADDR
> +bfd_lookup_symbol (bfd *abfd, char *symname)
> +{
> +  long storage_needed;
> +  asymbol **symbol_table;
> +  unsigned int number_of_symbols;
> +  unsigned int i;
> +  CORE_ADDR symaddr = 0;
> +
> +  storage_needed = bfd_get_symtab_upper_bound (abfd);
> +
> +  if (storage_needed > 0)
> +    {
> +      symbol_table = (asymbol **) xmalloc (storage_needed);
> +      number_of_symbols = bfd_canonicalize_symtab (abfd,  
> symbol_table);
> +
> +      for (i = 0; i < number_of_symbols; i++)
> +	{
> +	  asymbol *sym = symbol_table[i];
> +	  if (strcmp (sym->name, symname) == 0
> +              && (sym->section->flags & (SEC_CODE | SEC_DATA)) != 0)
> +	    {
> +	      /* BFD symbols are section relative.  */
> +	      symaddr = sym->value + sym->section->vma;
> +	      break;
> +	    }
> +	}
> +      xfree (symbol_table);
> +    }
> +
> +  return symaddr;
> +}
> +
> +/* Return program interpreter string.  */
> +static gdb_byte *
> +find_program_interpreter (void)
> +{
> +  gdb_byte *buf = NULL;
> +
> +  /* If we have an exec_bfd, use its section table.  */
> +  if (exec_bfd)
> +    {
> +      struct bfd_section *dylinker_sect;
> +
> +      dylinker_sect = bfd_get_section_by_name (exec_bfd,  
> "LC_LOAD_DYLINKER");
> +      if (dylinker_sect != NULL)
> +	{
> +	  int sect_size = bfd_section_size (exec_bfd, dylinker_sect);
> +
> +	  buf = xmalloc (sect_size);
> +	  if (bfd_get_section_contents (exec_bfd, dylinker_sect,
> +					buf, 0, sect_size))
> +	    return buf;
> +	  xfree (buf);
> +	}
> +    }
> +
> +  /* If we didn't find it, read from memory.
> +     FIXME: todo.  */
> +  return buf;
> +}
> +
> +/*  Not used.  I don't see how the main symbol file can be found: the
> +    interpreter name is needed and it is known from the executable  
> file.
> +    Note that darwin-nat.c implements pid_to_exec_file.  */
> +static int
> +open_symbol_file_object (void *from_ttyp)
> +{
> +  return 0;
> +}
> +
> +/* Build a list of currently loaded shared objects.  See solib- 
> svr4.c  */
> +static struct so_list *
> +darwin_current_sos (void)
> +{
> +  struct type *ptr_type = builtin_type (target_gdbarch)- 
> >builtin_data_ptr;
> +  int ptr_len = TYPE_LENGTH (ptr_type);
> +  unsigned int image_info_size;
> +  CORE_ADDR lm;
> +  struct so_list *head = NULL;
> +  struct so_list *tail = NULL;
> +  int i;
> +
> +  /* Be sure image infos are loaded.  */
> +  darwin_load_image_infos ();
> +
> +  if (dyld_all_image.version != DYLD_VERSION)
> +    return NULL;
> +
> +  image_info_size = ptr_len * 3;
> +
> +  /* Read infos for each solib.  */
> +  for (i = 0; i < dyld_all_image.count; i++)
> +    {
> +      CORE_ADDR info = dyld_all_image.info + i * image_info_size;
> +      char buf[image_info_size];
> +      CORE_ADDR load_addr;
> +      CORE_ADDR path_addr;
> +      char *file_path;
> +      int errcode;
> +      struct darwin_so_list *dnew;
> +      struct so_list *new;
> +      struct cleanup *old_chain;
> +
> +      /* Read image info from inferior.  */
> +      if (target_read_memory (info, buf, image_info_size))
> +	break;
> +
> +      load_addr = extract_typed_address (buf, ptr_type);
> +      path_addr = extract_typed_address (buf + ptr_len, ptr_type);
> +
> +      target_read_string (path_addr, &file_path,
> +			  SO_NAME_MAX_PATH_SIZE - 1, &errcode);
> +      if (errcode)
> +	break;
> +
> +      /* Ignore first entry as this is the executable itself.  */
> +      if (i == 0)
> +	continue;
> +
> +      /* Create and fill the new so_list element.  */
> +      dnew = XZALLOC (struct darwin_so_list);
> +      new = &dnew->sl;
> +      old_chain = make_cleanup (xfree, dnew);
> +
> +      new->lm_info = &dnew->li;
> +
> +      strncpy (new->so_name, file_path, SO_NAME_MAX_PATH_SIZE - 1);
> +      new->so_name[SO_NAME_MAX_PATH_SIZE - 1] = '\0';
> +      strcpy (new->so_original_name, new->so_name);
> +      xfree (file_path);
> +      new->lm_info->lm_addr = load_addr;
> +
> +      if (head == NULL)
> +	head = new;
> +      else
> +	tail->next = new;
> +      tail = new;
> +
> +      discard_cleanups (old_chain);
> +    }
> +
> +  return head;
> +}
> +
> +/* Return 1 if PC lies in the dynamic symbol resolution code of the
> +   run time loader.  */
> +int
> +darwin_in_dynsym_resolve_code (CORE_ADDR pc)
> +{
> +  return 0;
> +}
> +
> +
> +/* No special symbol handling.  */
> +static void
> +darwin_special_symbol_handling (void)
> +{
> +}
> +
> +/* Shared library startup support.  See documentation in solib- 
> svr4.c  */
> +static void
> +darwin_solib_create_inferior_hook (void)
> +{
> +  struct minimal_symbol *msymbol;
> +  char **bkpt_namep;
> +  asection *interp_sect;
> +  gdb_byte *interp_name;
> +  CORE_ADDR sym_addr;
> +  CORE_ADDR load_addr = 0;
> +  int load_addr_found = 0;
> +  int loader_found_in_list = 0;
> +  struct so_list *so;
> +  bfd *dyld_bfd = NULL;
> +  struct inferior *inf = current_inferior ();
> +
> +  /* First, remove all the solib event breakpoints.  Their addresses
> +     may have changed since the last time we ran the program.  */
> +  remove_solib_event_breakpoints ();
> +
> +  /* Find the program interpreter.  */
> +  interp_name = find_program_interpreter ();
> +  if (!interp_name)
> +    return;
> +
> +  /* Create a bfd for the interpreter.  */
> +  sym_addr = 0;
> +  dyld_bfd = bfd_openr (interp_name, gnutarget);
> +  if (dyld_bfd)
> +    {
> +      bfd *sub;
> +      sub = bfd_mach_o_fat_extract (dyld_bfd, bfd_object,
> +				    gdbarch_bfd_arch_info (current_gdbarch));
> +      if (sub)
> +	dyld_bfd = sub;
> +      else
> +	{
> +	  bfd_close (dyld_bfd);
> +	  dyld_bfd = NULL;
> +	}
> +    }
> +  if (!dyld_bfd)
> +    {
> +      xfree (interp_name);
> +      return;
> +    }
> +
> +  if (!inf->attach_flag)
> +    {
> +      /* We find the dynamic linker's base address by examining
> +	 the current pc (which should point at the entry point for the
> +	 dynamic linker) and subtracting the offset of the entry point.  */
> +      load_addr = (read_pc () - bfd_get_start_address (dyld_bfd));
> +    }
> +  else
> +    {
> +      /* FIXME: todo.
> +	 Get address of __DATA.__dyld in exec_bfd, read address at offset 0
> +      */
> +      xfree (interp_name);
> +      return;
> +    }
> +
> +  /* Now try to set a breakpoint in the dynamic linker.  */
> +  dyld_all_image_addr =
> +    bfd_lookup_symbol (dyld_bfd, "_dyld_all_image_infos");
> +
> +  bfd_close (dyld_bfd);
> +  xfree (interp_name);
> +
> +  if (dyld_all_image_addr == 0)
> +    return;
> +
> +  dyld_all_image_addr += load_addr;
> +
> +  darwin_load_image_infos ();
> +
> +  if (dyld_all_image.version == DYLD_VERSION)
> +    create_solib_event_breakpoint (dyld_all_image.notifier);
> +}
> +
> +static void
> +darwin_clear_solib (void)
> +{
> +  dyld_all_image_addr = 0;
> +  dyld_all_image.version = 0;
> +}
> +
> +static void
> +darwin_free_so (struct so_list *so)
> +{
> +}
> +
> +/* The section table is built from bfd sections using bfd VMAs.
> +   Relocate these VMAs according to solib info.  */
> +static void
> +darwin_relocate_section_addresses (struct so_list *so,
> +				   struct section_table *sec)
> +{
> +  sec->addr += so->lm_info->lm_addr;
> +  sec->endaddr += so->lm_info->lm_addr;
> +
> +  /* Best effort to set addr_high/addr_low.  This is used only by
> +     'info sharedlibary'.  */
> +  if (so->addr_high == 0)
> +    {
> +      so->addr_low = sec->addr;
> +      so->addr_high = sec->endaddr;
> +    }
> +  if (sec->endaddr > so->addr_high)
> +    so->addr_high = sec->endaddr;
> +  if (sec->addr < so->addr_low)
> +    so->addr_low = sec->addr;
> +}
> +\f
> +static struct symbol *
> +darwin_lookup_lib_symbol (const struct objfile *objfile,
> +			  const char *name,
> +			  const char *linkage_name,
> +			  const domain_enum domain)
> +{
> +  return NULL;
> +}
> +
> +static bfd *
> +darwin_bfd_open (char *pathname)
> +{
> +  char *found_pathname;
> +  int found_file;
> +  bfd *abfd;
> +  bfd *res;
> +
> +  /* Search for shared library file.  */
> +  found_pathname = solib_find (pathname, &found_file);
> +  if (found_pathname == NULL)
> +    perror_with_name (pathname);
> +
> +  /* Open bfd for shared library.  */
> +  abfd = solib_bfd_fopen (found_pathname, found_file);
> +
> +  res = bfd_mach_o_fat_extract (abfd, bfd_object,
> +				gdbarch_bfd_arch_info (current_gdbarch));
> +  if (!res)
> +    {
> +      bfd_close (abfd);
> +      make_cleanup (xfree, found_pathname);
> +      error (_("`%s': not a shared-library: %s"),
> +	     found_pathname, bfd_errmsg (bfd_get_error ()));
> +    }
> +  return res;
> +}
> +
> +extern initialize_file_ftype _initialize_svr4_solib; /* -Wmissing- 
> prototypes */
> +
> +struct target_so_ops darwin_so_ops;
> +
> +void
> +_initialize_darwin_solib (void)
> +{
> +  darwin_so_ops.relocate_section_addresses =  
> darwin_relocate_section_addresses;
> +  darwin_so_ops.free_so = darwin_free_so;
> +  darwin_so_ops.clear_solib = darwin_clear_solib;
> +  darwin_so_ops.solib_create_inferior_hook =  
> darwin_solib_create_inferior_hook;
> +  darwin_so_ops.special_symbol_handling =  
> darwin_special_symbol_handling;
> +  darwin_so_ops.current_sos = darwin_current_sos;
> +  darwin_so_ops.open_symbol_file_object = open_symbol_file_object;
> +  darwin_so_ops.in_dynsym_resolve_code =  
> darwin_in_dynsym_resolve_code;
> +  darwin_so_ops.lookup_lib_global_symbol = darwin_lookup_lib_symbol;
> +  darwin_so_ops.same = darwin_same;
> +  darwin_so_ops.bfd_open = darwin_bfd_open;
> +}
>


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

* Re: Ping: [RFA] Add support of shared lib for Darwin
  2009-01-23 15:54 ` Ping: " Tristan Gingold
@ 2009-02-03  2:04   ` Joel Brobecker
  2009-02-03 11:41     ` Tristan Gingold
  0 siblings, 1 reply; 4+ messages in thread
From: Joel Brobecker @ 2009-02-03  2:04 UTC (permalink / raw)
  To: Tristan Gingold; +Cc: gdb-patches

No review of this patch so far, so I took a look :)

> >2009-01-08  Tristan Gingold  <gingold@adacore.com>
> >
> >	* machoread.c (macho_symfile_read): Read minsymtab also from
> >	shared libraries.
> >	(macho_symfile_read): Try to read dwarf2 frame info from main
> >	object file, but not from OSO files.
> >	(macho_symfile_offsets): Update section names for latest BFD
> >	changes.
> >	* i386-darwin-tdep.c (i386_darwin_init_abi): Call set_solib_ops.
> >	(x86_darwin_init_abi_64): Ditto.
> >	* configure.tgt: Add solib.o solib-darwin.o for Darwin.

Generally speaking, this looks fine.  I have a few minor comments...

> >+/* Read dyld_all_image from inferior.  */
> >+static void
> >+darwin_load_image_infos (void)
> >+{
> >+  gdb_byte buf[24];

I'm always nervous when I see hard-coded constants like this in
buffer declarations? Would it make sense to use alloca? Or maybe
add an assertion that len <= sizeof (buf)?

> >+  len = 4 + 4 + 2 * ptr_type->length;

Can you explain the computation using little comments, maybe?

> >+/* Return non-zero if GDB_SO_NAME and INFERIOR_SO_NAME represent
> >+   the same shared library.  */
> >+
> >+static int
> >+darwin_same (struct so_list *gdb, struct so_list *inferior)
> >+{
> >+  return strcmp (gdb->so_original_name, inferior->so_original_name)  
> >== 0;
> >+}

I think that this function is not necessary. if so_ops.same is set to NULL,
then GDB falls back to using strcmp like you did... Perhaps we could add
a comment about that in solist.h, in fact.

> >+/* Lookup the value for a specific symbol.  */

                        of?


> >+static CORE_ADDR
> >+bfd_lookup_symbol (bfd *abfd, char *symname)

The name of this function annoys me a little. With GDB's current
conventions, it seems to suggest that this function is part of bfd.
Can we call is darwin_lookup_symbol or darwin_lookup_symbol_from_bfd?

> >+/* Return program interpreter string.  */
> >+static gdb_byte *
> >+find_program_interpreter (void)
> >+{
[...]
> >+  /* If we didn't find it, read from memory.
> >+     FIXME: todo.  */

Would it be complicated to do this now? I'm OK with looking at this
later, if you think it's easier.  I suppose this only really matter
in the "attach" case, right?

> >+/* Build a list of currently loaded shared objects.  See solib- 
> >svr4.c  */
> >+static struct so_list *
> >+darwin_current_sos (void)
> >+{
[...]
> >+  /* Read infos for each solib.  */
> >+  for (i = 0; i < dyld_all_image.count; i++)
> >+    {
> >+      CORE_ADDR info = dyld_all_image.info + i * image_info_size;
> >+      char buf[image_info_size];
> >+      CORE_ADDR load_addr;
[...]
> >+
> >+      /* Read image info from inferior.  */
> >+      if (target_read_memory (info, buf, image_info_size))
> >+	break;
> >+
> >+      load_addr = extract_typed_address (buf, ptr_type);
> >+      path_addr = extract_typed_address (buf + ptr_len, ptr_type);
> >+
> >+      target_read_string (path_addr, &file_path,
> >+			  SO_NAME_MAX_PATH_SIZE - 1, &errcode);
> >+      if (errcode)
> >+	break;
> >+
> >+      /* Ignore first entry as this is the executable itself.  */
> >+      if (i == 0)
> >+	continue;

Is there a reason for reading the info about the first entry at all?
Can we for instance start the loop with i = 1?

> >+  if (!inf->attach_flag)
> >+    {
> >+      /* We find the dynamic linker's base address by examining
> >+	 the current pc (which should point at the entry point for the
> >+	 dynamic linker) and subtracting the offset of the entry point.  */
> >+      load_addr = (read_pc () - bfd_get_start_address (dyld_bfd));
> >+    }
> >+  else
> >+    {
> >+      /* FIXME: todo.
> >+	 Get address of __DATA.__dyld in exec_bfd, read address at offset 0
> >+      */
> >+      xfree (interp_name);
> >+      return;
> >+    }

Can we implement this part as well? Same remark as above. OK to
push to a separate patch if it helps, but might as well if it's easy.

> >+extern initialize_file_ftype _initialize_svr4_solib; /* -Wmissing- 
> >prototypes */

Looks like an unused declaration. Unwanted copy/paste?  The
corresponding advance prototype for _initialize_darwin_solib
really isn't necessary - I think. We have lots of files that
don't provide this advance declaration.

-- 
Joel


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

* Re: Ping: [RFA] Add support of shared lib for Darwin
  2009-02-03  2:04   ` Joel Brobecker
@ 2009-02-03 11:41     ` Tristan Gingold
  0 siblings, 0 replies; 4+ messages in thread
From: Tristan Gingold @ 2009-02-03 11:41 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches


On Feb 3, 2009, at 3:04 AM, Joel Brobecker wrote:

> No review of this patch so far, so I took a look :)

Thank you for reviewing.

>>> +/* Read dyld_all_image from inferior.  */
>>> +static void
>>> +darwin_load_image_infos (void)
>>> +{
>>> +  gdb_byte buf[24];
>
> I'm always nervous when I see hard-coded constants like this in
> buffer declarations? Would it make sense to use alloca? Or maybe
> add an assertion that len <= sizeof (buf)?

I will add the assertion.

>>> +  len = 4 + 4 + 2 * ptr_type->length;
>
> Can you explain the computation using little comments, maybe?

Ok.

>>> +/* Return non-zero if GDB_SO_NAME and INFERIOR_SO_NAME represent
>>> +   the same shared library.  */
>>> +
>>> +static int
>>> +darwin_same (struct so_list *gdb, struct so_list *inferior)
>>> +{
>>> +  return strcmp (gdb->so_original_name, inferior->so_original_name)
>>> == 0;
>>> +}
>
> I think that this function is not necessary. if so_ops.same is set  
> to NULL,
> then GDB falls back to using strcmp like you did... Perhaps we could  
> add
> a comment about that in solist.h, in fact.

Ok.

>>> +/* Lookup the value for a specific symbol.  */
>
>                        of?
>
>
>>> +static CORE_ADDR
>>> +bfd_lookup_symbol (bfd *abfd, char *symname)
>
> The name of this function annoys me a little. With GDB's current
> conventions, it seems to suggest that this function is part of bfd.
> Can we call is darwin_lookup_symbol or darwin_lookup_symbol_from_bfd?

Ok, I changed it to lookup_symbol_from_bfd.

>>> +/* Return program interpreter string.  */
>>> +static gdb_byte *
>>> +find_program_interpreter (void)
>>> +{
> [...]
>>> +  /* If we didn't find it, read from memory.
>>> +     FIXME: todo.  */
>
> Would it be complicated to do this now? I'm OK with looking at this
> later, if you think it's easier.  I suppose this only really matter
> in the "attach" case, right?

I think this case won't happen once pid_to_exec_file is implemented  
(and I have an implementation for this
target op).

>>> +/* Build a list of currently loaded shared objects.  See solib-
>>> svr4.c  */
>>> +static struct so_list *
>>> +darwin_current_sos (void)
>>> +{
> [...]
>>> +  /* Read infos for each solib.  */
>>> +  for (i = 0; i < dyld_all_image.count; i++)
>>> +    {
>>> +      CORE_ADDR info = dyld_all_image.info + i * image_info_size;
>>> +      char buf[image_info_size];
>>> +      CORE_ADDR load_addr;
> [...]
>>> +
>>> +      /* Read image info from inferior.  */
>>> +      if (target_read_memory (info, buf, image_info_size))
>>> +	break;
>>> +
>>> +      load_addr = extract_typed_address (buf, ptr_type);
>>> +      path_addr = extract_typed_address (buf + ptr_len, ptr_type);
>>> +
>>> +      target_read_string (path_addr, &file_path,
>>> +			  SO_NAME_MAX_PATH_SIZE - 1, &errcode);
>>> +      if (errcode)
>>> +	break;
>>> +
>>> +      /* Ignore first entry as this is the executable itself.  */
>>> +      if (i == 0)
>>> +	continue;
>
> Is there a reason for reading the info about the first entry at all?
> Can we for instance start the loop with i = 1?

Yes.

>>> +  if (!inf->attach_flag)
>>> +    {
>>> +      /* We find the dynamic linker's base address by examining
>>> +	 the current pc (which should point at the entry point for the
>>> +	 dynamic linker) and subtracting the offset of the entry point.   
>>> */
>>> +      load_addr = (read_pc () - bfd_get_start_address (dyld_bfd));
>>> +    }
>>> +  else
>>> +    {
>>> +      /* FIXME: todo.
>>> +	 Get address of __DATA.__dyld in exec_bfd, read address at  
>>> offset 0
>>> +      */
>>> +      xfree (interp_name);
>>> +      return;
>>> +    }
>
> Can we implement this part as well? Same remark as above. OK to
> push to a separate patch if it helps, but might as well if it's easy.

Unfortunately this doesn't look very easy.  This method is not  
documented and I had to read the sources
of the dynamic loader to find it.  I really prefer to postpone the  
implementation.

>>> +extern initialize_file_ftype _initialize_svr4_solib; /* -Wmissing-
>>> prototypes */
>
> Looks like an unused declaration. Unwanted copy/paste?  The
> corresponding advance prototype for _initialize_darwin_solib
> really isn't necessary - I think. We have lots of files that
> don't provide this advance declaration.

Oops, this is a left over previous cleanups.

I plan to re-submit the patch soon.

Tristan.


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

end of thread, other threads:[~2009-02-03 11:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-08 14:09 [RFA] Add support of shared lib for Darwin Tristan Gingold
2009-01-23 15:54 ` Ping: " Tristan Gingold
2009-02-03  2:04   ` Joel Brobecker
2009-02-03 11:41     ` Tristan Gingold

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox