From: Cary Coutant <ccoutant@gmail.com>
To: "Rafael Espíndola" <rafael.espindola@gmail.com>
Cc: Binutils <binutils@sourceware.org>,
Sriraman Tallam <tmsriram@google.com>
Subject: Re: [patch] Ignore non relobj files in gc
Date: Mon, 27 Apr 2015 20:17:00 -0000 [thread overview]
Message-ID: <CAJimCsFzvW5u1BefoXZm79exW2=3kf+cHOH96DntfXouL-CCFw@mail.gmail.com> (raw)
In-Reply-To: <CAG3jReKWtG9v9VR4JTs-8VzeN-OqURA_ZOSQo=+bu=gnkxrGpQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2233 bytes --]
> If a relocation points to a dynamic object we can ignore it, since we
> cannot gc an already existing .so file.
>
> The attached patch produces an identical chromium binary, but does so
> a bit faster (see the attached perf logs).
I'd think the one-line patch below accomplishes the same result with
much less disruption:
@@ -340,7 +341,7 @@ gc_process_relocs(
src_obj));
}
- if (gsym->source() != Symbol::FROM_OBJECT)
+ if (dst_obj == NULL || dst_obj->is_dynamic())
continue;
if (!is_ordinary)
continue;
> Using Relobj also helps me in my patch for gcing parts of SHF_MERGE sections.
If that's the case, let's move those changes to a separate patch.
Nevertheless, I think it would be cleaner to change Section_id to use
a Relobj* directly. That has some cascading effects, but doesn't
affect quite as much target-independent code, and we really never need
a Section_id that refers to a non-relocatable object. The attached
patch does that.
Sri, would you mind taking a look to make sure I haven't done anything
in the attached patch that might break --icf? Here's the only place
where we might have created a Section_id with a pointer to a Dynobj,
but I don't think that makes sense:
@@ -324,8 +326,11 @@ gc_process_relocs(
if (is_icf_tracked)
{
Address symvalue = dst_off - addend;
- if (is_ordinary && gsym->source() == Symbol::FROM_OBJECT)
- (*secvec).push_back(Section_id(dst_obj, dst_indx));
+ if (is_ordinary
+ && gsym->source() == Symbol::FROM_OBJECT
+ && !dst_obj->is_dynamic())
+ (*secvec).push_back(Section_id(static_cast<Relobj*>(dst_obj),
+ dst_indx));
else
(*secvec).push_back(Section_id(NULL, 0));
(*symvec).push_back(gsym);
Perhaps it does make sense here to track a relocation pointing into a
dynamic object, but in this case, the static_cast might still be OK,
since I think at this point, we're only using the object pointer as an
opaque ID for the object.
-cary
[-- Attachment #2: gold-section-id.patch --]
[-- Type: application/octet-stream, Size: 13511 bytes --]
diff --git a/gold/gc.h b/gold/gc.h
index bf4023d..4db101a 100644
--- a/gold/gc.h
+++ b/gold/gc.h
@@ -87,7 +87,7 @@ class Garbage_collection
do_transitive_closure();
bool
- is_section_garbage(Object* obj, unsigned int shndx)
+ is_section_garbage(Relobj* obj, unsigned int shndx)
{ return (this->referenced_list().find(Section_id(obj, shndx))
== this->referenced_list().end()); }
@@ -103,8 +103,8 @@ class Garbage_collection
// Add a reference from the SRC_SHNDX-th section of SRC_OBJECT to
// DST_SHNDX-th section of DST_OBJECT.
void
- add_reference(Object* src_object, unsigned int src_shndx,
- Object* dst_object, unsigned int dst_shndx)
+ add_reference(Relobj* src_object, unsigned int src_shndx,
+ Relobj* dst_object, unsigned int dst_shndx)
{
Section_id src_id(src_object, src_shndx);
Section_id dst_id(dst_object, dst_shndx);
@@ -249,7 +249,7 @@ gc_process_relocs(
{
Address symvalue = dst_off - addend;
if (is_ordinary)
- (*secvec).push_back(Section_id(dst_obj, dst_indx));
+ (*secvec).push_back(Section_id(src_obj, dst_indx));
else
(*secvec).push_back(Section_id(NULL, 0));
(*symvec).push_back(NULL);
@@ -301,12 +301,14 @@ gc_process_relocs(
// of a function pointer being taken.
if (gsym->source() == Symbol::FROM_OBJECT
&& check_section_for_function_pointers
- && gsym->type() != elfcpp::STT_OBJECT
+ && dst_obj != NULL
+ && !dst_obj->is_dynamic()
&& (!is_ordinary
|| scan.global_reloc_may_be_function_pointer(
symtab, NULL, NULL, src_obj, src_indx, NULL, reloc,
r_type, gsym)))
- symtab->icf()->set_section_has_function_pointers(dst_obj, dst_indx);
+ symtab->icf()->set_section_has_function_pointers(
+ static_cast<Relobj*>(dst_obj), dst_indx);
// If the symbol name matches '__start_XXX' then the section with
// the C identifier like name 'XXX' should not be garbage collected.
@@ -324,8 +326,11 @@ gc_process_relocs(
if (is_icf_tracked)
{
Address symvalue = dst_off - addend;
- if (is_ordinary && gsym->source() == Symbol::FROM_OBJECT)
- (*secvec).push_back(Section_id(dst_obj, dst_indx));
+ if (is_ordinary
+ && gsym->source() == Symbol::FROM_OBJECT
+ && !dst_obj->is_dynamic())
+ (*secvec).push_back(Section_id(static_cast<Relobj*>(dst_obj),
+ dst_indx));
else
(*secvec).push_back(Section_id(NULL, 0));
(*symvec).push_back(gsym);
@@ -340,17 +345,19 @@ gc_process_relocs(
src_obj));
}
- if (gsym->source() != Symbol::FROM_OBJECT)
+ if (dst_obj == NULL || dst_obj->is_dynamic())
continue;
if (!is_ordinary)
continue;
}
if (parameters->options().gc_sections())
{
- symtab->gc()->add_reference(src_obj, src_indx, dst_obj, dst_indx);
+ symtab->gc()->add_reference(src_obj, src_indx,
+ static_cast<Relobj*>(dst_obj), dst_indx);
parameters->sized_target<size, big_endian>()
->gc_add_reference(symtab, src_obj, src_indx,
- dst_obj, dst_indx, dst_off);
+ static_cast<Relobj*>(dst_obj),
+ dst_indx, dst_off);
if (cident_section_name != NULL)
{
Garbage_collection::Cident_section_map::iterator ele =
diff --git a/gold/icf.cc b/gold/icf.cc
index 8de6386..96b7f2d 100644
--- a/gold/icf.cc
+++ b/gold/icf.cc
@@ -787,7 +787,7 @@ Icf::find_identical_sections(const Input_objects* input_objects,
else if (sym->source() == Symbol::FROM_OBJECT
&& !sym->object()->is_dynamic())
{
- Object* obj = sym->object();
+ Relobj* obj = static_cast<Relobj*>(sym->object());
bool is_ordinary;
unsigned int shndx = sym->shndx(&is_ordinary);
if (is_ordinary)
@@ -804,7 +804,7 @@ Icf::find_identical_sections(const Input_objects* input_objects,
// Unfolds the section denoted by OBJ and SHNDX if folded.
void
-Icf::unfold_section(Object* obj, unsigned int shndx)
+Icf::unfold_section(Relobj* obj, unsigned int shndx)
{
Section_id secn(obj, shndx);
Uniq_secn_id_map::iterator it = this->section_id_.find(secn);
@@ -821,7 +821,7 @@ Icf::unfold_section(Object* obj, unsigned int shndx)
// is different from this section.
bool
-Icf::is_section_folded(Object* obj, unsigned int shndx)
+Icf::is_section_folded(Relobj* obj, unsigned int shndx)
{
Section_id secn(obj, shndx);
Uniq_secn_id_map::iterator it = this->section_id_.find(secn);
@@ -835,7 +835,7 @@ Icf::is_section_folded(Object* obj, unsigned int shndx)
// This function returns the folded section for the given section.
Section_id
-Icf::get_folded_section(Object* dup_obj, unsigned int dup_shndx)
+Icf::get_folded_section(Relobj* dup_obj, unsigned int dup_shndx)
{
Section_id dup_secn(dup_obj, dup_shndx);
Uniq_secn_id_map::iterator it = this->section_id_.find(dup_secn);
diff --git a/gold/icf.h b/gold/icf.h
index d343fa5..7faf816 100644
--- a/gold/icf.h
+++ b/gold/icf.h
@@ -74,7 +74,7 @@ class Icf
// Returns the kept folded identical section corresponding to
// dup_obj and dup_shndx.
Section_id
- get_folded_section(Object* dup_obj, unsigned int dup_shndx);
+ get_folded_section(Relobj* dup_obj, unsigned int dup_shndx);
// Forms groups of identical sections where the first member
// of each group is the kept section during folding.
@@ -95,17 +95,17 @@ class Icf
// Unfolds the section denoted by OBJ and SHNDX if folded.
void
- unfold_section(Object* obj, unsigned int shndx);
+ unfold_section(Relobj* obj, unsigned int shndx);
// Returns the kept section corresponding to the
// given section.
bool
- is_section_folded(Object* obj, unsigned int shndx);
+ is_section_folded(Relobj* obj, unsigned int shndx);
// Given an object and a section index, this returns true if the
// pointer of the function defined in this section is taken.
bool
- section_has_function_pointers(Object* obj, unsigned int shndx)
+ section_has_function_pointers(Relobj* obj, unsigned int shndx)
{
return (this->fptr_section_id_.find(Section_id(obj, shndx))
!= this->fptr_section_id_.end());
@@ -114,7 +114,7 @@ class Icf
// Records that a pointer of the function defined in this section
// is taken.
void
- set_section_has_function_pointers(Object* obj, unsigned int shndx)
+ set_section_has_function_pointers(Relobj* obj, unsigned int shndx)
{
this->fptr_section_id_.insert(Section_id(obj, shndx));
}
diff --git a/gold/object.h b/gold/object.h
index fc93abd..a3d5d0e 100644
--- a/gold/object.h
+++ b/gold/object.h
@@ -2863,11 +2863,11 @@ struct Relocate_info
// This is used to represent a section in an object and is used as the
// key type for various section maps.
-typedef std::pair<Object*, unsigned int> Section_id;
+typedef std::pair<Relobj*, unsigned int> Section_id;
// This is similar to Section_id but is used when the section
// pointers are const.
-typedef std::pair<const Object*, unsigned int> Const_section_id;
+typedef std::pair<const Relobj*, unsigned int> Const_section_id;
// The hash value is based on the address of an object in memory during
// linking. It is okay to use this for looking up sections but never use
diff --git a/gold/output.cc b/gold/output.cc
index ee6c475..781fbfb 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -3501,7 +3501,7 @@ Output_section::update_section_layout(
if (p->is_input_section()
|| p->is_relaxed_input_section())
{
- Object* obj = (p->is_input_section()
+ Relobj* obj = (p->is_input_section()
? p->relobj()
: p->relaxed_input_section()->relobj());
unsigned int shndx = p->shndx();
diff --git a/gold/output.h b/gold/output.h
index 318af36..be72965 100644
--- a/gold/output.h
+++ b/gold/output.h
@@ -2904,7 +2904,7 @@ class Output_section_lookup_maps
// Find a relaxed input section of OBJECT with index SHNDX.
Output_relaxed_input_section*
- find_relaxed_input_section(const Object* object, unsigned int shndx) const
+ find_relaxed_input_section(const Relobj* object, unsigned int shndx) const
{
gold_assert(this->is_valid_);
Relaxed_input_sections_by_id::const_iterator p =
diff --git a/gold/plugin.cc b/gold/plugin.cc
index 68da8e3..1588f34 100644
--- a/gold/plugin.cc
+++ b/gold/plugin.cc
@@ -1731,10 +1731,10 @@ update_section_order(const struct ld_plugin_section* section_list,
{
Object* obj = parameters->options().plugins()->get_elf_object(
section_list[i].handle);
- if (obj == NULL)
+ if (obj == NULL || obj->is_dynamic())
return LDPS_BAD_HANDLE;
unsigned int shndx = section_list[i].shndx;
- Section_id secn_id(obj, shndx);
+ Section_id secn_id(static_cast<Relobj*>(obj), shndx);
(*order_map)[secn_id] = i + 1;
}
@@ -1800,10 +1800,10 @@ unique_segment_for_sections(const char* segment_name,
{
Object* obj = parameters->options().plugins()->get_elf_object(
section_list[i].handle);
- if (obj == NULL)
+ if (obj == NULL || obj->is_dynamic())
return LDPS_BAD_HANDLE;
unsigned int shndx = section_list[i].shndx;
- Const_section_id secn_id(obj, shndx);
+ Const_section_id secn_id(static_cast<Relobj*>(obj), shndx);
layout->insert_section_segment_map(secn_id, s);
}
diff --git a/gold/powerpc.cc b/gold/powerpc.cc
index fddf3fa..29abd5e 100644
--- a/gold/powerpc.cc
+++ b/gold/powerpc.cc
@@ -212,7 +212,7 @@ public:
// Add a reference from SRC_OBJ, SRC_INDX to this object's .opd
// section at DST_OFF.
void
- add_reference(Object* src_obj,
+ add_reference(Relobj* src_obj,
unsigned int src_indx,
typename elfcpp::Elf_types<size>::Elf_Addr dst_off)
{
@@ -780,9 +780,9 @@ class Target_powerpc : public Sized_target<size, big_endian>
// section of a function descriptor.
void
do_gc_add_reference(Symbol_table* symtab,
- Object* src_obj,
+ Relobj* src_obj,
unsigned int src_shndx,
- Object* dst_obj,
+ Relobj* dst_obj,
unsigned int dst_shndx,
Address dst_off) const;
@@ -6347,7 +6347,7 @@ Target_powerpc<size, big_endian>::gc_process_relocs(
typename Powerpc_relobj<size, big_endian>::Section_refs::iterator s;
for (s = p->second.begin(); s != p->second.end(); ++s)
{
- Object* src_obj = s->first;
+ Relobj* src_obj = s->first;
unsigned int src_indx = s->second;
symtab->gc()->add_reference(src_obj, src_indx,
ppc_object, dst_indx);
@@ -6384,9 +6384,9 @@ template<int size, bool big_endian>
void
Target_powerpc<size, big_endian>::do_gc_add_reference(
Symbol_table* symtab,
- Object* src_obj,
+ Relobj* src_obj,
unsigned int src_shndx,
- Object* dst_obj,
+ Relobj* dst_obj,
unsigned int dst_shndx,
Address dst_off) const
{
diff --git a/gold/symtab.cc b/gold/symtab.cc
index d2ed352..925296a 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -585,7 +585,7 @@ Symbol_table::Symbol_table_eq::operator()(const Symbol_table_key& k1,
}
bool
-Symbol_table::is_section_folded(Object* obj, unsigned int shndx) const
+Symbol_table::is_section_folded(Relobj* obj, unsigned int shndx) const
{
return (parameters->options().icf_enabled()
&& this->icf_->is_section_folded(obj, shndx));
@@ -650,10 +650,11 @@ Symbol_table::gc_mark_symbol(Symbol* sym)
// Add the object and section to the work list.
bool is_ordinary;
unsigned int shndx = sym->shndx(&is_ordinary);
- if (is_ordinary && shndx != elfcpp::SHN_UNDEF)
+ if (is_ordinary && shndx != elfcpp::SHN_UNDEF && !sym->object()->is_dynamic())
{
gold_assert(this->gc_!= NULL);
- this->gc_->worklist().push_back(Section_id(sym->object(), shndx));
+ Relobj* relobj = static_cast<Relobj*>(sym->object());
+ this->gc_->worklist().push_back(Section_id(relobj, shndx));
}
parameters->target().gc_mark_symbol(this, sym);
}
diff --git a/gold/symtab.h b/gold/symtab.h
index 6f47c5e..f6c0ac6 100644
--- a/gold/symtab.h
+++ b/gold/symtab.h
@@ -1372,7 +1372,7 @@ class Symbol_table
// Returns true if ICF determined that this is a duplicate section.
bool
- is_section_folded(Object* obj, unsigned int shndx) const;
+ is_section_folded(Relobj* obj, unsigned int shndx) const;
void
set_gc(Garbage_collection* gc)
diff --git a/gold/target.h b/gold/target.h
index 95bc57e..dfbc5ee 100644
--- a/gold/target.h
+++ b/gold/target.h
@@ -1056,9 +1056,9 @@ class Sized_target : public Target
// and DST_OFF.
void
gc_add_reference(Symbol_table* symtab,
- Object* src_obj,
+ Relobj* src_obj,
unsigned int src_shndx,
- Object* dst_obj,
+ Relobj* dst_obj,
unsigned int dst_shndx,
typename elfcpp::Elf_types<size>::Elf_Addr dst_off) const
{
@@ -1080,8 +1080,8 @@ class Sized_target : public Target
// Handle target specific gc actions when adding a gc reference.
virtual void
- do_gc_add_reference(Symbol_table*, Object*, unsigned int,
- Object*, unsigned int,
+ do_gc_add_reference(Symbol_table*, Relobj*, unsigned int,
+ Relobj*, unsigned int,
typename elfcpp::Elf_types<size>::Elf_Addr) const
{ }
next prev parent reply other threads:[~2015-04-27 20:17 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-23 2:27 Rafael Espíndola
2015-04-27 20:17 ` Cary Coutant [this message]
2015-04-27 22:03 ` Rafael Espíndola
2015-05-02 15:49 ` Cary Coutant
2015-04-28 18:16 ` Sriraman Tallam
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJimCsFzvW5u1BefoXZm79exW2=3kf+cHOH96DntfXouL-CCFw@mail.gmail.com' \
--to=ccoutant@gmail.com \
--cc=binutils@sourceware.org \
--cc=rafael.espindola@gmail.com \
--cc=tmsriram@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).