[gold RFC] Do not generate dynamic reloc for unresolved TLS symbol

[gold RFC] Do not generate dynamic reloc for unresolved TLS symbol

[Cary Coutant]

I have an internal bug report where they're trying to link an object
file with an unresolved TLS symbol (using --warn-unresolved-symbols).
For non-TLS symbols, when linking an executable, we statically resolve
the symbols to 0 and do not generate a dynamic relocation (this is
handled in Symbol::needs_dynamic_reloc()). But for TLS symbols, we do
not call this function, and we generate the dynamic relocation
regardless.

I believe that the right fix is to change
Symbol::final_value_is_known() to return true for undefined symbols
when the output is executable. We would then treat the relocation as
one that could be optimized to the LE model, and would statically
resolve the symbol to 0 with no dynamic relocation. This would match
Gnu ld behavior.

The following patch passes all tests on x86_64, but I wanted to give
you all a chance to comment on it before I commit, since the change
has a broader effect than just TLS symbols, and I might have missed
something.

(Before I commit, I'll add a test case for --warn-unresolved-symbols,
with unresolved TLS and non-TLS symbols.)

-cary


diff --git a/gold/symtab.cc b/gold/symtab.cc
index c433018..341e0fd 100644
--- a/gold/symtab.cc
+++ b/gold/symtab.cc
@@ -462,11 +462,11 @@ Symbol::final_value_is_known() const
        return true;
     }

-  // If the symbol is undefined, then whether the final value is known
-  // depends on whether we are doing a static link.  If we are doing a
-  // dynamic link, then the final value could be filled in at runtime.
-  // This could reasonably be the case for a weak undefined symbol.
-  return parameters->doing_static_link();
+  // The symbol is unresolved.  If we are building an executable,
+  // unresolved symbols are statically resolved to 0, so the final value
+  // is known.  If we're doing a shared link, they can be resolved
+  // dynamically, so the final values are not known.
+  return parameters->options().output_is_executable();
 }

 // Return the output section where this symbol is defined.

Re: [gold RFC] Do not generate dynamic reloc for unresolved TLS symbol

[Alan Modra]

On Thu, Dec 18, 2014 at 10:50:07AM -0800, Cary Coutant wrote:
> --- a/gold/symtab.cc
> +++ b/gold/symtab.cc
> @@ -462,11 +462,11 @@ Symbol::final_value_is_known() const
>         return true;
>      }
> 
> -  // If the symbol is undefined, then whether the final value is known
> -  // depends on whether we are doing a static link.  If we are doing a
> -  // dynamic link, then the final value could be filled in at runtime.
> -  // This could reasonably be the case for a weak undefined symbol.
> -  return parameters->doing_static_link();
> +  // The symbol is unresolved.  If we are building an executable,
> +  // unresolved symbols are statically resolved to 0, so the final value
> +  // is known.  If we're doing a shared link, they can be resolved
> +  // dynamically, so the final values are not known.
> +  return parameters->options().output_is_executable();
>  }
> 
>  // Return the output section where this symbol is defined.

As the comment says it "could reasonably be the case for a weak
undefined symbol".  If the reference to a weak symbol is in PIC, then
you can emit dynamic relocs and have an executable do something
reasonable based on whether the symbol is defined at runtime.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [gold RFC] Do not generate dynamic reloc for unresolved TLS symbol

[Cary Coutant]

> As the comment says it "could reasonably be the case for a weak
> undefined symbol".  If the reference to a weak symbol is in PIC, then
> you can emit dynamic relocs and have an executable do something
> reasonable based on whether the symbol is defined at runtime.

With my patch, I get an internal error for the unresolved TLS symbol
when trying to apply a (PIC) R_X86_64_TLSGD relocation.

For PIC code, Gnu ld will generate dynamic relocations for a non-TLS
symbol or function symbol, but not for a TLS symbol. Is that a bug, or
behavior that gold should match?

So, yeah, it looks like the expected behavior depends on the
relocation, so patching it in target-independent code isn't going to
work (unless I pass an is_pic_relocation flag to
final_value_is_known()).

Making the symbols weak vs. using --warn-unresolved-symbols doesn't
seem to make a difference -- Either way, Gnu ld will statically
resolve non-PIC references to 0, and will make dynamic relocs for PIC
non-TLS references.

-cary

Re: [gold RFC] Do not generate dynamic reloc for unresolved TLS symbol

[Cary Coutant]

[-- Attachment #1: Type: text/plain, Size: 627 bytes --]

> So, yeah, it looks like the expected behavior depends on the
> relocation, so patching it in target-independent code isn't going to
> work (unless I pass an is_pic_relocation flag to
> final_value_is_known()).

I've committed the following patch to make this work for x86_64.

Alan, do you think something similar is needed for PPC?

-cary


2015-01-28  Cary Coutant  <ccoutant@google.com>

gold/
        * x86_64.cc (Target_x86_64::Scan::global): Allow IE-to-LE optimization
        for undef TLS symbols.
        (Target_x86_64::Relocate::relocate_tls): Likewise.
        (Target_x86_64::Relocate::tls_ie_to_le): Likewise.

[-- Attachment #2: patch-undef-tls.txt --]
[-- Type: text/plain, Size: 2305 bytes --]

Allow undefined references to TLS symbols.

When --warn-unresolved-symbols is used, gold tries to create a dynamic relocation
for it, and gives an internal error if the TLS segment has not already been
created. This patch allows the IE-to-LE optimization for an undefined symbol
when building an executable, which suppresses the dynamic relocation, and
relaxes the requirement to have a TLS segment when applying a relocation for
an undefined symbol.


2015-01-28  Cary Coutant  <ccoutant@google.com>

gold/
	* x86_64.cc (Target_x86_64::Scan::global): Allow IE-to-LE optimization
	for undef TLS symbols.
	(Target_x86_64::Relocate::relocate_tls): Likewise.
	(Target_x86_64::Relocate::tls_ie_to_le): Likewise.


diff --git a/gold/x86_64.cc b/gold/x86_64.cc
index dd6c07b..a4aebc8 100644
--- a/gold/x86_64.cc
+++ b/gold/x86_64.cc
@@ -2983,7 +2983,12 @@ Target_x86_64<size>::Scan::global(Symbol_table* symtab,
     case elfcpp::R_X86_64_GOTTPOFF:         // Initial-exec
     case elfcpp::R_X86_64_TPOFF32:          // Local-exec
       {
-	const bool is_final = gsym->final_value_is_known();
+	// For the Initial-Exec model, we can treat undef symbols as final
+	// when building an executable.
+	const bool is_final = (gsym->final_value_is_known() ||
+			       (r_type == elfcpp::R_X86_64_GOTTPOFF &&
+			        gsym->is_undefined() &&
+				parameters->options().output_is_executable()));
 	const tls::Tls_optimization optimized_type
 	    = Target_x86_64<size>::optimize_tls_reloc(is_final, r_type);
 	switch (r_type)
@@ -3779,7 +3784,15 @@ Target_x86_64<size>::Relocate::relocate_tls(
       break;
 
     case elfcpp::R_X86_64_GOTTPOFF:         // Initial-exec
-      if (optimized_type == tls::TLSOPT_TO_LE)
+      if (gsym != NULL && gsym->is_undefined())
+	{
+	  Target_x86_64<size>::Relocate::tls_ie_to_le(relinfo, relnum,
+						      NULL, rela,
+						      r_type, value, view,
+						      view_size);
+	  break;
+	}
+      else if (optimized_type == tls::TLSOPT_TO_LE)
 	{
 	  if (tls_segment == NULL)
 	    {
@@ -4126,7 +4139,8 @@ Target_x86_64<size>::Relocate::tls_ie_to_le(
       view[-1] = 0x80 | reg | (reg << 3);
     }
 
-  value -= tls_segment->memsz();
+  if (tls_segment != NULL)
+    value -= tls_segment->memsz();
   Relocate_functions<size, false>::rela32(view, value, 0);
 }
 

Re: [gold RFC] Do not generate dynamic reloc for unresolved TLS symbol

[Alan Modra]

On Wed, Jan 28, 2015 at 03:57:54PM -0800, Cary Coutant wrote:
> created. This patch allows the IE-to-LE optimization for an undefined symbol
> when building an executable,

Thinking out loud here.  That looks like a contradiction to me.  If
the symbol is undefined, how can it be local-exec?

Oh, is this the undefined weak case?  Hmm, seems to me that undefined
weaks don't play well with TLS symbols..  Even for initial-exec you
won't have its address resolve to zero as you do with non-TLS symbols,
because tp is always added.  So
  if (foo)
    {
      /* Do something when foo is defined.  */
    }
will "Do something" even when foo is undefined.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [gold RFC] Do not generate dynamic reloc for unresolved TLS symbol

[Cary Coutant]

>> created. This patch allows the IE-to-LE optimization for an undefined symbol
>> when building an executable,
>
> Thinking out loud here.  That looks like a contradiction to me.  If
> the symbol is undefined, how can it be local-exec?

If it's undefined in an executable (and the reference isn't PIC), then
we statically resolve it to zero. That means it can be local-exec.

> Oh, is this the undefined weak case?  Hmm, seems to me that undefined
> weaks don't play well with TLS symbols..  Even for initial-exec you
> won't have its address resolve to zero as you do with non-TLS symbols,
> because tp is always added.  So
>   if (foo)
>     {
>       /* Do something when foo is defined.  */
>     }
> will "Do something" even when foo is undefined.

Yes, for TLS symbols, resolving to zero doesn't *really* resolve to
zero. We just resolve to TP+0 because that's the best we can do (it's
what BFD ld does).

The particular bug I'm responding to isn't a weak undefined -- it's a
normal undefined, linked with --warn-unresolved-symbols, and the code
containing the reference won't get executed (it's an artifact from
gcc's lightweight IPO implementation). In that case, gold was creating
a dynamic relocation, which caused an unsatisfied symbol error from
the dynamic loader. In addition, in a small test case with no other
TLS symbols, it caused gold to assert because we expect a TLS segment
when applying the relocation.

-cary