Repository navigation
i128 multiplication on Cortex M0+ unconditionally causes overflow in bitshift causing debug panic, when codegen-units = 1 #86063
Copy link
Copy link
Closed
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.Category: This is a bug.O-ArmTarget: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 stateTarget: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 stateP-mediumMedium priorityMedium priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-betaPerformance or correctness regression from stable to beta.Performance or correctness regression from stable to beta.
Milestone
Description
Minimal reproducible example, targeting the STM32G030F6 microcontroller:
#![no_main]
#![no_std]
use cortex_m_rt::entry;
use cortex_m_semihosting::*;
use panic_semihosting as _;
use stm32g0xx_hal as _;
#[entry]
fn main() -> ! {
let x = dbg!(1i128);
dbg!(x * x);
loop {}
}This panics in the computation of x * x when built with:
[profile.dev]
codegen-units = 1
opt-level = 1 # to fit in sizeNote: 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 the nightly-2021-05-22 and nightly-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=<optimized out>, other=<optimized out>)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:459
#4 0x0800394a in compiler_builtins::int::shift::Ashl::ashl<u64> (self=<optimized out>, shl=4294967264)
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/compiler_builtins-0.1.43/src/int/shift.rs:9
#5 0x08004642 in compiler_builtins::int::shift::__ashldi3 (a=2305873692996934600, b=134241300)
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/compiler_builtins-0.1.43/src/int/shift.rs:80
#6 0x0800464c in compiler_builtins::int::shift::__aeabi_llsl::__aeabi_llsl (a=2305873692996934600, b=134241300)
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/compiler_builtins-0.1.43/src/macros.rs:226
#7 0x08003456 in core::ops::bit::{{impl}}::shl (self=536878024, other=32)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/bit.rs:459
#8 0x08003680 in compiler_builtins::int::mul::Mul::mul<i128> (self=0, rhs=<optimized out>)
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/compiler_builtins-0.1.43/src/int/mul.rs:22
#9 0x08004564 in compiler_builtins::int::mul::__multi3 (a=10635484907954590928331707261963475912, b=<optimized out>)
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/compiler_builtins-0.1.43/src/int/mul.rs:108
#10 0x08004582 in compiler_builtins::int::mul::__multi3::__multi3 (a=10635484907954590928331707261963475912, b=<optimized out>)
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/compiler_builtins-0.1.43/src/macros.rs:270
#11 0x08001e26 in core::fmt::num::udiv_1e19 (n=<optimized out>)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/num.rs:663
#12 0x08001c1c in core::fmt::num::fmt_u128 (n=2305873692460056577, is_nonnegative=<optimized out>, f=0x20001e88)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/num.rs:599
#13 0x08001bfc in core::fmt::num::{{impl}}::fmt (self=<optimized out>, f=0x1)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/num.rs:586
#14 0x08000156 in core::fmt::num::{{impl}}::fmt (self=0x20001f98, f=0x20001e88)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/num.rs:191
#15 0x08000106 in core::fmt::{{impl}}::fmt<i128> (self=<optimized out>, f=0x20001be8)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:2030
#16 0x080021e4 in core::fmt::run (fmt=0x20001e88, arg=0x8004a08, args=...)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:1155
#17 0x080020a8 in core::fmt::write (output=..., args=...)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:1123
#18 0x08000494 in core::fmt::Write::write_fmt<cortex_m_semihosting::hio::HStderr> (self=0x20000004 <cortex_m_semihosting::export::HSTDERR+4>, args=...)
at /home/nmertin/.rustup/toolchains/nightly-2021-05-22-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/fmt/mod.rs:184
#19 0x08000810 in cortex_m_semihosting::export::hstderr_fmt::{{closure}} ()
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/cortex-m-semihosting-0.3.7/src/export.rs:49
#20 0x08000796 in cortex_m::interrupt::free<closure-0,core::result::Result<(), ()>> (f=...)
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/cortex-m-0.7.2/src/interrupt.rs:64
#21 0x080007ca in cortex_m_semihosting::export::hstderr_fmt (args=...)
at /home/nmertin/.cargo/registry/src/github.com-1ecc6299db9ec823/cortex-m-semihosting-0.3.7/src/export.rs:44
#22 0x0800021e in interface_firmware::__cortex_m_rt_main () at src/main.rs:32
#23 0x080001b0 in interface_firmware::__cortex_m_rt_main_trampoline () at src/main.rs:30
Full Cargo.toml to build
[package]
name = "interface-firmware"
version = "0.1.0"
readme = "README.md"
authors = ["Nick Mertin <nickmertin@gmail.com>"]
edition = "2018"
[dependencies]
cortex-m-semihosting = "0.3.3"
cortex-m-rt = "0.6.14"
panic-semihosting = "0.5.6"
[dependencies.stm32g0xx-hal]
version = "0.1.0"
default-features = false
features = ["stm32g030", "rt"]
[profile.dev]
codegen-units = 1
opt-level = 1 # to fit in size
[profile.release]
codegen-units = 1
debug = true
lto = truememory.x for the STM32G030F6
MEMORY
{
FLASH : ORIGIN = 0x08000000, LENGTH = 32K
RAM : ORIGIN = 0x20000000, LENGTH = 8K
}
Metadata
Metadata
Assignees
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.Category: This is a bug.O-ArmTarget: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 stateTarget: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 stateP-mediumMedium priorityMedium priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.regression-from-stable-to-betaPerformance or correctness regression from stable to beta.Performance or correctness regression from stable to beta.
Activity
nikic commentedon Jun 6, 2021
Would it be possible to provide the
--emit=llvm-iroutput for this test case?nagisa commentedon Jun 6, 2021
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}}::shlis what you'd expect. An ABI mismatch, maybe?nagisa commentedon Jun 6, 2021
To elaborate on my remark above: the LLVM IR will only reveal a plain
mulinstruction, 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=corebased 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:
jonas-schievink commentedon Jun 6, 2021
Cortex-M0+ uses
thumbv6m-none-eabi, which is also shipped via rustupjfrimmel commentedon Jun 7, 2021
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.
Reproduction
Shell
Cargo.toml.cargo/configmemory.xbuild.rssrc/main.rs$ 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 = 1nickmertin commentedon Jun 7, 2021
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.
nickmertin commentedon Jun 7, 2021
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.GDB backtrace at panic
LLVM IR
apiraino commentedon Jun 9, 2021
@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)
apiraino commentedon Jun 9, 2021
Assigning priority as discussed in the Zulip thread of the Prioritization Working Group.
@rustbot label -I-prioritize +P-medium
14 remaining items
emilyalbini commentedon Jul 26, 2021
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.
tschuett commentedon Jul 26, 2021
Is the build process documented somewhere? Is it a highly optimized binary?
emilyalbini commentedon Jul 26, 2021
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-eabiis not a Tier 1 target so a fix for this would likely not result in a rebuild, and would slip into 1.55.tschuett commentedon Jul 26, 2021
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.
Amanieu commentedon Jul 26, 2021
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
r6is only masked with127before being passed to__aeabi_llsl. This means that the shift value passed to__aeabi_llslmay be larger than 64, which is UB according to the specification for__aeabi_llsl(shift value must be < 64).adamgemmell commentedon Jul 27, 2021
A bisect on compiler-builtins leads to this commit as the incidental "cause": rust-lang/compiler-builtins@01eaf80
Reverting it does fix this bug if a quick & dirty fix is wanted.
Amanieu commentedon Jul 29, 2021
On second thought, I don't think this is an LLVM bug: it seems that the code calls
__aeabi_llslwith an out-of-range shift, but then doesn't actually use the result of that call in that case.I opened rust-lang/compiler-builtins#432 which should fix this, could someone test to see if this actually resolves the issue?
adamgemmell commentedon Jul 30, 2021
@Amanieu looks fixed to me. Nice work!
Update compiler_builtins to fix i128 shift/mul on thumbv6m
Rollup merge of rust-lang#87633 - Amanieu:fix-86063, r=Mark-Simulacrum
rust: Update to 1.54.0.