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]

Re: static link with gold + gc-sections does not flush stdout buffer to file.


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]