Patch to fix a bug with version scripts and gc-sections in the GOLD linker.

Patch to fix a bug with version scripts and gc-sections in the GOLD linker.

[Sriraman Tallam]

[-- 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();

Re: Patch to fix a bug with version scripts and gc-sections in the GOLD linker.

[Ian Lance Taylor]

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

Re: Patch to fix a bug with version scripts and gc-sections in the GOLD linker.

[Sriraman Tallam]

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
>