Re: [ld] section address : ALIGN(align) and the maximum of input section alignments

[Alan Modra <amodra@gmail.com>]

On Wed, Mar 04, 2020 at 10:41:28PM -0800, Fangrui Song wrote:
> For convenience, I will use some notations:
> 
> max_input_align: maximum of input section alignments.
> addr_tree: output section address
> 
> On 2020-03-04, Alan Modra wrote:
> > On Tue, Mar 03, 2020 at 10:39:45PM -0800, Fangrui Song wrote:
> > > The implementation is complex. For users to understand, I think it
> > > will be helpful to have something more detailed in
> > > https://sourceware.org/binutils/docs/ld/Output-Section-Address.html#Output-Section-Address
> > > 
> > > If my understanding is correct
> > > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=233bf4f847b136705247e2f7f11bae41c72448a4
> > >  makes the output section address override sh_addralign computed from
> > > the maximum of input section alignments.
> > 
> > Right.
> > 
> > > So, generally the rules are:
> > > * The max of ALIGN and (the maximum of input section alignments) is taken.
> > > * The output section address overrides the above. If sh_addr %
> > > alignment != 0, set sh_addralign to the largest alignment that makes
> > > sh_addr%alignment=0
> > >   In this case, should the linker emit a warning?
> > 
> > I don't think so.  The input sections are still aligned within the
> > output section to their required alignment.
> > 
> > > * ALIGN and the output section address cannot be specified at the same
> > > time. This is considered a linker script "undefined behavior". Users
> > > should not rely on a particular result.
> > 
> > I'm not going to make that change for ld.bfd.  I said it probably
> > would have been better if ALIGN for output section statements hadn't
> > been invented, but once there are users for a script feature it can't
> > be removed without a good reason.
> 
> I take ALIGN as a way to overalign an output section.
> When ALIGN < max_input_align, do we agree that sh_addralign = max(ALIGN, max_input_align) = max_input_align ?
> 
> When both addr_tree and ALIGN are specified (what I called "undefined behavior"), and addr_tree is misaligned,
> sh_addralign can be decreased from max(ALIGN,max_input_align) to
> (addr_tree|max(ALIGN,max_input_align)) & -(addr_tree|max(ALIGN,max_input_align))
> 
> Commit 233bf4f847b136705247e2f7f11bae41c72448a4 is made so that
> "The value of sh_addr must be congruent to 0, modulo the value of sh_addralign."
> is obeyed.
> 
> Another view is that the user intentionally breaks the ELF rule. We can keep
> sh_addralign as max(ALIGN,max_input_align) and emit a warning along the line of:
> 
>   warning: address (0x10010) of section .foo is not a multiple of alignment (32)

I'm not going to do that for BFD ld.  The user chose an address for an
output section, and all input sections are placed in that section
according to their alignment.  No warning needed, just a change in
sh_addralign calculation.   I'll note that ld could quite correctly
set sh_addralign to 0 or 1 for any final linked executable.

> > > --warn-section-align may be out of place. It can be noisy for normal
> > > output section descriptions like    .foo : ALIGN(16) { ... }  without
> > > a preceding dot advancing to a multiple of 16.
> 
>   /* Without this assignment, the ALIGN(16) below will likely report a warning */
>   . = ALIGN(16);
>      .foo : ALIGN(16) { ... }
> 
> Does this suggest that --warn-section-align is not very useful?
> Keep reading.
> 
> > It's even more noisy when relaxation is enabled..
> 
> https://sourceware.org/ml/binutils/2020-03/msg00107.html does not fix
> the --warn-section-align version of PR25570.
> 
> # My original example.
> cat > a.s <<e
>   .globl _start; _start: ret
>   .section .data.rel.ro,"aw"; .balign 8; .byte 0
>   .data; .byte 0
>   .section .data2,"aw"; .balign 8; .byte 0
>   .bss; .balign 32; .byte 0
> e
> as a.s -o a.o
> 
> % ./ld-new a.o -o a --warn-section-align            ./ld-new: warning: start
> of section .got changed by 7
> ./ld-new: warning: start of section .got.plt changed by 7
> ./ld-new: warning: start of section .data2 changed by 6
> ./ld-new: warning: start of section .bss changed by 23
> ./ld-new: warning: start of section .data.rel.ro changed by 4088
> ./ld-new: warning: start of section .got changed by 4088
> ./ld-new: warning: start of section .got.plt changed by 4088
> ./ld-new: warning: start of section .data2 changed by 4096
> ./ld-new: warning: start of section .bss changed by 4096
> ./ld-new: warning: start of section .rela.dyn changed by 56
> ./ld-new: warning: start of section .rela.plt changed by 56
> ./ld-new: warning: start of section .data.rel.ro changed by -4088
> ./ld-new: warning: start of section .got changed by -4088
> ./ld-new: warning: start of section .got.plt changed by -4088
> ./ld-new: warning: start of section .data2 changed by -4096
> ./ld-new: warning: start of section .bss changed by -4096
> ./ld-new: warning: start of section .data.rel.ro changed by 4088
> ./ld-new: warning: start of section .got changed by 4088
> ./ld-new: warning: start of section .got.plt changed by 4088
> ./ld-new: warning: start of section .data2 changed by 4096
> ./ld-new: warning: start of section .bss changed by 4096
> 
> This also demonstrates how annoying --warn-section-align can be.

	PR 25570
	* ldlang.c (lang_size_sections_1): Don't report changes on
	second and subsequent iterations that make no change in
	alignment from that already reported.

diff --git a/ld/ldlang.c b/ld/ldlang.c
index 6ffa7af575..63f9d182ea 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -5597,7 +5597,13 @@ lang_size_sections_1
 		    if (lang_sizing_iteration == 1)
 		      diff = dotdelta;
 		    else if (lang_sizing_iteration > 1)
-		      diff = newdot - os->bfd_section->vma;
+		      {
+			/* Only report adjustments that would change
+			   alignment from what we have already reported.  */
+			diff = newdot - os->bfd_section->vma;
+			if (!(diff & (((bfd_vma) 1 << section_alignment) - 1)))
+			  diff = 0;
+		      }
 		    if (diff != 0
 			&& (config.warn_section_align
 			    || os->addr_tree != NULL))


-- 
Alan Modra
Australia Development Lab, IBM