This is an archive of the discontinued LLVM Phabricator instance.

[ELF] LinkerScript: Don't assign zero to all regular symbols
ClosedPublic

Authored by arichardson on Apr 12 2017, 11:06 AM.

Details

Summary

This fixes an assertion `Align != 0u && "Align can't be 0."'
in llvm::alignTo() when a linker script references a globally
defined variable in an ALIGN() context.

Event Timeline

arichardson created this revision.Apr 12 2017, 11:06 AM
ruiu edited edge metadata.Apr 12 2017, 11:21 AM

The BFD linker doesn't accept variables in alignment expressions. I got this error.

nonconstant expression for section alignment

The assertion failure needs fixing, but we could accept it or reject it, so I wonder why you noticed it. Are you actually using this?

This happens when using the following linker scripts: https://github.com/CTSRD-CHERI/cheribsd/blob/master/lib/libc_cheri/sandbox.ld and https://github.com/CTSRD-CHERI/cherios/blob/master/ldscripts/sandbox.ld and these to work fine with the ancient (2007) FreeBSD BFD version.
The crash was discovered when we tried to link files using sandbox.ld with LLD instead of BFD. I just tried running the testcase with the FreeBSD BFD and you are right that it rejects it. I am not entirely sure what is different in the test case compared to sandbox.ld that would cause it to be rejected.

The reason we use the variable in our linker scripts is that we need either 16 or 32 byte alignment depending on which CPU configuration we run the binary on. To workaround this we could just hardcode it to 32 as that will work for both.
However, being able to use a variable makes it clear that we want to align the section to the size of a capability register and not just some random constant 0x20.

ruiu added a comment.Apr 12 2017, 3:12 PM

Thank you for the information. I don't know why the BFD linker rejects this test case, but I don't think we need to understand it. This expression itself makes sense, so we should allow it. You code looks fine. There are only a few nits.

ELF/LinkerScript.cpp
71–73

Please add a comment why we set symbol values early.

// We want to set symbol values early if we can. This allows us to use symbols 
// as variables in linker scripts. E.g. doing this allows us to write expressions
// like this: `alignment = 16; . = ALIGN(., alignment)`

I think I prefer SymValue over SymbolValue.

ruiu accepted this revision.Apr 12 2017, 3:36 PM

LGTM

ELF/LinkerScript.cpp
71

Add a blank space before comment.

This revision is now accepted and ready to land.Apr 12 2017, 3:36 PM

Could you commit for me please? I don't have commit access.

This revision was automatically updated to reflect the committed changes.