about summary refs log tree commit diff
diff options
context:
space:
mode:
author许杰友 Jieyou Xu (Joe) <39484203+jieyouxu@users.noreply.github.com>2024-10-18 12:00:50 +0100
committerGitHub <noreply@github.com>2024-10-18 12:00:50 +0100
commitebff1679664272ebeda1f2b753ae499d7dee35ee (patch)
treebf40303b3392be5c6a1d0564d5d4205245f77163
parentdae3076fa20f6a01b66681dbcd48ac6ad1b5863e (diff)
parenta35ed2f9eb52176b14e27f0f39229015cf035a30 (diff)
downloadrust-ebff1679664272ebeda1f2b753ae499d7dee35ee.tar.gz
rust-ebff1679664272ebeda1f2b753ae499d7dee35ee.zip
Rollup merge of #131755 - jfrimmel:avr-rjmp-offset-regression-test, r=jieyouxu
Regression test for AVR `rjmp` offset

This adds a regression test for #129301 by minimizing the code in the linked issue and putting it into a `#![no_core]`-compatible format, so that it can easily be used within an `rmake`-test. This needs to be a `rmake` test (opposed to a `tests/assembly` one), since the linked issue describes, that the problem only occurs if the code is directly compiled. Note, that `lld` is used instead of `avr-gcc`; see the [comments](https://github.com/rust-lang/rust/pull/131755#issuecomment-2416469675) [below](https://github.com/rust-lang/rust/pull/131755#issuecomment-2417160045).
Closes #129301.

To show, that the test actually catches the wrong behavior, this can be tested with a faulty rustc:
```bash
$ rustup install nightly-2024-08-19
$ rustc +nightly-2024-08-19 -C opt-level=s -C panic=abort --target avr-unknown-gnu-atmega328 -Clinker=build/x86_64-unknown-linux-gnu/ci-llvm/bin/lld -Clink-arg='--entry=main' -o compiled tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs
$ llvm-objdump -d compiled | grep '<main>' -A 6
000110b4 <main>:
   110b4: 81 e0         ldi     r24, 0x1
   110b6: 92 e0         ldi     r25, 0x2
   110b8: 85 b9         out     0x5, r24
   110ba: 95 b9         out     0x5, r25
   110bc: fe cf         rjmp    .-4
```
One can see, that the wrong label offset (`4` instead of `6`) is produced, which would trigger an assertion in the test case.

This would be a good candidate for using the `minicore` proposed in #130693. Since this is not yet merged, there is a FIXME.

r? Patryk27
I think, you are the yet-to-be official target maintainer, hence I'll assign to you.

`@rustbot` label +O-AVR
-rw-r--r--tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs47
-rw-r--r--tests/run-make/avr-rjmp-offset/rmake.rs60
2 files changed, 107 insertions, 0 deletions
diff --git a/tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs b/tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs
new file mode 100644
index 00000000000..2f97fc1ed95
--- /dev/null
+++ b/tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs
@@ -0,0 +1,47 @@
+//! This test case is a `#![no_core]`-version of the MVCE presented in #129301.
+//!
+//! The function [`delay()`] is removed, as it is not necessary to trigger the
+//! wrong behavior and would require some additional lang items.
+#![feature(no_core, lang_items, intrinsics, rustc_attrs)]
+#![no_core]
+#![no_main]
+#![allow(internal_features)]
+
+use minicore::ptr;
+
+#[no_mangle]
+pub fn main() -> ! {
+    let port_b = 0x25 as *mut u8; // the I/O-address of PORTB
+
+    // a simple loop with some trivial instructions within. This loop label has
+    // to be placed correctly before the `ptr::write_volatile()` (some LLVM ver-
+    // sions did place it after the first loop instruction, causing unsoundness)
+    loop {
+        unsafe { ptr::write_volatile(port_b, 1) };
+        unsafe { ptr::write_volatile(port_b, 2) };
+    }
+}
+
+// FIXME: replace with proper minicore once available (#130693)
+mod minicore {
+    #[lang = "sized"]
+    pub trait Sized {}
+
+    #[lang = "copy"]
+    pub trait Copy {}
+    impl Copy for u32 {}
+    impl Copy for &u32 {}
+    impl<T: ?Sized> Copy for *mut T {}
+
+    pub mod ptr {
+        #[inline]
+        #[rustc_diagnostic_item = "ptr_write_volatile"]
+        pub unsafe fn write_volatile<T>(dst: *mut T, src: T) {
+            extern "rust-intrinsic" {
+                #[rustc_nounwind]
+                pub fn volatile_store<T>(dst: *mut T, val: T);
+            }
+            unsafe { volatile_store(dst, src) };
+        }
+    }
+}
diff --git a/tests/run-make/avr-rjmp-offset/rmake.rs b/tests/run-make/avr-rjmp-offset/rmake.rs
new file mode 100644
index 00000000000..89cbca309be
--- /dev/null
+++ b/tests/run-make/avr-rjmp-offset/rmake.rs
@@ -0,0 +1,60 @@
+//@ needs-llvm-components: avr
+//@ needs-rust-lld
+//! Regression test for #129301/llvm-project#106722 within `rustc`.
+//!
+//! Some LLVM-versions had wrong offsets in the local labels, causing the first
+//! loop instruction to be missed. This test therefore contains a simple loop
+//! with trivial instructions in it, to see, where the label is placed.
+//!
+//! This must be a `rmake`-test and cannot be a `tests/assembly`-test, since the
+//! wrong output is only produced with direct assembly generation, but not when
+//! "emit-asm" is used, as described in the issue description of #129301:
+//! https://github.com/rust-lang/rust/issues/129301#issue-2475070770
+use run_make_support::{llvm_objdump, rustc};
+
+fn main() {
+    rustc()
+        .input("avr-rjmp-offsets.rs")
+        .opt_level("s")
+        .panic("abort")
+        .target("avr-unknown-gnu-atmega328")
+        // normally one links with `avr-gcc`, but this is not available in CI,
+        // hence this test diverges from the default behavior to enable linking
+        // at all, which is necessary for the test (to resolve the labels). To
+        // not depend on a special linker script, the main-function is marked as
+        // the entry function, causing the linker to not remove it.
+        .linker("rust-lld")
+        .link_arg("--entry=main")
+        .output("compiled")
+        .run();
+
+    let disassembly = llvm_objdump().disassemble().input("compiled").run().stdout_utf8();
+
+    // search for the following instruction sequence:
+    // ```disassembly
+    // 00000080 <main>:
+    // 80: 81 e0         ldi     r24, 0x1
+    // 82: 92 e0         ldi     r25, 0x2
+    // 84: 85 b9         out     0x5, r24
+    // 86: 95 b9         out     0x5, r25
+    // 88: fd cf         rjmp    .-6
+    // ```
+    // This matches on all instructions, since the size of the instructions be-
+    // fore the relative jump has an impact on the label offset. Old versions
+    // of the Rust compiler did produce a label `rjmp .-4` (misses the first
+    // instruction in the loop).
+    assert!(disassembly.contains("<main>"), "no main function in output");
+    disassembly
+        .trim()
+        .lines()
+        .skip_while(|&line| !line.contains("<main>"))
+        .inspect(|line| println!("{line}"))
+        .skip(1)
+        .zip(["ldi\t", "ldi\t", "out\t", "out\t", "rjmp\t.-6"])
+        .for_each(|(line, expected_instruction)| {
+            assert!(
+                line.contains(expected_instruction),
+                "expected instruction `{expected_instruction}`, got `{line}`"
+            );
+        });
+}