Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linker script: implement SIZEOF_HEADERS #29062

Closed
emaste opened this issue Jul 25, 2016 · 8 comments
Closed

linker script: implement SIZEOF_HEADERS #29062

emaste opened this issue Jul 25, 2016 · 8 comments
Labels
bugzilla Issues migrated from bugzilla lld:ELF

Comments

@emaste
Copy link
Member

emaste commented Jul 25, 2016

Bugzilla Link 28688
Resolution FIXED
Resolved on Aug 10, 2016 11:43
Version unspecified
OS FreeBSD
Blocks #23588
CC @rui314

Extended Description

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.

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 25, 2016

Take.

@emaste
Copy link
Member Author

emaste commented Jul 25, 2016

Also used in the main FreeBSD kernel link script.
https://svnweb.freebsd.org/base/head/sys/conf/ldscript.amd64?view=markup#l10

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 25, 2016

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

@rui314
Copy link
Member

rui314 commented Jul 26, 2016

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?

@emaste
Copy link
Member Author

emaste commented Jul 26, 2016

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.

@rui314
Copy link
Member

rui314 commented Jul 26, 2016

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.

@llvmbot
Copy link
Collaborator

llvmbot commented Jul 26, 2016

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.

@emaste
Copy link
Member Author

emaste commented Aug 10, 2016

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla lld:ELF
Projects
None yet
Development

No branches or pull requests

3 participants