This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 1/2] gold: extend Output_file to support read/write access to existing files.
- From: Ian Lance Taylor <iant at google dot com>
- To: Mikolaj Zalewski <mikolajz at google dot com>
- Cc: binutils at sourceware dot org
- Date: Tue, 01 Sep 2009 10:33:06 -0700
- Subject: Re: [PATCH 1/2] gold: extend Output_file to support read/write access to existing files.
- References: <7ebec9e70909010910g5999fe4cg150949ee267d348@mail.gmail.com>
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();