Re: ld -r breakage

Re: ld -r breakage

[Nick Clifton]

Hi Alan,

: > Although this solution will probably work, it does not strike me as
: > being the right thing to do.
: 
: I think it is.  Why should "ld -r" change section attributes?

Hmm, I agree that it shouldn't.  But 'ld -r -N' should.  (I am not
sure why anyone would ever want to combine these switches but you
never know).   So what I am suggesting is that the SEC_READONLY
attribite of the text section should only be controlled by the
config.text_read_only flag, and that by default doing 'ld -r' should
not change this flag.

Cheers
	Nick

Re: ld -r breakage

[Richard Henderson]

On Mon, Jun 12, 2000 at 03:30:08PM -0700, Nick Clifton wrote:
> Hmm, I agree that it shouldn't.  But 'ld -r -N' should.

I don't agree.  That combination of options is nonsense.


r~

Re: ld -r breakage

[Nick Clifton]

Hi Alan,

: And vice versa.  Since OMAGIC should be used for object files, we
: ought to continue to clear config.text_read_only for "ld -r".
: 
: config.text_read_only clear
:   => WP_TEXT cleared in ldlang.c:ldlang_open_output
:   => o_magic selected in aoutx.h:adjust_sizes_and_vmas
: 
: I've checked in the fix I posted earlier.

Thanks.

Cheers
	Nick

Re: ld -r breakage

[Alan Modra]

On 14 Jun 2000, Ian Lance Taylor wrote:

>    Any recollection about this comment Ian?
>    /* Text section is write protected (if D_PAGED is not set, this is
>       like an a.out NMAGIC file) (the linker sets this by default, but
>       clears it for -r or -N).  */
>    #define WP_TEXT     	0x80
> 
> This is an a.out thing, really.  In a.out, section flags are implied
> by the magic number.

And vice versa.  Since OMAGIC should be used for object files, we
ought to continue to clear config.text_read_only for "ld -r".

config.text_read_only clear
  => WP_TEXT cleared in ldlang.c:ldlang_open_output
  => o_magic selected in aoutx.h:adjust_sizes_and_vmas

I've checked in the fix I posted earlier.

Regards, Alan Modra
-- 
Linuxcare.  Support for the Revolution.

Re: ld -r breakage

[Nick Clifton]

Hi Alan,

: > OK, but I still think that the reason that 'ld -r' does not change the
: > SEC_READONLY attribute of the text section should be a consequence of
: > the fact that config.text_read_only is true, and not of the fact that
: > link_info.relocatable is true.
: 
: Hmm.  We started with (1)
: config.text_read_only true => make .text read only
: config.text_read_only false => don't change .text attributes
: 
: This was buggy because "ld -N" needs to mark .text read/write
: 
: Then we went to (2)
: config.text_read_only true => make .text read only
: config.text_read_only false => make .text read/write
: 
: This is again incorrect because "ld -r" cleared config.text_read_only so
: as to leave the section attributes unchanged as per (1).  Under (2), we
: get a read/write .text section.
: 
: I suggested testing link_info.relocatable so that "ld -r" would again
: leave the section attributes unchanged.  It's true that the patch I
: submitted should have removed the unnecessary clearing of
: config.text_read_only for `-r' (and `-Ur').
: 
: Now you seem to be saying (3)
: config.text_read_only true => don't change .text attributes
: config.text_read_only false => make .text read/write
: 
: ??

OK - sorry for all this confusion.  I actually agree with scenario (2)
above, although I obviously did not express myself clearly.  If you
would care to resubmit your patch (with the fix to -r and -Ur that you
mention above) then I will be happy yto approve it.

Cheers
	Nick

Re: ld -r breakage

[Alan Modra]

On Mon, 12 Jun 2000, Nick Clifton wrote:

> OK, but I still think that the reason that 'ld -r' does not change the
> SEC_READONLY attribute of the text section should be a consequence of
> the fact that config.text_read_only is true, and not of the fact that
> link_info.relocatable is true.

Hmm.  We started with (1)
config.text_read_only true => make .text read only
config.text_read_only false => don't change .text attributes

This was buggy because "ld -N" needs to mark .text read/write

Then we went to (2)
config.text_read_only true => make .text read only
config.text_read_only false => make .text read/write

This is again incorrect because "ld -r" cleared config.text_read_only so
as to leave the section attributes unchanged as per (1).  Under (2), we
get a read/write .text section.

I suggested testing link_info.relocatable so that "ld -r" would again
leave the section attributes unchanged.  It's true that the patch I
submitted should have removed the unnecessary clearing of
config.text_read_only for `-r' (and `-Ur').

Now you seem to be saying (3)
config.text_read_only true => don't change .text attributes
config.text_read_only false => make .text read/write

??

-- 
Linuxcare.  Support for the Revolution.

Re: ld -r breakage

[Nick Clifton]

Hi Guys,

: > Hmm, I agree that it shouldn't.  But 'ld -r -N' should.
: 
: I don't agree.  That combination of options is nonsense.

OK, but I still think that the reason that 'ld -r' does not change the
SEC_READONLY attribute of the text section should be a consequence of
the fact that config.text_read_only is true, and not of the fact that
link_info.relocatable is true.

Cheers
	Nick

Re: ld -r breakage

[Alan Modra]

On Mon, 12 Jun 2000, Nick Clifton wrote:

> Although this solution will probably work, it does not strike me as
> being the right thing to do.

I think it is.  Why should "ld -r" change section attributes?

-- 
Linuxcare.  Support for the Revolution.

Re: ld -r breakage

[Nick Clifton]

Hi Guys,

: On Mon, 12 Jun 2000, Jakub Jelinek wrote:
: 
: > ld -r no longer preserves SEC_READONLY attribute for some sections.
: 
: I suspect it's this patch (presumably for correct -N operation)
: 
: 2000-05-22  Igor Shevlyakov  <igor@windriver.com>
: 
: 	* ldmain.c (main): When deciding if ".text" section should be
:         read-only, don't forget to reset SEC_READONLY because it
:         could be already set.
: 
: This will fix it
: 
: 2000-06-12  Alan Modra  <alan@linuxcare.com.au>
: 
: 	* ldmain.c (main): Only change .text SEC_READONLY for final link.
: 
: Index: ldmain.c
: ===================================================================
: RCS file: /cvs/src/src/ld/ldmain.c,v
: retrieving revision 1.7
: diff -u -p -r1.7 ldmain.c
: --- ldmain.c	2000/05/26 13:11:57	1.7
: +++ ldmain.c	2000/06/12 09:40:03
: @@ -349,17 +349,18 @@ main (argc, argv)
:       symbols, and possibly multiple definitions */
:  
:    /* Look for a text section and switch the readonly attribute in it.  */
: -  {
: -    asection * found = bfd_get_section_by_name (output_bfd, ".text");
: +  if (!link_info.relocateable)
: +    {
: +      asection * found = bfd_get_section_by_name (output_bfd, ".text");
:      
: -    if (found != (asection *) NULL)
: -      {
: -	if (config.text_read_only)
: -	  found->flags |= SEC_READONLY;
: -	else
: -	  found->flags &= ~SEC_READONLY;
: -      }
: -  }
: +      if (found != (asection *) NULL)
: +	{
: +	  if (config.text_read_only)
: +	    found->flags |= SEC_READONLY;
: +	  else
: +	    found->flags &= ~SEC_READONLY;
: +	}
: +    }
:  
:    if (link_info.relocateable)
:      output_bfd->flags &= ~EXEC_P;


Although this solution will probably work, it does not strike me as
being the right thing to do.  Surely the correct thing to do is to set
the config.text_read_only flag correctly, even for a partial link,
rather than ignoring it.  The real bug is probably this line from
parse_args() in lexsup.c:

	case 'r':
	  link_info.relocateable = true;
	  config.build_constructors = false;
	  config.magic_demand_paged = false;
	  config.text_read_only = false;       <=== Why ???
	  config.dynamic_link = false;
	  break;

This problem was recently reported by Tom de Lellis:

  http://sourceware.cygnus.com/ml/binutils/2000-06/msg00085.html

I plan to check in a patch that removes this line, unless anyone
objects.

Cheers
	Nick

Re: ld -r breakage

[Alan Modra]

On Mon, 12 Jun 2000, Jakub Jelinek wrote:

> ld -r no longer preserves SEC_READONLY attribute for some sections.

I suspect it's this patch (presumably for correct -N operation)

2000-05-22  Igor Shevlyakov  <igor@windriver.com>

	* ldmain.c (main): When deciding if ".text" section should be
        read-only, don't forget to reset SEC_READONLY because it
        could be already set.

This will fix it

2000-06-12  Alan Modra  <alan@linuxcare.com.au>

	* ldmain.c (main): Only change .text SEC_READONLY for final link.

Index: ldmain.c
===================================================================
RCS file: /cvs/src/src/ld/ldmain.c,v
retrieving revision 1.7
diff -u -p -r1.7 ldmain.c
--- ldmain.c	2000/05/26 13:11:57	1.7
+++ ldmain.c	2000/06/12 09:40:03
@@ -349,17 +349,18 @@ main (argc, argv)
      symbols, and possibly multiple definitions */
 
   /* Look for a text section and switch the readonly attribute in it.  */
-  {
-    asection * found = bfd_get_section_by_name (output_bfd, ".text");
+  if (!link_info.relocateable)
+    {
+      asection * found = bfd_get_section_by_name (output_bfd, ".text");
     
-    if (found != (asection *) NULL)
-      {
-	if (config.text_read_only)
-	  found->flags |= SEC_READONLY;
-	else
-	  found->flags &= ~SEC_READONLY;
-      }
-  }
+      if (found != (asection *) NULL)
+	{
+	  if (config.text_read_only)
+	    found->flags |= SEC_READONLY;
+	  else
+	    found->flags &= ~SEC_READONLY;
+	}
+    }
 
   if (link_info.relocateable)
     output_bfd->flags &= ~EXEC_P;

-- 
Linuxcare.  Support for the Revolution.

ld -r breakage

[Jakub Jelinek]

Hi!

ld -r no longer preserves SEC_READONLY attribute for some sections.
A test case can be say
cat > foo.c <<EOF
void foo() { }
EOF
gcc -c -o foo.o foo.c
ld -r -o foo.lo foo.o

It used to work correctly, at least in H.J.'s 2.9.5.0.22.
Seen on both sparc and i386.
I haven't yet had time to track this down, if someone gets to it before
myself, I'll appreciate it.

	Jakub