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] |
I made the changes and committed this patch. 2010-01-06 Sriraman Tallam <tmsriram@google.com> * gc.h (Garbage_collection::Cident_section_map): New typedef. (Garbage_collection::cident_sections): New function. (Garbage_collection::add_cident_section): New function. (Garbage_collection::cident_sections_): New member. (gc_process_relocs): Add references to sections whose names are C identifiers. * gold.h (cident_section_start_prefix): New constant. (cident_section_stop_prefix): New constant. (is_cident): New function. * layout.cc (Layout::define_section_symbols): Replace string constants with the newly defined constants. * object.cc (Sized_relobj::do_layout): Track sections whose names are C identifiers. * testsuite/Makefile.am: Add gc_orphan_section_test. * testsuite/Makefile.in: Regenerate. * testsuite/gc_orphan_section_test.cc: New file. * testsuite/gc_orphan_section_test.sh: New file. On Wed, Jan 6, 2010 at 9:26 PM, Ian Lance Taylor <iant@google.com> wrote: > Sriraman Tallam <tmsriram@google.com> writes: > >> 2010-01-06 ?Sriraman Tallam ?<tmsriram@google.com> >> >> ? ? ? * gc.h (Garbage_collection::Cident_section_map): New typedef. >> ? ? ? (Garbage_collection::cident_sections): New function. >> ? ? ? (Garbage_collection::cident_sections_): New member. >> ? ? ? (gc_process_relocs): Add references to sections whose names are C >> ? ? ? identifiers. >> ? ? ? * gold.cc (cident_section_start_prefix): New constant. >> ? ? ? (cident_section_stop_prefix): New constant. >> ? ? ? * gold.h (cident_section_start_prefix): New constant. >> ? ? ? (cident_section_stop_prefix): New constant. >> ? ? ? * layout.cc (Layout::define_section_symbols): Replace string constants >> ? ? ? with the newly defined constants. >> ? ? ? * object.cc (Sized_relobj::do_layout): Track sections whose names are >> ? ? ? C identifiers. >> ? ? ? * testsuite/Makefile.am: Add gc_orphan_section_test. >> ? ? ? * testsuite/Makefile.in: Regenerate. >> ? ? ? * testsuite/gc_orphan_section_test.cc: New file. >> ? ? ? * testsuite/gc_orphan_section_test.sh: New file. > > >> + ?Cident_section_map& >> + ?cident_sections() >> + ?{ return cident_sections_; } > > I don't know if this is always followed, but the general rule should > be that if a class returns a reference, it should be a const > reference. ?Since code needs to modify this map, this should return a > pointer, not a reference. ?Alternatively and probably preferably, you > should write tiny functions which do the operations which need to be > done on cident_sections_. > > >> + ? ? ? ? ?if (cident_section_name) > > Write if (cident_section_name != NULL) > >> + ? ? ? ? ? ?{ >> + ? ? ? ? ? ? ?Garbage_collection::Sections_reachable& >> + ? ? ? ? ? ? ? ?v(symtab->gc()->section_reloc_map()[src_id]); >> + ? ? ? ? ? ? ?Garbage_collection::Cident_section_map::iterator ele = >> + ? ? ? ? ? ? ? ?symtab->gc()->cident_sections().find(std::string(cident_section_name)); >> + ? ? ? ? ? ? ?if (ele == symtab->gc()->cident_sections().end()) >> + ? ? ? ? ? ? ? ?continue; >> + ? ? ? ? ? ? ?Garbage_collection::Sections_reachable& cident_secn(ele->second); >> + ? ? ? ? ? ? ?for (Garbage_collection::Sections_reachable::iterator it_v >> + ? ? ? ? ? ? ? ? ? ? = cident_secn.begin(); >> + ? ? ? ? ? ? ? ? ? it_v != cident_secn.end(); >> + ? ? ? ? ? ? ? ? ? ++it_v) >> + ? ? ? ? ? ? ? ?{ >> + ? ? ? ? ? ? ? ? ?v.insert(*it_v); >> + ? ? ? ? ? ? ? ?} >> + ? ? ? ? ? ?} > > Move the definition of v to below the continue statement. > > >> Index: gold.cc >> =================================================================== >> RCS file: /cvs/src/src/gold/gold.cc,v >> retrieving revision 1.76 >> diff -u -u -p -r1.76 gold.cc >> --- gold.cc ? 6 Jan 2010 05:30:24 -0000 ? ? ? 1.76 >> +++ gold.cc ? 7 Jan 2010 05:04:33 -0000 >> @@ -49,6 +49,8 @@ namespace gold >> ?{ >> >> ?const char* program_name; >> +const char* cident_section_start_prefix = "__start_"; >> +const char* cident_section_stop_prefix = "__stop_"; >> >> ?void >> ?gold_exit(bool status) > > Move these to gold.h and write them as > > const char* const cident_section_start_prefix = "__start_"; > const char* const cident_section_stop_prefix = "__stop_"; > > >> + ? ? ? ? ?// If the section name XXX can be represented as a C identifier >> + ? ? ? ? ?// it cannot be discarded if there are references to >> + ? ? ? ? ?// __start_XXX and __stop_XXX symbols. ?These need to be >> + ? ? ? ? ?// specially handled. >> + ? ? ? ? ?if (is_cident(name)) >> + ? ? ? ? ? ?{ >> + ? ? ? ? ? ? ?Garbage_collection::Cident_section_map::iterator it = >> + ? ? ? ? ? ? ? ?symtab->gc()->cident_sections().find(std::string(name)); >> + ? ? ? ? ? ? ?if (it == symtab->gc()->cident_sections().end()) >> + ? ? ? ? ? ? ? ?{ >> + ? ? ? ? ? ? ? ? ?symtab->gc()->cident_sections()[name].insert( >> + ? ? ? ? ? ? ? ? ? ?Section_id(this, i)); >> + ? ? ? ? ? ? ? ?} >> + ? ? ? ? ? ? ?else >> + ? ? ? ? ? ? ? ?{ >> + ? ? ? ? ? ? ? ? ?Garbage_collection::Sections_reachable& v(it->second); >> + ? ? ? ? ? ? ? ? ?v.insert(Section_id(this, i)); >> + ? ? ? ? ? ? ? ?} >> + ? ? ? ? ? ?} > > This is a case where you are always going to insert, and you are > always going to do the same thing if you do insert, so here I don't > think it helps to do a find. ?I think you can just write > > symtab->gc()->cident_sections()[name].insert(Section_id(this, i)); > > except it should probably be more like > > symtab->gc()->add_cident_section(name, this, i); > > > This is OK with those changes. > > Thanks. > > Ian >
Attachment:
gold_cident_section_patch.txt
Description: Text document
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |