Skip to content
This repository was archived by the owner on Jan 24, 2022. It is now read-only.

refactor the linker script #84

Merged
merged 3 commits into from
Aug 7, 2018
Merged

refactor the linker script #84

merged 3 commits into from
Aug 7, 2018

Conversation

japaric
Copy link
Member

@japaric japaric commented Aug 7, 2018

to make it more compatible with LLD. This commit contains no functional changes.

fixes #70

Overview of changes:

  • Alignment checks are enabled now that rust-lld (LLD 7.0) supports the modulo
    operator.

  • Removed some private symbols (e.g. __foo) in favor of ADDR and SIZEOF.

  • Turned .got into a NOLOAD section now that rust-lld supports it.

  • Replaced ABSOLUTE(.) with . as an old LLD overlap bug seems to be gone and
    ABSOLUTE seems to cause problems, like Invalid .vector_table.exceptions section #70, on bigger programs.

  • Made the linker assertion messages more uniform.

  • Extended test suite to check that linking works with both rust-lld and GNU
    LD.

r? therealprof (chosen at random)

to make it more compatible with LLD. This commit contains no functional changes.

fixes #70

Overview of changes:

- Alignment checks are enabled now that rust-lld (LLD 7.0) supports the modulo
operator.

- Removed some private symbols (e.g. __foo) in favor of ADDR and SIZEOF.

- Turned .got into a NOLOAD section now that rust-lld supports it.

- Replaced `ABSOLUTE(.)` with `.` as an old LLD overlap bug seems to be gone and
ABSOLUTE seems to cause problems, like #70, on bigger programs.

- Made the linker assertion messages more uniform.

- Extended test suite to check that linking works with both rust-lld and GNU
LD.
@japaric japaric requested a review from a team as a code owner August 7, 2018 20:30
@japaric
Copy link
Member Author

japaric commented Aug 7, 2018

r? therealprof (chosen at random)

err, I meant @therealprof

@therealprof
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 7, 2018

👎 Rejected by too few approved reviews

therealprof
therealprof previously approved these changes Aug 7, 2018
Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@therealprof
Copy link
Contributor

bors retry

bors bot added a commit that referenced this pull request Aug 7, 2018
84: refactor the linker script r=therealprof a=japaric

to make it more compatible with LLD. This commit contains no functional changes.

fixes #70

Overview of changes:

- Alignment checks are enabled now that rust-lld (LLD 7.0) supports the modulo
operator.

- Removed some private symbols (e.g. __foo) in favor of ADDR and SIZEOF.

- Turned .got into a NOLOAD section now that rust-lld supports it.

- Replaced `ABSOLUTE(.)` with `.` as an old LLD overlap bug seems to be gone and
ABSOLUTE seems to cause problems, like #70, on bigger programs.

- Made the linker assertion messages more uniform.

- Extended test suite to check that linking works with both rust-lld and GNU
LD.

r? therealprof (chosen at random)

Co-authored-by: Jorge Aparicio <[email protected]>
@japaric
Copy link
Member Author

japaric commented Aug 7, 2018

Travis is failing (https://travis-ci.org/rust-embedded/cortex-m-rt/jobs/413297498#L544). I can't repro the error locally; I wonder if the problem is that the Travis builds are using an old gcc.

@bors
Copy link
Contributor

bors bot commented Aug 7, 2018

Build failed

@therealprof
Copy link
Contributor

gcc 4.8.2? That's ancient. Is there any way to use something a bit more 2018?

@korken89
Copy link
Contributor

korken89 commented Aug 7, 2018

The arm-none-eabi-gcc on Launchpad is v7, which should work fine on Travis: https://docs.travis-ci.com/user/installing-dependencies/
gcc: https://launchpad.net/gcc-arm-embedded

Adapted example, but untested:

before_install:
  - sudo add-apt-repository ppa:team-gcc-arm-embedded/ppa -y
  - sudo apt-get update -q
  - sudo apt-get install gcc-arm-embedded -y

@japaric
Copy link
Member Author

japaric commented Aug 7, 2018

The arm-none-eabi-gcc on Launchpad is v7

This one seems to work but it also made some CI builds 4 times slower 😂.

@korken89
Copy link
Contributor

korken89 commented Aug 7, 2018

This one seems to work but it also made some CI builds 4 times slower

Haha, sorry about that!
I am not very used to caching, but perhaps downloading the tarbal and caching is better?

@adamgreig
Copy link
Member

adamgreig commented Aug 7, 2018

I bet even without caching it would be quicker to download the tarball of pre-built arm-gcc-embedded executables each time, since using sudo: true means the tests run in a full VM which takes way longer to start up than the Docker images you get otherwise.

edit: Travis even suggests not caching this sort of thing in their docs.

@japaric
Copy link
Member Author

japaric commented Aug 7, 2018

I'll switch to the tarball then.

@japaric
Copy link
Member Author

japaric commented Aug 7, 2018

Hey, it worked on the first try :-).

re-r? @therealprof

@therealprof
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 7, 2018

👎 Rejected by too few approved reviews

@therealprof
Copy link
Contributor

bors retry

bors bot added a commit that referenced this pull request Aug 7, 2018
84: refactor the linker script r=therealprof a=japaric

to make it more compatible with LLD. This commit contains no functional changes.

fixes #70

Overview of changes:

- Alignment checks are enabled now that rust-lld (LLD 7.0) supports the modulo
operator.

- Removed some private symbols (e.g. __foo) in favor of ADDR and SIZEOF.

- Turned .got into a NOLOAD section now that rust-lld supports it.

- Replaced `ABSOLUTE(.)` with `.` as an old LLD overlap bug seems to be gone and
ABSOLUTE seems to cause problems, like #70, on bigger programs.

- Made the linker assertion messages more uniform.

- Extended test suite to check that linking works with both rust-lld and GNU
LD.

r? therealprof (chosen at random)

Co-authored-by: Jorge Aparicio <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 7, 2018

Build succeeded

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid .vector_table.exceptions section
4 participants