rust-embedded-community / ssd1306

SSD1306 OLED driver
Apache License 2.0
283 stars 69 forks source link

memory.x & build.rs in the library's crate #143

Closed mexus closed 3 years ago

mexus commented 3 years ago

Description of the problem/feature request/other

Seems like the provided memory.x file overrides the user-provided memory.x file when building a binary using the cortex-m-rt crate. Do you think it might be possible to exclude both build.rs and memory.x files from the crate's library package?

Test case

Add ssd1306 = "0.5" to the target's Cargo.toml

#[cortex_m_rt::entry]
fn main() -> ! {
    const LARGE_ARRAY: [u8; 40 * 1024] = [0; 40 * 1024];
    loop {}
}

Expect a hardfault. One might need to use flip-link to actually see the hardfault when the seemingly segmentation fault occurs.

Removing ssd1306 from dependencies resolves the problem.

It seems like the link.x script provided by the cortex-m-rt crate simply includes any memory.x found, and since the build.rs you provide adds a viable link search path, the user-defined memory.x is ignored. It can be simply proved by removing the user's memory.x and observe the compilation/linking succeeds.

mexus commented 3 years ago

Please see #144 for a possible solution

therealprof commented 3 years ago

Something seems off here. A user provided memory.x should never be overridden and the included one should never be used in user projects. In fact I've used the published crates in a quite a few projects (including one which runs on an MCU which only has 6kB of RAM and 32kB of flash) and non-STM MCUs which have different addresses so a use of the included memory.x would have resulted in easily noticeable fatal errors. This would mean there has either been a severe regression in this crate or something else is wrong in your environment.

mexus commented 3 years ago

A user provided memory.x should never be overridden

How could that be enforced? link.x from the cortex-m-rt simply includes a memory.x without any path hints: https://github.com/rust-embedded/cortex-m-rt/blob/3a2ae355a272246dd3650f415fbb3895deb19571/link.x.in#L23

And I guess it is up to the linker to find the file, perhaps it simply tries various paths it is provided with. The build.rs provided by ssd1306 adds a linker search path: https://github.com/jamwaffles/ssd1306/blob/09e7982c08505121b7d54fa6d9aef979aa67b53b/build.rs#L10

So in the end it is up to the order in which paths are scanned to decide which memory.x is finally included. But it's just my speculations, I might be absolutely mistaking.

Anyhow, I'll try to set up a clean environment and reiterate the building process to find out whether my local environment is somehow twisted.

Thanks for a fast response btw :)

mexus commented 3 years ago

So, steps to reproduce:

  1. Set up a fresh project:
    $ cargo install cargo-generate
    $ cargo generate \
    --git https://github.com/knurling-rs/app-template \
    --branch main \
    --name test-ssd1306
  2. Install flip-link: $ cargo install flip-link
  3. Add the required target: $ rustup target add thumbv7em-none-eabihf (in my case)
  4. Modify Cargo.toml, .cargo/config and other files to use the required MCU. All the required changes can be found by running $git grep TODO
  5. Try to build the project: $ cargo build. The build will fail with the following error:
    = note: rust-lld: error: /test-ssd1306/target/thumbv7em-none-eabihf/debug/build/cortex-m-rt-c8fd83f10c447ef5/out/link.x:23: cannot find linker script memory.x
          >>> INCLUDE memory.x
          >>>         ^

    So far it is expected.

  6. Add ssd1306 = "0.5" dependency to the project's Cargo.toml
  7. Build the project with $ cargo build. The build succeeds!
    $ cargo build
    Updating crates.io index
    Downloaded byte-slice-cast v0.3.5
    Downloaded display-interface-i2c v0.4.0
    Downloaded display-interface-spi v0.4.0
    Downloaded display-interface v0.4.0
    Downloaded embedded-graphics v0.6.2
    Downloaded ssd1306 v0.5.0
    Downloaded 6 crates (2.6 MB) in 1.08s (largest was `ssd1306` at 2.5 MB)
    Compiling display-interface v0.4.0
    Compiling byte-slice-cast v0.3.5
    Compiling ssd1306 v0.5.0
    Compiling embedded-graphics v0.6.2
    Compiling display-interface-i2c v0.4.0
    Compiling display-interface-spi v0.4.0
    Compiling test-ssd1306 v0.1.0 (/test-ssd1306)
    Finished dev [optimized + debuginfo] target(s) in 4.37s

    Please note, that no user-provided memory.x exists in the project.

  8. Let's verify that a memory.x file provided by the ssd1306 crate exists and remove it:
    $ find . -name 'memory.x'
    ./target/thumbv7em-none-eabihf/debug/build/ssd1306-7856f396f8c230ad/out/memory.x
    $ find . -name 'memory.x' -delete
  9. Force re-linking by touching all the rust files in the project and try to build it again:
    $ touch **/*.rs
    $ cargo build

    And it fails with the linking error as before:

    = note: rust-lld: error: /test-ssd1306/target/thumbv7em-none-eabihf/debug/build/cortex-m-rt-c8fd83f10c447ef5/out/link.x:23: cannot find linker script memory.x
          >>> INCLUDE memory.x
          >>>         ^

I've performed all the steps in a docker container, so the environment is as clean as possible :)

mexus commented 3 years ago

I know it's a little bit late, but let me thank you @therealprof @jamwaffles and other people involved for the great crate!

jamwaffles commented 3 years ago

Any time! Thank you for the kind words :blush: