[gold patch] Incremental 8/18: Initial support for incremental update

[Cary Coutant]

>> +      if (options.has_plugins())
>> +     gold_error(_("incremental linking is incompatible with --plugin"));
>
> Will this be possible at some future date?

It could be, but I haven't thought through all the implications. With
plugins, it's hard to guarantee that all the restrictions we impose
are met. What should we do with an IR file marked
--incremental-unchanged? The LTO plugin would probably still prefer to
claim it, and would have to if it's a pure IR file. The use cases for
LTO and for incremental linking seem opposed to each other -- LTO
wants to see and recompile as much as possible, while incremental
linking wants to see minimal changes.

>> +      // Incremental update link.  Process the list of input files
>> +      // stored in the base file, and queue a task for each file:
>> +      // a Read_symbols task for a changed file, and an Add_symbols task
>> +      // for an unchanged file.  We need to mark all the space used by
>> +      // unchanged files before we can start any tasks running.
>> +      std::list<Task*> tasks;
>
> Add tasks.reserve(input_file_count);

OK.

>> +// Process an incremental input file: if it is unchanged from the previous
>> +// link, return a task to add its symbols from the base file's incremental
>> +// info; if it has changed, return a normal Read_symbols task.  We create a
>> +// task for every input file, if only to report the file for rebuilding the
>> +// incremental info.
>> +
>> +Task*
>> +process_incremental_input(Incremental_binary* ibase,
>> +                       unsigned int input_file_index,
>> +                       Input_objects* input_objects,
>> +                       Symbol_table* symtab,
>> +                       Layout* layout,
>> +                       Dirsearch* search_path,
>> +                       Mapfile* mapfile,
>> +                       Task_token* this_blocker,
>> +                       Task_token* next_blocker)
>> +{
>
> This function should be static.

OK.

>> +  gold_debug(DEBUG_INCREMENTAL,   "Incremental object: %s, type %d",
>> +          input_reader->filename(), input_type);
>
> s/   / /

OK.

>> +  if (input_type == INCREMENTAL_INPUT_ARCHIVE)
>> +    {
>> +      Incremental_library* lib = ibase->get_library(input_file_index);
>> +      gold_assert(lib != NULL);
>> +      if (lib->filename() == "<group>"
>> +       || !ibase->file_has_changed(input_file_index))
>
> Hmmm.  For extreme cleanliness, suppose we change "<group>" to "/group/"
> or some other name containing a '/', so that we know for sure that there
> is no input file with that name.

Good idea. I was bothered by that myself, and intended to come back to
this with a cleaner solution. I'll use "/group/" for now.

>> @@ -396,7 +586,8 @@ queue_middle_tasks(const General_options& options,
>>    // generate an empty file.  Existing builds depend on being able to
>>    // pass an empty archive to the linker and get an empty object file
>>    // out.  In order to do this we need to use a default target.
>> -  if (input_objects->number_of_input_objects() == 0)
>> +  if (input_objects->number_of_input_objects() == 0
>> +      && layout->incremental_base() == NULL)
>>      parameters_force_valid_target();
>
> Why this change?

If there are no changed files at all, we want to base the target on
the base file rather than the arbitrary target that
parameters_force_valid_target() would choose.

Hmmm, if there are no changed files, we have no work to do, and we
should have just exited the link by this point. I'll take a closer
look at this.

>> +// Allocate an incremental object of the appropriate size and endianness.
>> +Object*
>> +make_sized_incremental_object(
>> +    Incremental_binary* base,
>> +    unsigned int input_file_index,
>> +    Incremental_input_type input_type,
>> +    const Incremental_binary::Input_reader* input_reader);
>
> Add "extern".

OK.

>> @@ -869,7 +1086,9 @@ Layout::layout_eh_frame(Sized_relobj<size, big_endian>* object,
>>        this->eh_frame_section_ = os;
>>        this->eh_frame_data_ = new Eh_frame();
>>
>> -      if (parameters->options().eh_frame_hdr())
>> +      // For incremental linking, we do not optimize .eh_frame sections
>> +      // or create a .eh_frame_hdr section.
>> +      if (parameters->options().eh_frame_hdr() && !parameters->incremental())
>>       {
>>         Output_section* hdr_os =
>>           this->choose_output_section(NULL, ".eh_frame_hdr",
>> @@ -901,14 +1120,15 @@ Layout::layout_eh_frame(Sized_relobj<size, big_endian>* object,
>>
>>    gold_assert(this->eh_frame_section_ == os);
>>
>> -  if (this->eh_frame_data_->add_ehframe_input_section(object,
>> -                                                   symbols,
>> -                                                   symbols_size,
>> -                                                   symbol_names,
>> -                                                   symbol_names_size,
>> -                                                   shndx,
>> -                                                   reloc_shndx,
>> -                                                   reloc_type))
>> +  if (!parameters->incremental()
>> +      && this->eh_frame_data_->add_ehframe_input_section(object,
>> +                                                      symbols,
>> +                                                      symbols_size,
>> +                                                      symbol_names,
>> +                                                      symbol_names_size,
>> +                                                      shndx,
>> +                                                      reloc_shndx,
>> +                                                      reloc_type))
>>      {
>>        os->update_flags_for_input_section(shdr.get_sh_flags());
>>
>
> Where do these things happen when doing an incremental link?

I haven't yet implemented the .eh_frame_hdr section for incremental
linking -- my plan is to rebuild it from scratch rather than try to
update it in place, but that means it'll need to rescan all the
.eh_frame sections from the unchanged files as well as changed files.

> This is OK with those changes.

Thanks. I'll post an updated patch when I resolve the
parameters_force_valid_target() issue. Or, if it's OK with you, I can
commit this patch with the changes you've noted and follow up later
with another patch to improve the no-changed-files case.

-cary