* [gold patch] Incremental 22/25: Add patch space and --incremental-patch option
@ 2011-06-01 21:53 Cary Coutant
2011-07-05 20:51 ` Ian Lance Taylor
0 siblings, 1 reply; 2+ messages in thread
From: Cary Coutant @ 2011-06-01 21:53 UTC (permalink / raw)
To: Ian Lance Taylor, Binutils
[-- Attachment #1: Type: text/plain, Size: 2005 bytes --]
This patch finally adds patch space during an incremental-full link.
I've also added a --incremental-patch option that lets you specify how
much patch space to add, as a percentage of the original size. I also
added an ability for class Free_list to extend beyond the original
allocation, which lets me extend the output file if necessary. I
modified one of the existing test cases to start with an empty version
of the t18() function, so that the --incremental-update link would
fail without additional patch space.
-cary
2011-06-01 Cary Coutant <ccoutant@google.com>
* incremental.cc (Incremental_inputs::report_command_line): Ignore
--incremental-patch option.
* layout.cc (Free_list::allocate): Extend allocation beyond original
end if enabled.
(Layout::make_output_section): Mark sections that should get
patch space.
* options.cc (parse_percent): New function.
* options.h (parse_percent): New function.
(DEFINE_percent): New macro.
(General_options): Add --incremental-patch option.
* output.cc (Output_section::Output_section): Initialize new data
members.
(Output_section::add_input_section): Print section name when out
of patch space.
(Output_section::add_output_section_data): Likewise.
(Output_section::set_final_data_size): Add patch space when
doing --incremental-full.
(Output_section::do_reset_address_and_file_offset): Remove patch
space.
(Output_segment::set_section_list_addresses): Print debug output
only if --incremental-update.
* output.h (Output_section::set_is_patch_space_allowed): New function.
(Output_section::is_patch_space_allowed_): New data member.
(Output_section::patch_space_): New data member.
* parameters.cc (Parameters::incremental_full): New function.
* parameters.h (Parameters::incremental_full): New function
* testsuite/Makefile.am (incremental_test_2): Add test for
--incremental-patch option.
* testsuite/Makefile.in: Regenerate.
* testsuite/two_file_test_1_v1.cc (t1, t2, t3): Add comments.
(t18): Remove function body.
[-- Attachment #2: incr-patch-22b.txt --]
[-- Type: text/plain, Size: 17243 bytes --]
Patch 22: Add patch space and --incremental-patch option; extend file when
no free space is left; modify test case to test use of patch space.
Suppress adding patch space for certain sections.
2011-06-01 Cary Coutant <ccoutant@google.com>
* incremental.cc (Incremental_inputs::report_command_line): Ignore
--incremental-patch option.
* layout.cc (Free_list::allocate): Extend allocation beyond original
end if enabled.
(Layout::make_output_section): Mark sections that should get
patch space.
* options.cc (parse_percent): New function.
* options.h (parse_percent): New function.
(DEFINE_percent): New macro.
(General_options): Add --incremental-patch option.
* output.cc (Output_section::Output_section): Initialize new data
members.
(Output_section::add_input_section): Print section name when out
of patch space.
(Output_section::add_output_section_data): Likewise.
(Output_section::set_final_data_size): Add patch space when
doing --incremental-full.
(Output_section::do_reset_address_and_file_offset): Remove patch
space.
(Output_segment::set_section_list_addresses): Print debug output
only if --incremental-update.
* output.h (Output_section::set_is_patch_space_allowed): New function.
(Output_section::is_patch_space_allowed_): New data member.
(Output_section::patch_space_): New data member.
* parameters.cc (Parameters::incremental_full): New function.
* parameters.h (Parameters::incremental_full): New function
* testsuite/Makefile.am (incremental_test_2): Add test for
--incremental-patch option.
* testsuite/Makefile.in: Regenerate.
* testsuite/two_file_test_1_v1.cc (t1, t2, t3): Add comments.
(t18): Remove function body.
diff --git a/gold/incremental.cc b/gold/incremental.cc
index 1dac8ec..db28c76 100644
--- a/gold/incremental.cc
+++ b/gold/incremental.cc
@@ -925,9 +925,11 @@ Incremental_inputs::report_command_line(int argc, const char* const* argv)
|| strcmp(argv[i], "--incremental-unchanged") == 0
|| strcmp(argv[i], "--incremental-unknown") == 0
|| is_prefix_of("--incremental-base=", argv[i])
+ || is_prefix_of("--incremental-patch=", argv[i])
|| is_prefix_of("--debug=", argv[i]))
continue;
if (strcmp(argv[i], "--incremental-base") == 0
+ || strcmp(argv[i], "--incremental-patch") == 0
|| strcmp(argv[i], "--debug") == 0)
{
// When these options are used without the '=', skip the
diff --git a/gold/layout.cc b/gold/layout.cc
index 4c0f1f1..639e7e5 100644
--- a/gold/layout.cc
+++ b/gold/layout.cc
@@ -167,6 +167,11 @@ Free_list::allocate(off_t len, uint64_t align, off_t minoff)
off_t start = p->start_ > minoff ? p->start_ : minoff;
start = align_address(start, align);
off_t end = start + len;
+ if (end > p->end_ && p->end_ == this->length_ && this->extend_)
+ {
+ this->length_ = end;
+ p->end_ = end;
+ }
if (end <= p->end_)
{
if (p->start_ + 3 >= start && p->end_ <= end + 3)
@@ -185,6 +190,12 @@ Free_list::allocate(off_t len, uint64_t align, off_t minoff)
return start;
}
}
+ if (this->extend_)
+ {
+ off_t start = align_address(this->length_, align);
+ this->length_ = start + len;
+ return start;
+ }
return -1;
}
@@ -1312,6 +1323,21 @@ Layout::make_output_section(const char* name, elfcpp::Elf_Word type,
&& strcmp(name + strlen(name) - 3, "str") == 0)
this->have_stabstr_section_ = true;
+ // During a full incremental link, we add patch space to most
+ // PROGBITS and NOBITS sections. Flag those that may be
+ // arbitrarily padded.
+ if ((type == elfcpp::SHT_PROGBITS || type == elfcpp::SHT_NOBITS)
+ && order != ORDER_INTERP
+ && order != ORDER_INIT
+ && order != ORDER_PLT
+ && order != ORDER_FINI
+ && order != ORDER_RELRO_LAST
+ && order != ORDER_NON_RELRO_FIRST
+ && strcmp(name, ".ctors") != 0
+ && strcmp(name, ".dtors") != 0
+ && strcmp(name, ".jcr") != 0)
+ os->set_is_patch_space_allowed();
+
// If we have already attached the sections to segments, then we
// need to attach this one now. This happens for sections created
// directly by the linker.
diff --git a/gold/options.cc b/gold/options.cc
index 3ec76a8..b41daa2 100644
--- a/gold/options.cc
+++ b/gold/options.cc
@@ -226,6 +226,17 @@ parse_double(const char* option_name, const char* arg, double* retval)
}
void
+parse_percent(const char* option_name, const char* arg, double* retval)
+{
+ char* endptr;
+ *retval = strtod(arg, &endptr) / 100.0;
+ if (*endptr != '\0')
+ gold_fatal(_("%s: invalid option value "
+ "(expected a floating point number): %s"),
+ option_name, arg);
+}
+
+void
parse_string(const char* option_name, const char* arg, const char** retval)
{
if (*arg == '\0')
diff --git a/gold/options.h b/gold/options.h
index b3b51dc..f261622 100644
--- a/gold/options.h
+++ b/gold/options.h
@@ -98,6 +98,9 @@ extern void
parse_double(const char* option_name, const char* arg, double* retval);
extern void
+parse_percent(const char* option_name, const char* arg, double* retval);
+
+extern void
parse_string(const char* option_name, const char* arg, const char** retval);
extern void
@@ -372,6 +375,12 @@ struct Struct_special : public Struct_var
#default_value__, helpstring__, helparg__, false, \
double, double, options::parse_double)
+#define DEFINE_percent(varname__, dashes__, shortname__, default_value__, \
+ helpstring__, helparg__) \
+ DEFINE_var(varname__, dashes__, shortname__, default_value__ / 100.0, \
+ #default_value__, helpstring__, helparg__, false, \
+ double, double, options::parse_percent)
+
#define DEFINE_string(varname__, dashes__, shortname__, default_value__, \
helpstring__, helparg__) \
DEFINE_var(varname__, dashes__, shortname__, default_value__, \
@@ -801,6 +810,10 @@ class General_options
DEFINE_special(incremental_unknown, options::TWO_DASHES, '\0',
N_("Use timestamps to check files (default)"), NULL);
+ DEFINE_percent(incremental_patch, options::TWO_DASHES, '\0', 10,
+ N_("Amount of extra space to allocate for patches"),
+ N_("PERCENT"));
+
DEFINE_string(init, options::ONE_DASH, '\0', "_init",
N_("Call SYMBOL at load-time"), N_("SYMBOL"));
diff --git a/gold/output.cc b/gold/output.cc
index 4babe5f..8444264 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -2152,10 +2152,12 @@ Output_section::Output_section(const char* name, elfcpp::Elf_Word type,
is_noload_(false),
always_keeps_input_sections_(false),
has_fixed_layout_(false),
+ is_patch_space_allowed_(false),
tls_offset_(0),
checkpoint_(NULL),
lookup_maps_(new Output_section_lookup_maps),
- free_list_()
+ free_list_(),
+ patch_space_(0)
{
// An unallocated section has no address. Forcing this means that
// we don't need special treatment for symbols defined in debug
@@ -2270,7 +2272,9 @@ Output_section::add_input_section(Layout* layout,
offset_in_section = this->free_list_.allocate(input_section_size,
addralign, 0);
if (offset_in_section == -1)
- gold_fallback(_("out of patch space; relink with --incremental-full"));
+ gold_fallback(_("out of patch space in section %s; "
+ "relink with --incremental-full"),
+ this->name());
aligned_offset_in_section = offset_in_section;
}
else
@@ -2374,8 +2378,9 @@ Output_section::add_output_section_data(Output_section_data* posd)
offset_in_section = this->free_list_.allocate(posd->data_size(),
posd->addralign(), 0);
if (offset_in_section == -1)
- gold_fallback(_("out of patch space; "
- "relink with --incremental-full"));
+ gold_fallback(_("out of patch space in section %s; "
+ "relink with --incremental-full"),
+ this->name());
// Finalize the address and offset now.
uint64_t addr = this->address();
off_t offset = this->offset();
@@ -2945,30 +2950,48 @@ Output_section::update_data_size()
void
Output_section::set_final_data_size()
{
+ off_t data_size;
+
if (this->input_sections_.empty())
+ data_size = this->current_data_size_for_child();
+ else
{
- this->set_data_size(this->current_data_size_for_child());
- return;
- }
+ if (this->must_sort_attached_input_sections()
+ || this->input_section_order_specified())
+ this->sort_attached_input_sections();
- if (this->must_sort_attached_input_sections()
- || this->input_section_order_specified())
- this->sort_attached_input_sections();
+ uint64_t address = this->address();
+ off_t startoff = this->offset();
+ off_t off = startoff + this->first_input_offset_;
+ for (Input_section_list::iterator p = this->input_sections_.begin();
+ p != this->input_sections_.end();
+ ++p)
+ {
+ off = align_address(off, p->addralign());
+ p->set_address_and_file_offset(address + (off - startoff), off,
+ startoff);
+ off += p->data_size();
+ }
+ data_size = off - startoff;
+ }
- uint64_t address = this->address();
- off_t startoff = this->offset();
- off_t off = startoff + this->first_input_offset_;
- for (Input_section_list::iterator p = this->input_sections_.begin();
- p != this->input_sections_.end();
- ++p)
+ // For full incremental links, we want to allocate some patch space
+ // in most sections for subsequent incremental updates.
+ if (this->is_patch_space_allowed_ && parameters->incremental_full())
{
- off = align_address(off, p->addralign());
- p->set_address_and_file_offset(address + (off - startoff), off,
- startoff);
- off += p->data_size();
+ double pct = parameters->options().incremental_patch();
+ off_t extra = static_cast<off_t>(data_size * pct);
+ off_t new_size = align_address(data_size + extra, this->addralign());
+ this->patch_space_ = new_size - data_size;
+ gold_debug(DEBUG_INCREMENTAL,
+ "set_final_data_size: %08lx + %08lx: section %s",
+ static_cast<long>(data_size),
+ static_cast<long>(this->patch_space_),
+ this->name());
+ data_size = new_size;
}
- this->set_data_size(off - startoff);
+ this->set_data_size(data_size);
}
// Reset the address and file offset.
@@ -2987,8 +3010,16 @@ Output_section::do_reset_address_and_file_offset()
p != this->input_sections_.end();
++p)
p->reset_address_and_file_offset();
+
+ // Remove any patch space that was added in set_final_data_size.
+ if (this->patch_space_ > 0)
+ {
+ this->set_current_data_size_for_child(this->current_data_size_for_child()
+ - this->patch_space_);
+ this->patch_space_ = 0;
+ }
}
-
+
// Return true if address and file offset have the values after reset.
bool
@@ -4224,14 +4255,15 @@ Output_segment::set_section_list_addresses(Layout* layout, bool reset,
(*p)->finalize_data_size();
}
- gold_debug(DEBUG_INCREMENTAL,
- "set_section_list_addresses: %08lx %08lx %s",
- static_cast<long>(off),
- static_cast<long>((*p)->data_size()),
- ((*p)->output_section() != NULL
- ? (*p)->output_section()->name() : "(special)"));
-
- // We want to ignore the size of a SHF_TLS or SHT_NOBITS
+ if (parameters->incremental_update())
+ gold_debug(DEBUG_INCREMENTAL,
+ "set_section_list_addresses: %08lx %08lx %s",
+ static_cast<long>(off),
+ static_cast<long>((*p)->data_size()),
+ ((*p)->output_section() != NULL
+ ? (*p)->output_section()->name() : "(special)"));
+
+ // We want to ignore the size of a SHF_TLS SHT_NOBITS
// section. Such a section does not affect the size of a
// PT_LOAD segment.
if (!(*p)->is_section_flag_set(elfcpp::SHF_TLS)
diff --git a/gold/output.h b/gold/output.h
index 72d1dba..7fb87de 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -3427,6 +3427,12 @@ class Output_section : public Output_data
has_fixed_layout() const
{ return this->has_fixed_layout_; }
+ // Set flag to allow patch space for this section. Used for full
+ // incremental links.
+ void
+ set_is_patch_space_allowed()
+ { this->is_patch_space_allowed_ = true; }
+
// Reserve space within the fixed layout for the section. Used for
// incremental update links.
void
@@ -3890,6 +3896,8 @@ class Output_section : public Output_data
bool always_keeps_input_sections_ : 1;
// Whether this section has a fixed layout, for incremental update links.
bool has_fixed_layout_ : 1;
+ // True if we can add patch space to this section.
+ bool is_patch_space_allowed_ : 1;
// For SHT_TLS sections, the offset of this section relative to the base
// of the TLS segment.
uint64_t tls_offset_;
@@ -3900,6 +3908,8 @@ class Output_section : public Output_data
// List of available regions within the section, for incremental
// update links.
Free_list free_list_;
+ // Amount added as patch space for incremental linking.
+ off_t patch_space_;
};
// An output segment. PT_LOAD segments are built from collections of
diff --git a/gold/parameters.cc b/gold/parameters.cc
index 194c81b..ed13bb3 100644
--- a/gold/parameters.cc
+++ b/gold/parameters.cc
@@ -248,6 +248,14 @@ Parameters::incremental() const
return this->incremental_mode_ != General_options::INCREMENTAL_OFF;
}
+// Return true if we are doing a full incremental link.
+
+bool
+Parameters::incremental_full() const
+{
+ return (this->incremental_mode_ == General_options::INCREMENTAL_FULL);
+}
+
// Return true if we are doing an incremental update.
bool
diff --git a/gold/parameters.h b/gold/parameters.h
index 7867503..09b0516 100644
--- a/gold/parameters.h
+++ b/gold/parameters.h
@@ -159,6 +159,10 @@ class Parameters
bool
incremental() const;
+ // Return true if we are doing a full incremental link.
+ bool
+ incremental_full() const;
+
// Return true if we are doing an incremental update.
bool
incremental_update() const;
diff --git a/gold/testsuite/Makefile.am b/gold/testsuite/Makefile.am
index fb20f46..209c363 100644
--- a/gold/testsuite/Makefile.am
+++ b/gold/testsuite/Makefile.am
@@ -1825,7 +1825,7 @@ MOSTLYCLEANFILES += two_file_test_tmp_2.o
incremental_test_2: two_file_test_1_v1.o two_file_test_1.o two_file_test_1b.o \
two_file_test_2.o two_file_test_main.o gcctestdir/ld
cp -f two_file_test_1_v1.o two_file_test_tmp_2.o
- $(CXXLINK) -Wl,--incremental-full -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b.o two_file_test_2.o two_file_test_main.o
+ $(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b.o two_file_test_2.o two_file_test_main.o
@sleep 1
cp -f two_file_test_1.o two_file_test_tmp_2.o
$(CXXLINK) -Wl,--incremental-update -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b.o two_file_test_2.o two_file_test_main.o
diff --git a/gold/testsuite/Makefile.in b/gold/testsuite/Makefile.in
index 9da0841..17ab1a0 100644
--- a/gold/testsuite/Makefile.in
+++ b/gold/testsuite/Makefile.in
@@ -4640,7 +4640,7 @@ uninstall-am:
@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@incremental_test_2: two_file_test_1_v1.o two_file_test_1.o two_file_test_1b.o \
@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ two_file_test_2.o two_file_test_main.o gcctestdir/ld
@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ cp -f two_file_test_1_v1.o two_file_test_tmp_2.o
-@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -Wl,--incremental-full -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b.o two_file_test_2.o two_file_test_main.o
+@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -Wl,--incremental-full,--incremental-patch=100 -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b.o two_file_test_2.o two_file_test_main.o
@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ @sleep 1
@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ cp -f two_file_test_1.o two_file_test_tmp_2.o
@DEFAULT_TARGET_X86_64_TRUE@@GCC_TRUE@@NATIVE_LINKER_TRUE@ $(CXXLINK) -Wl,--incremental-update -Bgcctestdir/ two_file_test_tmp_2.o two_file_test_1b.o two_file_test_2.o two_file_test_main.o
diff --git a/gold/testsuite/two_file_test_1_v1.cc b/gold/testsuite/two_file_test_1_v1.cc
index 6a43d9b..2a23654 100644
--- a/gold/testsuite/two_file_test_1_v1.cc
+++ b/gold/testsuite/two_file_test_1_v1.cc
@@ -62,7 +62,7 @@
bool
t1()
{
- return t1_2() == 0;
+ return t1_2() == 0; // Intentionally wrong.
}
// 2 Code in file 1 refers to global data in file 2.
@@ -70,7 +70,7 @@ t1()
bool
t2()
{
- return v2 == 0;
+ return v2 == 0; // Intentionally wrong.
}
// 3 Code in file 1 referes to common symbol in file 2.
@@ -78,7 +78,7 @@ t2()
bool
t3()
{
- return v3 == 0;
+ return v3 == 0; // Intentionally wrong.
}
// 4 Code in file 1 refers to offset within global data in file 2.
@@ -231,13 +231,6 @@ t17()
bool
t18()
{
- char c = 'a';
- for (int i = 0; i < T17_COUNT; ++i)
- {
- const char* s = f18(i);
- if (s[0] != c || s[1] != '\0')
- return false;
- ++c;
- }
+ // Stubbed out; full implementation in two_file_test_1.cc.
return true;
}
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [gold patch] Incremental 22/25: Add patch space and --incremental-patch option
2011-06-01 21:53 [gold patch] Incremental 22/25: Add patch space and --incremental-patch option Cary Coutant
@ 2011-07-05 20:51 ` Ian Lance Taylor
0 siblings, 0 replies; 2+ messages in thread
From: Ian Lance Taylor @ 2011-07-05 20:51 UTC (permalink / raw)
To: Cary Coutant; +Cc: Binutils
Cary Coutant <ccoutant@google.com> writes:
> 2011-06-01 Cary Coutant <ccoutant@google.com>
>
> * incremental.cc (Incremental_inputs::report_command_line): Ignore
> --incremental-patch option.
> * layout.cc (Free_list::allocate): Extend allocation beyond original
> end if enabled.
> (Layout::make_output_section): Mark sections that should get
> patch space.
> * options.cc (parse_percent): New function.
> * options.h (parse_percent): New function.
> (DEFINE_percent): New macro.
> (General_options): Add --incremental-patch option.
> * output.cc (Output_section::Output_section): Initialize new data
> members.
> (Output_section::add_input_section): Print section name when out
> of patch space.
> (Output_section::add_output_section_data): Likewise.
> (Output_section::set_final_data_size): Add patch space when
> doing --incremental-full.
> (Output_section::do_reset_address_and_file_offset): Remove patch
> space.
> (Output_segment::set_section_list_addresses): Print debug output
> only if --incremental-update.
> * output.h (Output_section::set_is_patch_space_allowed): New function.
> (Output_section::is_patch_space_allowed_): New data member.
> (Output_section::patch_space_): New data member.
> * parameters.cc (Parameters::incremental_full): New function.
> * parameters.h (Parameters::incremental_full): New function
> * testsuite/Makefile.am (incremental_test_2): Add test for
> --incremental-patch option.
> * testsuite/Makefile.in: Regenerate.
> * testsuite/two_file_test_1_v1.cc (t1, t2, t3): Add comments.
> (t18): Remove function body.
> +// Return true if we are doing a full incremental link.
> +
> +bool
> +Parameters::incremental_full() const
> +{
> + return (this->incremental_mode_ == General_options::INCREMENTAL_FULL);
> +}
> +
No need for parentheses here--omit them.
This is OK with that change.
Thanks.
Ian
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-07-05 20:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-01 21:53 [gold patch] Incremental 22/25: Add patch space and --incremental-patch option Cary Coutant
2011-07-05 20:51 ` Ian Lance Taylor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).