How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Martin Walter]

Hello!

We have an existing toolchain for our own architecture that should be
extended by my binutils port. Now, some of these tools we would like
to reuse make the assumption that a byte in our target's ELF-binary
contains 16 bits. I have taken a closer look at 'bits_per_byte'=16 of
the 'bfd_arch_info_type' in the 'bfd/cpu-target.c' file:

const bfd_arch_info_type bfd_spear32_arch =
{
  32,                   /* 32 bits per word */
  32,                   /* 32 bits per address */
  16,                   /* 16 bits per byte */
  ...
};

It seems that this is not quite it. I could see from an unlinked
object file that 'fixP->where' must be divided by 2 in
spear32_md_apply_fix() in 'gas/config/tc-spear32.c' in order for
fixups to appear at their right positions:

void
spear32_md_apply_fix(fixS *fixP, valueT * valP, segT seg)
{
  long value = *valP;

  fixP->where /= 2;

  if (fixP->fx_pcrel == 1)
    value -= 1;

  gas_cgen_md_apply_fix(fixP, (valueT *)&value, seg);
}

Now I experience a problem in '_bfd_relocate_contents()' in
'bfd/reloc.c' where the address of a relocatable instruction is given
as an index to a 16-bit instruction, but 'bfd_get16(location)' seems
to be interpreting the location value as a byte index:

-- Begin crt0.o --
0000000 <_start>:
_start():
   0:   01 10           ldhi r1,0       0: R_SPEAR32_4TH      __bss_start
   1:   01 20           ldliu r1,0x0    1: R_SPEAR32_3RD    __bss_start
   2:   81 a1           sli r1,0x8
   3:   01 20           ldliu r1,0x0    3: R_SPEAR32_HI       __bss_start
   ...
-- End crt0.o --

-- Begin Debug --
in spear32_final_link_relocate()
R_SPEAR32_4TH
in _bfd_relocate_contents()
Reading value: 1001 from 0110 (little endian) // Correct!
---------------------------
in spear32_final_link_relocate()
R_SPEAR32_3RD
in bfd_relocate_contents()
Reading value: 0110 at location: 1001 (little endian) // Not correct,
location should be 0120
---------------------------
in spear32_final_link_relocate()
R_SPEAR32_HI
in bfd_relocate_contents()
Reading value: 8120 at location: 2081 (little endian) // Not correct,
location should be 0120
...
-- End Debug --

Where is my mistake? Are there any other parts of the binutils that
would need to be changed in order to get this work?

Cheers,
Martin

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Alan Modra]

On Thu, Aug 06, 2009 at 07:39:17PM +0200, Martin Walter wrote:
> Now I experience a problem in '_bfd_relocate_contents()' in
> 'bfd/reloc.c' where the address of a relocatable instruction is given
> as an index to a 16-bit instruction,

Yes.

> but 'bfd_get16(location)' seems
> to be interpreting the location value as a byte index:

Well why not?  After all, "location" just points into the section
contents, an array of bytes.

> Where is my mistake? Are there any other parts of the binutils that
> would need to be changed in order to get this work?

There are at least two other binutils ports that work with
OCTETS_PER_BYTE greater than one, tic4x and tic54x.  You should look
at them if you have not done so already.  Unfortunately, they are both
COFF.  The generic ELF code is missing some conversions between octets
and bytes (one such is on calls to bfd_set_section_contents in
elflink.c).  You get to fix it.  :-)

-- 
Alan Modra
Australia Development Lab, IBM

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Martin Walter]

Hi Alan,

>> but 'bfd_get16(location)' seems
>> to be interpreting the location value as a byte index:
>
> Well why not?  After all, "location" just points into the section
> contents, an array of bytes.
Anyway, shouldn't location point to the right instruction? After all,
location := contents + address in _bfd_final_link_relocate and is not
easy manipulatable externally, since address is sanity checked at the
beginning of _bfd_final_link_relocate.

> There are at least two other binutils ports that work with
> OCTETS_PER_BYTE greater than one, tic4x and tic54x.  You should look
> at them if you have not done so already.  Unfortunately, they are both
> COFF.  The generic ELF code is missing some conversions between octets
> and bytes (one such is on calls to bfd_set_section_contents in
> elflink.c).  You get to fix it.  :-)
I know about these architectures, but mine uses ELF so I did not dig
too much into them. I am not that much into the binutils and it does
not seem like this can be fixed easily. Have you got some more details
on where these missing conversions could reside?

Cheers,
Martin

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Alan Modra]

On Fri, Aug 07, 2009 at 02:28:21AM +0200, Martin Walter wrote:
> Hi Alan,
> 
> >> but 'bfd_get16(location)' seems
> >> to be interpreting the location value as a byte index:
> >
> > Well why not?  After all, "location" just points into the section
> > contents, an array of bytes.
> Anyway, shouldn't location point to the right instruction?

Certainly.  If not, you have found a bug in the caller of
_bfd_final_link_relocate.

> > There are at least two other binutils ports that work with
> > OCTETS_PER_BYTE greater than one, tic4x and tic54x.  You should look
> > at them if you have not done so already.  Unfortunately, they are both
> > COFF.  The generic ELF code is missing some conversions between octets
> > and bytes (one such is on calls to bfd_set_section_contents in
> > elflink.c).  You get to fix it.  :-)
> I know about these architectures, but mine uses ELF so I did not dig
> too much into them. I am not that much into the binutils and it does
> not seem like this can be fixed easily. Have you got some more details
> on where these missing conversions could reside?

Apart from saying they are likely in elflink.c, no.  I found the
errors on bfd_set_section_contents calls by inspection when responding
to your email.

-- 
Alan Modra
Australia Development Lab, IBM

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Martin Walter]

Hello,

>> I know about these architectures, but mine uses ELF so I did not dig
>> too much into them. I am not that much into the binutils and it does
>> not seem like this can be fixed easily. Have you got some more details
>> on where these missing conversions could reside?
>
> Apart from saying they are likely in elflink.c, no.  I found the
> errors on bfd_set_section_contents calls by inspection when responding
> to your email.

I have yet failed to find the relevant hotspots in the code. Some
changes give improvements in one place, but create a total mess in the
object file in other places. I am doing the Binutils port for a
practical work at University and I am afraid not to have the required
insight into the BFD to fix this on my own. Is there a possibility to
request a fix by a maintainer directly?

Cheers,
Martin

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Alan Modra]

On Mon, Aug 10, 2009 at 02:04:03AM +0200, Martin Walter wrote:
> Hello,
> 
> >> I know about these architectures, but mine uses ELF so I did not dig
> >> too much into them. I am not that much into the binutils and it does
> >> not seem like this can be fixed easily. Have you got some more details
> >> on where these missing conversions could reside?
> >
> > Apart from saying they are likely in elflink.c, no.  I found the
> > errors on bfd_set_section_contents calls by inspection when responding
> > to your email.
> 
> I have yet failed to find the relevant hotspots in the code. Some
> changes give improvements in one place, but create a total mess in the
> object file in other places. I am doing the Binutils port for a
> practical work at University and I am afraid not to have the required
> insight into the BFD to fix this on my own. Is there a possibility to
> request a fix by a maintainer directly?

I don't have time spare to do this at the moment, sorry.  I started
looking just at occurrences of output_offset (which is an address),
and found rather a lot of places that assume octet/byte == 1.

eg.
	p = sort + o->output_offset / ext_size * sort_elt;
and
      offset += sections[n]->size;

besides the calls to bfd_set_section_contents, which ought to be fixed
by the following patch.  I'm loathe to apply this patch to the
official sources until we have a contributed port that needs it,
because bfd_octets_per_bytes is not exactly a cheap function.  A
proper fix would involve adding an opb_shift or somesuch to the bfd
struct, and using that.

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.351
diff -u -p -r1.351 elflink.c
--- bfd/elflink.c	10 Aug 2009 06:14:04 -0000	1.351
+++ bfd/elflink.c	10 Aug 2009 11:56:30 -0000
@@ -9696,15 +9696,16 @@ elf_link_input_bfd (struct elf_final_lin
 	  }
 	  break;
 	default:
-	  {
-	    if (! (o->flags & SEC_EXCLUDE)
-		&& ! (o->output_section->flags & SEC_NEVER_LOAD)
-		&& ! bfd_set_section_contents (output_bfd, o->output_section,
-					       contents,
-					       (file_ptr) o->output_offset,
-					       o->size))
-	      return FALSE;
-	  }
+	  if (!(o->flags & SEC_EXCLUDE)
+	      && !(o->output_section->flags & SEC_NEVER_LOAD))
+	    {
+	      file_ptr loc;
+
+	      loc = o->output_offset * bfd_octets_per_byte (output_bfd);
+	      if (!bfd_set_section_contents (output_bfd, o->output_section,
+					     contents, loc, o->size))
+		return FALSE;
+	    }
 	  break;
 	}
     }
@@ -9803,6 +9804,7 @@ elf_reloc_link_order (bfd *output_bfd,
       bfd_byte *buf;
       bfd_boolean ok;
       const char *sym_name;
+      file_ptr loc;
 
       size = bfd_get_reloc_size (howto);
       buf = bfd_zmalloc (size);
@@ -9833,8 +9835,9 @@ elf_reloc_link_order (bfd *output_bfd,
 	    }
 	  break;
 	}
+      loc = link_order->offset * bfd_octets_per_byte (output_bfd);
       ok = bfd_set_section_contents (output_bfd, output_section, buf,
-				     link_order->offset, size);
+				     loc, size);
       free (buf);
       if (! ok)
 	return FALSE;
@@ -10999,10 +11002,11 @@ bfd_elf_final_link (bfd *abfd, struct bf
 	       != SHT_STRTAB)
 	      || strcmp (bfd_get_section_name (abfd, o), ".dynstr") != 0)
 	    {
+	      file_ptr loc;
+
+	      loc = o->output_offset * bfd_octets_per_byte (abfd);
 	      if (! bfd_set_section_contents (abfd, o->output_section,
-					      o->contents,
-					      (file_ptr) o->output_offset,
-					      o->size))
+					      o->contents, loc, o->size))
 		goto error_return;
 	    }
 	  else

-- 
Alan Modra
Australia Development Lab, IBM

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Dave Korn]

Alan Modra wrote:

> besides the calls to bfd_set_section_contents, which ought to be fixed
> by the following patch.  I'm loathe to apply this patch to the
> official sources until we have a contributed port that needs it,
> because bfd_octets_per_bytes is not exactly a cheap function.  A
> proper fix would involve adding an opb_shift or somesuch to the bfd
> struct, and using that.

  I'm working on a (private) port where octets_per_byte isn't even a power of 2 :-)

  I have a couple of binutils bugfixes to feed upstream, but obviously no
testcases.  I was considering submitting a toy CPU port that we could use to
keep an eye open for these sort of problems creeping in, if there was interest.

  (Alan, you appear to have suddenly started setting Mail-Followups-To: headers;
does this mean you don't want to be Cc'd on list posts any more?)

    cheers,
      DaveK

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Alan Modra]

On Mon, Aug 10, 2009 at 06:19:12PM +0100, Dave Korn wrote:
> Alan Modra wrote:
> 
> > besides the calls to bfd_set_section_contents, which ought to be fixed
> > by the following patch.  I'm loathe to apply this patch to the
> > official sources until we have a contributed port that needs it,
> > because bfd_octets_per_bytes is not exactly a cheap function.  A
> > proper fix would involve adding an opb_shift or somesuch to the bfd
> > struct, and using that.
> 
>   I'm working on a (private) port where octets_per_byte isn't even a power of 2 :-)

Fun.  ELF or COFF?

>   (Alan, you appear to have suddenly started setting Mail-Followups-To: headers;
> does this mean you don't want to be Cc'd on list posts any more?)

I haven't made any deliberate changes to my mail setup (and I don't
care one way or the other whether I'm Cc'd on binutils posts).

-- 
Alan Modra
Australia Development Lab, IBM

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Dave Korn]

Alan Modra wrote:
> On Mon, Aug 10, 2009 at 06:19:12PM +0100, Dave Korn wrote:
>> Alan Modra wrote:
>>
>>> besides the calls to bfd_set_section_contents, which ought to be fixed
>>> by the following patch.  I'm loathe to apply this patch to the
>>> official sources until we have a contributed port that needs it,
>>> because bfd_octets_per_bytes is not exactly a cheap function.  A
>>> proper fix would involve adding an opb_shift or somesuch to the bfd
>>> struct, and using that.
>>   I'm working on a (private) port where octets_per_byte isn't even a power of 2 :-)
> 
> Fun.  ELF or COFF?

  COFF.  ELF is left as an exercise for the reader :-)

  Re: "Fun", I'll just add that I've been pleasantly surprised by how smoothly
it all went; BFD and binutils in general handled it correctly and there were
only trivial bugs.  And CGEN makes it really easy, once you've done it once you
can knock out another port in a matter of days.

  It's mostly working so far but I've got the simulator and debugger yet to do.
 I've been given permission to invent a toy CPU of my own (which will have only
one thing in common, 24-bit bytes) to send upstream so as to be able to
contribute bugfixes with some kind of evidence that they're needed and work; do
you think it would be worth adding the full port to the repository?

    cheers,
      DaveK

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Alan Modra]

On Tue, Aug 11, 2009 at 08:53:54AM +0930, Alan Modra wrote:
> On Mon, Aug 10, 2009 at 06:19:12PM +0100, Dave Korn wrote:
> >   (Alan, you appear to have suddenly started setting Mail-Followups-To: headers;

Suddenly?  I went back to 2003 and found I had follow-ups set.
Apparently added by mutt.  (.muttrc subscribed configuration command.)

-- 
Alan Modra
Australia Development Lab, IBM

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Dave Korn]

Alan Modra wrote:
> On Tue, Aug 11, 2009 at 08:53:54AM +0930, Alan Modra wrote:
>> On Mon, Aug 10, 2009 at 06:19:12PM +0100, Dave Korn wrote:
>>>   (Alan, you appear to have suddenly started setting Mail-Followups-To: headers;
> 
> Suddenly?  I went back to 2003 and found I had follow-ups set.

  So it is.  I suddenly noticed it.  I think that's because often enough in the
past I've replied to a thread where you were on the Cc line, but not to one of
your posts, so it didn't have the f'ups-to set and led me to unconsciously infer
that when I Reply-All'd a mail with your name on it it would /always/ Cc you a
copy.  Nothing to see here, I guess!

    cheers,
      DaveK

Re: How to use bits_per_byte (~OCTETS_PER_BYTE) of bfd_arch_info_type in binutils port?

[Alan Modra]

On Tue, Aug 11, 2009 at 01:28:48AM +0100, Dave Korn wrote:
> do you think it would be worth adding the full port to the repository?

You're welcome to as far as I'm concerned.

-- 
Alan Modra
Australia Development Lab, IBM