* Patch to fix a bug with version scripts and gc-sections in the GOLD linker.
@ 2011-07-22 22:39 Sriraman Tallam
2011-07-23 10:52 ` Ian Lance Taylor
0 siblings, 1 reply; 3+ messages in thread
From: Sriraman Tallam @ 2011-07-22 22:39 UTC (permalink / raw)
To: binutils, Ian Lance Taylor
[-- Attachment #1: Type: text/plain, Size: 769 bytes --]
Hi,
Gold garbage collection does not do the right thing with this program:
foobar.cc
--------------
extern "C"
int bar()
{
return 144;
}
extern "C"
int foo()
{
return 121;
}
sym.ver (version_script)
-----------
{
global:
foo;
local:
bar;
};
$ g++ -fPIC -shared -Wl,--gc-sections foobar.cc -o foobar.so
-Wl,--version-script,sym.ver
$ nm foobar.so | grep bar
00000000000066a t bar
Function bar is not garbage collected even though it is local.
This simple patch fixes it.
* symtab.cc (Symbol_table::gc_mark_symbol_for_shlib): Check if symbol
is not forced local before marking it as referenced.
(Symbol_table::add_from_relobj): Mark symbol as referenced only after
checking if they cannot be forced local.
Ok to submit?
Thanks,
-Sri.
[-- Attachment #2: gc_version_script.txt --]
[-- Type: text/plain, Size: 1230 bytes --]
Index: symtab.cc
===================================================================
RCS file: /cvs/src/src/gold/symtab.cc,v
retrieving revision 1.157
diff -u -u -p -r1.157 symtab.cc
--- symtab.cc 8 Jul 2011 23:49:11 -0000 1.157
+++ symtab.cc 22 Jul 2011 21:40:19 -0000
@@ -605,7 +605,8 @@ void
Symbol_table::gc_mark_symbol_for_shlib(Symbol* sym)
{
if (!sym->is_from_dynobj()
- && sym->is_externally_visible())
+ && sym->is_externally_visible()
+ && !sym->is_forced_local())
{
//Add the object and section to the work list.
Relobj* obj = static_cast<Relobj*>(sym->object());
@@ -1238,15 +1239,15 @@ Symbol_table::add_from_relobj(
is_default_version, *psym, st_shndx,
is_ordinary, orig_st_shndx);
+ if (is_forced_local)
+ this->force_local(res);
+
// If building a shared library using garbage collection, do not
// treat externally visible symbols as garbage.
if (parameters->options().gc_sections()
&& parameters->options().shared())
this->gc_mark_symbol_for_shlib(res);
- if (is_forced_local)
- this->force_local(res);
-
if (is_defined_in_discarded_section)
res->set_is_defined_in_discarded_section();
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Patch to fix a bug with version scripts and gc-sections in the GOLD linker.
2011-07-22 22:39 Patch to fix a bug with version scripts and gc-sections in the GOLD linker Sriraman Tallam
@ 2011-07-23 10:52 ` Ian Lance Taylor
2011-07-24 14:23 ` Sriraman Tallam
0 siblings, 1 reply; 3+ messages in thread
From: Ian Lance Taylor @ 2011-07-23 10:52 UTC (permalink / raw)
To: Sriraman Tallam; +Cc: binutils
Sriraman Tallam <tmsriram@google.com> writes:
> Function bar is not garbage collected even though it is local.
>
> This simple patch fixes it.
>
> * symtab.cc (Symbol_table::gc_mark_symbol_for_shlib): Check if symbol
> is not forced local before marking it as referenced.
> (Symbol_table::add_from_relobj): Mark symbol as referenced only after
> checking if they cannot be forced local.
I would like to suggest that you instead change
Symbol::is_externally_visible to return false if this->is_forced_local()
is true. I think that will fix your problem, and I don't think think
that will break anything--but, then, I haven't tried it.
If that patch works, it is preapproved.
Thanks.
Ian
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Patch to fix a bug with version scripts and gc-sections in the GOLD linker.
2011-07-23 10:52 ` Ian Lance Taylor
@ 2011-07-24 14:23 ` Sriraman Tallam
0 siblings, 0 replies; 3+ messages in thread
From: Sriraman Tallam @ 2011-07-24 14:23 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: binutils
Hi Ian,
That change does not break any test case. I submitted it after changing it.
Thanks,
-Sri.
2011-07-22 Sriraman Tallam <tmsriram@google.com>
* symtab.cc (Symbol_table::add_from_relobj): Mark symbol as referenced
only after checking if it cannot be forced local.
* symtab.h (is_externally_visible): Check if the symbol is not forced
local.
Index: symtab.cc
===================================================================
RCS file: /cvs/src/src/gold/symtab.cc,v
retrieving revision 1.157
diff -u -u -p -r1.157 symtab.cc
--- symtab.cc 8 Jul 2011 23:49:11 -0000 1.157
+++ symtab.cc 22 Jul 2011 22:30:19 -0000
@@ -1238,15 +1238,15 @@ Symbol_table::add_from_relobj(
is_default_version, *psym, st_shndx,
is_ordinary, orig_st_shndx);
+ if (is_forced_local)
+ this->force_local(res);
+
// If building a shared library using garbage collection, do not
// treat externally visible symbols as garbage.
if (parameters->options().gc_sections()
&& parameters->options().shared())
this->gc_mark_symbol_for_shlib(res);
- if (is_forced_local)
- this->force_local(res);
-
if (is_defined_in_discarded_section)
res->set_is_defined_in_discarded_section();
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gold/symtab.h,v
retrieving revision 1.124
diff -u -u -p -r1.124 symtab.h
--- symtab.h 8 Jul 2011 23:49:11 -0000 1.124
+++ symtab.h 22 Jul 2011 22:30:19 -0000
@@ -538,8 +538,9 @@ class Symbol
bool
is_externally_visible() const
{
- return (this->visibility_ == elfcpp::STV_DEFAULT
- || this->visibility_ == elfcpp::STV_PROTECTED);
+ return ((this->visibility_ == elfcpp::STV_DEFAULT
+ || this->visibility_ == elfcpp::STV_PROTECTED)
+ && !this->is_forced_local_);
}
Thanks,
-Sri.
On Fri, Jul 22, 2011 at 3:09 PM, Ian Lance Taylor <iant@google.com> wrote:
> Sriraman Tallam <tmsriram@google.com> writes:
>
>> Function bar is not garbage collected even though it is local.
>>
>> This simple patch fixes it.
>>
>> * symtab.cc (Symbol_table::gc_mark_symbol_for_shlib): Check if symbol
>> is not forced local before marking it as referenced.
>> (Symbol_table::add_from_relobj): Mark symbol as referenced only after
>> checking if they cannot be forced local.
>
> I would like to suggest that you instead change
> Symbol::is_externally_visible to return false if this->is_forced_local()
> is true. I think that will fix your problem, and I don't think think
> that will break anything--but, then, I haven't tried it.
>
> If that patch works, it is preapproved.
>
> Thanks.
>
> Ian
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-07-22 22:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-22 22:39 Patch to fix a bug with version scripts and gc-sections in the GOLD linker Sriraman Tallam
2011-07-23 10:52 ` Ian Lance Taylor
2011-07-24 14:23 ` Sriraman Tallam
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).