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 1/2] gold: extend Output_file to support read/write access to existing files.


Mikolaj Zalewski <mikolajz@google.com> writes:

> Added options to open file without removing its contents and made
> map_anonymous behave similarly to other *map* methods, e.g. to modify
> this->base_ on success.
>
> 2009-09-01  Mikolaj Zalewski  <mikolajz@google.com>
>
> 	* output.cc (Output_file::Output_file): Initialize
> map_include_original_content_ .
> 	(Output_file::open_for_modification): New method.
> 	(Output_file::try_map_anonymous): Renamed from
> Output_file::map_anonymous, changed behavior to similar to other
> *map*.
> 	(Output_file::map): Don't try anonymous mapping it
> this->map_include_original_content_.
> 	(Output_file::try_map_no_anonymous): New method (most code from
> Output_file::map).
> 	* output.h (Output_file::open_for_modification): New method.
> 	(Output_file::open): Update comment.
> 	(Output_file::resize): Update comment.
> 	(Output_file::close): Update comment.
> 	(Output_file::map): Update comment.
> 	(Output_file::try_map_anonymous): Renamed from Output_file::map_anonymous.
> 	(Output_file::try_map_no_anonymous): New method.
> 	(Output_file::map_include_original_content_): New field.

Thanks.  I made a bunch of minor changes to this patch.

1) I didn't like the names "try_map_anonymous" and
   "try_map_no_anonymous".  The functions don't try to do maps: they
   actually do maps.

2) I didn't see any reason to include the map_include_original_content_
   field.

3) GNU formatting puts the open brace on a line by itself.

I included the other patch mostly as is.

This is the patch I committed.

Thanks.

Ian


2009-09-01  Mikolaj Zalewski  <mikolajz@google.com>

	* gold.cc: Include "incremental.h".
	(queue_initial_tasks): Call Incremental_checker methods.
	* incremental.cc: Include "output.h".
	(Incremental_checker::can_incrementally_link_output_file): New
	method.
	* incremental.h (Incremental_checker): New class.

	* output.cc (Output_file::open_for_modification): New method.
	(Output_file::map_anonymous): Changed return type to bool.  Record
	map in base_ field.
	(Output_file::map_no_anonymous): New method, broken out of map.
	(Output_file::map): Use map_no_anonymous and map_anonymous.
	* output.h (class Output_file): Update declarations.


Index: gold.cc
===================================================================
RCS file: /cvs/src/src/gold/gold.cc,v
retrieving revision 1.66
diff -p -u -r1.66 gold.cc
--- gold.cc	5 Aug 2009 20:51:56 -0000	1.66
+++ gold.cc	1 Sep 2009 17:27:59 -0000
@@ -42,6 +42,7 @@
 #include "defstd.h"
 #include "plugin.h"
 #include "icf.h"
+#include "incremental.h"
 
 namespace gold
 {
@@ -176,6 +177,20 @@ queue_initial_tasks(const General_option
     thread_count = cmdline.number_of_input_files();
   workqueue->set_thread_count(thread_count);
 
+  if (cmdline.options().incremental())
+    {
+      Incremental_checker incremental_checker(
+          parameters->options().output_file_name());
+      if (incremental_checker.can_incrementally_link_output_file())
+        {
+          // TODO: remove when incremental linking implemented.
+          printf("Incremental linking might be possible "
+              "(not implemented yet)\n");
+        }
+      // TODO: If we decide on an incremental build, fewer tasks
+      // should be scheduled.
+    }
+
   // Read the input files.  We have to add the symbols to the symbol
   // table in order.  We do this by creating a separate blocker for
   // each input file.  We associate the blocker with the following
@@ -229,8 +244,8 @@ queue_initial_tasks(const General_option
     }
 }
 
-// Queue up a set of tasks to be done before queueing the middle set 
-// of tasks.  This is only necessary when garbage collection 
+// Queue up a set of tasks to be done before queueing the middle set
+// of tasks.  This is only necessary when garbage collection
 // (--gc-sections) of unused sections is desired.  The relocs are read
 // and processed here early to determine the garbage sections before the
 // relocs can be scanned in later tasks.
Index: incremental.cc
===================================================================
RCS file: /cvs/src/src/gold/incremental.cc,v
retrieving revision 1.5
diff -p -u -r1.5 incremental.cc
--- incremental.cc	6 Jul 2009 23:11:21 -0000	1.5
+++ incremental.cc	1 Sep 2009 17:27:59 -0000
@@ -25,6 +25,7 @@
 #include "output.h"
 #include "incremental.h"
 #include "archive.h"
+#include "output.h"
 
 using elfcpp::Convert;
 
@@ -149,6 +150,18 @@ class Incremental_inputs_entry_write
   internal::Incremental_inputs_entry_data* p_;
 };
 
+// Analyzes the output file to check if incremental linking is possible and
+// (to be done) what files need to be relinked.
+
+bool
+Incremental_checker::can_incrementally_link_output_file()
+{
+  Output_file output(this->output_name_);
+  if (!output.open_for_modification())
+    return false;
+  return true;
+}
+
 // Add the command line to the string table, setting
 // command_line_key_.  In incremental builds, the command line is
 // stored in .gnu_incremental_inputs so that the next linker run can
Index: incremental.h
===================================================================
RCS file: /cvs/src/src/gold/incremental.h,v
retrieving revision 1.3
diff -p -u -r1.3 incremental.h
--- incremental.h	6 Jul 2009 23:11:21 -0000	1.3
+++ incremental.h	1 Sep 2009 17:27:59 -0000
@@ -50,6 +50,24 @@ enum Incremental_input_type
   INCREMENTAL_INPUT_SCRIPT = 4
 };
 
+// Code invoked early during an incremental link that checks what files need
+// to be relinked.
+class Incremental_checker
+{
+ public:
+  Incremental_checker(const char* output_name)
+    : output_name_(output_name)
+  { }
+
+  // Analyzes the output file to check if incremental linking is possible and
+  // what files needs to be relinked.
+  bool
+  can_incrementally_link_output_file();
+
+ private:
+  const char* output_name_;
+};
+
 // This class contains the information needed during an incremental
 // build about the inputs necessary to build the .gnu_incremental_inputs.
 class Incremental_inputs
@@ -127,11 +145,11 @@ class Incremental_inputs
     {
       // Present if type == INCREMENTAL_INPUT_ARCHIVE.
       Archive* archive;
-  
+
       // Present if type == INCREMENTAL_INPUT_OBJECT or
       // INCREMENTAL_INPUT_SHARED_LIBRARY.
       Object* object;
-  
+
       // Present if type == INCREMENTAL_INPUT_SCRIPT.
       Script_info* script;
     };
@@ -141,7 +159,7 @@ class Incremental_inputs
 
     // Position of the entry information in the output section.
     unsigned int index;
-    
+
     // Last modification time of the file.
     Timespec mtime;
   };
@@ -151,7 +169,7 @@ class Incremental_inputs
   // A lock guarding access to inputs_ during the first phase of linking, when
   // report_ function may be called from multiple threads.
   Lock* lock_;
-  
+
   // The list of input arguments obtained from parsing the command line.
   const Input_arguments* inputs_;
 
Index: output.cc
===================================================================
RCS file: /cvs/src/src/gold/output.cc,v
retrieving revision 1.92
diff -p -u -r1.92 output.cc
--- output.cc	24 Jun 2009 19:48:51 -0000	1.92
+++ output.cc	1 Sep 2009 17:27:59 -0000
@@ -3397,6 +3397,42 @@ Output_file::Output_file(const char* nam
 {
 }
 
+// Try to open an existing file.  Returns false if the file doesn't
+// exist, has a size of 0 or can't be mmapped.
+
+bool
+Output_file::open_for_modification()
+{
+  // The name "-" means "stdout".
+  if (strcmp(this->name_, "-") == 0)
+    return false;
+
+  // Don't bother opening files with a size of zero.
+  struct stat s;
+  if (::stat(this->name_, &s) != 0 || s.st_size == 0)
+    return false;
+
+  int o = open_descriptor(-1, this->name_, O_RDWR, 0);
+  if (o < 0)
+    gold_fatal(_("%s: open: %s"), this->name_, strerror(errno));
+  this->o_ = o;
+  this->file_size_ = s.st_size;
+
+  // If the file can't be mmapped, copying the content to an anonymous
+  // map will probably negate the performance benefits of incremental
+  // linking.  This could be helped by using views and loading only
+  // the necessary parts, but this is not supported as of now.
+  if (!this->map_no_anonymous())
+    {
+      release_descriptor(o, true);
+      this->o_ = -1;
+      this->file_size_ = 0;
+      return false;
+    }
+
+  return true;
+}
+
 // Open the output file.
 
 void
@@ -3465,21 +3501,27 @@ Output_file::resize(off_t file_size)
     }
 }
 
-// Map a block of memory which will later be written to the file.
-// Return a pointer to the memory.
+// Map an anonymous block of memory which will later be written to the
+// file.  Return whether the map succeeded.
 
-void*
+bool
 Output_file::map_anonymous()
 {
-  this->map_is_anonymous_ = true;
-  return ::mmap(NULL, this->file_size_, PROT_READ | PROT_WRITE,
-		MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  void* base = ::mmap(NULL, this->file_size_, PROT_READ | PROT_WRITE,
+		      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+  if (base != MAP_FAILED)
+    {
+      this->map_is_anonymous_ = true;
+      this->base_ = static_cast<unsigned char*>(base);
+      return true;
+    }
+  return false;
 }
 
-// Map the file into memory.
+// Map the file into memory.  Return whether the mapping succeeded.
 
-void
-Output_file::map()
+bool
+Output_file::map_no_anonymous()
 {
   const int o = this->o_;
 
@@ -3492,38 +3534,52 @@ Output_file::map()
       || ::fstat(o, &statbuf) != 0
       || !S_ISREG(statbuf.st_mode)
       || this->is_temporary_)
-    base = this->map_anonymous();
-  else
-    {
-      // Ensure that we have disk space available for the file.  If we
-      // don't do this, it is possible that we will call munmap,
-      // close, and exit with dirty buffers still in the cache with no
-      // assigned disk blocks.  If the disk is out of space at that
-      // point, the output file will wind up incomplete, but we will
-      // have already exited.  The alternative to fallocate would be
-      // to use fdatasync, but that would be a more significant
-      // performance hit.
-      if (::posix_fallocate(o, 0, this->file_size_) < 0)
-	gold_fatal(_("%s: %s"), this->name_, strerror(errno));
-
-      // Map the file into memory.
-      this->map_is_anonymous_ = false;
-      base = ::mmap(NULL, this->file_size_, PROT_READ | PROT_WRITE,
-                    MAP_SHARED, o, 0);
-
-      // The mmap call might fail because of file system issues: the
-      // file system might not support mmap at all, or it might not
-      // support mmap with PROT_WRITE.  I'm not sure which errno
-      // values we will see in all cases, so if the mmap fails for any
-      // reason try for an anonymous map.
-      if (base == MAP_FAILED)
-	base = this->map_anonymous();
-    }
+    return false;
+
+  // Ensure that we have disk space available for the file.  If we
+  // don't do this, it is possible that we will call munmap, close,
+  // and exit with dirty buffers still in the cache with no assigned
+  // disk blocks.  If the disk is out of space at that point, the
+  // output file will wind up incomplete, but we will have already
+  // exited.  The alternative to fallocate would be to use fdatasync,
+  // but that would be a more significant performance hit.
+  if (::posix_fallocate(o, 0, this->file_size_) < 0)
+    gold_fatal(_("%s: %s"), this->name_, strerror(errno));
+
+  // Map the file into memory.
+  base = ::mmap(NULL, this->file_size_, PROT_READ | PROT_WRITE,
+		MAP_SHARED, o, 0);
+
+  // The mmap call might fail because of file system issues: the file
+  // system might not support mmap at all, or it might not support
+  // mmap with PROT_WRITE.
   if (base == MAP_FAILED)
-    gold_fatal(_("%s: mmap: failed to allocate %lu bytes for output file: %s"),
-	       this->name_, static_cast<unsigned long>(this->file_size_),
-	       strerror(errno));
+    return false;
+
+  this->map_is_anonymous_ = false;
   this->base_ = static_cast<unsigned char*>(base);
+  return true;
+}
+
+// Map the file into memory.
+
+void
+Output_file::map()
+{
+  if (this->map_no_anonymous())
+    return;
+
+  // The mmap call might fail because of file system issues: the file
+  // system might not support mmap at all, or it might not support
+  // mmap with PROT_WRITE.  I'm not sure which errno values we will
+  // see in all cases, so if the mmap fails for any reason and we
+  // don't care about file contents, try for an anonymous map.
+  if (this->map_anonymous())
+    return;
+
+  gold_fatal(_("%s: mmap: failed to allocate %lu bytes for output file: %s"),
+             this->name_, static_cast<unsigned long>(this->file_size_),
+             strerror(errno));
 }
 
 // Unmap the file from memory.
Index: output.h
===================================================================
RCS file: /cvs/src/src/gold/output.h,v
retrieving revision 1.80
diff -p -u -r1.80 output.h
--- output.h	24 Jun 2009 19:48:51 -0000	1.80
+++ output.h	1 Sep 2009 17:27:59 -0000
@@ -3093,16 +3093,24 @@ class Output_file
   set_is_temporary()
   { this->is_temporary_ = true; }
 
+  // Try to open an existing file. Returns false if the file doesn't
+  // exist, has a size of 0 or can't be mmaped.  This method is
+  // thread-unsafe.
+  bool
+  open_for_modification();
+
   // Open the output file.  FILE_SIZE is the final size of the file.
+  // If the file already exists, it is deleted/truncated.  This method
+  // is thread-unsafe.
   void
   open(off_t file_size);
 
-  // Resize the output file.
+  // Resize the output file.  This method is thread-unsafe.
   void
   resize(off_t file_size);
 
   // Close the output file (flushing all buffered data) and make sure
-  // there are no errors.
+  // there are no errors.  This method is thread-unsafe.
   void
   close();
 
@@ -3153,14 +3161,19 @@ class Output_file
   { }
 
  private:
-  // Map the file into memory.
+  // Map the file into memory or, if that fails, allocate anonymous
+  // memory.
   void
   map();
 
   // Allocate anonymous memory for the file.
-  void*
+  bool
   map_anonymous();
 
+  // Map the file into memory.
+  bool
+  map_no_anonymous();
+
   // Unmap the file from memory (and flush to disk buffers).
   void
   unmap();

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