This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[rfa] Fix signed modulo arrith in elf.c


Hello,

Ref: http://sources.redhat.com/ml/binutils/2004-01/msg00325.html

the attached patch replaces a number of cases of signed modulo arrithmetic in elf.c with a call to a more robust function.

I've tested it on Fedora Core 1 (i386, little endian) and NetBSD (PPC, big endian) without regressions (this change won't really have an effect until after 64-bit off_t is enabled).

ok to commit?

Andrew

(This hopefully leaves me with just a patch to make file_ptr 64-bit. I'll send that one out tomorrow ...).
2004-02-12  Andrew Cagney  <cagney@redhat.com>

	* elf.c: Update copyright.
	(vma_page_aligned_bias): New function.
	(assign_file_positions_except_relocs)
	(assign_file_positions_for_segments): Replace broken modulo
	arithmetic with call to vma_page_aligned_bias.

Index: elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.216
diff -u -r1.216 elf.c
--- elf.c	9 Feb 2004 08:04:00 -0000	1.216
+++ elf.c	12 Feb 2004 17:48:02 -0000
@@ -1,6 +1,7 @@
 /* ELF executable support for BFD.
-   Copyright 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002,
-   2003 Free Software Foundation, Inc.
+
+   Copyright 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001,
+   2002, 2003, 2004 Free Software Foundation, Inc.
 
    This file is part of BFD, the Binary File Descriptor library.
 
@@ -3571,6 +3572,35 @@
   return sec1->target_index - sec2->target_index;
 }
 
+/* Ian Lance Taylor writes:
+
+   We shouldn't be using % with a negative signed number.  That's just
+   not good.  We have to make sure either that the number is not
+   negative, or that the number has an unsigned type.  When the types
+   are all the same size they wind up as unsigned.  When file_ptr is a
+   larger signed type, the arithmetic winds up as signed long long,
+   which is wrong.
+
+   What we're trying to say here is something like ``increase OFF by
+   the least amount that will cause it to be equal to the VMA modulo
+   the page size.''  */
+/* In other words, something like:
+
+   vma_offset = m->sections[0]->vma % bed->maxpagesize;
+   off_offset = off % bed->maxpagesize;
+   if (vma_offset < off_offset)
+     adjustment = vma_offset + bed->maxpagesize - off_offset;
+   else
+     adjustment = vma_offset - off_offset;
+     
+   which can can be collapsed into the expression below.  */
+
+static file_ptr
+vma_page_aligned_bias (bfd_vma vma, ufile_ptr off, bfd_vma maxpagesize)
+{
+  return ((vma - off) % maxpagesize);
+}
+
 /* Assign file positions to the sections based on the mapping from
    sections to segments.  This function also sets up some fields in
    the file header, and writes out the program headers.  */
@@ -3698,7 +3728,8 @@
 	  && (m->sections[0]->flags & SEC_ALLOC) != 0)
 	{
 	  if ((abfd->flags & D_PAGED) != 0)
-	    off += (m->sections[0]->vma - off) % bed->maxpagesize;
+	    off += vma_page_aligned_bias (m->sections[0]->vma, off,
+					  bed->maxpagesize);
 	  else
 	    {
 	      bfd_size_type align;
@@ -3713,7 +3744,8 @@
 		    align = secalign;
 		}
 
-	      off += (m->sections[0]->vma - off) % (1 << align);
+	      off += vma_page_aligned_bias (m->sections[0]->vma, off,
+					    1 << align);
 	    }
 	}
 
@@ -3875,9 +3907,11 @@
 		     not have the SEC_LOAD case just above, and then
 		     this was necessary, but now I'm not sure.  */
 		  if ((abfd->flags & D_PAGED) != 0)
-		    adjust = (sec->vma - voff) % bed->maxpagesize;
+		    adjust = vma_page_aligned_bias (sec->vma, voff,
+						    bed->maxpagesize);
 		  else
-		    adjust = (sec->vma - voff) % align;
+		    adjust = vma_page_aligned_bias (sec->vma, voff,
+						    align);
 		}
 	      else
 		adjust = 0;
@@ -4211,9 +4245,11 @@
 		 ? "*unknown*"
 		 : hdr->bfd_section->name)));
 	      if ((abfd->flags & D_PAGED) != 0)
-		off += (hdr->sh_addr - off) % bed->maxpagesize;
+		off += vma_page_aligned_bias (hdr->sh_addr, off,
+					      bed->maxpagesize);
 	      else
-		off += (hdr->sh_addr - off) % hdr->sh_addralign;
+		off += vma_page_aligned_bias (hdr->sh_addr, off,
+					      hdr->sh_addralign);
 	      off = _bfd_elf_assign_file_position_for_section (hdr, off,
 							       FALSE);
 	    }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]