This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Assign absolute values in linkerscript correctly #1.
AbandonedPublic

Authored by grimar on Apr 18 2017, 9:21 AM.

Details

Reviewers
ruiu
rafael
Summary

This is PR32664. Current LLD behavior - segfault.

I found 2 ways of solving the issue.
One of them is this patch, second is D32174.

Problem itself is next. Imagine next script and code:
SECTIONS { . = 0x1000; aaa = ABSOLUTE(foo - 1); .text : { *(.text*) } };

.section .text
.globl foo
foo:
nop

At the moment of assignment to aaa we do not know the address of output section .text.
We are unable to evaluate the absolute value aaa properly therefore.

This patch suggests to handle forced absolute symbols like regular symbols until we assign
all output sections addresses. After that it converts them into absolute.

Diff Detail

Event Timeline

grimar created this revision.Apr 18 2017, 9:21 AM
ruiu edited edge metadata.Apr 19 2017, 5:36 AM

Is your example linker script valid for GNU linkers? I'm curious why you noticed this in the first place.

grimar added a comment.EditedApr 19 2017, 5:41 AM
In D32173#730327, @ruiu wrote:

Is your example linker script valid for GNU linkers? I'm curious why you noticed this in the first place.

I noticed that when tried to link linux kernel. It has next script at start:

SECTIONS
{
 . = (0xffffffff80000000 + ALIGN(0x1000000, 0x200000));
 phys_startup_64 = ABSOLUTE(startup_64 - 0xffffffff80000000);
 /* Text and read-only data */
 .text : AT(ADDR(.text) - 0xffffffff80000000) {
.....
  /* bootstrapping code */
  *(.head.text)

Where startup_64 is in .head.text. Currently we segfault, but even with change at LinkerScript.cpp::75 which fixes crash we do
produce wrong symbol value for phys_startup_64.
(we produce 0 - 0xffffffff80000000 == 0x0000000080000000 instead of 0xffffffff81000000 - 0xffffffff80000000 == 0x0000000001000000).

And this patch fixes that I believe.

Linux kernel script for this place is: https://github.com/torvalds/linux/blob/master/arch/x86/kernel/vmlinux.lds.S#L88

ruiu added a comment.Apr 24 2017, 8:57 PM

Sorry for the delay. I was on vacation last week.

So, this patch allows you compute absolute symbols addresses lazily. With that, you can create cycles like this.

foo = ABSOLUTE(bar - 0xff);
bar = ABSOLUTE(foo - 0xff);

What do you think it should be handled?

Also, you can create not a cycle but a series of expressions that should be calculated backwards, like this.

foo = ABSOLUTE(bar);
bar = ABSOLUTE(baz);
baz = 0xff;

What do you think about this?

grimar added a comment.EditedApr 25 2017, 3:43 AM
In D32173#736295, @ruiu wrote:

Sorry for the delay. I was on vacation last week.

So, this patch allows you compute absolute symbols addresses lazily. With that, you can create cycles like this.

foo = ABSOLUTE(bar - 0xff);
bar = ABSOLUTE(foo - 0xff);

What do you think it should be handled?

Also, you can create not a cycle but a series of expressions that should be calculated backwards, like this.

foo = ABSOLUTE(bar);
bar = ABSOLUTE(baz);
baz = 0xff;

What do you think about this?

Honestly I think I do not like idea to calculate something absolute for symbol which section did not
get VA assigned yet at all. And I dont like idea to evaluate something lazily too.
I just don't sure how to handle such situations properly, thats why prepared 2 different patches
with approaches I can imagine.

May be we just should produce an error in such cases ? That will be inconsistent behavior
with GNU linkers, but simplifies things.

Comment above should be: "Honestly I think I do not like idea to calculate something..."

ruiu added a comment.Apr 25 2017, 9:55 AM

Since this patch doesn't handle the issue in general but just fixes one specific error, I'm inclined to just not to crash when the linker sees this pattern.

In D32173#736989, @ruiu wrote:

Since this patch doesn't handle the issue in general but just fixes one specific error, I'm inclined to just not to crash when the linker sees this pattern.

Sometimes I think its better to crash than produce broken output silently. For this case it would be much harder to find what is wrong with linux kernel if LLD would not crash.
I think it does not make much sense fix crash and keep producing wrong symbol address silently. I would suggest at least to error out in addition.

But what about solution used in D32174 ? I just rebased it and it is free from cycles problems you mentioned and looks solves issue in general.

ruiu added a comment.Apr 26 2017, 6:28 AM

What I was trying to say is to make LLD not segfault but fail with an error message.

grimar abandoned this revision.May 12 2017, 12:48 AM

D32793 was landed instead.