public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@bigpond.net.au>
To: binutils@sourceware.org
Subject: Fix regression introduced 2006-04-21
Date: Wed, 17 May 2006 15:32:00 -0000	[thread overview]
Message-ID: <20060517050852.GL19700@bubble.grove.modra.org> (raw)
In-Reply-To: <20060517003519.GJ19700@bubble.grove.modra.org>

When looking at Etienne's testcase, I saw some very wierd readelf
output.  It turns out I introduced a bug with my 2006-04-21 change to
assign_file_positions_for_segments.  I hadn't noticed an early exit from
the function when no section is mapped to segments, a possibility when
--gc-sections removes everything.  The early exit meant that .shstrtab,
.symtab, and .strtab did not have their file positions set, and thus
overwrote the headers.  Oops.

	* elf.c (assign_file_positions_for_segments): Split into..
	(assign_file_positions_for_load_sections): ..this, and..
	(assign_file_positions_for_non_load_sections): ..this new function,..
	(assign_file_positions_except_relocs): ..writing program headers here.

Index: bfd/elf.c
===================================================================
RCS file: /cvs/src/src/bfd/elf.c,v
retrieving revision 1.336
diff -u -p -r1.336 elf.c
--- bfd/elf.c	11 May 2006 12:34:46 -0000	1.336
+++ bfd/elf.c	17 May 2006 03:56:02 -0000
@@ -4106,24 +4106,20 @@ print_segment_map (bfd *abfd)
 
 /* 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.  */
+   the file header.  */
 
 static bfd_boolean
-assign_file_positions_for_segments (bfd *abfd, struct bfd_link_info *link_info)
+assign_file_positions_for_load_sections (bfd *abfd,
+					 struct bfd_link_info *link_info)
 {
   const struct elf_backend_data *bed = get_elf_backend_data (abfd);
-  unsigned int count;
   struct elf_segment_map *m;
-  unsigned int alloc;
   Elf_Internal_Phdr *phdrs;
-  file_ptr off, voff;
-  bfd_vma filehdr_vaddr, filehdr_paddr;
-  bfd_vma phdrs_vaddr, phdrs_paddr;
   Elf_Internal_Phdr *p;
-  Elf_Internal_Shdr **i_shdrpp;
-  Elf_Internal_Shdr **hdrpp;
+  file_ptr off, voff;
+  unsigned int count;
+  unsigned int alloc;
   unsigned int i;
-  unsigned int num_sec;
 
   if (elf_tdata (abfd)->segment_map == NULL)
     {
@@ -4196,20 +4192,19 @@ assign_file_positions_for_segments (bfd 
     }
 
   if (alloc == 0)
-    alloc = count;
+    {
+      alloc = count;
+      elf_tdata (abfd)->program_header_size = alloc * bed->s->sizeof_phdr;
+    }
 
   phdrs = bfd_alloc2 (abfd, alloc, sizeof (Elf_Internal_Phdr));
+  elf_tdata (abfd)->phdr = phdrs;
   if (phdrs == NULL)
     return FALSE;
 
   off = bed->s->sizeof_ehdr;
   off += alloc * bed->s->sizeof_phdr;
 
-  filehdr_vaddr = 0;
-  filehdr_paddr = 0;
-  phdrs_vaddr = 0;
-  phdrs_paddr = 0;
-
   for (m = elf_tdata (abfd)->segment_map, p = phdrs;
        m != NULL;
        m = m->next, p++)
@@ -4345,11 +4340,6 @@ assign_file_positions_for_segments (bfd 
 	      if (! m->p_paddr_valid)
 		p->p_paddr -= off;
 	    }
-	  if (p->p_type == PT_LOAD)
-	    {
-	      filehdr_vaddr = p->p_vaddr;
-	      filehdr_paddr = p->p_paddr;
-	    }
 	}
 
       if (m->includes_phdrs)
@@ -4357,15 +4347,7 @@ assign_file_positions_for_segments (bfd 
 	  if (! m->p_flags_valid)
 	    p->p_flags |= PF_R;
 
-	  if (m->includes_filehdr)
-	    {
-	      if (p->p_type == PT_LOAD)
-		{
-		  phdrs_vaddr = p->p_vaddr + bed->s->sizeof_ehdr;
-		  phdrs_paddr = p->p_paddr + bed->s->sizeof_ehdr;
-		}
-	    }
-	  else
+	  if (!m->includes_filehdr)
 	    {
 	      p->p_offset = bed->s->sizeof_ehdr;
 
@@ -4376,14 +4358,6 @@ assign_file_positions_for_segments (bfd 
 		  if (! m->p_paddr_valid)
 		    p->p_paddr -= off - p->p_offset;
 		}
-
-	      if (p->p_type == PT_LOAD)
-		{
-		  phdrs_vaddr = p->p_vaddr;
-		  phdrs_paddr = p->p_paddr;
-		}
-	      else
-		phdrs_vaddr = bed->maxpagesize + bed->s->sizeof_ehdr;
 	    }
 
 	  p->p_filesz += alloc * bed->s->sizeof_phdr;
@@ -4538,9 +4512,39 @@ assign_file_positions_for_segments (bfd 
 	}
     }
 
-  /* Assign file positions for the other sections.  */
+  /* Clear out any program headers we allocated but did not use.  */
+  for (; count < alloc; count++, p++)
+    {
+      memset (p, 0, sizeof *p);
+      p->p_type = PT_NULL;
+    }
+
+  elf_tdata (abfd)->next_file_pos = off;
+  return TRUE;
+}
+
+/* Assign file positions for the other sections.  */
+
+static bfd_boolean
+assign_file_positions_for_non_load_sections (bfd *abfd,
+					     struct bfd_link_info *link_info)
+{
+  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
+  Elf_Internal_Shdr **i_shdrpp;
+  Elf_Internal_Shdr **hdrpp;
+  Elf_Internal_Phdr *phdrs;
+  Elf_Internal_Phdr *p;
+  struct elf_segment_map *m;
+  bfd_vma filehdr_vaddr, filehdr_paddr;
+  bfd_vma phdrs_vaddr, phdrs_paddr;
+  file_ptr off;
+  unsigned int num_sec;
+  unsigned int i;
+  unsigned int count;
+
   i_shdrpp = elf_elfsections (abfd);
   num_sec = elf_numsections (abfd);
+  off = elf_tdata (abfd)->next_file_pos;
   for (i = 1, hdrpp = i_shdrpp + 1; i < num_sec; i++, hdrpp++)
     {
       struct elf_obj_tdata *tdata = elf_tdata (abfd);
@@ -4585,6 +4589,37 @@ assign_file_positions_for_segments (bfd 
 
   /* Now that we have set the section file positions, we can set up
      the file positions for the non PT_LOAD segments.  */
+  count = 0;
+  filehdr_vaddr = 0;
+  filehdr_paddr = 0;
+  phdrs_vaddr = bed->maxpagesize + bed->s->sizeof_ehdr;
+  phdrs_paddr = 0;
+  phdrs = elf_tdata (abfd)->phdr;
+  for (m = elf_tdata (abfd)->segment_map, p = phdrs;
+       m != NULL;
+       m = m->next, p++)
+    {
+      ++count;
+      if (p->p_type != PT_LOAD)
+	continue;
+
+      if (m->includes_filehdr)
+	{
+	  filehdr_vaddr = p->p_vaddr;
+	  filehdr_paddr = p->p_paddr;
+	}
+      if (m->includes_phdrs)
+	{
+	  phdrs_vaddr = p->p_vaddr;
+	  phdrs_paddr = p->p_paddr;
+	  if (m->includes_filehdr)
+	    {
+	      phdrs_vaddr += bed->s->sizeof_ehdr;
+	      phdrs_paddr += bed->s->sizeof_ehdr;
+	    }
+	}
+    }
+
   for (m = elf_tdata (abfd)->segment_map, p = phdrs;
        m != NULL;
        m = m->next, p++)
@@ -4657,22 +4692,8 @@ assign_file_positions_for_segments (bfd 
 	}
     }
 
-  /* Clear out any program headers we allocated but did not use.  */
-  for (; count < alloc; count++, p++)
-    {
-      memset (p, 0, sizeof *p);
-      p->p_type = PT_NULL;
-    }
-
-  elf_tdata (abfd)->phdr = phdrs;
-
   elf_tdata (abfd)->next_file_pos = off;
 
-  /* Write out the program headers.  */
-  if (bfd_seek (abfd, (bfd_signed_vma) bed->s->sizeof_ehdr, SEEK_SET) != 0
-      || bed->s->write_out_phdrs (abfd, phdrs, alloc) != 0)
-    return FALSE;
-
   return TRUE;
 }
 
@@ -4844,9 +4865,21 @@ assign_file_positions_except_relocs (bfd
     }
   else
     {
+      unsigned int alloc;
+
       /* Assign file positions for the loaded sections based on the
          assignment of sections to segments.  */
-      if (! assign_file_positions_for_segments (abfd, link_info))
+      if (!assign_file_positions_for_load_sections (abfd, link_info))
+	return FALSE;
+
+      /* And for non-load sections.  */
+      if (!assign_file_positions_for_non_load_sections (abfd, link_info))
+	return FALSE;
+
+      /* Write out the program headers.  */
+      alloc = tdata->program_header_size / bed->s->sizeof_phdr;
+      if (bfd_seek (abfd, (bfd_signed_vma) bed->s->sizeof_ehdr, SEEK_SET) != 0
+	  || bed->s->write_out_phdrs (abfd, tdata->phdr, alloc) != 0)
 	return FALSE;
 
       off = tdata->next_file_pos;
 
-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

  reply	other threads:[~2006-05-17  5:09 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-05-16 15:28 `.sym' referenced in section `reloc_sym' of file.o: defined in discarded section `.text.sym' of file.o Etienne Lorrain
2006-05-16 22:39 ` Alan Modra
2006-05-16 22:47   ` Etienne Lorrain
2006-05-17 11:34     ` Alan Modra
2006-05-17 15:32       ` Alan Modra [this message]
2006-05-17 16:19       ` Etienne Lorrain
2006-05-19 11:50         ` Alan Modra
2006-05-19 15:30           ` Etienne Lorrain
2006-05-19 15:37           ` Alan Modra
2006-05-19 15:57           ` Etienne Lorrain
2006-05-20 13:56             ` Alan Modra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060517050852.GL19700@bubble.grove.modra.org \
    --to=amodra@bigpond.net.au \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).