Closed nickmertin closed 3 years ago
Would it be possible to provide the --emit=llvm-ir
output for this test case?
This is inside compiler-builtins. The arguments in the stack trace seem all sorts of wrong all over the place, though. But the shift width passed into core::ops::bit::{{impl}}::shl
is what you'd expect. An ABI mismatch, maybe?
To elaborate on my remark above: the LLVM IR will only reveal a plain mul
instruction, which becomes a libcall only in codegen.
To the issue author: can you please also post the exact command you're building? I'm not able to reproduce a similar issue when using a -Zbuild-std=core
based approach – in particular I see many more functions being inlined away, functions which in your case don't appear to be.
This is what I was trying:
cargo rustc -Z build-std=core --target=thumbv7m-none-eabi -- -Clinker=arm-none-eabi-gcc -C link-arg=-nostartfiles -C link-arg=-Tlink.x
Cortex-M0+ uses thumbv6m-none-eabi
, which is also shipped via rustup
Hm, I cannot reproduce inside QEMU (using nightly-2021-05-22
, steps described below), but maybe I'm missing something (maybe the error only shows up on the real hardware and not in QEMU).
Please correct me, if I'm wrong.
$ cargo run
Finished dev [optimized + debuginfo] target(s) in 0.01s
Running `qemu-system-arm -cpu cortex-m3 -machine lm3s6965evb -nographic -semihosting-config enable=on,target=native -kernel target/thumbv6m-none-eabi/debug/mcve`
Timer with period zero, disabling
[src/main.rs:9] 1i128 = 1
[src/main.rs:11] x * x = 1
$ cargo run --release
Finished release [optimized + debuginfo] target(s) in 0.01s
Running `qemu-system-arm -cpu cortex-m3 -machine lm3s6965evb -nographic -semihosting-config enable=on,target=native -kernel target/thumbv6m-none-eabi/release/mcve`
Timer with period zero, disabling
[src/main.rs:9] 1i128 = 1
[src/main.rs:11] x * x = 1
Sorry, I will put together a package that fully demonstrates it and upload a tarball later today. I'll also provide the LLVM and executable output.
To be clear, I wasn't building using any direct invocation of rustc, just with cargo.
So I managed to simplify the minimal example further a bit, as it turns out that simply dbg!(1i128)
panics too, since the formatter for integers involves multiplication. This is reflected in the following attachments. I have tested these on a live STM32G030 chip; the GDB backtrace is from that (over openocd through the SWD port). I don't consider myself knowledgable enough with QEMU to try to reproduce it there.
@nickmertin does it also reproduce on Rust stable (or beta)? Just to understand here if it's a regression or this issue has "always" been here (I suspect the latter)
Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.
@rustbot label -I-prioritize +P-medium
@nickmertin does it also reproduce on Rust stable (or beta)? Just to understand here if it's a regression or this issue has "always" been here (I suspect the latter)
Good thinking... I just tested it on stable and it appears to not be present there (or at least not with all else equal). I confirmed the issue on nightly-2021-06-05
again just to be sure I didn't change anything else. I'm not sure if there's a good way to bisect the rust toolchain to find the regression? If there is I may be able to script it to see if GDB reports a panic and use that. Of course QEMU would be better-suited for that, so I may try to repro in it as well.
cool @nickmertin then probably we can try to find the regression
To bisect we use cargo-bisect, it will download nightlies in a range of time and bisect them to find where the regression starts. You'll need to download the Rust sources beforehand (this repo) and then run it like:
$ cd /path/to/your/project
$ RUST_SRC_REPO=/path/to/rust/src cargo-bisect-rustc --preserve --start=2021-04-01 --script=./test.sh
test.sh
should be 755 and something like:
#!/bin/sh
... your code to test the regression ...
cargo-bisect will then download nightlies. The first one must not reproduce the issue, so if it does, move --start
a bit back in time until you hit one of the last good nightly. If everything goes well you should get an ouput similar to this
Thanks a lot for trying to bisect the issue! :-)
@rustbot label regression-from-stable-to-nightly
Here's the report:
searched nightlies: from nightly-2021-04-01 to nightly-2021-06-05 regressed nightly: nightly-2021-05-15 searched commits: from https://github.com/rust-lang/rust/commit/6d395a1c2946c79966490f5b1e6e619d3a713e6b to https://github.com/rust-lang/rust/commit/1025db84a68b948139b5adcd55da31bce32da8f3 regressed commit: https://github.com/rust-lang/rust/commit/17f30e5451f581d753899d2f628e5be354df33cd
That is a huge commit, but if it helps, I have a hunch that the regression may actually be in compiler-builtins
, since a) the version of it changed as part of that commit and b) that's where the implementaiton of this is. On the other hand, the connection to codegen-units = 1
is weird. Before reporting the issue I did delve into the compiler-builtins
source but most of the arithmetic implementation stuff kinda went over my head unfortunately.
This is highly likely to slip into stable next week -- let's @rustbot ping arm to at least get some more eyes on it (doesn't look like that was done yet).
Error: Parsing ping command in comment failed: ...'t ping arm' | error: expected end of command at >| ' to at lea'...
Please let @rust-lang/release
know if you're having trouble with this bot.
@rustbot ping arm
Hey ARM Group! This bug has been identified as a good "ARM candidate". In case it's useful, here are some instructions for tackling these sorts of bugs. Maybe take a look? Thanks! <3
cc @adamgemmell @hug-dev @jacobbramley @JamieCunliffe @joaopaulocarreiro @raw-bin @Stammark
I can reproduce this on qemu. I'm not sure precisely what I'm doing differently to @jfrimmel, but I'm using the source tarball @nickmertin uploaded (plus a modified memory layout for the QEMU machine). Possibly build-std = ["core", "alloc"]
in the cargo config would do it.
qemu-system-arm -cpu cortex-m3 -M lm3s6965evb -kernel bug-example/target/thumbv6m-none-eabi/debug/bug-example -gdb tcp::3333 -S --semihosting-config enable=on,target=native -nographic
From the last working rust commit bumping compiler-builtins to 0.1.40 (from 0.1.39) is enough to trigger this. Unfortunately, patching out compiler_builtins to a local path (checked out to 0.1.39) also seems to reliably trigger it so I haven't been able to narrow down to a specific compiler_builtins commit.
Cargo.toml:
[patch.crates-io]
compiler_builtins = { path = "../compiler-builtins" }
Additionally I can't reproduce on thumbv7m-none-eabi
, only v6.
Additionally I can't reproduce on
thumbv7m-none-eabi
, only v6.
That matches what I observed on real hardware as well.
1.54 stable will be built today and released on Thursday, this will unfortunately have to slip into stable :(
The target is not tier 1 so this won't block the release, but it will be added in the compatibility notes section.
1.54 stable will be built today and released on Thursday, this will unfortunately have to slip into stable :(
Is the build process documented somewhere? Is it a highly optimized binary?
The build itself is done by our normal CI and the full release process is documented here.
The build doesn't take that long to finish though. The Release Team prepares the final release on Monday to be able to publish a pre-release on Monday evening, allowing testing from the wider community before the actual release goes out. It's possible to do a rebuild after Monday if critical issues are found and need to be fixed, but thumbv6m-none-eabi
is not a Tier 1 target so a fix for this would likely not result in a rebuild, and would slip into 1.55.
Thanks! I noted that the python scripts use the aws cli instead of the excellent boto3 library. But probably the scripts are battle proof by now.
This is a bug in LLVM's expansion of 128-bit shifts: https://rust.godbolt.org/z/3nz1fnTE5
The 128-bit shift is expanded to a series of 64-bit shift via __aeabi_llsl
.
Note how the shift value in r6
is only masked with 127
before being passed to __aeabi_llsl
. This means that the shift value passed to __aeabi_llsl
may be larger than 64, which is UB according to the specification for __aeabi_llsl
(shift value must be < 64).
A bisect on compiler-builtins leads to this commit as the incidental "cause": https://github.com/rust-lang/compiler-builtins/commit/01eaf808c6e491632492c7d67f239ad0086d0cbd
Reverting it does fix this bug if a quick & dirty fix is wanted.
On second thought, I don't think this is an LLVM bug: it seems that the code calls __aeabi_llsl
with an out-of-range shift, but then doesn't actually use the result of that call in that case.
I opened https://github.com/rust-lang/compiler-builtins/pull/432 which should fix this, could someone test to see if this actually resolves the issue?
@Amanieu looks fixed to me. Nice work!
Minimal reproducible example, targeting the STM32G030F6 microcontroller:
This panics in the computation of
x * x
when built with:Note: this had to be tested with
opt-level = 1
in the dev profile, in order to fit on the small flash size on the chip. This was confirmed on thenightly-2021-05-22
andnightly-2021-06-05
toolchains.GDB backtrace at the panic
``` #0 panic_semihosting::panic (info=0x20001bc8) at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/panic-semihosting-0.5.6/src/lib.rs:79 #1 0x08001098 in core::panicking::panic_fmt (fmt=...) at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panicking.rs:92 #2 0x0800107a in core::panicking::panic (expr=...) at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/panicking.rs:50 #3 0x0800332a in core::ops::bit::{{impl}}::shl (self=Full Cargo.toml to build
```toml [package] name = "interface-firmware" version = "0.1.0" readme = "README.md" authors = ["Nick Mertinmemory.x for the STM32G030F6
``` MEMORY { FLASH : ORIGIN = 0x08000000, LENGTH = 32K RAM : ORIGIN = 0x20000000, LENGTH = 8K } ```