Skip to content

Commit

Permalink
Rollup merge of #131755 - jfrimmel:avr-rjmp-offset-regression-test, r…
Browse files Browse the repository at this point in the history
…=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](#131755 (comment)) [below](#131755 (comment)).
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
  • Loading branch information
jieyouxu authored Oct 18, 2024
2 parents dae3076 + a35ed2f commit ebff167
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 0 deletions.
47 changes: 47 additions & 0 deletions tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs
Original file line number Diff line number Diff line change
@@ -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) };
}
}
}
60 changes: 60 additions & 0 deletions tests/run-make/avr-rjmp-offset/rmake.rs
Original file line number Diff line number Diff line change
@@ -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}`"
);
});
}

0 comments on commit ebff167

Please sign in to comment.