[gold][patch] Fix non-PIC warning to print only when building position-independent output

Cary Coutant ccoutant@google.com
Thu Mar 26 18:33:00 GMT 2009


>> If a missing -fPIC really is the problem, we will still print that. If
>> you're building a non-position-independent executable, however, you
>> shouldn't be told to recompile the code with -fPIC, because that's not
>> supposed to be required for code in an executable. Instead, it points
>> to a problem either with hand-written assembly code (most likely, I
>> think), or with the compiler, or with the linker. If the only
>> possibility were a linker bug, I could just put an assert here and let
>> the linker die:
>>
>> +      gold_assert(parameters->options().output_is_position_independent());
>> +      object->error(_("requires unsupported dynamic reloc; "
>> +                      "recompile with -fPIC"));
>> +      this->issued_unsupp_reloc_error_ = true;
>>
>> I don't think that's the only (or even the most likely) possibility, though.
>
> How can hand-written or compiler-generated assembly code require an
> unsupported dynamic relocation in a non-PIC executable?  On all
> platforms I can think of except for VxWorks RTPs, both PIC and non-PIC
> inputs are valid for executables.

I'm not actually sure -- I just haven't yet convinced myself that it
can't happen. It would have to be a reference to a data symbol in a
shared library for which we wouldn't choose to create a COPY
relocation, using a relocation type that the dynamic linker doesn't
support. Looking at the code we have now in the existing targets, it
does indeed look like we'll always generate a COPY relocation in cases
where we would otherwise try to produce an unsupported dynamic
relocation, so I think it would be safe to go with the assert. In
fact, there are only a couple of places where check_non_pic was called
from places where output_is_position_independent isn't obviously true
-- a better patch might just be to leave check_non_pic as it is, and
put the assert in front of the few calls to check_non_pic where we
haven't already established the condition explicitly. That patch is
attached below for comparison. Tested on x86_64.

Having seen the comments from you and Hans-Peter, and having thought
about it overnight, I think I like this patch better.

-cary


       * powerpc.cc (Target_powerpc::Scan::global): Add assertion
before calls to check_non_pic.
       * sparc.cc (Target_sparc::Scan::global): Likewise.
       * x86_64.cc (Target_x86_64::Scan::global): Likewise.


Index: powerpc.cc
===================================================================
RCS file: /cvs/src/src/gold/powerpc.cc,v
retrieving revision 1.11
diff -u -p -r1.11 powerpc.cc
--- powerpc.cc	24 Mar 2009 04:50:32 -0000	1.11
+++ powerpc.cc	26 Mar 2009 18:26:54 -0000
@@ -1313,6 +1313,8 @@ Target_powerpc<size, big_endian>::Scan::
               {
                 Reloc_section* rela_dyn = target->rela_dyn_section(layout);

+                gold_assert(parameters->
+                  options().output_is_position_independent());
 		check_non_pic(object, r_type);
 		if (gsym->is_from_dynobj()
 		    || gsym->is_undefined()
@@ -1356,6 +1358,8 @@ Target_powerpc<size, big_endian>::Scan::
 	    else
 	      {
 		Reloc_section* rela_dyn = target->rela_dyn_section(layout);
+                gold_assert(parameters->
+                  options().output_is_position_independent());
 		check_non_pic(object, r_type);
 		rela_dyn->add_global(gsym, r_type, output_section, object,
 				     data_shndx, reloc.get_r_offset(),
Index: sparc.cc
===================================================================
RCS file: /cvs/src/src/gold/sparc.cc,v
retrieving revision 1.15
diff -u -p -r1.15 sparc.cc
--- sparc.cc	24 Mar 2009 04:50:32 -0000	1.15
+++ sparc.cc	26 Mar 2009 18:26:54 -0000
@@ -1984,6 +1984,8 @@ Target_sparc<size, big_endian>::Scan::gl
 	    else
 	      {
 		Reloc_section* rela_dyn = target->rela_dyn_section(layout);
+                gold_assert(parameters->
+                  options().output_is_position_independent());
 		check_non_pic(object, r_type);
 		rela_dyn->add_global(gsym, orig_r_type, output_section, object,
 				     data_shndx, reloc.get_r_offset(),
@@ -2050,6 +2052,8 @@ Target_sparc<size, big_endian>::Scan::gl
               {
                 Reloc_section* rela_dyn = target->rela_dyn_section(layout);

+                gold_assert(parameters->
+                  options().output_is_position_independent());
 		check_non_pic(object, r_type);
 		if (gsym->is_from_dynobj()
 		    || gsym->is_undefined()
Index: x86_64.cc
===================================================================
RCS file: /cvs/src/src/gold/x86_64.cc,v
retrieving revision 1.80
diff -u -p -r1.80 x86_64.cc
--- x86_64.cc	24 Mar 2009 00:31:29 -0000	1.80
+++ x86_64.cc	26 Mar 2009 18:26:54 -0000
@@ -1322,6 +1322,8 @@ Target_x86_64::Scan::global(const Genera
               }
             else
               {
+                gold_assert(parameters->
+                  options().output_is_position_independent());
                 this->check_non_pic(object, r_type);
                 Reloc_section* rela_dyn = target->rela_dyn_section(layout);
                 rela_dyn->add_global(gsym, r_type, output_section, object,
@@ -1353,6 +1355,8 @@ Target_x86_64::Scan::global(const Genera
               }
             else
               {
+                gold_assert(parameters->
+                  options().output_is_position_independent());
                 this->check_non_pic(object, r_type);
                 Reloc_section* rela_dyn = target->rela_dyn_section(layout);
                 rela_dyn->add_global(gsym, r_type, output_section, object,



More information about the Binutils mailing list