Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c)
@ 2008-11-14 18:33 Tristan Gingold
  2008-11-18 17:20 ` Stan Shebs
  0 siblings, 1 reply; 7+ messages in thread
From: Tristan Gingold @ 2008-11-14 18:33 UTC (permalink / raw)
  To: gdb-patches


/* Darwin support for GDB, the GNU debugger.
    Copyright (C) 2008 Free Software Foundation, Inc.

    Contributed by AdaCore.

    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 "gdbtypes.h"
#include "bfd.h"
#include "symfile.h"
#include "objfiles.h"
#include "buildsym.h"
#include "gdbcmd.h"
#include "gdbcore.h"
#include "mach-o.h"
#include "gdb_assert.h"
#include "aout/stab_gnu.h"
#include "vec.h"

#include <string.h>

static int mach_o_debug_level = 0;

static void
macho_new_init (struct objfile *objfile)
{
}

static void
macho_symfile_init (struct objfile *objfile)
{
   objfile->flags |= OBJF_REORDERED;
   init_entry_point_info (objfile);
}

typedef struct oso_el
{
   /* Object file name.  */
   const char *name;

   unsigned long mtime;

   int num_sections;

   asymbol **symbols;
   bfd_vma *offsets;
}
oso_el;

DEF_VEC_O(oso_el);
static VEC(oso_el) *oso_vector;

static void
macho_add_oso (const asymbol *oso_sym, int nbr_sections,
	       asymbol **symbols, bfd_vma *offsets)
{
   oso_el el;

   el.name = oso_sym->name;
   el.mtime = oso_sym->value;
   el.num_sections = nbr_sections;
   el.symbols = symbols;
   el.offsets = offsets;
   VEC_safe_push(oso_el, oso_vector, &el);
}

static void
macho_symtab_read (struct objfile *objfile,
		   long number_of_symbols, asymbol **symbol_table)
{
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   long storage_needed;
   asymbol *sym;
   long i, j;
   CORE_ADDR offset;
   enum minimal_symbol_type ms_type;
   unsigned int nbr_sections = bfd_count_sections (objfile->obfd);
   asymbol **first_symbol = NULL;
   bfd_vma *first_offset = NULL;
   const asymbol *oso_file = NULL;

   for (i = 0; i < number_of_symbols; i++)
     {
       sym = symbol_table[i];
       offset = ANOFFSET (objfile->section_offsets, sym->section- 
 >index);

       if (sym->flags & BSF_DEBUGGING)
	{
	  unsigned char type = BFD_MACH_O_SYM_NTYPE(sym);
	  bfd_vma addr;

	  switch (type)
	    {
	    case N_SO:
	      if ((sym->name == NULL || sym->name[0] == 0)
		  && oso_file != NULL)
		{
		  macho_add_oso (oso_file, nbr_sections,
				 first_symbol, first_offset);
		  first_symbol = NULL;
		  first_offset = NULL;
		  oso_file = NULL;
		}
	      break;
	    case N_FUN:
	    case N_STSYM:
	      if (sym->name == NULL || sym->name[0] == '\0')
		break;
	      /* Fall through.  */
	    case N_BNSYM:
	      gdb_assert (oso_file != NULL);
	      addr = sym->value
		+ bfd_get_section_vma (sym->section->bfd, sym->section);
	      if (addr != 0
		  && first_symbol[sym->section->index] == NULL)
		{
		  first_symbol[sym->section->index] = sym;
		  first_offset[sym->section->index] = addr + offset;
		}
	      break;
	    case N_GSYM:
	      gdb_assert (oso_file != NULL);
	      if (first_symbol[sym->section->index] == NULL)
		first_symbol[sym->section->index] = sym;
	      break;
	    case N_OSO:
	      gdb_assert (oso_file == NULL);
	      first_symbol = (asymbol **)xmalloc (nbr_sections
						  * sizeof (asymbol *));
	      first_offset = (bfd_vma *)xmalloc (nbr_sections
						 * sizeof (bfd_vma));
	      for (j = 0; j < nbr_sections; j++)
		first_symbol[j] = NULL;
	      oso_file = sym;
	      break;
	    }
	  continue;
	}

       if (sym->name == NULL || *sym->name == '\0')
	{
	  /* Skip names that don't exist (shouldn't happen), or names
	     that are null strings (may happen). */
	  continue;
	}

       if (sym->flags & (BSF_GLOBAL | BSF_LOCAL | BSF_WEAK))
	{
	  struct minimal_symbol *msym;
	  CORE_ADDR symaddr;

	  /* Bfd symbols are section relative. */
	  symaddr = sym->value + sym->section->vma;

	  /* Select global/local/weak symbols.  Note that bfd puts abs
	     symbols in their own section, so all symbols we are
	     interested in will have a section. */
	  /* Relocate all non-absolute and non-TLS symbols by the
	     section offset.  */
	  if (sym->section != &bfd_abs_section
	      && !(sym->section->flags & SEC_THREAD_LOCAL))
	    symaddr += offset;

	  if (sym->section == &bfd_abs_section)
	    ms_type = mst_abs;
	  else if (sym->section->flags & SEC_CODE)
	    {
	      if (sym->flags & (BSF_GLOBAL | BSF_WEAK))
		ms_type = mst_text;
	      else
		ms_type = mst_file_text;
	    }
	  else if (sym->section->flags & SEC_ALLOC)
	    {
	      if (sym->flags & (BSF_GLOBAL | BSF_WEAK))
		{
		  if (sym->section->flags & SEC_LOAD)
		    ms_type = mst_data;
		  else
		    ms_type = mst_bss;
		}
	      else if (sym->flags & BSF_LOCAL)
		{
		  /* Not a special stabs-in-elf symbol, do regular
		     symbol processing.  */
		  if (sym->section->flags & SEC_LOAD)
		    ms_type = mst_file_data;
		  else
		    ms_type = mst_file_bss;
		}
	      else
		ms_type = mst_unknown;
	    }
	  else
	    continue;	/* Skip this symbol. */

	  gdb_assert (sym->section->index < nbr_sections);
	  if (oso_file != NULL
	      && first_symbol[sym->section->index] == NULL)
	    {
	      first_symbol[sym->section->index] = sym;
	      first_offset[sym->section->index] = symaddr;
	    }

	  msym = prim_record_minimal_symbol_and_info
	    (sym->name, symaddr, ms_type, sym->section->index,
	     sym->section, objfile);
	}
     }

   if (oso_file != NULL)
     macho_add_oso (oso_file, nbr_sections, first_symbol, first_offset);
}

/* If NAME describes an archive member (ie: ARCHIVE '(' MEMBER ')'),
    returns the length of the archive name.
    Returns -1 otherwise.  */
static int
get_archive_prefix_len (const char *name)
{
   char *lparen;
   int name_len = strlen (name);

   if (name_len == 0 || name[name_len - 1] != ')')
     return -1;

   lparen = strrchr (name, '(');
   if (lparen == NULL || lparen == name)
     return -1;
   return lparen - name;
}

static void
macho_oso_symfile (struct objfile *main_objfile)
{
   int ix;
   VEC(oso_el) *vec;
   oso_el *oso;
   char leading_char;

   /* TODO: Sort them, group library search.  */

   vec = oso_vector;
   oso_vector = NULL;

   leading_char = bfd_get_symbol_leading_char (main_objfile->obfd);

   for (ix = 0; VEC_iterate(oso_el, vec, ix, oso); ix++)
     {
       struct section_addr_info *addrs;
       int pfx_len;
       int len;
       int i;
       oso_el el;

       if (mach_o_debug_level > 0)
	printf_unfiltered (_("Loading symbols from oso: %s\n"), oso->name);

       /* Compute addr length.  */
       len = 0;
       for (i = 0; i < oso->num_sections; i++)
	if (oso->symbols[i] != NULL)
	  len++;

       addrs = alloc_section_addr_info (len);

       len = 0;
       for (i = 0; i < oso->num_sections; i++)
	if (oso->symbols[i] != NULL)
	  {
	    if (oso->offsets[i])
	      addrs->other[len].addr = oso->offsets[i];
	    else
	      {
		struct minimal_symbol *msym;
		const char *name = oso->symbols[i]->name;

		if (name[0] == leading_char)
		  ++name;

		if (mach_o_debug_level > 3)
		  printf_unfiltered (_("resolv sec %s with %s\n"),
				     oso->symbols[i]->section->name,
				     oso->symbols[i]->name);
		msym = lookup_minimal_symbol (name, NULL, main_objfile);
		if (msym == NULL)
		  {
		    warning (_("can't find symbol '%s' in minsymtab"),
			     oso->symbols[i]->name);
		    addrs->other[len].addr = 0;
		  }
		else
		  addrs->other[len].addr = SYMBOL_VALUE_ADDRESS (msym);
	      }
	    addrs->other[len].name = (char *)oso->symbols[i]->section->name;
	    len++;
	  }

       if (mach_o_debug_level > 1)
	{
	  int j;
	  for (j = 0; j < addrs->num_sections; j++)
	    printf_unfiltered
	      (_("  %s: %s\n"),
	       core_addr_to_string (addrs->other[j].addr),
	       addrs->other[j].name);
	}

       /* Check if this is a library name.  */
       pfx_len = get_archive_prefix_len (oso->name);
       if (pfx_len > 0)
	{
	  bfd *archive_bfd;
	  bfd *member_bfd;
	  char *archive_name = (char *) alloca (pfx_len + 1);
	  int member_len;

	  member_len = strlen (oso->name + pfx_len + 1) - 1;
	  memcpy (archive_name, oso->name, pfx_len);
	  archive_name[pfx_len] = '\0';

	  /* Open the archive and check the format.  */
	  archive_bfd = bfd_openr (archive_name, gnutarget);
	  if (archive_bfd == NULL)
	    {
	      warning (_("Could not open OSO archive file \"%s\""),
		       archive_name);
	      continue;
	    }
	  if (!bfd_check_format (archive_bfd, bfd_archive))
	    {
	      warning (_("OSO archive file \"%s\" not an archive."),
		       archive_name);
	      bfd_close (archive_bfd);
	      continue;
	    }
	  member_bfd = bfd_openr_next_archived_file (archive_bfd, NULL);
	
	  if (member_bfd == NULL)
	    {
	      warning (_("Could not read archive members out of "
			 "OSO archive \"%s\""), archive_name);
	      bfd_close (archive_bfd);
	      continue;
	    }
	
	  while (member_bfd != NULL)
	    {
	      bfd *prev = member_bfd;
	      const char *member_name = member_bfd->filename;
	      if (strlen (member_name) == member_len
		  && !memcmp (member_name, oso->name + pfx_len + 1, member_len))
		break;
	      member_bfd = bfd_openr_next_archived_file
		(archive_bfd, member_bfd);
	      bfd_close (prev);
	    }
	  if (member_bfd == NULL)
	    {
	      warning (_("Could not find specified archive member "
			 "for OSO name \"%s\""), oso->name);
	      bfd_close (archive_bfd);
	      continue;
	    }
	
	  bfd_set_cacheable (member_bfd, 1);

	  if (!bfd_check_format (member_bfd, bfd_object))
	    {
	      warning (_("`%s': can't read symbols: %s."), oso->name,
		       bfd_errmsg (bfd_get_error ()));
	      bfd_close (member_bfd);
	    }
	    else
	      symbol_file_add_from_bfd (member_bfd, 0, addrs, 0, 0);
	}
       else
	{
	  bfd *abfd;

	  abfd = bfd_openr (oso->name, gnutarget);
	  if (!abfd)
	    {
	      warning (_("`%s': can't open to read symbols: %s."), oso->name,
		       bfd_errmsg (bfd_get_error ()));
	      continue;
	    }
	  bfd_set_cacheable (abfd, 1);

	  if (!bfd_check_format (abfd, bfd_object))
	    {
	      bfd_close (abfd);
	      warning (_("`%s': can't read symbols: %s."), oso->name,
		       bfd_errmsg (bfd_get_error ()));
	      continue;
	    }

	  symbol_file_add_from_bfd (abfd, 0, addrs, 0, 0);
	}
       xfree (oso->symbols);
       xfree (oso->offsets);
     }
   VEC_free(oso_el, vec);
}

#define DSYM_SUFFIX ".dSYM/Contents/Resources/DWARF/"

static bfd *
macho_check_dsym (struct objfile *objfile)
{
   size_t name_len = strlen (objfile->name);
   size_t dsym_len = strlen (DSYM_SUFFIX);
   const char *base_name = lbasename (objfile->name);
   size_t base_len = strlen (base_name);
   char *dsym_filename = alloca (name_len + dsym_len + base_len + 1);
   bfd *dsym_bfd;
   asection *sect;
   bfd_byte main_uuid[16];
   bfd_byte dsym_uuid[16];

   strcpy (dsym_filename, objfile->name);
   strcpy (dsym_filename + name_len, DSYM_SUFFIX);
   strcpy (dsym_filename + name_len + dsym_len, base_name);

   if (access (dsym_filename, R_OK) != 0)
     return NULL;

   sect = bfd_get_section_by_name (objfile->obfd, "LC_UUID");
   if (sect == NULL)
     {
       warning (_("can't find UUID in %s"), objfile->name);
       return NULL;
     }
   if (!bfd_get_section_contents (objfile->obfd, sect, main_uuid,
				 0, sizeof (main_uuid)))
     {
       warning (_("can't read UUID in %s"), objfile->name);
       return NULL;
     }

   dsym_filename = xstrdup (dsym_filename);
   dsym_bfd = bfd_openr (dsym_filename, gnutarget);
   if (dsym_bfd == NULL)
     {
       warning (_("can't open dsym file %s"), dsym_filename);
       xfree (dsym_filename);
       return NULL;
     }

   if (!bfd_check_format (dsym_bfd, bfd_object))
     {
       bfd_close (dsym_bfd);
       warning (_("bad dsym file format: %s"), bfd_errmsg  
(bfd_get_error ()));
       xfree (dsym_filename);
       return NULL;
     }

   sect = bfd_get_section_by_name (dsym_bfd, "LC_UUID");
   if (sect == NULL)
     {
       warning (_("can't find UUID in %s"), dsym_filename);
       bfd_close (dsym_bfd);
       xfree (dsym_filename);
       return NULL;
     }
   if (!bfd_get_section_contents (dsym_bfd, sect, dsym_uuid,
				 0, sizeof (dsym_uuid)))
     {
       warning (_("can't read UUID in %s"), dsym_filename);
       bfd_close (dsym_bfd);
       xfree (dsym_filename);
       return NULL;
     }
   if (memcmp (dsym_uuid, main_uuid, sizeof (main_uuid)))
     {
       warning (_("dsym file UUID doesn't match the one in %s"),  
objfile->name);
       bfd_close (dsym_bfd);
       xfree (dsym_filename);
       return NULL;
     }
   return dsym_bfd;

}

static void
macho_symfile_read (struct objfile *objfile, int mainline)
{
   bfd *abfd = objfile->obfd;
   struct cleanup *back_to;
   CORE_ADDR offset;
   long storage_needed;
   bfd *dsym_bfd;

   init_minimal_symbol_collection ();
   back_to = make_cleanup_discard_minimal_symbols ();

   /* 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)
     {
       /* Process the normal symbol table first.  */
       storage_needed = bfd_get_symtab_upper_bound (objfile->obfd);
       if (storage_needed < 0)
	error (_("Can't read symbols from %s: %s"),
	       bfd_get_filename (objfile->obfd),
	       bfd_errmsg (bfd_get_error ()));

       if (storage_needed > 0)
	{
	  asymbol **symbol_table;
	  long symcount;

	  symbol_table = (asymbol **) xmalloc (storage_needed);
	  make_cleanup (xfree, symbol_table);
	  symcount = bfd_canonicalize_symtab (objfile->obfd, symbol_table);
	
	  if (symcount < 0)
	    error (_("Can't read symbols from %s: %s"),
		   bfd_get_filename (objfile->obfd),
		   bfd_errmsg (bfd_get_error ()));
	
	  macho_symtab_read (objfile, symcount, symbol_table);
	}

       install_minimal_symbols (objfile);

       /* Check for dsym file.  */
       dsym_bfd = macho_check_dsym (objfile);
       if (dsym_bfd != NULL)
	{
	  int ix;
	  oso_el *oso;

	  if (mach_o_debug_level > 0)
	    printf_unfiltered (_("dsym file found\n"));

	  /* Remove oso.  They won't be used.  */
	  for (ix = 0; VEC_iterate(oso_el, oso_vector, ix, oso); ix++)
	    {
	      xfree (oso->symbols);
	      xfree (oso->offsets);
	    }
	  VEC_free(oso_el, oso_vector);
	  oso_vector = NULL;

	  /* 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.  */
	  return;
	}
     }

   if (dwarf2_has_info (objfile))
     {
       /* DWARF 2 sections */
       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);

   /* Then the oso.  */
   if (oso_vector != NULL)
     macho_oso_symfile (objfile);
}

static void
macho_symfile_finish (struct objfile *objfile)
{
}

static void
macho_symfile_offsets (struct objfile *objfile,
                        struct section_addr_info *addrs)
{
   unsigned int i;
   unsigned int num_sections;
   struct obj_section *osect;

   /* Allocate section_offsets.  */
   objfile->num_sections = bfd_count_sections (objfile->obfd);
   objfile->section_offsets = (struct section_offsets *)
     obstack_alloc (&objfile->objfile_obstack,
                    SIZEOF_N_SECTION_OFFSETS (objfile->num_sections));
   memset (objfile->section_offsets, 0,
           SIZEOF_N_SECTION_OFFSETS (objfile->num_sections));

   /* This code is run when we first add the objfile with
      symfile_add_with_addrs_or_offsets, when "addrs" not "offsets" are
      passed in.  The place in symfile.c where the addrs are applied
      depends on the addrs having section names.  But in the dyld code
      we build an anonymous array of addrs, so that code is a no-op.
      Because of that, we have to apply the addrs to the sections here.
      N.B. if an objfile slides after we've already created it, then it
      goes through objfile_relocate.  */

   for (i = 0; i < addrs->num_sections; i++)
     {
       if (addrs->other[i].name == NULL)
	continue;

       ALL_OBJFILE_OSECTIONS (objfile, osect)
	{
	  const char *bfd_sect_name = osect->the_bfd_section->name;

	  if (strcmp (bfd_sect_name, addrs->other[i].name) == 0)
	    {
	      obj_section_offset (osect) = addrs->other[i].addr;
	      break;
	    }
	}
     }

   objfile->sect_index_text = 0;

   ALL_OBJFILE_OSECTIONS (objfile, osect)
     {
       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)
	objfile->sect_index_text = sect_index;
     }
}

static struct sym_fns macho_sym_fns = {
   bfd_target_mach_o_flavour,

   macho_new_init,               /* sym_new_init: init anything gbl to  
entire symtab */
   macho_symfile_init,           /* sym_init: read initial info, setup  
for sym_read() */
   macho_symfile_read,          /* sym_read: read a symbol file into  
symtab */
   macho_symfile_finish,         /* sym_finish: finished with file,  
cleanup */
   macho_symfile_offsets,        /* sym_offsets:  xlate external to  
internal form */
   NULL                          /* next: pointer to next struct  
sym_fns */
};

void
_initialize_machoread ()
{
   add_symtab_fns (&macho_sym_fns);

   add_setshow_zinteger_cmd ("mach-o", class_obscure,
			    &mach_o_debug_level, _("\
Set if printing mach-o symbols processing."), _("\
Show if printing mach-o symbols processing."), NULL,
			    NULL, NULL,
			    &setdebuglist, &showdebuglist);
}


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

* Re: [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c)
  2008-11-14 18:33 [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c) Tristan Gingold
@ 2008-11-18 17:20 ` Stan Shebs
  2008-11-18 19:54   ` Daniel Jacobowitz
  2008-11-19 21:21   ` Tristan Gingold
  0 siblings, 2 replies; 7+ messages in thread
From: Stan Shebs @ 2008-11-18 17:20 UTC (permalink / raw)
  To: Tristan Gingold; +Cc: gdb-patches

Tristan Gingold wrote:
>
> typedef struct oso_el
> {
Something about what this struct is for, please? And while you and I 
know that "oso" is short for "other source" and refers to the collection 
of debug info from .o files, it's new to everybody else. :-) (Getting 
function bounds set correctly is going to be loads of fun, ahem, I had 
to look at Apple's hairy code for this several months ago...)
> DEF_VEC_O(oso_el);
> static VEC(oso_el) *oso_vector;
The vector macros should follow the "space before paren" just as 
everybody else does, although I note that the doc in vec.h confuses 
things by not following the general rule.
> #define DSYM_SUFFIX ".dSYM/Contents/Resources/DWARF/"  
Again, need to say what dSYM files are, and maybe mention that the yucky 
wired-in string is guaranteed to have that form (or at least that we're 
doing the same thing as Apple's GDB).

In comments, we should prefer to say "Mach-O" rather than "macho" or 
"mach-o", since that is the proper name of the file format.

With these doc and formatting changes, machoread.c is OK to go into the 
trunk.

(Ideally we will migrate this file to the list of generic files, but we 
will need to check that it compiles on other hosts first.)

Stan


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

* Re: [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c)
  2008-11-18 17:20 ` Stan Shebs
@ 2008-11-18 19:54   ` Daniel Jacobowitz
  2008-11-18 21:46     ` Stan Shebs
  2008-11-19 21:21   ` Tristan Gingold
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Jacobowitz @ 2008-11-18 19:54 UTC (permalink / raw)
  To: Stan Shebs; +Cc: Tristan Gingold, gdb-patches

On Mon, Nov 17, 2008 at 05:31:21PM -0800, Stan Shebs wrote:
>> DEF_VEC_O(oso_el);
>> static VEC(oso_el) *oso_vector;
> The vector macros should follow the "space before paren" just as  
> everybody else does, although I note that the doc in vec.h confuses  
> things by not following the general rule.

FWIW, I decided to skip this space when writing other code using VEC.
I put it in after VEC_index, but not VEC.  It made things much simpler
to read.  But it's not strictly proper, so either way is fine with me.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c)
  2008-11-18 19:54   ` Daniel Jacobowitz
@ 2008-11-18 21:46     ` Stan Shebs
  2008-11-18 21:50       ` Daniel Jacobowitz
  0 siblings, 1 reply; 7+ messages in thread
From: Stan Shebs @ 2008-11-18 21:46 UTC (permalink / raw)
  To: Stan Shebs, Tristan Gingold, gdb-patches

Daniel Jacobowitz wrote:
> On Mon, Nov 17, 2008 at 05:31:21PM -0800, Stan Shebs wrote:
>   
>>> DEF_VEC_O(oso_el);
>>> static VEC(oso_el) *oso_vector;
>>>       
>> The vector macros should follow the "space before paren" just as  
>> everybody else does, although I note that the doc in vec.h confuses  
>> things by not following the general rule.
>>     
>
> FWIW, I decided to skip this space when writing other code using VEC.
> I put it in after VEC_index, but not VEC.  It made things much simpler
> to read.  But it's not strictly proper, so either way is fine with me.
>   
The whole space-or-no-space issue is so pedantic I can barely type this 
message :-) , but the reality is that if you complicate the rules to 
allow some exceptions, it opens the door to interminable debates over 
whether this or that construct is exception-worthy. We minimize the time 
we spend thinking about this by just having the one simple rule that 
mechanically applies to everything.

Stan


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

* Re: [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c)
  2008-11-18 21:46     ` Stan Shebs
@ 2008-11-18 21:50       ` Daniel Jacobowitz
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Jacobowitz @ 2008-11-18 21:50 UTC (permalink / raw)
  To: Stan Shebs; +Cc: Tristan Gingold, gdb-patches

On Tue, Nov 18, 2008 at 10:47:22AM -0800, Stan Shebs wrote:
> The whole space-or-no-space issue is so pedantic I can barely type this  
> message :-) , but the reality is that if you complicate the rules to  
> allow some exceptions, it opens the door to interminable debates over  
> whether this or that construct is exception-worthy. We minimize the time  
> we spend thinking about this by just having the one simple rule that  
> mechanically applies to everything.

The coding standards are practically a set of codified exceptions
anyway :-)  Like I said, whichever others prefer - I found this
style more readable at the time but it may have been the wrong thing
to do.

-- 
Daniel Jacobowitz
CodeSourcery


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

* Re: [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c)
  2008-11-18 17:20 ` Stan Shebs
  2008-11-18 19:54   ` Daniel Jacobowitz
@ 2008-11-19 21:21   ` Tristan Gingold
  2008-11-26 16:30     ` Stan Shebs
  1 sibling, 1 reply; 7+ messages in thread
From: Tristan Gingold @ 2008-11-19 21:21 UTC (permalink / raw)
  To: Stan Shebs; +Cc: gdb-patches


On Nov 18, 2008, at 2:31 AM, Stan Shebs wrote:

> Tristan Gingold wrote:
>>
>> typedef struct oso_el
>> {
> Something about what this struct is for, please?

Comments added.

> And while you and I know that "oso" is short for "other source" and  
> refers to the collection of debug info from .o files, it's new to  
> everybody else. :-)

I thought it means "object source".  Anyway, I added comments.

> (Getting function bounds set correctly is going to be loads of fun,  
> ahem, I had to look at Apple's hairy code for this several months  
> ago...)

Yes, there is work to do in this area!

>> DEF_VEC_O(oso_el);
>> static VEC(oso_el) *oso_vector;
> The vector macros should follow the "space before paren" just as  
> everybody else does, although I note that the doc in vec.h confuses  
> things by not following the general rule.

I added a space.

>> #define DSYM_SUFFIX ".dSYM/Contents/Resources/DWARF/"
> Again, need to say what dSYM files are, and maybe mention that the  
> yucky wired-in string is guaranteed to have that form (or at least  
> that we're doing the same thing as Apple's GDB).

Done.

> In comments, we should prefer to say "Mach-O" rather than "macho" or  
> "mach-o", since that is the proper name of the file format.

I think I fixed all.  'macho' is still used in functions name.

> With these doc and formatting changes, machoread.c is OK to go into  
> the trunk.

Thanks.

> (Ideally we will migrate this file to the list of generic files, but  
> we will need to check that it compiles on other hosts first.)

Tristan.


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

* Re: [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c)
  2008-11-19 21:21   ` Tristan Gingold
@ 2008-11-26 16:30     ` Stan Shebs
  0 siblings, 0 replies; 7+ messages in thread
From: Stan Shebs @ 2008-11-26 16:30 UTC (permalink / raw)
  To: Tristan Gingold; +Cc: Stan Shebs, gdb-patches

Tristan Gingold wrote:
>> And while you and I know that "oso" is short for "other source" and 
>> refers to the collection of debug info from .o files, it's new to 
>> everybody else. :-)
>
> I thought it means "object source".  Anyway, I added comments.
Hmm, you could be right, maybe I'm misremembering!

Stan


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

end of thread, other threads:[~2008-11-25 18:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-14 18:33 [RFA] Darwin/x86 port (v4 - part 1/4: machoread.c) Tristan Gingold
2008-11-18 17:20 ` Stan Shebs
2008-11-18 19:54   ` Daniel Jacobowitz
2008-11-18 21:46     ` Stan Shebs
2008-11-18 21:50       ` Daniel Jacobowitz
2008-11-19 21:21   ` Tristan Gingold
2008-11-26 16:30     ` Stan Shebs

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