[PATCH] D74286: [ELF] Respect output section alignment for AT> (non-null lmaRegion)

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D74286#1866537 <https://reviews.llvm.org/D74286#1866537>, @psmith wrote:

> Do you happen to know how this relates to the ALIGN_WITH_INPUT linker script keyword? Apologies I've got stuck in an airport due to flight cancellations and can't check easily. I had thought that in BFD if you wanted InputSection alignment to affect LMA you needed to add ALIGN_WITH_INPUT.
>
> For example, taken from one of the few pages I could find that mentioned it: https://www.openstm32.org/forumthread4866
>
>   don’t forget to use ALIGN_WITH_INPUT section attribute, or you’d have aligned sections slipping if previous section doesn’t completely fill the aligned size.
>   ...
>     /* The startup code goes first into FLASH */
>     .isr_vector : ALIGN_WITH_INPUT
>     {
>       . = ALIGN(4);
>       KEEP(*(.isr_vector)) /* Startup code */
>       . = ALIGN(4);
>     } >FLASH AT > AXIFLASH
>
>
> A brief look at BFD implies that LMA is only aligned for an OS that goes into a memory region if ALIGN_WITH_INPUT is true. I suspect that not aligning LMA can give some size savings, for example if at load time the OutputSections are byte-copied from their LMA to their aligned VMA then it doesn't matter if they are not aligned in LMA.
>
> If I'm right it may be worth following BFD here as if we make ALIGN_WITH_INPUT the default, which is safe, then we can't then introduce ALIGN_WITH_INPUT without risking breaking some programs.


Thanks for mentioning `ALIGN_WITH_INPUT`. I investigated it a bit. The following is my reconstruction of its logic after omitting unimportant things:

  if (!bfd_is_abs_section (os->bfd_section)) {
    if (os->addr_tree) // output_section_address is specified
      section_alignment = exp_get_power (os->section_alignment, "section alignment");
    else {
      ...
      newdot = os->region->current;
      /// Maximum of input section alignments
      section_alignment = os->bfd_section->alignment_power;
    }
  
    if (section_alignment > 0) {
      dotdelta = align_power (newdot, section_alignment) - newdot;
      if (dotdelta != 0 && os->addr_tree)
        einfo("warning: changing start of section %s by %lu bytes", ...);
    }
  
    os->bfd_section->vma = os->bfd_section->lma = newdot;
  }
  
  if (os->load_base) {
    bfd_vma lma = exp_get_abs_int (os->load_base, 0, "load base");
    os->bfd_section->lma = lma;
  } else if (os->lma_region != NULL) {
    bfd_vma lma = os->lma_region->current;
  
    if (os->align_lma_with_input) // ALIGN_WITH_INPUT
      lma += dotdelta;
    else {
      if (os->lma_region != os->region)
        section_alignment = exp_get_power (os->section_alignment, "section alignment");
      if (section_alignment > 0)
        lma = align_power (lma, section_alignment);
    }
    os->bfd_section->lma = lma;
  }

lld does not support `ALIGN_WITH_INPUT`.

I think BFD either uses maximum input section alignments (addr_tree == NULL), or the ALIGN value (addr_tree != NULL) (not investigated carefully). lld's `sec->alignment` is the maximum of both:

  // LinkerScript.cpp
      // Handle align (e.g. ".foo : ALIGN(16) { ... }").
      if (sec->alignExpr)
        sec->alignment =
            std::max<uint32_t>(sec->alignment, sec->alignExpr().getValue());

I will do more investigations whether `max` can be dropped.

`ctx->lmaOffset = alignTo(mr->curPos, sec->alignment) - dot;` should work.

After this patch, we may also need a similar `changing start of section .foo by 15 bytes` warning.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74286/new/

https://reviews.llvm.org/D74286