don't over-align file positions of PE executable sections
authorJan Beulich <jbeulich@suse.com>
Thu, 19 May 2022 10:43:10 +0000 (12:43 +0200)
committerJan Beulich <jbeulich@suse.com>
Thu, 19 May 2022 10:43:10 +0000 (12:43 +0200)
When a sufficiently small alignment was specified via --file-alignment,
individual section alignment shouldn't affect placement within the file.
This involves first of all clearing D_PAGED for images when section and
file alignment together don't permit paging of the image. The involved
comparison against COFF_PAGE_SIZE in turn helped point out (through a
compiler warning) that 'page_size' should be of unsigned type (as in
particular FileAlignment is). This yet in turn pointed out a dubious
error condition (which is being deleted).

For the D_PAGED case I think the enforced file alignment may still be
too high, but I'm wary of changing that logic without knowing of
possible corner cases.

Furthermore file positions in PE should be independent of the alignment
recorded in section headers anyway. Otherwise there are e.g. anomalies
following commit 6f8f6017a0c4 ("PR27567, Linking PE files adds alignment
section flags to executables") in that linking would use information a
subsequent processing step (e.g. stripping) wouldn't have available
anymore, and hence a binary could change in that 2nd step for no actual
reason. (Similarly stripping a binary linked with a linker pre-dating
that commit would change the binary again when stripping it a 2nd time.)

bfd/coffcode.h

index dde8a649e433bd085220aff6109aa6e06f01b267..0a6605191383c2eb7346a3b7e295a929bb8b4c27 100644 (file)
@@ -2962,7 +2962,7 @@ coff_compute_section_file_positions (bfd * abfd)
 #endif
 
 #ifdef COFF_IMAGE_WITH_PE
-  int page_size;
+  unsigned int page_size;
 
   if (coff_data (abfd)->link_info
       || (pe_data (abfd) && pe_data (abfd)->pe_opthdr.FileAlignment))
@@ -2973,22 +2973,12 @@ coff_compute_section_file_positions (bfd * abfd)
         This repairs 'ld -r' for arm-wince-pe target.  */
       if (page_size == 0)
        page_size = 1;
-
-      /* PR 17512: file: 0ac816d3.  */
-      if (page_size < 0)
-       {
-         bfd_set_error (bfd_error_file_too_big);
-         _bfd_error_handler
-           /* xgettext:c-format */
-           (_("%pB: page size is too large (0x%x)"), abfd, page_size);
-         return false;
-       }
     }
   else
     page_size = PE_DEF_FILE_ALIGNMENT;
 #else
 #ifdef COFF_PAGE_SIZE
-  int page_size = COFF_PAGE_SIZE;
+  unsigned int page_size = COFF_PAGE_SIZE;
 #endif
 #endif
 
@@ -3070,9 +3060,10 @@ coff_compute_section_file_positions (bfd * abfd)
     bfd_size_type amt;
 
 #ifdef COFF_PAGE_SIZE
-    /* Clear D_PAGED if section alignment is smaller than
-       COFF_PAGE_SIZE.  */
-   if (pe_data (abfd)->pe_opthdr.SectionAlignment < COFF_PAGE_SIZE)
+    /* Clear D_PAGED if section / file alignment aren't suitable for
+       paging at COFF_PAGE_SIZE granularity.  */
+   if (pe_data (abfd)->pe_opthdr.SectionAlignment < COFF_PAGE_SIZE
+       || page_size < COFF_PAGE_SIZE)
      abfd->flags &= ~D_PAGED;
 #endif
 
@@ -3193,7 +3184,11 @@ coff_compute_section_file_positions (bfd * abfd)
             padding the previous section up if necessary.  */
          old_sofar = sofar;
 
+#ifdef COFF_IMAGE_WITH_PE
+         sofar = BFD_ALIGN (sofar, page_size);
+#else
          sofar = BFD_ALIGN (sofar, 1 << current->alignment_power);
+#endif
 
 #ifdef RS6000COFF_C
          /* Make sure the file offset and the vma of .text/.data are at the
@@ -3269,7 +3264,11 @@ coff_compute_section_file_positions (bfd * abfd)
       else
        {
          old_sofar = sofar;
+#ifdef COFF_IMAGE_WITH_PE
+         sofar = BFD_ALIGN (sofar, page_size);
+#else
          sofar = BFD_ALIGN (sofar, 1 << current->alignment_power);
+#endif
          align_adjust = sofar != old_sofar;
          current->size += sofar - old_sofar;
        }