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

data_overflow.rs example/test is failing #404

Closed
jonas-schievink opened this issue Nov 21, 2019 · 2 comments
Closed

data_overflow.rs example/test is failing #404

jonas-schievink opened this issue Nov 21, 2019 · 2 comments

Comments

@jonas-schievink
Copy link
Contributor

Or rather, it builds successfully. The reason is that these ! do not work with errexit, so a successful build has no consequences:

https://github.com/rust-embedded/cortex-m-rt/blob/5ac023dd9261d67aafbeab018241b4bcc4742152/ci/script.sh#L44-L48

Caught by shellcheck SC2251.

@adamgreig adamgreig transferred this issue from rust-embedded/cortex-m-rt Jan 23, 2022
@newAM
Copy link
Member

newAM commented Jun 30, 2024

I'm triaging Cortex-M issues, I think this one is fixed as of #505, please re-open if not!

@newAM newAM closed this as completed Jun 30, 2024
newAM added a commit to newAM/cortex-m that referenced this issue Jun 30, 2024
Long story:

* This test was introduced
* `ci/script.sh` did not correctly check for failures, [SC2251]
  * Thanks rust-embedded#404 for the tip!
* The FLASH size was increased, and the test incorrectly passed, but
  nobody noticed
* I modified the test in rust-embedded#505 which made it fail again, but for the
  wrong reason.  `ptr::read_volatile(ptr::addr_of!(RODATA))` reads the
  entire array, which is not equivalent to the original code
  `ptr::read_volatile(&RODATA as *const u8)` which read a single element
  of the array.
* The test now failed, but the stack related overflow takes rustc a LONG
  time to compile, and pushed our CI times to >30m.

These changes fix ci/scripts.sh to exit with a non-zero code if
data_overflow is passing, and makes data_overflow fail to compile for
the original reason, updating RODATA to reflect the increased FLASH
size.

[SC2251](https://www.shellcheck.net/wiki/SC2251)
newAM added a commit to newAM/cortex-m that referenced this issue Jun 30, 2024
Long story:

* This test was introduced
* `ci/script.sh` did not correctly check for failures, [SC2251]
  * Thanks rust-embedded#404 for the tip!
* The FLASH size was increased, and the test incorrectly passed, but
  nobody noticed.
* I modified the test in rust-embedded#505 which made it fail again, but for the
  wrong reason.  `ptr::read_volatile(ptr::addr_of!(RODATA))` reads the
  entire array, which is not equivalent to the original code
  `ptr::read_volatile(&RODATA as *const u8)` which read a single element
  of the array.
* The test now failed, but the stack related overflow takes rustc a LONG
  time to compile, and pushed our CI times to >30m.

These changes fix ci/scripts.sh to exit with a non-zero code if
data_overflow is passing, and makes data_overflow fail to compile for
the original reason, updating RODATA to reflect the increased FLASH
size.

[SC2251]: https://www.shellcheck.net/wiki/SC2251
@newAM
Copy link
Member

newAM commented Jun 30, 2024

Actually I didn't fix it in #505, have #542 open to fix.

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

No branches or pull requests

2 participants