This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Decompress section before applying simple relocations


This interim fix simply kills direct use of the caller's buffer.  Yes,
it's inefficient to not write directly, but this is safe and avoids
the decompression code having tentacles everywhere.

A proper solution would be to rewrite this code to not cache anything.
I say that because bfd_get_full_section_contents is a replacement for
bfd_get_section_contents at some call sites, so it ought to behave
similarly.  bfd_get_section_contents just reads from disk, with
caching done by its callers.  So the decompression code ought to do
the same, even so far as to decompress each time the function is
called.  This would likely require fixing some bugs in code that set
rawsize, eg. merge.c, but I think you then could do without
section->compressed_size.

	* compress.c (decompress_contents): Style.
	(bfd_get_full_section_contents): Do not decompress directly into
	caller buffer or directly return cached section contents.
	Check malloc return for compressed_buffer.

note diff -w to better show changes.

Index: bfd/compress.c
===================================================================
RCS file: /cvs/src/src/bfd/compress.c,v
retrieving revision 1.6
diff -u -p -w -r1.6 compress.c
--- bfd/compress.c	29 Oct 2010 19:56:00 -0000	1.6
+++ bfd/compress.c	23 Dec 2010 12:05:08 -0000
@@ -59,7 +59,7 @@ decompress_contents (bfd_byte *compresse
       rc = inflateReset (&strm);
     }
   rc = inflateEnd (&strm);
-  return rc != Z_OK || strm.avail_out != 0 ? FALSE: TRUE;
+  return rc == Z_OK && strm.avail_out == 0;
 }
 #endif
 
@@ -157,8 +157,8 @@ bfd_get_full_section_contents (bfd *abfd
 {
   bfd_size_type sz = sec->rawsize ? sec->rawsize : sec->size;
   bfd_byte *p = *ptr;
-  bfd_boolean need_free, ret;
 #ifdef HAVE_ZLIB_H
+  bfd_boolean ret;
   bfd_size_type compressed_size;
   bfd_size_type uncompressed_size;
   bfd_size_type rawsize;
@@ -177,30 +177,17 @@ bfd_get_full_section_contents (bfd *abfd
 	  p = (bfd_byte *) bfd_malloc (sz);
 	  if (p == NULL)
 	    return FALSE;
-	  need_free = TRUE;
-	  *ptr = p;
 	}
-      else
-	need_free = FALSE;
-      ret = bfd_get_section_contents (abfd, sec, p, 0, sz);
-      if (!ret && need_free)
+      if (!bfd_get_section_contents (abfd, sec, p, 0, sz))
+	{
+	  if (*ptr != p)
 	free (p);
-      return ret;
-
-    case COMPRESS_SECTION_DONE:
-      if (p)
-	memcpy (p, sec->contents, sz);
-      else
-	*ptr = sec->contents;
+	  return FALSE;
+	}
+      *ptr = p;
       return TRUE;
 
     case DECOMPRESS_SECTION_SIZED:
-      break;
-
-    default:
-      abort ();
-    }
-
 #ifndef HAVE_ZLIB_H
   bfd_set_error (bfd_error_invalid_operation);
   return FALSE;
@@ -209,6 +196,8 @@ bfd_get_full_section_contents (bfd *abfd
   uncompressed_size = sec->size;
   compressed_size = sec->compressed_size;
   compressed_buffer = (bfd_byte *) bfd_malloc (compressed_size);
+      if (compressed_buffer == NULL)
+	return FALSE;
   rawsize = sec->rawsize;
   /* Clear rawsize, set size to compressed size and set compress_status
      to COMPRESS_SECTION_NONE.  If the compressed size is bigger than
@@ -221,44 +210,44 @@ bfd_get_full_section_contents (bfd *abfd
   /* Restore rawsize and size.  */
   sec->rawsize = rawsize;
   sec->size = uncompressed_size;
-  if (!ret)
-    {
-fail_compressed:
       sec->compress_status = DECOMPRESS_SECTION_SIZED;
-      free (compressed_buffer);
-      return ret;
-    }
+      if (!ret)
+	goto fail_compressed;
 
-  /* Decompress to caller buffer directly if it is provided. */
-  if (p)
-    uncompressed_buffer = p;
-  else
-    {
       uncompressed_buffer = (bfd_byte *) bfd_malloc (uncompressed_size);
       if (uncompressed_buffer == NULL)
 	goto fail_compressed;
-    }
 
   if (!decompress_contents (compressed_buffer, compressed_size,
 			    uncompressed_buffer, uncompressed_size))
     {
-      sec->compress_status = DECOMPRESS_SECTION_SIZED;
-      free (compressed_buffer);
-      if (p == NULL)
-	free (uncompressed_buffer);
       bfd_set_error (bfd_error_bad_value);
+	  free (uncompressed_buffer);
+	fail_compressed:
+	  free (compressed_buffer);
       return FALSE;
     }
 
   free (compressed_buffer);
-  if (p == NULL)
-    *ptr = uncompressed_buffer;
-
   sec->contents = uncompressed_buffer;
   sec->compress_status = COMPRESS_SECTION_DONE;
+      /* Fall thru */
+#endif
 
+    case COMPRESS_SECTION_DONE:
+      if (p == NULL)
+	{
+	  p = (bfd_byte *) bfd_malloc (sz);
+	  if (p == NULL)
+	    return FALSE;
+	  *ptr = p;
+	}
+      memcpy (p, sec->contents, sz);
   return TRUE;
-#endif
+
+    default:
+      abort ();
+    }
 }
 
 /*

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]