Problem with ld for PDP-11

Problem with ld for PDP-11

[Stephen Casner]

Recently Nick Clifton applied a fix for Bug 20694 - "PDP11
TARGET_PAGE_SIZE is incorrect" that I filed 3+ year ago.  Thanks!

In testing that fix I discoverd another bug in the PDP11 code for
which I have a proposed fix and would like your feedback.  I also have
some questions about the command line options that control the various
magic numbers, but I will raise those questions separately.

The newly discovered bug is that when I add the -s option to ld to
skip output of the symbol table, the last byte of the .data section is
been replaced by zero if that section is of even size (a multiple of 2
bytes long).

I found the code that does this.  Specifically for the case when no
symbols are written, the 0 byte is written by this code in the else
clause at line 3926 of bfd/pdp11.c:

  /* Write out the string table, unless there are no symbols.  */
  if (abfd->symcount > 0)
    {
      if (bfd_seek (abfd, obj_str_filepos (abfd), SEEK_SET) != 0
	  || ! emit_stringtab (abfd, aout_info.strtab))
	goto error_return;
    }
  else if (obj_textsec (abfd)->reloc_count == 0
	   && obj_datasec (abfd)->reloc_count == 0)
    {
      bfd_byte b;

      b = 0;
      if (bfd_seek (abfd,
		    (file_ptr) (obj_datasec (abfd)->filepos
				+ exec_hdr (abfd)->a_data
				- 1),
		    SEEK_SET) != 0
	  || bfd_bwrite (&b, (bfd_size_type) 1, abfd) != 1)
	goto error_return;
    }

If the size of the .data section is odd, then the result of this code
is to write a padding byte to make the size of the a.out file even.
That is probably the intent of the code.  But when I tested commenting
out this else clause the a.out file still contained a zero padding
byte there, which suggests that it may be safe to just remove this
else clause.

My first idea was to make the else clause conditional on whether the
size of the .data section was odd, but from what I can see the size
has already been rounded up by the time this code is reached and I
could not find another variable that would hold the value before
rounding.

It appears that the bfd/pdp11.c file was created by copying and
modifying aoutx.h; this work was done by Lars Brinkhoff years ago.
The corresponding else clause in aoutx.h always appends one word's
worth of zero bytes (typically 4) to the output.  It looks like the
intention is to pad the length of the ouput to a word boundary, but
that would only be effective if there is some operation that truncates
the output to the last whole word.  I don't know the code well enough
to find that.  For the pdpd11 case, it looks like the last odd byte
does get written with a zero pad even when the else clause is
commented out.

I asked Lars about deleting this else clause.  He said he didn't
remember the code well enough to say why that clause was included, but
he agreed with my suggestion to remove it.  Do any of you readers know
how the padding and truncation in aoutx.h is implemented so you can
direct me to the right part of the code to compare to the pdpd11 case?

If you agree with the suggestion to delete this else clause in
pdp11.c, how can I make that happen?  Should I file a bug and include
the change as a patch, or push it back from git somehow (I'm not a git
expert)?

It took a long time for bug 20694 to be addressed, and the fix was
only committed after I happened to ask Lars about it, so I'm hoping to
establish a more efficient and effective path for fixing this bug and
for addressing the magic changes I'd like to propose.

Thanks for your time.

Steve Casner

p.s.  I'd be happy to provide the details of my testing if you care to
see them.

Re: Problem with ld for PDP-11

[Alan Modra]

On Wed, Feb 12, 2020 at 12:29:50PM -0800, Stephen Casner wrote:
> Recently Nick Clifton applied a fix for Bug 20694 - "PDP11
> TARGET_PAGE_SIZE is incorrect" that I filed 3+ year ago.  Thanks!
> 
> In testing that fix I discoverd another bug in the PDP11 code for
> which I have a proposed fix and would like your feedback.  I also have
> some questions about the command line options that control the various
> magic numbers, but I will raise those questions separately.
> 
> The newly discovered bug is that when I add the -s option to ld to
> skip output of the symbol table, the last byte of the .data section is
> been replaced by zero if that section is of even size (a multiple of 2
> bytes long).
> 
> I found the code that does this.  Specifically for the case when no
> symbols are written, the 0 byte is written by this code in the else
> clause at line 3926 of bfd/pdp11.c:
> 
>   /* Write out the string table, unless there are no symbols.  */
>   if (abfd->symcount > 0)
>     {
>       if (bfd_seek (abfd, obj_str_filepos (abfd), SEEK_SET) != 0
> 	  || ! emit_stringtab (abfd, aout_info.strtab))
> 	goto error_return;
>     }
>   else if (obj_textsec (abfd)->reloc_count == 0
> 	   && obj_datasec (abfd)->reloc_count == 0)
>     {
>       bfd_byte b;
> 
>       b = 0;
>       if (bfd_seek (abfd,
> 		    (file_ptr) (obj_datasec (abfd)->filepos
> 				+ exec_hdr (abfd)->a_data
> 				- 1),
> 		    SEEK_SET) != 0
> 	  || bfd_bwrite (&b, (bfd_size_type) 1, abfd) != 1)
> 	goto error_return;
>     }
> 
> If the size of the .data section is odd, then the result of this code
> is to write a padding byte to make the size of the a.out file even.
> That is probably the intent of the code.

Yes.  The layout of a typical a.out file is header, text, data,
relocs, symbols, string table.  Since the code in question is handling
the case where there are no relocs, symbols or string table, ie. the
last thing in the file is data, I would guess that when the data
section size is odd that the file size does not include the padding
byte to make the data size even.  But of course if the original data
size is even you don't want to overwrite the last byte.

The correct fix then is to add a check that the original data size was
odd.  That might not be easy as you may have lost the original data
size by the time this code runs.  The AOUT code is a bit of a mess.
Oh, and if there is no data then it's the text section that might need
extending.

>  But when I tested commenting
> out this else clause the a.out file still contained a zero padding
> byte there, which suggests that it may be safe to just remove this
> else clause.
> 
> My first idea was to make the else clause conditional on whether the
> size of the .data section was odd, but from what I can see the size
> has already been rounded up by the time this code is reached and I
> could not find another variable that would hold the value before
> rounding.
> 
> It appears that the bfd/pdp11.c file was created by copying and
> modifying aoutx.h; this work was done by Lars Brinkhoff years ago.
> The corresponding else clause in aoutx.h always appends one word's
> worth of zero bytes (typically 4) to the output.  It looks like the
> intention is to pad the length of the ouput to a word boundary, but
> that would only be effective if there is some operation that truncates
> the output to the last whole word.  I don't know the code well enough
> to find that.  For the pdpd11 case, it looks like the last odd byte
> does get written with a zero pad even when the else clause is
> commented out.
> 
> I asked Lars about deleting this else clause.  He said he didn't
> remember the code well enough to say why that clause was included, but
> he agreed with my suggestion to remove it.  Do any of you readers know
> how the padding and truncation in aoutx.h is implemented so you can
> direct me to the right part of the code to compare to the pdpd11 case?
> 
> If you agree with the suggestion to delete this else clause in
> pdp11.c, how can I make that happen?  Should I file a bug and include
> the change as a patch, or push it back from git somehow (I'm not a git
> expert)?
> 
> It took a long time for bug 20694 to be addressed, and the fix was
> only committed after I happened to ask Lars about it, so I'm hoping to
> establish a more efficient and effective path for fixing this bug and
> for addressing the magic changes I'd like to propose.
> 
> Thanks for your time.
> 
> Steve Casner
> 
> p.s.  I'd be happy to provide the details of my testing if you care to
> see them.

-- 
Alan Modra
Australia Development Lab, IBM

Re: Problem with ld for PDP-11

[Stephen Casner]

On Thu, 13 Feb 2020, Alan Modra wrote:
> On Wed, Feb 12, 2020 at 12:29:50PM -0800, Stephen Casner wrote:
> > If the size of the .data section is odd, then the result of this code
> > is to write a padding byte to make the size of the a.out file even.
> > That is probably the intent of the code.
>
> Yes.  The layout of a typical a.out file is header, text, data,
> relocs, symbols, string table.  Since the code in question is handling
> the case where there are no relocs, symbols or string table, ie. the
> last thing in the file is data, I would guess that when the data
> section size is odd that the file size does not include the padding
> byte to make the data size even.  But of course if the original data
> size is even you don't want to overwrite the last byte.
>
> The correct fix then is to add a check that the original data size was
> odd.  That might not be easy as you may have lost the original data
> size by the time this code runs.  The AOUT code is a bit of a mess.
> Oh, and if there is no data then it's the text section that might need
> extending.

Thanks for your prompt reply.

Indeed, as I said in the later half of my previous message, my first
idea was to make the else clause conditional on whether the size of
the .data section was odd, but from what I could see the size had
already been rounded up by the time this code is reached and I could
not find another variable that would hold the value before rounding.

So now I went digging my way back through the code sequence using
printfs to find where the original data size gets rounded up.  It
turns out that the size of the data section is always even because the
assembler "as" or "gas" always pads the data section with a zero byte
if the size is odd, so the object file header always contains an even
size for the .data section.  That's because there is always relocation
and symbol information following it.

Therefore the size of the .data section in ld is always even so just
removing that "else" clause would be a valid fix.  How can I best make
that happen?  Should I file a bug and include the change as a patch,
or push it back from git somehow (I'm not a git expert)?

Steve Casner

Re: Problem with ld for PDP-11

[Alan Modra]

On Wed, Feb 12, 2020 at 10:02:02PM -0800, Stephen Casner wrote:
> So now I went digging my way back through the code sequence using
> printfs to find where the original data size gets rounded up.  It
> turns out that the size of the data section is always even because the
> assembler "as" or "gas" always pads the data section with a zero byte
> if the size is odd, so the object file header always contains an even
> size for the .data section.  That's because there is always relocation
> and symbol information following it.
> 
> Therefore the size of the .data section in ld is always even

No, a linker script might contribute to .data output.

-- 
Alan Modra
Australia Development Lab, IBM

Re: Problem with ld for PDP-11

[Stephen Casner]

On Thu, 13 Feb 2020, Alan Modra wrote:

> On Wed, Feb 12, 2020 at 10:02:02PM -0800, Stephen Casner wrote:
> > So now I went digging my way back through the code sequence using
> > printfs to find where the original data size gets rounded up.  It
> > turns out that the size of the data section is always even because the
> > assembler "as" or "gas" always pads the data section with a zero byte
> > if the size is odd, so the object file header always contains an even
> > size for the .data section.  That's because there is always relocation
> > and symbol information following it.
> >
> > Therefore the size of the .data section in ld is always even
>
> No, a linker script might contribute to .data output.

Oh, that's something I don't know about.  Can you point me to an
example so I can try it?

-- Steve

Re: Problem with ld for PDP-11

[Alan Modra]

On Wed, Feb 12, 2020 at 11:18:50PM -0800, Stephen Casner wrote:
> On Thu, 13 Feb 2020, Alan Modra wrote:
> 
> > On Wed, Feb 12, 2020 at 10:02:02PM -0800, Stephen Casner wrote:
> > > So now I went digging my way back through the code sequence using
> > > printfs to find where the original data size gets rounded up.  It
> > > turns out that the size of the data section is always even because the
> > > assembler "as" or "gas" always pads the data section with a zero byte
> > > if the size is odd, so the object file header always contains an even
> > > size for the .data section.  That's because there is always relocation
> > > and symbol information following it.
> > >
> > > Therefore the size of the .data section in ld is always even
> >
> > No, a linker script might contribute to .data output.
> 
> Oh, that's something I don't know about.  Can you point me to an
> example so I can try it?

You could use ld/ldscripts/pdp11.x (which is the default script you'll
be using for normal executables), and add the line

    BYTE (0)

after CONSTRUCTORS.  Then link using -T.

-- 
Alan Modra
Australia Development Lab, IBM

Re: Problem with ld for PDP-11

[Stephen Casner]

On Thu, 13 Feb 2020, Alan Modra wrote:
> You could use ld/ldscripts/pdp11.x (which is the default script you'll
> be using for normal executables), and add the line
>
>     BYTE (0)
>
> after CONSTRUCTORS.  Then link using -T.

Thanks for the pointer.  You are right, if I comment out the "else"
clause that I proposed to remove and link using a script modified as
you suggest then I can create an output file with an odd length that
is one bye shorter than what the header indicates.  With the code
intact a padding byte is appended such that the length then matches.

But that begs the question, does it matter?  What would be the
motivation to create such a script?

As an alternative to removing the else clause, I could change it to be
similar to the corresponding else clause in the aoutx.h file from
which pdp11.c was adapted.  The code in aoutx.h always appends
BYTES_IN_WORD bytes of zero to the .data section for this "else" case
where no symbols will follow.  That always leaves one or two
extraneous bytes of 0 after then end of the proper length of the .data
section, even for normal production with the default linker script.
Would that be a better solution than just removing the padding code?
It appears that is what targets based on aoutx.h would do unless there
is some other step that truncates the file to the correct length.

                                                        -- Steve

Re: Problem with ld for PDP-11

[Stephen Casner]

On Thu, 13 Feb 2020, Alan Modra wrote:

> The correct fix then is to add a check that the original data size was
> odd.  That might not be easy as you may have lost the original data
> size by the time this code runs.  The AOUT code is a bit of a mess.

Here's an alternative idea, if it would not be a logical scope
violation to access the "where" member:  check abfd->where to see if
an odd byte has been written, and if so add a padding byte.

> Oh, and if there is no data then it's the text section that might need
> extending.

That should not be a concern because PDP11 instruction must always be
a multiple of words long.

                                                        -- Steve

Re: Problem with ld for PDP-11

[Stephen Casner]

I have submitted bug 25569 with a patch for this proposed fix.

                                                        -- Steve

On Fri, 14 Feb 2020, Stephen Casner wrote:

> On Thu, 13 Feb 2020, Alan Modra wrote:
>
> > The correct fix then is to add a check that the original data size was
> > odd.  That might not be easy as you may have lost the original data
> > size by the time this code runs.  The AOUT code is a bit of a mess.
>
> Here's an alternative idea, if it would not be a logical scope
> violation to access the "where" member:  check abfd->where to see if
> an odd byte has been written, and if so add a padding byte.
>
> > Oh, and if there is no data then it's the text section that might need
> > extending.
>
> That should not be a concern because PDP11 instruction must always be
> a multiple of words long.
>
>                                                         -- Steve