On 4. Apr 2023, at 16:03, Ard Biesheuvel <ardb@kernel.org> wrote:
On Tue, 4 Apr 2023 at 13:10, Rebecca Cran <rebecca@bsdio.com> wrote:
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.
Ah right, fair enough.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.
I don't have a preference for one over the other, although I fail tosee how this fixes the -n problem above.Since .text is aligned to COMMONPAGESIZE, I think I can remove the
ALIGN(ALIGNOF(.text)) and keep the ALIGN(CONSTANT(COMMONPAGESIZE))?
No. The alignment of .text could exceed COMMONPAGESIZE; this happense.g., with AArch64 vector tables which are 2k aligned, when they areemitted into a SEC or PEIM module which only has 32 byte alignment bydefault.In this case, the PE/COFF section alignment will be 2k, and so .datashould start at a 2k aligned address as well.
'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."
Let me know if you manage to reproduce that - I'd like to understandwhat exactly is happening there.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.
Yes, that would be preferable.