Re: Linker scripts use of "-z common-page-size=0x20" etc.

["Rebecca Cran" <rebecca@bsdio.com>]

On 4/4/23 1:22 AM, Ard Biesheuvel wrote:
> On Mon, 3 Apr 2023 at 22:33, Rebecca Cran <rebecca@bsdio.com> wrote:
>> As part of my work on the toolchain definitions, I've come across a
>> situation where ld.lld fails to align sections correctly, due to it
>> being invoked via clang with the '-n' option, which causes GenFw to fail
>> with "Section address not aligned to its own alignment.".
>>
> Stupid question: if it breaks stuff, why do you use -n ?

As far as I can see, clang adds it automatically when it invokes ld.lld.

>> The following messages are printed:
>>
>>
>> ld.lld: warning: -z common-page-size set, but paging disabled by omagic
>> or nmagic ld.lld: warning: address (0x558) of section .data is not a
>> multiple of alignment (16)
>>
>> I tracked the problem down to GccBase.lds and ClangBase.lds, which have:
>>
>> /* * The alignment of the .data section should be less than or equal to
>> the * alignment of the .text section. This ensures that the relative
>> offset * between these sections is the same in the ELF and the PE/COFF
>> versions of * this binary. */ .data ALIGN(ALIGNOF(.text)) :
>> ALIGN(CONSTANT(COMMONPAGESIZE)) { *(.data .data.* .gnu.linkonce.d.*)
>> *(.bss .bss.*) }
>>
>> I can work around the problem by removing "ALIGN(ALIGNOF(.text)", but
>> I'm wondering if our use of COMMONPAGESIZE/MAXPAGESIZE is correct.
>>
> What do you mean by 'correct'? The intent is clearly to declare the
> mapping granule size, and for SEC/PEI binaries that execute in place
> from flash, the MMU page size is not the most useful quantum here.

On Discord, I think Marvin was saying that we shouldn't be using 
COMMONPAGESIZE, but MAXPAGESIZE.

Since .text is aligned to COMMONPAGESIZE, I think I can remove the 
ALIGN(ALIGNOF(.text)) and keep the ALIGN(CONSTANT(COMMONPAGESIZE))?


> 'Page size' is highly context specific, and toolchain people are
> (imho) usually quite quick to call something abuse if it does not
> match their narrow definition of how a compiler or linker should be
> used. For the same reason, it has been so difficult to get a compiler
> to understand that the desire for position independent code does not
> imply that we want GOTs, or care about ELF symbol preemption or
> copy-on-write footprint of relocatable segments. In general, the bare
> metal use case (which includes EFI) is quite poorly understood by many
> people working on toolchains.
>
>> I'm wondering what the correct approach is here: should we do something
>> similar to how we set PECOFF_HEADER_SIZE and define a SECTION_ALIGNMENT
>> symbol?
> We cannot, that is the reason for using the page size switches here:
> using a symbol to set the location pointer is fine, but using a symbol
> to set the alignment of a section is not.
>
>> Or, as discussed on Discord should we just use
>> CONSTANT(MAXPAGESIZE) and ignore how it's normally used to specify the
>> maximum allowable page size for an executable?
>>
> Note that (when I last checked), the only effect of setting -z
> xxx-page-size is that those macros assume the associated value in the
> linker script. Nothing else in the linker changes behavior (with the
> exception of the warning you are seeing)
>
> So claiming abuse because the provided value does not match the page
> size of an OS that might also run on the same system is strenuous, and
> I think our use of it is fine. AIUI, the reason for having
> ClangBase.lds in addition to GccBase.lds is the fact that LLD does not
> support common but only max page size, so I think it should be fine to
> merge the two, and use max-page-size everywhere.

Thanks for the explanation. This is probably an error on my part, but 
when I tried to use max-page-size everywhere, the AARCH64 build of 
ArmVirtQemu failed, with GenFw saying "AARCH64 small code model requires 
identical ELF and PE/COFF section offsets modulo 4 KB."

LLD added support for common-page-size in 2019 
(https://reviews.llvm.org/D56205), so I think we can keep the current 
usage of common and max page size and just combine GccBase.lds and 
ClangBase.lds.


-- 
Rebecca Cran