Re: [PATCH] add --reverse option to objcopy

[Nathan Froyd <froydnj@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

On Fri, Apr 20, 2007 at 02:32:56PM +0100, Nick Clifton wrote:
>   * H.J. pointed out that the name of the switch is slightly confusing.
>     --reverse-endian or --reverse-bytes might be a more reasonable
>     name.

I have changed the option to --reverse-bytes in the code and the
documentation.

>   * I think that this paragraph in the documentation is rather
>     confusing:
> 
> +By using @samp{--reverse=2} followed by @samp{--reverse=4}, then bytes
> +ordered @code{34127856} would be generated.
> 
>     To me this implied that you can use the --reverse switch twice on 
> the same command line in order to achieve the effect you describe.  If I 
> have read the code correctly however, only the last instance of the 
> switch on the command line will be honored, and so you need to run the 
> objcopy command *twice* in order to achieve the desired effect.

Yes, that's not particularly clear.  I have altered the paragraph to
make it more obvious that you need to run the command twice.

>   * The code should check to see if --reverse if issued more than once 
> on the command line and (if they have different values) warn the user 
> that the earlier value(s) are being ignored.

Done.

OK?

-Nathan

[-- Attachment #2: reverse.patch --]
[-- Type: text/plain, Size: 8539 bytes --]

binutils/
2007-04-20  Nathan Froyd  <froydnj@codesourcery.com>
	    Phil Edwards  <phil@codesourcery.com>
	    Thomas de Lellis <tdel@windriver.com>

        * objcopy.c (reverse_bytes):  New variable.
        (command_line_switch, copy_main):  Add OPTION_REVERSE_ENDIAN.
        (copy_options, copy_usage):  Add "reverse-bytes" entry.
        (copy_section):  Reverse bytes within output sections.
        * doc/binutils.texi:  Document new objcopy option.

binutils/testsuite/
2007-04-20  Nathan Froyd  <froydnj@codesourcery.com>
	    Phil Edwards  <phil@codesourcery.com>
	    Thomas de Lellis <tdel@windriver.com>

	* binutils-all/objcopy.exp: Add test for --reverse-bytes.

Index: binutils/objcopy.c
===================================================================
RCS file: /cvs/src/src/binutils/objcopy.c,v
retrieving revision 1.107
diff -u -r1.107 objcopy.c
--- binutils/objcopy.c	12 Apr 2007 19:20:46 -0000	1.107
+++ binutils/objcopy.c	20 Apr 2007 16:58:18 -0000
@@ -218,6 +218,11 @@
 /* True if --extract-symbol was passed on the command line.  */
 static bfd_boolean extract_symbol = FALSE;
 
+/* If `reverse_bytes' is nonzero, then reverse the order of every chunk
+   of <reverse_bytes> bytes within each output section.  */
+static int reverse_bytes = 0;
+
+
 /* 150 isn't special; it's just an arbitrary non-ASCII char value.  */
 enum command_line_switch
   {
@@ -265,7 +270,8 @@
     OPTION_WRITABLE_TEXT,
     OPTION_PURE,
     OPTION_IMPURE,
-    OPTION_EXTRACT_SYMBOL
+    OPTION_EXTRACT_SYMBOL,
+    OPTION_REVERSE_BYTES
   };
 
 /* Options to handle if running as "strip".  */
@@ -358,6 +364,7 @@
   {"remove-leading-char", no_argument, 0, OPTION_REMOVE_LEADING_CHAR},
   {"remove-section", required_argument, 0, 'R'},
   {"rename-section", required_argument, 0, OPTION_RENAME_SECTION},
+  {"reverse-bytes", required_argument, 0, OPTION_REVERSE_BYTES},
   {"set-section-flags", required_argument, 0, OPTION_SET_SECTION_FLAGS},
   {"set-start", required_argument, 0, OPTION_SET_START},
   {"srec-len", required_argument, 0, OPTION_SREC_LEN},
@@ -471,6 +478,7 @@
      --rename-section <old>=<new>[,<flags>] Rename section <old> to <new>\n\
      --change-leading-char         Force output format's leading character style\n\
      --remove-leading-char         Remove leading character from global symbols\n\
+     --reverse-bytes=<num>         Reverse <num> bytes at a time, in output sections with content\n\
      --redefine-sym <old>=<new>    Redefine symbol name <old> to <new>\n\
      --redefine-syms <file>        --redefine-sym for all symbol pairs \n\
                                      listed in <file>\n\
@@ -2383,6 +2391,32 @@
       if (!bfd_get_section_contents (ibfd, isection, memhunk, 0, size))
 	RETURN_NONFATAL (bfd_get_filename (ibfd));
 
+      if (reverse_bytes)
+	{
+	  /* We don't handle leftover bytes (too many possible behaviors,
+	     and we don't know what the user wants).  The section length
+	     must be a multiple of the number of bytes to swap.  */
+	  if ((size % reverse_bytes) == 0)
+	    {
+	      unsigned long i, j;
+	      bfd_byte b;
+
+	      for (i = 0; i < size; i += reverse_bytes)
+		for (j = 0; j < (unsigned long)(reverse_bytes / 2); j++)
+		  {
+		    bfd_byte *m = (bfd_byte*)memhunk;
+
+		    b = m[i + j];
+		    m[i + j] = m[(i + reverse_bytes) - (j + 1)];
+		    m[(i + reverse_bytes) - (j + 1)] = b;
+		  }
+	    }
+	  else
+	    /* User must pad the section up in order to do this.  */
+	    fatal (_("cannot reverse bytes: length of section %s must be evenly divisible by %d"),
+		bfd_section_name (ibfd, isection), reverse_bytes);
+	}
+
       if (copy_byte >= 0)
 	{
 	  /* Keep only every `copy_byte'th byte in MEMHUNK.  */
@@ -3256,6 +3290,20 @@
 	  extract_symbol = TRUE;
 	  break;
 
+	case OPTION_REVERSE_BYTES:
+          {
+            int prev = reverse_bytes;
+
+            reverse_bytes = atoi (optarg);
+            if ((reverse_bytes <= 0) || ((reverse_bytes % 2) != 0))
+              fatal (_("number of bytes to reverse must be positive and even"));
+            break;
+
+            if (prev && prev != reverse_bytes)
+              non_fatal (_("Warning: ignoring previous --reverse-bytes value of %d"),
+                         prev);
+          }
+
 	case 0:
 	  /* We've been given a long option.  */
 	  break;
Index: binutils/doc/binutils.texi
===================================================================
RCS file: /cvs/src/src/binutils/doc/binutils.texi,v
retrieving revision 1.104
diff -u -r1.104 binutils.texi
--- binutils/doc/binutils.texi	28 Mar 2007 09:08:53 -0000	1.104
+++ binutils/doc/binutils.texi	20 Apr 2007 16:58:18 -0000
@@ -987,6 +987,7 @@
         [@option{--add-section} @var{sectionname}=@var{filename}]
         [@option{--rename-section} @var{oldname}=@var{newname}[,@var{flags}]]
         [@option{--change-leading-char}] [@option{--remove-leading-char}]
+        [@option{--reverse-bytes=}@var{num}]
         [@option{--srec-len=}@var{ival}] [@option{--srec-forceS3}]
         [@option{--redefine-sym} @var{old}=@var{new}]
         [@option{--redefine-syms=}@var{filename}]
@@ -1052,6 +1053,7 @@
 files.  If the input format has an endianness (some formats do not),
 @command{objcopy} can only copy the inputs into file formats that have the
 same endianness or which have no endianness (e.g., @samp{srec}).
+(However, see the @option{--reverse-bytes} option.)
 
 @c man end
 
@@ -1342,6 +1344,30 @@
 when appropriate, regardless of the object file format of the output
 file.
 
+@item --reverse-bytes=@var{num}
+Reverse the bytes in a section with output contents.  A section length must
+be evenly divisible by the value given in order for the swap to be able to
+take place. Reversing takes place before the interleaving is performed.
+
+This option is used typically in generating ROM images for problematic
+target systems.  For example, on some target boards, the 32-bit words
+fetched from 8-bit ROMs are re-assembled in little-endian byte order
+regardless of the CPU byte order.  Depending on the programming model, the
+endianness of the ROM may need to be modified.
+
+Consider a simple file with a section containing the following eight
+bytes:  @code{12345678}.
+
+Using @samp{--reverse-bytes=2} for the above example, the bytes in the
+output file would be ordered @code{21436587}.
+
+Using @samp{--reverse-bytes=4} for the above example, the bytes in the
+output file would be ordered @code{43218765}.
+
+By using @samp{--reverse-bytes=2} for the above example, followed by
+@samp{--reverse-bytes=4} on the output file, the bytes in the second
+output file would be ordered @code{34127856}.
+
 @item --srec-len=@var{ival}
 Meaningful only for srec output.  Set the maximum length of the Srecords
 being produced to @var{ival}.  This length covers both address, data and
Index: binutils/testsuite/binutils-all/objcopy.exp
===================================================================
RCS file: /cvs/src/src/binutils/testsuite/binutils-all/objcopy.exp,v
retrieving revision 1.41
diff -u -r1.41 objcopy.exp
--- binutils/testsuite/binutils-all/objcopy.exp	12 Apr 2007 19:20:46 -0000	1.41
+++ binutils/testsuite/binutils-all/objcopy.exp	20 Apr 2007 16:58:18 -0000
@@ -113,5 +113,39 @@
 
 objcopy_test "simple copy" bintest.s
 
+# Test reversing bytes in a section.
+
+set reversed ${tempfile}-reversed
+set got [binutils_run $OBJCOPY "$OBJCOPYFLAGS -j .data --reverse-bytes=4 $tempfile $reversed"]
+
+if ![string match "" $got] then {
+    fail "objcopy --reverse-bytes"
+} else {
+    if [is_remote host] {
+        remote_upload host ${reversed} tmpdir/copy-reversed.o
+        set reversed tmpdir/copy-reversed.o
+    }
+
+    set origdata [binutils_run $OBJDUMP "$OBJDUMPFLAGS -s -j .data $tempfile"]
+    set revdata [binutils_run $OBJDUMP "$OBJDUMPFLAGS -s -j .data $reversed"]
+
+    set want "^ \[0-9\]+ (\[0-9\]+)"
+    set found_orig [regexp -lineanchor $want $origdata -> origdata]
+    set found_rev [regexp -lineanchor $want $revdata -> revdata]
+
+    if {$found_orig == 0 || $found_rev == 0} then {
+        fail "objcopy --reverse-bytes"
+    } else {
+        scan $origdata "%2x%2x%2x%2x" b1 b2 b3 b4
+        scan $revdata "%2x%2x%2x%2x" c4 c3 c2 c1
+
+        if {$b1 == $c1 && $b2 == $c2 && $b3 == $c3 && $b4 == $c4} then {
+            pass "objcopy --reverse-bytes"
+        } else {
+            fail "objcopy --reverse-bytes"
+        }
+    }
+}
+
 # Test generating S records.
 
 # We make the srec filename 8.3 compatible. Note that the header string