Re: [RFC PATCH] pad out code segment for NaCl targets

[Roland McGrath <mcgrathr at google dot com>]

On Fri, Aug 16, 2013 at 9:49 AM, Roland McGrath <mcgrathr@google.com> wrote:
> Perhaps I can have the modify_segment_map hook synthesize a fake section
> that isn't really there or something.  I'll play with that while I wait for
> a better suggestion.

So I did that.  It seems to work.  It doesn't have the bad material effects
I was hoping to avoid (extra output section, potential bad interactions
with input sections of the magic name).  It is a dismal, dismal kludge,
clearly worse in that regard than my earlier hack.  But, you know, BFD.

It also causes several test suite failures that I have not yet figured out.
I'll investigate those now.  In the meantime, I'd appreciate any feedback
anyone has on this.


Thanks,
Roland


bfd/
2013-08-16  Roland McGrath  <mcgrathr@google.com>

        * elf-nacl.c (nacl_modify_segment_map): Fix logic reordering the
        elf_segment_map list.  If an executable segment is page-aligned
        but does not end with a full page, then append a fake section into
        the segment map entry that pads out the page.
        (nacl_final_write_processing): New function.  Write the code fill
        laid out in nacl_modify_segment_map.
        * elf-nacl.h: Declare it.
        * elf32-arm.c (elf32_arm_nacl_final_write_processing): New function.
        (elf_backend_final_write_processing): Define it for NaCl backend.
        * elf32-i386.c (elf_backend_final_write_processing): Likewise.
        * elf64-x86-64.c (elf_backend_final_write_processing): Likewise.

--- a/bfd/elf-nacl.c
+++ b/bfd/elf-nacl.c
@@ -1,5 +1,5 @@
 /* Native Client support for ELF
-   Copyright 2012 Free Software Foundation, Inc.
+   Copyright 2012, 2013 Free Software Foundation, Inc.

    This file is part of BFD, the Binary File Descriptor library.

@@ -20,6 +20,7 @@

 #include "sysdep.h"
 #include "bfd.h"
+#include "libbfd.h"
 #include "elf-bfd.h"
 #include "elf-nacl.h"
 #include "elf/common.h"
@@ -89,14 +90,84 @@ nacl_modify_segment_map (bfd *abfd, struct
bfd_link_info *info)

       if (seg->p_type == PT_LOAD)
        {
+         bfd_boolean executable = segment_executable (seg);
+
+         if (executable
+             && seg->count > 0
+             && seg->sections[0]->vma % maxpagesize == 0)
+           {
+             asection *lastsec = seg->sections[seg->count - 1];
+             bfd_vma end = lastsec->vma + lastsec->size;
+             if (end % maxpagesize != 0)
+               {
+                 /* This is an executable segment that starts on a page
+                    boundary but does not end on a page boundary.  Fill
+                    it out to a whole page with code fill (the tail of
+                    the segment will not be within any section).  Thus
+                    the entire code segment can be mapped from the file
+                    as whole pages and that mapping will contain only
+                    valid instructions.
+
+                    To accomplish this, we must fake out the code in
+                    assign_file_positions_for_load_sections (elf.c) so
+                    that it advances past the rest of the final page,
+                    rather than trying to put the next (unaligned, or
+                    unallocated) section.  We do this by appending a
+                    dummy section record to this element in the segment
+                    map.  No such output section ever actually exists,
+                    but this gets the layout logic to advance the file
+                    positions past this partial page.  Since we are
+                    lying to BFD like this, nothing will ever know to
+                    write the section contents.  So we do that by hand
+                    after the fact, in nacl_final_write_processing, below.  */
+
+                 struct elf_segment_map *newseg;
+                 asection *sec;
+                 struct bfd_elf_section_data *secdata;
+
+                 BFD_ASSERT (!seg->p_size_valid);
+
+                 secdata = bfd_zalloc (abfd, sizeof *secdata);
+                 if (secdata == NULL)
+                   return FALSE;
+
+                 sec = bfd_zalloc (abfd, sizeof *sec);
+                 if (sec == NULL)
+                   return FALSE;
+
+                 /* Fill in only the fields that actually affect the logic
+                    in assign_file_positions_for_load_sections.  */
+                 sec->vma = sec->lma = end;
+                 sec->size = maxpagesize - (end % maxpagesize);
+                 sec->flags = (SEC_ALLOC | SEC_LOAD
+                               | SEC_READONLY | SEC_CODE | SEC_LINKER_CREATED);
+                 sec->used_by_bfd = secdata;
+
+                 secdata->this_hdr.sh_type = SHT_PROGBITS;
+                 secdata->this_hdr.sh_flags = SHF_ALLOC | SHF_EXECINSTR;
+                 secdata->this_hdr.sh_addr = sec->vma;
+                 secdata->this_hdr.sh_size = sec->size;
+
+                 newseg = bfd_alloc (abfd,
+                                     sizeof *newseg + ((seg->count + 1)
+                                                       * sizeof (asection *)));
+                 if (newseg == NULL)
+                   return FALSE;
+                 memcpy (newseg, seg,
+                         sizeof *newseg + (seg->count * sizeof (asection *)));
+                 newseg->sections[newseg->count++] = sec;
+                 *m = seg = newseg;
+               }
+           }
+
          /* First, we're just finding the earliest PT_LOAD.
             By the normal rules, this will be the lowest-addressed one.
             We only have anything interesting to do if it's executable.  */
          last_load = m;
          if (first_load == NULL)
            {
-              if (!segment_executable (*m))
-                return TRUE;
+             if (!executable)
+               goto next;
              first_load = m;
            }
          /* Now that we've noted the first PT_LOAD, we're looking for
@@ -127,6 +198,7 @@ nacl_modify_segment_map (bfd *abfd, struct
bfd_link_info *info)
            }
        }

+    next:
       m = &seg->next;
     }

@@ -183,7 +255,7 @@ nacl_modify_program_headers (bfd *abfd, struct
bfd_link_info *info)
       m = &(*m)->next;
       ++p;

-      while ((*m) != NULL)
+      while (*m != NULL)
        {
          if (p->p_type == PT_LOAD && p->p_vaddr < first_load_phdr->p_vaddr)
            {
@@ -207,12 +279,24 @@ nacl_modify_program_headers (bfd *abfd, struct
bfd_link_info *info)
          struct elf_segment_map *first_next = first_seg->next;
          struct elf_segment_map *next_next = next_seg->next;

-          first_seg->next = next_next;
+         if (next_load_seg == &first_seg->next)
+           {
              *first_load_seg = next_seg;
+             next_seg->next = first_seg;
+             first_seg->next = next_next;
+           }
+         else
+           {
+             *first_load_seg = first_next;
+             *next_load_seg = next_next;

-          next_seg->next = first_next;
+             first_seg->next = *next_load_seg;
              *next_load_seg = first_seg;

+             next_seg->next = *first_load_seg;
+             *first_load_seg = next_seg;
+           }
+
          move_phdr = *next_load_phdr;
          memmove (first_load_phdr + 1, first_load_phdr,
                   (next_load_phdr - first_load_phdr) * sizeof move_phdr);
@@ -222,3 +306,35 @@ nacl_modify_program_headers (bfd *abfd, struct
bfd_link_info *info)

   return TRUE;
 }
+
+void
+nacl_final_write_processing (bfd *abfd, bfd_boolean linker ATTRIBUTE_UNUSED)
+{
+  struct elf_segment_map *seg;
+  for (seg = elf_seg_map (abfd); seg != NULL; seg = seg->next)
+    if (seg->p_type == PT_LOAD
+       && seg->count > 1
+       && seg->sections[seg->count - 1]->owner == NULL)
+      {
+       asection *sec = seg->sections[seg->count - 1];
+       char *fill;
+
+       BFD_ASSERT (sec->flags & SEC_LINKER_CREATED);
+       BFD_ASSERT (sec->flags & SEC_CODE);
+       BFD_ASSERT (sec->size > 0);
+
+       fill = abfd->arch_info->fill (sec->size, bfd_big_endian (abfd), TRUE);
+
+       if (fill == NULL
+           || bfd_seek (abfd, sec->filepos, SEEK_SET) != 0
+           || bfd_bwrite (fill, sec->size, abfd) != sec->size)
+         {
+           /* We don't have a proper way to report an error here.  So
+              instead fudge things so that elf_write_shdrs_and_ehdr will
+              fail.  */
+           elf_elfheader (abfd)->e_shoff = (file_ptr) -1;
+         }
+
+       free (fill);
+      }
+}
--- a/bfd/elf-nacl.h
+++ b/bfd/elf-nacl.h
@@ -1,5 +1,5 @@
 /* Native Client support for ELF
-   Copyright 2012 Free Software Foundation, Inc.
+   Copyright 2012, 2013 Free Software Foundation, Inc.

    This file is part of BFD, the Binary File Descriptor library.

@@ -22,3 +22,4 @@

 bfd_boolean nacl_modify_segment_map (bfd *, struct bfd_link_info *);
 bfd_boolean nacl_modify_program_headers (bfd *, struct bfd_link_info *);
+void nacl_final_write_processing (bfd *, bfd_boolean linker);
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -15860,6 +15860,14 @@ elf32_arm_nacl_modify_segment_map (bfd *abfd,
struct bfd_link_info *info)
          && nacl_modify_segment_map (abfd, info));
 }

+static void
+elf32_arm_nacl_final_write_processing (bfd *abfd, bfd_boolean linker)
+{
+  elf32_arm_final_write_processing (abfd, linker);
+  nacl_final_write_processing (abfd, linker);
+}
+
+
 #undef elf32_bed
 #define elf32_bed                      elf32_arm_nacl_bed
 #undef  bfd_elf32_bfd_link_hash_table_create
@@ -15871,6 +15879,8 @@ elf32_arm_nacl_modify_segment_map (bfd *abfd,
struct bfd_link_info *info)
 #define        elf_backend_modify_segment_map
elf32_arm_nacl_modify_segment_map
 #undef elf_backend_modify_program_headers
 #define        elf_backend_modify_program_headers
nacl_modify_program_headers
+#undef  elf_backend_final_write_processing
+#define elf_backend_final_write_processing
elf32_arm_nacl_final_write_processing

 #undef ELF_MAXPAGESIZE
 #define ELF_MAXPAGESIZE                        0x10000
@@ -15882,6 +15892,8 @@ elf32_arm_nacl_modify_segment_map (bfd *abfd,
struct bfd_link_info *info)
 #undef elf_backend_modify_segment_map
 #define elf_backend_modify_segment_map         elf32_arm_modify_segment_map
 #undef elf_backend_modify_program_headers
+#undef  elf_backend_final_write_processing
+#define elf_backend_final_write_processing     elf32_arm_final_write_processing

 /* VxWorks Targets.  */

--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -5302,12 +5302,15 @@ static const struct elf_i386_backend_data
elf_i386_nacl_arch_bed =
 #define        elf_backend_modify_segment_map          nacl_modify_segment_map
 #undef elf_backend_modify_program_headers
 #define        elf_backend_modify_program_headers
nacl_modify_program_headers
+#undef elf_backend_final_write_processing
+#define elf_backend_final_write_processing     nacl_final_write_processing

 #include "elf32-target.h"

 /* Restore defaults.  */
 #undef elf_backend_modify_segment_map
 #undef elf_backend_modify_program_headers
+#undef elf_backend_final_write_processing

 /* VxWorks support.  */

--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -5397,6 +5397,8 @@ static const struct elf_x86_64_backend_data
elf_x86_64_nacl_arch_bed =
 #define        elf_backend_modify_segment_map          nacl_modify_segment_map
 #undef elf_backend_modify_program_headers
 #define        elf_backend_modify_program_headers
nacl_modify_program_headers
+#undef elf_backend_final_write_processing
+#define elf_backend_final_write_processing     nacl_final_write_processing

 #include "elf64-target.h"

@@ -5441,6 +5443,7 @@ static const struct elf_x86_64_backend_data
elf_x86_64_nacl_arch_bed =
 #undef elf_backend_size_info
 #undef elf_backend_modify_segment_map
 #undef elf_backend_modify_program_headers
+#undef elf_backend_final_write_processing

 /* Intel L1OM support.  */