Re: [PATCH 1/2] don't share per-BFD data if relocations are needed

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> @@ -139,7 +139,10 @@ get_objfile_bfd_data (struct objfile *objfile, struct bfd *abfd)
>> 
>> if (storage == NULL)
>> {
>> -      if (abfd != NULL)
>> +      /* If the object requires gdb to do relocations, we simply fall
>> +	 back to not sharing data across users.  These cases are rare
>> +	 enough that this seems reasonable.  */
>> +      if (abfd != NULL && !gdb_bfd_requires_relocations (abfd))
>> {
>> storage = bfd_zalloc (abfd, sizeof (struct objfile_per_bfd_storage));
>> set_bfd_data (abfd, objfiles_bfd_data, storage);
>> 

Pedro> Shouldn't we still set storage->gdbarch to the bfd's gdbarch?

Pedro> Otherwise looks fine to me.

Yes, good catch.

I'm checking in the appended updated patch instead.

Tom

commit 1e907c9922ff15d72d301cc3aa3586f85dbfe6b0
Author: Tom Tromey <tromey@redhat.com>
Date:   Wed Aug 14 14:06:53 2013 -0600

    don't share per-BFD data if relocations are needed
    
    Right now we always share per-BFD data across objfiles, if there is a
    BFD.  This works fine.  However, we're going to start sharing more
    data, and sometimes this data will come directly from sections of the
    BFD.  If such a section has SEC_RELOC set, then the data coming from
    that section will not be truly sharable -- the section will be
    program-space-dependent, and re-read by gdb for each objfile.
    
    This patch disallows per-BFD sharing in this case.  This is a bit
    "heavy" in that we could in theory examine each bit of shared data for
    suitability.  However, that is more complicated, and SEC_RELOC is rare
    enough that I think we needn't bother.
    
    Note that the "no sharing" case is equivalent to "gdb works as it
    historically did".  That is, the sharing is a new(-ish) optimization.
    
    Built and regtested on x86-64 Fedora 18.
    
    	* gdb_bfd.c (struct gdb_bfd_data) <relocation_computed,
    	needs_relocations>: New fields.
    	(gdb_bfd_requires_relocations): New function.
    	* gdb_bfd.h (gdb_bfd_requires_relocations): Declare.
    	* objfiles.c (get_objfile_bfd_data): Disallow sharing if
    	the BFD needs relocations applied.

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 7ba120e..25cee5c 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -82,6 +82,13 @@ struct gdb_bfd_data
   /* The mtime of the BFD at the point the cache entry was made.  */
   time_t mtime;
 
+  /* This is true if we have determined whether this BFD has any
+     sections requiring relocation.  */
+  unsigned int relocation_computed : 1;
+
+  /* This is true if any section needs relocation.  */
+  unsigned int needs_relocations : 1;
+
   /* This is true if we have successfully computed the file's CRC.  */
   unsigned int crc_computed : 1;
 
@@ -634,6 +641,30 @@ gdb_bfd_count_sections (bfd *abfd)
   return bfd_count_sections (abfd) + 4;
 }
 
+/* See gdb_bfd.h.  */
+
+int
+gdb_bfd_requires_relocations (bfd *abfd)
+{
+  struct gdb_bfd_data *gdata = bfd_usrdata (abfd);
+
+  if (gdata->relocation_computed == 0)
+    {
+      asection *sect;
+
+      for (sect = abfd->sections; sect != NULL; sect = sect->next)
+	if ((sect->flags & SEC_RELOC) != 0)
+	  {
+	    gdata->needs_relocations = 1;
+	    break;
+	  }
+
+      gdata->relocation_computed = 1;
+    }
+
+  return gdata->needs_relocations;
+}
+
 
 
 /* A callback for htab_traverse that prints a single BFD.  */
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index ca2eddc..d28b29e 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -135,4 +135,9 @@ int gdb_bfd_section_index (bfd *abfd, asection *section);
 
 int gdb_bfd_count_sections (bfd *abfd);
 
+/* Return true if any section requires relocations, false
+   otherwise.  */
+
+int gdb_bfd_requires_relocations (bfd *abfd);
+
 #endif /* GDB_BFD_H */
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index b10f803..b9bcfd7 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -137,18 +137,22 @@ get_objfile_bfd_data (struct objfile *objfile, struct bfd *abfd)
 
   if (storage == NULL)
     {
-      if (abfd != NULL)
+      /* If the object requires gdb to do relocations, we simply fall
+	 back to not sharing data across users.  These cases are rare
+	 enough that this seems reasonable.  */
+      if (abfd != NULL && !gdb_bfd_requires_relocations (abfd))
 	{
 	  storage = bfd_zalloc (abfd, sizeof (struct objfile_per_bfd_storage));
 	  set_bfd_data (abfd, objfiles_bfd_data, storage);
-
-	  /* Look up the gdbarch associated with the BFD.  */
-	  storage->gdbarch = gdbarch_from_bfd (abfd);
 	}
       else
 	storage = OBSTACK_ZALLOC (&objfile->objfile_obstack,
 				  struct objfile_per_bfd_storage);
 
+      /* Look up the gdbarch associated with the BFD.  */
+      if (abfd != NULL)
+	storage->gdbarch = gdbarch_from_bfd (abfd);
+
       obstack_init (&storage->storage_obstack);
       storage->filename_cache = bcache_xmalloc (NULL, NULL);
       storage->macro_cache = bcache_xmalloc (NULL, NULL);