public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [gold][patch] Fix non-PIC warning to print only when building   position-independent output
@ 2009-03-26  0:01 Cary Coutant
  2009-03-26  0:12 ` Hans-Peter Nilsson
  0 siblings, 1 reply; 10+ messages in thread
From: Cary Coutant @ 2009-03-26  0:01 UTC (permalink / raw)
  To: Binutils

This patch renames check_non_pic to check_dynamic_reloc, and in the
case of an unsupported dynamic relocation, prints the hint to
"recompile with -fPIC" only when actually building a
position-independent output file. Under normal circumstances (after
yesterday's patch to close the bug where an unsatisfied data reference
could trigger this), we should never actually try to generated an
unsupported dynamic relocation when building a
non-position-independent file -- non-PIC references to dynamic objects
generally will be transformed into COPY relocations (for data) or
route through the PLT (for code). If we ever do hit this case in a
non-position-independent link, however, I hope it'll be the compiler's
fault, and we shouldn't print a message that implies that -fPIC is
actually required. (Maybe we should print something instead about a
possible bug in the compiler or assembly code?)

Compiles clean with --enable-targets=all. Tested on x86_64.

OK?

-cary


	* powerpc.cc (Target_powerpc::issued_unsupp_reloc_error_): Renamed,
	was issued_non_pic_error_.
	(Target_powerpc::check_dynamic_reloc): Renamed, was check_non_pic.
	Mention "-fPIC" only if building position-independent output.
	* sparc.cc (Target_sparc::issued_unsupp_reloc_error_): Renamed,
	was issued_non_pic_error_.
	(Target_sparc::check_dynamic_reloc): Renamed, was check_non_pic.
	Mention "-fPIC" only if building position-independent output.
	* x86_64.cc (Target_x86_64::issued_unsupp_reloc_error_): Renamed,
	was issued_non_pic_error_.
	(Target_x86_64::check_dynamic_reloc): Renamed, was check_non_pic.
	Mention "-fPIC" only if building position-independent output.


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	25 Mar 2009 23:41:12 -0000
@@ -165,7 +165,7 @@ class Target_powerpc : public Sized_targ
   {
   public:
     Scan()
-      : issued_non_pic_error_(false)
+      : issued_unsupp_reloc_error_(false)
     { }

     inline void
@@ -200,10 +200,10 @@ class Target_powerpc : public Sized_targ
 		      Target_powerpc* target);

     void
-    check_non_pic(Relobj*, unsigned int r_type);
+    check_dynamic_reloc(Relobj*, unsigned int r_type);

     // Whether we have issued an error about a non-PIC compilation.
-    bool issued_non_pic_error_;
+    bool issued_unsupp_reloc_error_;
   };

   // The class which implements relocation.
@@ -1013,7 +1013,7 @@ Target_powerpc<size, big_endian>::Scan::

 template<int size, bool big_endian>
 void
-Target_powerpc<size, big_endian>::Scan::check_non_pic(Relobj* object,
+Target_powerpc<size, big_endian>::Scan::check_dynamic_reloc(Relobj* object,
 						      unsigned int r_type)
 {
   gold_assert(r_type != elfcpp::R_POWERPC_NONE);
@@ -1092,11 +1092,14 @@ Target_powerpc<size, big_endian>::Scan::
   // This prevents us from issuing more than one error per reloc
   // section.  But we can still wind up issuing more than one
   // error per object file.
-  if (this->issued_non_pic_error_)
+  if (this->issued_unsupp_reloc_error_)
     return;
-  object->error(_("requires unsupported dynamic reloc; "
-		  "recompile with -fPIC"));
-  this->issued_non_pic_error_ = true;
+  if (parameters->options().output_is_position_independent())
+    object->error(_("requires unsupported dynamic reloc; "
+                    "recompile with -fPIC"));
+  else
+    object->error(_("requires unsupported dynamic reloc"));
+  this->issued_unsupp_reloc_error_ = true;
   return;
 }

@@ -1134,7 +1137,7 @@ Target_powerpc<size, big_endian>::Scan::
         {
           Reloc_section* rela_dyn = target->rela_dyn_section(layout);

-	  check_non_pic(object, r_type);
+	  check_dynamic_reloc(object, r_type);
           if (lsym.get_st_type() != elfcpp::STT_SECTION)
             {
               unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
@@ -1313,7 +1316,7 @@ Target_powerpc<size, big_endian>::Scan::
               {
                 Reloc_section* rela_dyn = target->rela_dyn_section(layout);

-		check_non_pic(object, r_type);
+		check_dynamic_reloc(object, r_type);
 		if (gsym->is_from_dynobj()
 		    || gsym->is_undefined()
 		    || gsym->is_preemptible())
@@ -1356,7 +1359,7 @@ Target_powerpc<size, big_endian>::Scan::
 	    else
 	      {
 		Reloc_section* rela_dyn = target->rela_dyn_section(layout);
-		check_non_pic(object, r_type);
+		check_dynamic_reloc(object, r_type);
 		rela_dyn->add_global(gsym, r_type, output_section, object,
 				     data_shndx, reloc.get_r_offset(),
 				     reloc.get_r_addend());
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	25 Mar 2009 23:41:12 -0000
@@ -175,7 +175,7 @@ class Target_sparc : public Sized_target
   {
   public:
     Scan()
-      : issued_non_pic_error_(false)
+      : issued_unsupp_reloc_error_(false)
     { }

     inline void
@@ -210,10 +210,10 @@ class Target_sparc : public Sized_target
 		      Target_sparc* target);

     void
-    check_non_pic(Relobj*, unsigned int r_type);
+    check_dynamic_reloc(Relobj*, unsigned int r_type);

     // Whether we have issued an error about a non-PIC compilation.
-    bool issued_non_pic_error_;
+    bool issued_unsupp_reloc_error_;
   };

   // The class which implements relocation.
@@ -1509,7 +1509,7 @@ Target_sparc<size, big_endian>::Scan::un

 template<int size, bool big_endian>
 void
-Target_sparc<size, big_endian>::Scan::check_non_pic(Relobj* object,
unsigned int r_type)
+Target_sparc<size, big_endian>::Scan::check_dynamic_reloc(Relobj*
object, unsigned int r_type)
 {
   gold_assert(r_type != elfcpp::R_SPARC_NONE);

@@ -1587,11 +1587,14 @@ Target_sparc<size, big_endian>::Scan::ch
   // This prevents us from issuing more than one error per reloc
   // section.  But we can still wind up issuing more than one
   // error per object file.
-  if (this->issued_non_pic_error_)
+  if (this->issued_unsupp_reloc_error_)
     return;
-  object->error(_("requires unsupported dynamic reloc; "
-		  "recompile with -fPIC"));
-  this->issued_non_pic_error_ = true;
+  if (parameters->options().output_is_position_independent())
+    object->error(_("requires unsupported dynamic reloc; "
+                    "recompile with -fPIC"));
+  else
+    object->error(_("requires unsupported dynamic reloc"));
+  this->issued_unsupp_reloc_error_ = true;
   return;
 }

@@ -1669,7 +1672,7 @@ Target_sparc<size, big_endian>::Scan::lo
         {
           Reloc_section* rela_dyn = target->rela_dyn_section(layout);

-	  check_non_pic(object, r_type);
+	  check_dynamic_reloc(object, r_type);
           if (lsym.get_st_type() != elfcpp::STT_SECTION)
             {
               unsigned int r_sym = elfcpp::elf_r_sym<size>(reloc.get_r_info());
@@ -1984,7 +1987,7 @@ Target_sparc<size, big_endian>::Scan::gl
 	    else
 	      {
 		Reloc_section* rela_dyn = target->rela_dyn_section(layout);
-		check_non_pic(object, r_type);
+		check_dynamic_reloc(object, r_type);
 		rela_dyn->add_global(gsym, orig_r_type, output_section, object,
 				     data_shndx, reloc.get_r_offset(),
 				     reloc.get_r_addend());
@@ -2050,7 +2053,7 @@ Target_sparc<size, big_endian>::Scan::gl
               {
                 Reloc_section* rela_dyn = target->rela_dyn_section(layout);

-		check_non_pic(object, r_type);
+		check_dynamic_reloc(object, r_type);
 		if (gsym->is_from_dynobj()
 		    || gsym->is_undefined()
 		    || gsym->is_preemptible())
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	25 Mar 2009 23:41:12 -0000
@@ -172,7 +172,7 @@ class Target_x86_64 : public Target_free
   {
   public:
     Scan()
-      : issued_non_pic_error_(false)
+      : issued_unsupp_reloc_error_(false)
     { }

     inline void
@@ -202,10 +202,10 @@ class Target_x86_64 : public Target_free
 			     Symbol*);

     void
-    check_non_pic(Relobj*, unsigned int r_type);
+    check_dynamic_reloc(Relobj*, unsigned int r_type);

     // Whether we have issued an error about a non-PIC compilation.
-    bool issued_non_pic_error_;
+    bool issued_unsupp_reloc_error_;
   };

   // The class which implements relocation.
@@ -951,7 +951,7 @@ Target_x86_64::Scan::unsupported_reloc_l
 // error even if the section is not read-only.

 void
-Target_x86_64::Scan::check_non_pic(Relobj* object, unsigned int r_type)
+Target_x86_64::Scan::check_dynamic_reloc(Relobj* object, unsigned int r_type)
 {
   switch (r_type)
     {
@@ -972,11 +972,14 @@ Target_x86_64::Scan::check_non_pic(Relob
       // This prevents us from issuing more than one error per reloc
       // section.  But we can still wind up issuing more than one
       // error per object file.
-      if (this->issued_non_pic_error_)
+      if (this->issued_unsupp_reloc_error_)
         return;
-      object->error(_("requires unsupported dynamic reloc; "
-                      "recompile with -fPIC"));
-      this->issued_non_pic_error_ = true;
+      if (parameters->options().output_is_position_independent())
+        object->error(_("requires unsupported dynamic reloc; "
+                        "recompile with -fPIC"));
+      else
+        object->error(_("requires unsupported dynamic reloc"));
+      this->issued_unsupp_reloc_error_ = true;
       return;

     case elfcpp::R_X86_64_NONE:
@@ -1034,7 +1037,7 @@ Target_x86_64::Scan::local(const General
       // because that is always a 64-bit relocation.
       if (parameters->options().output_is_position_independent())
         {
-          this->check_non_pic(object, r_type);
+          this->check_dynamic_reloc(object, r_type);

           Reloc_section* rela_dyn = target->rela_dyn_section(layout);
 	  unsigned int r_sym = elfcpp::elf_r_sym<64>(reloc.get_r_info());
@@ -1105,7 +1108,7 @@ Target_x86_64::Scan::local(const General
                       object->local_got_offset(r_sym, GOT_TYPE_STANDARD), 0);
                 else
                   {
-                    this->check_non_pic(object, r_type);
+                    this->check_dynamic_reloc(object, r_type);

                     gold_assert(lsym.get_st_type() != elfcpp::STT_SECTION);
                     rela_dyn->add_local(
@@ -1322,7 +1325,7 @@ Target_x86_64::Scan::global(const Genera
               }
             else
               {
-                this->check_non_pic(object, r_type);
+                this->check_dynamic_reloc(object, r_type);
                 Reloc_section* rela_dyn = target->rela_dyn_section(layout);
                 rela_dyn->add_global(gsym, r_type, output_section, object,
                                      data_shndx, reloc.get_r_offset(),
@@ -1353,7 +1356,7 @@ Target_x86_64::Scan::global(const Genera
               }
             else
               {
-                this->check_non_pic(object, r_type);
+                this->check_dynamic_reloc(object, r_type);
                 Reloc_section* rela_dyn = target->rela_dyn_section(layout);
                 rela_dyn->add_global(gsym, r_type, output_section, object,
                                      data_shndx, reloc.get_r_offset(),

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building    position-independent output
  2009-03-26  0:01 [gold][patch] Fix non-PIC warning to print only when building position-independent output Cary Coutant
@ 2009-03-26  0:12 ` Hans-Peter Nilsson
  2009-03-26  0:26   ` Cary Coutant
  0 siblings, 1 reply; 10+ messages in thread
From: Hans-Peter Nilsson @ 2009-03-26  0:12 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Binutils

On Wed, 25 Mar 2009, Cary Coutant wrote:

> If we ever do hit this case in a
> non-position-independent link, however, I hope it'll be the compiler's
> fault, and we shouldn't print a message that implies that -fPIC is
> actually required. (Maybe we should print something instead about a
> possible bug in the compiler or assembly code?)

IMHO don't just hint at a gcc bug: missing -fPIC is much too
common.  Or if that's not applicable here, maybe the user didn't
use the correct linker option?

brgds, H-P

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building   position-independent output
  2009-03-26  0:12 ` Hans-Peter Nilsson
@ 2009-03-26  0:26   ` Cary Coutant
  2009-03-26 14:56     ` Daniel Jacobowitz
  0 siblings, 1 reply; 10+ messages in thread
From: Cary Coutant @ 2009-03-26  0:26 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: Binutils

>> If we ever do hit this case in a
>> non-position-independent link, however, I hope it'll be the compiler's
>> fault, and we shouldn't print a message that implies that -fPIC is
>> actually required. (Maybe we should print something instead about a
>> possible bug in the compiler or assembly code?)
>
> IMHO don't just hint at a gcc bug: missing -fPIC is much too
> common.  Or if that's not applicable here, maybe the user didn't
> use the correct linker option?

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.

-cary

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building  position-independent output
  2009-03-26  0:26   ` Cary Coutant
@ 2009-03-26 14:56     ` Daniel Jacobowitz
  2009-03-26 18:33       ` Cary Coutant
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Jacobowitz @ 2009-03-26 14:56 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Hans-Peter Nilsson, Binutils

On Wed, Mar 25, 2009 at 05:26:22PM -0700, Cary Coutant wrote:
> 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.

-- 
Daniel Jacobowitz
CodeSourcery

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building   position-independent output
  2009-03-26 14:56     ` Daniel Jacobowitz
@ 2009-03-26 18:33       ` Cary Coutant
  2009-03-27 14:47         ` Ian Lance Taylor
  0 siblings, 1 reply; 10+ messages in thread
From: Cary Coutant @ 2009-03-26 18:33 UTC (permalink / raw)
  To: Cary Coutant, Hans-Peter Nilsson, Binutils

>> 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,

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building   position-independent output
  2009-03-26 18:33       ` Cary Coutant
@ 2009-03-27 14:47         ` Ian Lance Taylor
  2009-03-27 17:35           ` Cary Coutant
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2009-03-27 14:47 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Hans-Peter Nilsson, Binutils

Cary Coutant <ccoutant@google.com> writes:

> +                gold_assert(parameters->
> +                  options().output_is_position_independent());
>  		check_non_pic(object, r_type);

I can see the path you took to get to this patch, but now that you are
here, why not just put the assert in check_non_pic in all cases?

Ian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building   position-independent output
  2009-03-27 14:47         ` Ian Lance Taylor
@ 2009-03-27 17:35           ` Cary Coutant
  2009-03-27 17:57             ` Cary Coutant
  0 siblings, 1 reply; 10+ messages in thread
From: Cary Coutant @ 2009-03-27 17:35 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Hans-Peter Nilsson, Binutils

>> +                gold_assert(parameters->
>> +                  options().output_is_position_independent());
>>               check_non_pic(object, r_type);
>
> I can see the path you took to get to this patch, but now that you are
> here, why not just put the assert in check_non_pic in all cases?

That would be just as good in my view -- that's what I originally
thought of in my first response to Hans-Peter, but then I realized
that we've already established that we're PIC in all but two cases, so
I thought it might be a bit clearer to establish the assertion more
locally for those two cases. Keeping the name check_non_pic does kind
of make that clear, though, so I'm fine either way.

-cary

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building   position-independent output
  2009-03-27 17:35           ` Cary Coutant
@ 2009-03-27 17:57             ` Cary Coutant
  2009-03-27 18:07               ` Ian Lance Taylor
  0 siblings, 1 reply; 10+ messages in thread
From: Cary Coutant @ 2009-03-27 17:57 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Binutils

>> I can see the path you took to get to this patch, but now that you are
>> here, why not just put the assert in check_non_pic in all cases?
>
> That would be just as good in my view -- that's what I originally
> thought of in my first response to Hans-Peter, but then I realized
> that we've already established that we're PIC in all but two cases, so
> I thought it might be a bit clearer to establish the assertion more
> locally for those two cases. Keeping the name check_non_pic does kind
> of make that clear, though, so I'm fine either way.

And here's a patch for that...

(I should note that I'm also fine leaving the code as is -- I think
I'm now satisfied that we can't take this path unless we are
generating a position-independent output file. It might be good as
documentation for future targets, though.)

-cary


      * powerpc.cc (Target_powerpc::check_non_pic): Assert that output is
      position independent.
      * sparc.cc (Target_sparc::check_non_pic): Likewise.
      * x86_64.cc (Target_x86_64::check_non_pic): 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	27 Mar 2009 17:41:08 -0000
@@ -1094,6 +1094,7 @@ Target_powerpc<size, big_endian>::Scan::
   // error per object file.
   if (this->issued_non_pic_error_)
     return;
+  gold_assert(parameters->options().output_is_position_independent());
   object->error(_("requires unsupported dynamic reloc; "
 		  "recompile with -fPIC"));
   this->issued_non_pic_error_ = true;
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	27 Mar 2009 17:41:08 -0000
@@ -1589,6 +1589,7 @@ Target_sparc<size, big_endian>::Scan::ch
   // error per object file.
   if (this->issued_non_pic_error_)
     return;
+  gold_assert(parameters->options().output_is_position_independent());
   object->error(_("requires unsupported dynamic reloc; "
 		  "recompile with -fPIC"));
   this->issued_non_pic_error_ = true;
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	27 Mar 2009 17:41:08 -0000
@@ -974,6 +974,7 @@ Target_x86_64::Scan::check_non_pic(Relob
       // error per object file.
       if (this->issued_non_pic_error_)
         return;
+      gold_assert(parameters->options().output_is_position_independent());
       object->error(_("requires unsupported dynamic reloc; "
                       "recompile with -fPIC"));
       this->issued_non_pic_error_ = true;

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building  position-independent output
  2009-03-27 17:57             ` Cary Coutant
@ 2009-03-27 18:07               ` Ian Lance Taylor
  2009-03-27 18:21                 ` Cary Coutant
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Lance Taylor @ 2009-03-27 18:07 UTC (permalink / raw)
  To: Cary Coutant; +Cc: Binutils

Cary Coutant <ccoutant@google.com> writes:

>       * powerpc.cc (Target_powerpc::check_non_pic): Assert that output is
>       position independent.
>       * sparc.cc (Target_sparc::check_non_pic): Likewise.
>       * x86_64.cc (Target_x86_64::check_non_pic): Likewise.

This is OK.

Thanks.

Ian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [gold][patch] Fix non-PIC warning to print only when building   position-independent output
  2009-03-27 18:07               ` Ian Lance Taylor
@ 2009-03-27 18:21                 ` Cary Coutant
  0 siblings, 0 replies; 10+ messages in thread
From: Cary Coutant @ 2009-03-27 18:21 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Binutils

>>       * powerpc.cc (Target_powerpc::check_non_pic): Assert that output is
>>       position independent.
>>       * sparc.cc (Target_sparc::check_non_pic): Likewise.
>>       * x86_64.cc (Target_x86_64::check_non_pic): Likewise.
>
> This is OK.

Committed. I also fixed a typo in a previous ChangeLog entry
(s/readsymc.cc/readsyms.cc/).

-cary

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-03-27 18:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-03-26  0:01 [gold][patch] Fix non-PIC warning to print only when building position-independent output Cary Coutant
2009-03-26  0:12 ` Hans-Peter Nilsson
2009-03-26  0:26   ` Cary Coutant
2009-03-26 14:56     ` Daniel Jacobowitz
2009-03-26 18:33       ` Cary Coutant
2009-03-27 14:47         ` Ian Lance Taylor
2009-03-27 17:35           ` Cary Coutant
2009-03-27 17:57             ` Cary Coutant
2009-03-27 18:07               ` Ian Lance Taylor
2009-03-27 18:21                 ` Cary Coutant

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).