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

[Fangrui Song <i@maskray.me>]

On 2020-03-05, Alan Modra wrote:
>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.

Hi, Alan,

https://reviews.llvm.org/D75724 includes the output section
address/alignment changes I intended for lld.

The comment https://reviews.llvm.org/D75724#1915342 includes useful user feedback:

> If anything, I think the GNU linkers should warn about violating input sh_addralign requirements if they're going to have that behavior.
>
> But it seems likely that the most common use of explicit alignments is
> exactly when setting the minimum alignment of the output section is
> the intent and that being increased by input sh_addralign requirements
> is normal and expected, and not a case like mine.

>> > > --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