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

[Sriraman Tallam <tmsriram at google dot com>]

On Wed, Jan 6, 2010 at 5:48 PM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>> + ? ? ?// Add the orphan sections referenced by this section.
>
> Orphan sections is GNU ld phrasing. ?The sections are orphaned because
> they are not explicitly mentioned in the linker script. ?In gold these
> sections should have a different name.
>
>
>> + ? ? ?Garbage_collection::Tracked_orphan_section_range key_range =
>> + ? ? ? ?this->tracked_orphan_sections_.equal_range(entry);
>> + ? ? ?Garbage_collection::Tracked_orphan_section_map::iterator it;
>> + ? ? ?for (it = key_range.first; it != key_range.second; ++it)
>> + ? ? ? ?{
>> + ? ? ? ? ?std::string orphan_section_name = it->second;
>> + ? ? ? ? ?Garbage_collection::Orphan_section_map::iterator ele =
>> + ? ? ? ? ? ?this->orphan_sections_.find(orphan_section_name);
>> + ? ? ? ? ?gold_assert (ele != this->orphan_sections_.end());
>
> Remove space before parenthesis.
>
>> + ? ? ? ? ?Garbage_collection::Sections_reachable v = ele->second;
>> + ? ? ? ? ?for (Garbage_collection::Sections_reachable::iterator it_v
>> + ? ? ? ? ? ? ? = v.begin();
>> + ? ? ? ? ? ? ? it_v != v.end();
>> + ? ? ? ? ? ? ? ++it_v)
>
> Indent "= v.begin();" two more spaces, I think.
>
>> + ? ? ? ? ? ?{
>> + ? ? ? ? ? ? ?if (it_v->first == NULL)
>> + ? ? ? ? ? ? ? ?continue;
>> + ? ? ? ? ? ? ?this->worklist().push(*it_v);
>> + ? ? ? ? ? ?}
>> + ? ? ? ?}
>
>
>
>> +void
>> +Garbage_collection::track_orphan_section(Section_id secn,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? std::string section_name)
>> +{
>> + ?this->tracked_orphan_sections_.insert(std::make_pair(secn, section_name));
>> + ?if (this->orphan_sections_.find(section_name)
>> + ? ? ?== this->orphan_sections_.end())
>> + ? ?this->orphan_sections_[section_name].insert(Section_id(NULL, 0));
>> +}
>> +
>> ?} // End namespace gold.
>>
>> Index: gc.h
>> ===================================================================
>> RCS file: /cvs/src/src/gold/gc.h,v
>> retrieving revision 1.6
>> diff -u -u -p -r1.6 gc.h
>> --- gc.h ? ? ?4 Jan 2010 19:08:39 -0000 ? ? ? 1.6
>> +++ gc.h ? ? ?7 Jan 2010 00:46:50 -0000
>> @@ -60,6 +60,17 @@ class Garbage_collection
>> ? ?typedef Unordered_set<Section_id, Section_id_hash> Sections_reachable;
>> ? ?typedef std::map<Section_id, Sections_reachable> Section_ref;
>> ? ?typedef std::queue<Section_id> Worklist_type;
>> + ?// This maps a section to the name of the orphan section it references.
>> + ?// A section can reference more than one orphan section.
>> + ?typedef Unordered_multimap<Section_id, std::string, Section_id_hash>
>> + ? ?Tracked_orphan_section_map;
>> + ?typedef std::pair<Tracked_orphan_section_map::iterator,
>> + ? ? ? ? ? ? ? ? ? ?Tracked_orphan_section_map::iterator>
>> + ? ?Tracked_orphan_section_range;
>> + ?// This maps the name of the orphan section to the list of sections that
>> + ?// have that name. ?Different object files can have orphan sections with
>> + ?// the same name.
>> + ?typedef std::map<std::string, Sections_reachable> Orphan_section_map;
>>
>> ? ?Garbage_collection()
>> ? ?: is_worklist_ready_(false)
>> @@ -94,12 +105,26 @@ class Garbage_collection
>> ? ?is_section_garbage(Object* obj, unsigned int shndx)
>> ? ?{ return (this->referenced_list().find(Section_id(obj, shndx))
>> ? ? ? ? ? ? ?== this->referenced_list().end()); }
>> +
>> + ?void
>> + ?track_orphan_section(Section_id secn, std::string section_name);
>> +
>> + ?Tracked_orphan_section_map&
>> + ?tracked_orphan_sections()
>> + ?{ return tracked_orphan_sections_; }
>> +
>> + ?Orphan_section_map&
>> + ?orphan_sections()
>> + ?{ return orphan_sections_; }
>> +
>> ? private:
>>
>> ? ?Worklist_type work_list_;
>> ? ?bool is_worklist_ready_;
>> ? ?Section_ref section_reloc_map_;
>> ? ?Sections_reachable referenced_list_;
>> + ?Tracked_orphan_section_map tracked_orphan_sections_;
>> + ?Orphan_section_map orphan_sections_;
>> ?};
>>
>> ?// Data to pass between successive invocations of do_layout
>> @@ -217,7 +242,23 @@ gc_process_relocs(
>> ? ? ? ? ? ?dst_indx = gsym->shndx(&is_ordinary);
>> ? ? ? ? ? ?if (!is_ordinary)
>> ? ? ? ? ? ? ?continue;
>> + ? ? ? ? ?Section_id src_id(src_obj, src_indx);
>> ? ? ? ? ? ?Section_id dst_id(dst_obj, dst_indx);
>> + ? ? ? ? ?// If the symbol name matches '__start_XXX' then the orphan
>> + ? ? ? ? ?// section with the name 'XXX' should not be garbage collected.
>> + ? ? ? ? ?// A similar treatment to symbols with the name '__stop_XXX'.
>> + ? ? ? ? ?if (is_prefix_of("__start_", gsym->name()))
>
> Let's make __start and __stop_ constants somewhere, rather than
> repeating them here and in layout.cc.
>
>> + ? ? ? ? ? ?{
>> + ? ? ? ? ? ? ?symtab->gc()->track_orphan_section(src_id,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? std::string(gsym->name()
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? + strlen("__start_")));
>
> Use parentheses around the + expression so that the + is indented one space.
>
>> + ? ? ? ? ? ?}
>> + ? ? ? ? ?else if (is_prefix_of("__stop_", gsym->name()))
>> + ? ? ? ? ? ?{
>> + ? ? ? ? ? ? ?symtab->gc()->track_orphan_section(src_id,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? std::string(gsym->name()
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? + strlen("__stop_")));
>
> Parentheses here too.
>
>
>> Index: symtab.cc
>> ===================================================================
>> RCS file: /cvs/src/src/gold/symtab.cc,v
>> retrieving revision 1.136
>> diff -u -u -p -r1.136 symtab.cc
>> --- symtab.cc 5 Jan 2010 19:29:15 -0000 ? ? ? 1.136
>> +++ symtab.cc 7 Jan 2010 00:46:50 -0000
>> @@ -524,6 +524,28 @@ Symbol_table::is_section_folded(Object*
>> ? ? ? ? ? ?&& this->icf_->is_section_folded(obj, shndx));
>> ?}
>>
>> +// Find the orphan sections that are tracked.
>> +
>> +void
>> +Symbol_table::gc_update_orphan_sections(const Input_objects* input_objects)
>> +{
>> + ?for (Input_objects::Relobj_iterator p = input_objects->relobj_begin();
>> + ? ? ? p != input_objects->relobj_end();
>> + ? ? ? ++p)
>> + ? ?{
>> + ? ? ?for (unsigned int i = 0;i < (*p)->shnum(); ++i)
>
> Space after first semicolon.
>
>> + ? ? ? ?{
>> + ? ? ? ? ?std::string section_name = (*p)->section_name(i);
>> + ? ? ? ? ?Garbage_collection::Orphan_section_map::iterator it =
>> + ? ? ? ? ? ?this->gc_->orphan_sections().find(section_name);
>> + ? ? ? ? ?if (it == this->gc_->orphan_sections().end())
>> + ? ? ? ? ? ?continue;
>> + ? ? ? ? ?Garbage_collection::Sections_reachable& v(it->second);
>> + ? ? ? ? ?v.insert(Section_id(*p, i));
>> + ? ? ? ?}
>> + ? ?}
>> +}
>
> This loop is going to be very slow, because section_name() is a slow
> call. ?This is not a good way to do this.
>
> For the case that matters, you can look for an output section with the
> name that matters. ?Then you need to find all the input sections that
> are attached to that output section. ?You can do this if you change
> Output_section::add_input_section to use the
> this->input_sections_.push_back line when you are garbage collecting

Thanks for the quick feed-back.

I am not sure I understand this. The output_sections are not assigned
in the first pass of do_layout and I need to find the orphaned
sections well before gc can start its transitive closure, right ?
add_input_section kicks in only after gc is finished and during when
do_layout is called the second time.

>
>
>> +
>> ?// For symbols that have been listed with -u option, add them to the
>> ?// work list to avoid gc'ing them.
>>
>> Index: symtab.h
>> ===================================================================
>> RCS file: /cvs/src/src/gold/symtab.h,v
>> retrieving revision 1.103
>> diff -u -u -p -r1.103 symtab.h
>> --- symtab.h ?31 Dec 2009 05:07:21 -0000 ? ? ?1.103
>> +++ symtab.h ?7 Jan 2010 00:46:50 -0000
>> @@ -1,6 +1,6 @@
>> ?// symtab.h -- the gold symbol table ? -*- C++ -*-
>>
>> -// Copyright 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
>> +// Copyright 2006, 2007, 2008, 2009, 2010 Free Software Foundation, Inc.
>> ?// Written by Ian Lance Taylor <iant@google.com>.
>>
>> ?// This file is part of gold.
>> @@ -1230,6 +1230,9 @@ class Symbol_table
>> ? ?gc() const
>> ? ?{ return this->gc_; }
>>
>> + ?void
>> + ?gc_update_orphan_sections(const Input_objects* input_objects);
>> +
>> ? ?// During garbage collection, this keeps undefined symbols.
>> ? ?void
>> ? ?gc_mark_undef_symbols();
>> cvs diff: Diffing po
>> cvs diff: Diffing testsuite
>> Index: testsuite/Makefile.am
>> ===================================================================
>> RCS file: /cvs/src/src/gold/testsuite/Makefile.am,v
>> retrieving revision 1.115
>> diff -u -u -p -r1.115 Makefile.am
>> --- testsuite/Makefile.am ? ? 31 Dec 2009 05:07:22 -0000 ? ? ?1.115
>> +++ testsuite/Makefile.am ? ? 7 Jan 2010 00:46:51 -0000
>> @@ -139,6 +139,16 @@ gc_tls_test:gc_tls_test.o gcctestdir/ld
>> ?gc_tls_test.stdout: gc_tls_test
>> ? ? ? $(TEST_NM) -C gc_tls_test > gc_tls_test.stdout
>>
>> +check_SCRIPTS += gc_orphan_section_test.sh
>> +check_DATA += gc_orphan_section_test.stdout
>> +MOSTLYCLEANFILES += gc_orphan_section_test
>> +gc_orphan_section_test.o: gc_orphan_section_test.cc
>> + ? ? $(CXXCOMPILE) -O0 -c -g -o $@ $<
>> +gc_orphan_section_test:gc_orphan_section_test.o gcctestdir/ld
>> + ? ? $(CXXLINK) -Bgcctestdir/ -Wl,--gc-sections gc_orphan_section_test.o
>> +gc_orphan_section_test.stdout: gc_orphan_section_test
>> + ? ? $(TEST_NM) gc_orphan_section_test > gc_orphan_section_test.stdout
>> +
>> ?check_SCRIPTS += icf_test.sh
>> ?check_DATA += icf_test.stdout
>> ?MOSTLYCLEANFILES += icf_test
>> Index: testsuite/gc_orphan_section_test.cc
>> ===================================================================
>> RCS file: testsuite/gc_orphan_section_test.cc
>> diff -N testsuite/gc_orphan_section_test.cc
>> --- /dev/null 1 Jan 1970 00:00:00 -0000
>> +++ testsuite/gc_orphan_section_test.cc ? ? ? 7 Jan 2010 00:46:51 -0000
>> @@ -0,0 +1,36 @@
>> +// gc_orphan_section_test.cc -- a test case for gold
>> +
>> +// Copyright 2010 Free Software Foundation, Inc.
>> +// Written by Sriraman Tallam <tmsriram@google.com>.
>> +
>> +// This file is part of gold.
>> +
>> +// This program is free software; you can redistribute it and/or modify
>> +// it under the terms of the GNU General Public License as published by
>> +// the Free Software Foundation; either version 3 of the License, or
>> +// (at your option) any later version.
>> +
>> +// This program is distributed in the hope that it will be useful,
>> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
>> +// GNU General Public License for more details.
>> +
>> +// You should have received a copy of the GNU General Public License
>> +// along with this program; if not, write to the Free Software
>> +// Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
>> +// MA 02110-1301, USA.
>> +
>> +// The goal of this program is to verify if garbage collection does not
>> +// discard orphan sections when references to them through __start_XXX
>> +// and __stop_XXX are present. ?Here section _foo must not be gc'ed but
>> +// _boo should be gc'ed.
>> +
>> +extern const int *__start__foo;
>> +int foo __attribute__((__section__("_foo"))) = 1;
>> +int boo __attribute__((__section__("_boo"))) = 1;
>> +
>> +int main()
>> +{
>> + ?return *__start__foo;
>> +}
>> +
>> Index: testsuite/gc_orphan_section_test.sh
>> ===================================================================
>> RCS file: testsuite/gc_orphan_section_test.sh
>> diff -N testsuite/gc_orphan_section_test.sh
>> --- /dev/null 1 Jan 1970 00:00:00 -0000
>> +++ testsuite/gc_orphan_section_test.sh ? ? ? 7 Jan 2010 00:46:51 -0000
>> @@ -0,0 +1,46 @@
>> +#!/bin/sh
>> +
>> +# gc_orphan_section_test.sh -- test --gc-sections
>> +
>> +# Copyright 2010 Free Software Foundation, Inc.
>> +# Written by Sriraman Tallam <tmsriram@google.com>.
>> +
>> +# This file is part of gold.
>> +
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 3 of the License, or
>> +# (at your option) any later version.
>> +
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. ?See the
>> +# GNU General Public License for more details.
>> +
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write to the Free Software
>> +# Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
>> +# MA 02110-1301, USA.
>> +
>> +# The goal of this program is to verify if gc-sections works as expected
>> +# with orphan sections.
>> +# File gc_orphan_sections_test.cc is in this test. This program checks if
>> +# the orphan sections are retained when they are referenced through
>> +# __start_XXX and __stop_XXX symbols.
>> +
>> +check()
>> +{
>> + ? ?if grep -q " boo" "$1"
>> + ? ?then
>> + ? ? ? ?echo "Garbage collection failed to collect boo"
>> + ? ? exit 1
>> + ? ?fi
>> + ? ?grep_foo=`grep -q " foo" $1`
>> + ? ?if [ $? != 0 ];
>> + ? ?then
>> + ? ? ? ?echo "Garbage collection should not discard foo"
>> + ? ? exit 1
>> + ? ?fi
>> +}
>> +
>> +check gc_orphan_section_test.stdout
>