LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 28688 - linker script: implement SIZEOF_HEADERS
Summary: linker script: implement SIZEOF_HEADERS
Status: RESOLVED FIXED
Alias: None
Product: lld
Classification: Unclassified
Component: ELF (show other bugs)
Version: unspecified
Hardware: PC FreeBSD
: P normal
Assignee: Davide Italiano
URL:
Keywords:
Depends on:
Blocks: 23214
  Show dependency tree
 
Reported: 2016-07-24 19:19 PDT by emaste
Modified: 2016-08-10 11:43 PDT (History)
5 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description emaste 2016-07-24 19:19:39 PDT
SIZEOF_HEADERS is used by the linker script in FreeBSD's linux emulation layer:

SECTIONS
{
        . = . + SIZEOF_HEADERS;

        .hash           : { *(.hash) }                  :text
        .gnu.hash       : { *(.gnu.hash) }
...

`SIZEOF_HEADERS'
`sizeof_headers'
     Return the size in bytes of the output file's headers.  You can
     use this number as the start address of the first section, if you
     choose, to facilitate paging.
Comment 1 Davide Italiano 2016-07-24 19:27:25 PDT
Take.
Comment 2 emaste 2016-07-24 20:05:57 PDT
Also used in the main FreeBSD kernel link script.
https://svnweb.freebsd.org/base/head/sys/conf/ldscript.amd64?view=markup#l10
Comment 3 Davide Italiano 2016-07-25 12:56:26 PDT
So, I'm taking a shot at implementing this but I'd like to discuss the implementation a little bit before digging into code.
`SIZEOF_HEADERS` should be set equal to ehdr_size + phdr_size * segment count.
The issue here is that we don't know exactly the number of segments that we're going to create because the `SECTION` clause of the linker script might change that. 
My idea is to have a preprocessing step, parsing the linker script in order to get the section lists from it (and use that to compute the correct value of SIZEOF_HEADERS).
This code changed quite a bit recently, os I might miss something here. Comments appreciated =)
Comment 4 Rui Ueyama 2016-07-25 20:43:37 PDT
SIZEOF_HEADERS seems pretty tricky to me and doesn't seem to be recommended to use without PHDRS command. I think it's a reasonable stance.

https://sourceware.org/binutils/docs/ld/Builtin-Functions.html#Builtin-Functions

I'm wondering what the FreeBSD's linker script is doing with SIZEOF_HEADERS. I don't understand why it is needed for Xen. Could you give me an insight?
Comment 5 emaste 2016-07-25 21:20:32 PDT
> I'm wondering what the FreeBSD's linker script is doing with SIZEOF_HEADERS. I > don't understand why it is needed for Xen. Could you give me an insight?

It's not for just for Xen; SIZEOF_HEADERS has been there since the beginning of the FreeBSD/amd64 project. For reference the default (built-in) userland linker script in GNU ld uses it like so:

SECTIONS
{
  /* Read-only sections, merged into text segment: */
  PROVIDE (__executable_start = 0x400000); . = 0x400000 + SIZEOF_HEADERS;

and the kernel use is equivalent:

SECTIONS
{
  /* Read-only sections, merged into text segment: */
  kernphys = CONSTANT (MAXPAGESIZE);
  . = kernbase + kernphys + SIZEOF_HEADERS;

Xen support added the "AT (kernphys + SIZEOF_HEADERS)" but that use is no different really.

I don't really have an objection to the binutils restrictions:

To avoid this error, you must avoid using the SIZEOF_HEADERS function, or you must rework your linker script to avoid forcing the linker to use additional program headers, or you must define the program headers yourself using the PHDRS command (see PHDRS).

except that I think "additional program headers" in the GNU ld case is based on the default layout it assumes, which we don't really want to do.

For the FreeBSD kernel we could certainly add PHDRS if that's the best way forward.
Comment 6 Rui Ueyama 2016-07-25 21:32:10 PDT
I'm inclined to allow SIZEOF_HEADER only when PHDRS commands are used to give us a full definition of the program headers. It is simple and easy to understand. Otherwise, we could either (1) assume some PHDRS and report an error if the program needs more PHDRS or (2) walk over all the SECTIONS command twice as Davide said. I think both are a bit too much for this variable.
Comment 7 George Rimar 2016-07-26 01:38:01 PDT
(In reply to comment #6)
> I'm inclined to allow SIZEOF_HEADER only when PHDRS commands are used to
> give us a full definition of the program headers. It is simple and easy to
> understand. Otherwise, we could either (1) assume some PHDRS and report an
> error if the program needs more PHDRS or (2) walk over all the SECTIONS
> command twice as Davide said. I think both are a bit too much for this
> variable.

At fact we are already walk over SECTIONS twice. First pass is createSections(), the second is assignAddresses(). Can't we use that ? I would try to estimate segments amount during the first pass.
And we need SIZEOF_HEADER only on second pass when we assign addresses I think.
Comment 8 emaste 2016-08-10 11:43:15 PDT
Review: https://reviews.llvm.org/D23165
Commit: https://reviews.llvm.org/rL278204