From da44e3fdceb46b8d53b2a6e93bca3b80d4243a17 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Mon, 14 Oct 2024 23:06:32 +0200 Subject: [PATCH 01/14] Start test case for `rjmp` regression test This commit introduces a minimal `![no_core]`-test case running on AVR, that contains the MCWE mentioned in [129301]. The test case itself does not have any assertions yet, but it shows the minimal set an language items necessary within the test case. [129301]: https://github.com/rust-lang/rust/issues/129301#issuecomment-2301399472 --- tests/assembly/avr-rjmp-offsets.rs | 62 ++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tests/assembly/avr-rjmp-offsets.rs diff --git a/tests/assembly/avr-rjmp-offsets.rs b/tests/assembly/avr-rjmp-offsets.rs new file mode 100644 index 0000000000000..fafa2b43138da --- /dev/null +++ b/tests/assembly/avr-rjmp-offsets.rs @@ -0,0 +1,62 @@ +//@ compile-flags: -Copt-level=s --target=avr-unknown-gnu-atmega328 -C panic=abort +//@ needs-llvm-components: avr +//@ assembly-output: emit-asm + +#![feature( + no_core, + lang_items, + intrinsics, + rustc_attrs, + arbitrary_self_types, + asm_experimental_arch +)] +#![crate_type = "rlib"] +#![no_core] + +#[rustc_builtin_macro] +macro_rules! asm { + () => {}; +} + +use minicore::ptr; + +// CHECK-LABEL: pin_toggling +#[no_mangle] +pub fn pin_toggling() { + let port_b = 0x25 as *mut u8; // the I/O-address of PORTB + loop { + unsafe { ptr::write_volatile(port_b, 1) }; + delay(500_0000); + unsafe { ptr::write_volatile(port_b, 2) }; + delay(500_0000); + } +} + +#[inline(never)] +fn delay(_: u32) { + unsafe { asm!("nop") }; +} + +// 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 Copy for *mut T {} + + pub mod ptr { + #[inline] + #[rustc_diagnostic_item = "ptr_write_volatile"] + pub unsafe fn write_volatile(dst: *mut T, src: T) { + extern "rust-intrinsic" { + #[rustc_nounwind] + pub fn volatile_store(dst: *mut T, val: T); + } + unsafe { volatile_store(dst, src) }; + } + } +} From 652ba6699c30f036be07fead0df63a1e5f5ddbf8 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Tue, 15 Oct 2024 12:23:35 +0200 Subject: [PATCH 02/14] Add check-annotations ensuring correct label The issue was, that the disassembled label was placed one instruction further down than expected. Therefore the test annotations check, that the label is placed above the loop-contents (writing the one value, then writing the other one). --- tests/assembly/avr-rjmp-offsets.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/assembly/avr-rjmp-offsets.rs b/tests/assembly/avr-rjmp-offsets.rs index fafa2b43138da..bf43e016f92b7 100644 --- a/tests/assembly/avr-rjmp-offsets.rs +++ b/tests/assembly/avr-rjmp-offsets.rs @@ -21,6 +21,12 @@ macro_rules! asm { use minicore::ptr; // CHECK-LABEL: pin_toggling +// CHECK: .LBB0_1: +// CHECK-NEXT: out 5, r17 +// CHECK-NEXT: call delay +// CHECK-NEXT: out 5, r16 +// CHECK-NEXT: call delay +// CHECK-NEXT: rjmp .LBB0_1 #[no_mangle] pub fn pin_toggling() { let port_b = 0x25 as *mut u8; // the I/O-address of PORTB @@ -33,6 +39,7 @@ pub fn pin_toggling() { } #[inline(never)] +#[no_mangle] fn delay(_: u32) { unsafe { asm!("nop") }; } From db6c736da3df6ab7be13c90ac0c13c21b2ba6920 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Tue, 15 Oct 2024 12:30:55 +0200 Subject: [PATCH 03/14] Allow the compiler to select any register --- tests/assembly/avr-rjmp-offsets.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/assembly/avr-rjmp-offsets.rs b/tests/assembly/avr-rjmp-offsets.rs index bf43e016f92b7..0acf54fada46a 100644 --- a/tests/assembly/avr-rjmp-offsets.rs +++ b/tests/assembly/avr-rjmp-offsets.rs @@ -21,10 +21,12 @@ macro_rules! asm { use minicore::ptr; // CHECK-LABEL: pin_toggling +// CHECK: ldi [[REG_1:r[0-9]+]], 1 +// CHECK: ldi [[REG_2:r[0-9]+]], 2 // CHECK: .LBB0_1: -// CHECK-NEXT: out 5, r17 +// CHECK-NEXT: out 5, [[REG_1]] // CHECK-NEXT: call delay -// CHECK-NEXT: out 5, r16 +// CHECK-NEXT: out 5, [[REG_2]] // CHECK-NEXT: call delay // CHECK-NEXT: rjmp .LBB0_1 #[no_mangle] From ab008414d4b4919c1344ef0d542fc9b0d354a505 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Tue, 15 Oct 2024 20:46:23 +0200 Subject: [PATCH 04/14] Convert to a `rmake`-test Since the `tests/assembly` use `emit=asm`, the issue is not observable as reported in the linked issue. Therefore the existing test case is converted and a simple `rmake`-test is added. The test only checks, if the correct `rjmp`-offset is used. --- .../avr-rjmp-offset}/avr-rjmp-offsets.rs | 35 +++++++------------ tests/run-make/avr-rjmp-offset/rmake.rs | 28 +++++++++++++++ 2 files changed, 40 insertions(+), 23 deletions(-) rename tests/{assembly => run-make/avr-rjmp-offset}/avr-rjmp-offsets.rs (63%) create mode 100644 tests/run-make/avr-rjmp-offset/rmake.rs diff --git a/tests/assembly/avr-rjmp-offsets.rs b/tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs similarity index 63% rename from tests/assembly/avr-rjmp-offsets.rs rename to tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs index 0acf54fada46a..d0e5ef1997380 100644 --- a/tests/assembly/avr-rjmp-offsets.rs +++ b/tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs @@ -1,17 +1,11 @@ -//@ compile-flags: -Copt-level=s --target=avr-unknown-gnu-atmega328 -C panic=abort -//@ needs-llvm-components: avr -//@ assembly-output: emit-asm - -#![feature( - no_core, - lang_items, - intrinsics, - rustc_attrs, - arbitrary_self_types, - asm_experimental_arch -)] -#![crate_type = "rlib"] +//! This test case is a `#![no_core]`-version of the MVCE presented in #129301. +//! +//! The function [`delay()`] is minimized and does not actually contain a loop +//! in order to remove the need for additional lang items. +#![feature(no_core, lang_items, intrinsics, rustc_attrs, asm_experimental_arch)] #![no_core] +#![no_main] +#![allow(internal_features)] #[rustc_builtin_macro] macro_rules! asm { @@ -20,18 +14,13 @@ macro_rules! asm { use minicore::ptr; -// CHECK-LABEL: pin_toggling -// CHECK: ldi [[REG_1:r[0-9]+]], 1 -// CHECK: ldi [[REG_2:r[0-9]+]], 2 -// CHECK: .LBB0_1: -// CHECK-NEXT: out 5, [[REG_1]] -// CHECK-NEXT: call delay -// CHECK-NEXT: out 5, [[REG_2]] -// CHECK-NEXT: call delay -// CHECK-NEXT: rjmp .LBB0_1 #[no_mangle] -pub fn pin_toggling() { +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) }; delay(500_0000); 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 0000000000000..666aa41ef4176 --- /dev/null +++ b/tests/run-make/avr-rjmp-offset/rmake.rs @@ -0,0 +1,28 @@ +//@ needs-llvm-components: avr +//! 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") + .output("compiled") + .run(); + + llvm_objdump() + .disassemble() + .input("compiled") + .run() + .assert_stdout_contains_regex(r"rjmp.*\.-14"); +} From bb8db13892ced757faf1c0206c70f313cd4b23c0 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Tue, 15 Oct 2024 21:04:06 +0200 Subject: [PATCH 05/14] Simplify test and make it more reliable The new `rmake`-content asserts the exact assembly sequence for the loop preventing false-negatives if some instructions would change and thus the label offset might need to change. --- .../avr-rjmp-offset/avr-rjmp-offsets.rs | 19 ++--------- tests/run-make/avr-rjmp-offset/rmake.rs | 33 ++++++++++++++++--- 2 files changed, 31 insertions(+), 21 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 index d0e5ef1997380..2f97fc1ed95ab 100644 --- a/tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs +++ b/tests/run-make/avr-rjmp-offset/avr-rjmp-offsets.rs @@ -1,17 +1,12 @@ //! This test case is a `#![no_core]`-version of the MVCE presented in #129301. //! -//! The function [`delay()`] is minimized and does not actually contain a loop -//! in order to remove the need for additional lang items. -#![feature(no_core, lang_items, intrinsics, rustc_attrs, asm_experimental_arch)] +//! 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)] -#[rustc_builtin_macro] -macro_rules! asm { - () => {}; -} - use minicore::ptr; #[no_mangle] @@ -23,18 +18,10 @@ pub fn main() -> ! { // sions did place it after the first loop instruction, causing unsoundness) loop { unsafe { ptr::write_volatile(port_b, 1) }; - delay(500_0000); unsafe { ptr::write_volatile(port_b, 2) }; - delay(500_0000); } } -#[inline(never)] -#[no_mangle] -fn delay(_: u32) { - unsafe { asm!("nop") }; -} - // FIXME: replace with proper minicore once available (#130693) mod minicore { #[lang = "sized"] diff --git a/tests/run-make/avr-rjmp-offset/rmake.rs b/tests/run-make/avr-rjmp-offset/rmake.rs index 666aa41ef4176..28018f59c5bb6 100644 --- a/tests/run-make/avr-rjmp-offset/rmake.rs +++ b/tests/run-make/avr-rjmp-offset/rmake.rs @@ -20,9 +20,32 @@ fn main() { .output("compiled") .run(); - llvm_objdump() - .disassemble() - .input("compiled") - .run() - .assert_stdout_contains_regex(r"rjmp.*\.-14"); + let disassembly = llvm_objdump().disassemble().input("compiled").run().stdout_utf8(); + + // search for the following instruction sequence: + // ```disassembly + // 00000080
: + // 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). + disassembly + .trim() + .lines() + .skip_while(|&line| !line.contains("
")) + .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}`" + ); + }); } From 24810b0036f87178cdce6876f3b3ee6c9b2b5313 Mon Sep 17 00:00:00 2001 From: George Bateman Date: Mon, 9 Sep 2024 08:43:03 +0100 Subject: [PATCH 06/14] Partially stabilize const_pin --- library/alloc/src/lib.rs | 1 - library/core/src/lib.rs | 2 +- library/core/src/pin.rs | 20 ++++++++++---------- library/core/tests/lib.rs | 2 +- library/core/tests/pin.rs | 4 ++++ 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 0a4a5160d827a..ae9b3739858cb 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -112,7 +112,6 @@ #![feature(const_eval_select)] #![feature(const_heap)] #![feature(const_maybe_uninit_write)] -#![feature(const_pin)] #![feature(const_size_of_val)] #![feature(const_vec_string_slice)] #![feature(core_intrinsics)] diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs index 7060da172f330..ad0acca0d568a 100644 --- a/library/core/src/lib.rs +++ b/library/core/src/lib.rs @@ -129,7 +129,7 @@ #![feature(const_nonnull_new)] #![feature(const_num_midpoint)] #![feature(const_option_ext)] -#![feature(const_pin)] +#![feature(const_pin_2)] #![feature(const_pointer_is_aligned)] #![feature(const_ptr_is_null)] #![feature(const_ptr_sub_ptr)] diff --git a/library/core/src/pin.rs b/library/core/src/pin.rs index fac789dbd99fb..5d5733d38fca1 100644 --- a/library/core/src/pin.rs +++ b/library/core/src/pin.rs @@ -1186,7 +1186,7 @@ impl> Pin { /// let mut pinned: Pin<&mut u8> = Pin::new(&mut val); /// ``` #[inline(always)] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_stable(feature = "const_pin", since = "CURRENT_RUSTC_VERSION")] #[stable(feature = "pin", since = "1.33.0")] pub const fn new(pointer: Ptr) -> Pin { // SAFETY: the value pointed to is `Unpin`, and so has no requirements @@ -1214,7 +1214,7 @@ impl> Pin { /// assert_eq!(*r, 5); /// ``` #[inline(always)] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_unstable(feature = "const_pin_2", issue = "76654")] #[stable(feature = "pin_into_inner", since = "1.39.0")] pub const fn into_inner(pin: Pin) -> Ptr { pin.__pointer @@ -1351,7 +1351,7 @@ impl Pin { /// [`pin` module docs]: self #[lang = "new_unchecked"] #[inline(always)] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_stable(feature = "const_pin", since = "CURRENT_RUSTC_VERSION")] #[stable(feature = "pin", since = "1.33.0")] pub const unsafe fn new_unchecked(pointer: Ptr) -> Pin { Pin { __pointer: pointer } @@ -1503,7 +1503,7 @@ impl Pin { /// If the underlying data is [`Unpin`], [`Pin::into_inner`] should be used /// instead. #[inline(always)] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_unstable(feature = "const_pin_2", issue = "76654")] #[stable(feature = "pin_into_inner", since = "1.39.0")] pub const unsafe fn into_inner_unchecked(pin: Pin) -> Ptr { pin.__pointer @@ -1559,7 +1559,7 @@ impl<'a, T: ?Sized> Pin<&'a T> { /// ["pinning projections"]: self#projections-and-structural-pinning #[inline(always)] #[must_use] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_stable(feature = "const_pin", since = "CURRENT_RUSTC_VERSION")] #[stable(feature = "pin", since = "1.33.0")] pub const fn get_ref(self) -> &'a T { self.__pointer @@ -1570,7 +1570,7 @@ impl<'a, T: ?Sized> Pin<&'a mut T> { /// Converts this `Pin<&mut T>` into a `Pin<&T>` with the same lifetime. #[inline(always)] #[must_use = "`self` will be dropped if the result is not used"] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_stable(feature = "const_pin", since = "CURRENT_RUSTC_VERSION")] #[stable(feature = "pin", since = "1.33.0")] pub const fn into_ref(self) -> Pin<&'a T> { Pin { __pointer: self.__pointer } @@ -1588,7 +1588,7 @@ impl<'a, T: ?Sized> Pin<&'a mut T> { #[inline(always)] #[must_use = "`self` will be dropped if the result is not used"] #[stable(feature = "pin", since = "1.33.0")] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_stable(feature = "const_pin", since = "CURRENT_RUSTC_VERSION")] pub const fn get_mut(self) -> &'a mut T where T: Unpin, @@ -1609,7 +1609,7 @@ impl<'a, T: ?Sized> Pin<&'a mut T> { #[inline(always)] #[must_use = "`self` will be dropped if the result is not used"] #[stable(feature = "pin", since = "1.33.0")] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_stable(feature = "const_pin", since = "CURRENT_RUSTC_VERSION")] pub const unsafe fn get_unchecked_mut(self) -> &'a mut T { self.__pointer } @@ -1652,7 +1652,7 @@ impl Pin<&'static T> { /// This is safe because `T` is borrowed immutably for the `'static` lifetime, which /// never ends. #[stable(feature = "pin_static_ref", since = "1.61.0")] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_stable(feature = "const_pin", since = "CURRENT_RUSTC_VERSION")] pub const fn static_ref(r: &'static T) -> Pin<&'static T> { // SAFETY: The 'static borrow guarantees the data will not be // moved/invalidated until it gets dropped (which is never). @@ -1666,7 +1666,7 @@ impl Pin<&'static mut T> { /// This is safe because `T` is borrowed for the `'static` lifetime, which /// never ends. #[stable(feature = "pin_static_ref", since = "1.61.0")] - #[rustc_const_unstable(feature = "const_pin", issue = "76654")] + #[rustc_const_stable(feature = "const_pin", since = "CURRENT_RUSTC_VERSION")] pub const fn static_mut(r: &'static mut T) -> Pin<&'static mut T> { // SAFETY: The 'static borrow guarantees the data will not be // moved/invalidated until it gets dropped (which is never). diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index bfc0b638b7e6c..443090097c0eb 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -21,7 +21,7 @@ #![feature(const_likely)] #![feature(const_nonnull_new)] #![feature(const_option_ext)] -#![feature(const_pin)] +#![feature(const_pin_2)] #![feature(const_pointer_is_aligned)] #![feature(const_three_way_compare)] #![feature(const_trait_impl)] diff --git a/library/core/tests/pin.rs b/library/core/tests/pin.rs index 7a6af46a74323..026d2ca8de26a 100644 --- a/library/core/tests/pin.rs +++ b/library/core/tests/pin.rs @@ -19,6 +19,10 @@ fn pin_const() { const REF: &'static usize = PINNED.get_ref(); assert_eq!(REF, POINTER); + const INT: u8 = 42; + const STATIC_REF: Pin<&'static u8> = Pin::static_ref(&INT); + assert_eq!(*STATIC_REF, INT); + // Note: `pin_mut_const` tests that the methods of `Pin<&mut T>` are usable in a const context. // A const fn is used because `&mut` is not (yet) usable in constants. const fn pin_mut_const() { From 3ed5d5590e12d972acf4ca6a503d0b00f233ad0e Mon Sep 17 00:00:00 2001 From: Collin O'Connor Date: Wed, 16 Oct 2024 19:52:50 -0500 Subject: [PATCH 07/14] Fix predicate signatures in retain_mut docs --- library/alloc/src/collections/linked_list.rs | 2 +- library/alloc/src/collections/vec_deque/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/library/alloc/src/collections/linked_list.rs b/library/alloc/src/collections/linked_list.rs index 0cd410c0fb7c1..ca0ea1ec8b2ba 100644 --- a/library/alloc/src/collections/linked_list.rs +++ b/library/alloc/src/collections/linked_list.rs @@ -1082,7 +1082,7 @@ impl LinkedList { /// Retains only the elements specified by the predicate. /// - /// In other words, remove all elements `e` for which `f(&e)` returns false. + /// In other words, remove all elements `e` for which `f(&mut e)` returns false. /// This method operates in place, visiting each element exactly once in the /// original order, and preserves the order of the retained elements. /// diff --git a/library/alloc/src/collections/vec_deque/mod.rs b/library/alloc/src/collections/vec_deque/mod.rs index 54739c50d1d84..cf51a84bb6f24 100644 --- a/library/alloc/src/collections/vec_deque/mod.rs +++ b/library/alloc/src/collections/vec_deque/mod.rs @@ -2122,7 +2122,7 @@ impl VecDeque { /// Retains only the elements specified by the predicate. /// - /// In other words, remove all elements `e` for which `f(&e)` returns false. + /// In other words, remove all elements `e` for which `f(&mut e)` returns false. /// This method operates in place, visiting each element exactly once in the /// original order, and preserves the order of the retained elements. /// From a35ed2f9eb52176b14e27f0f39229015cf035a30 Mon Sep 17 00:00:00 2001 From: Julian Frimmel Date: Tue, 15 Oct 2024 23:07:00 +0200 Subject: [PATCH 08/14] Use `rust-lld` instead of `avr-gcc` as the linker This fixes the [build error] caused by the `avr-gcc` (used as linker) not being available in the Rust CI. This is a viable solution, which shows the wrong/right behavior and, since no functions from `libgcc` are called, does not produce errors. This was discussed [here]. Another small problem is, that `lld` doesn't link the correct startup-code by default. This is not a problem for this test (since it does not actually use anything the startup code is needed for (no variables, no stack, no interrupts)), but this causes the `main`-function to be removed by the default flag `--gc-sections`. Therefore the `rmake`-driver also adds the linker flag `--entry=main` to mark the `main`-function as the entry point and thus preventing it from getting removed. The code would work on a real AVR device. [build error]: https://github.com/rust-lang/rust/pull/131755#issuecomment-2415127952 [here]: https://github.com/rust-lang/rust/pull/131755#issuecomment-2416469675 --- tests/run-make/avr-rjmp-offset/rmake.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/run-make/avr-rjmp-offset/rmake.rs b/tests/run-make/avr-rjmp-offset/rmake.rs index 28018f59c5bb6..89cbca309be09 100644 --- a/tests/run-make/avr-rjmp-offset/rmake.rs +++ b/tests/run-make/avr-rjmp-offset/rmake.rs @@ -1,4 +1,5 @@ //@ 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 @@ -17,6 +18,13 @@ fn main() { .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(); @@ -35,6 +43,7 @@ fn main() { // 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("
"), "no main function in output"); disassembly .trim() .lines() From cdacdae01f44763603d797e310c056015d1f0e1d Mon Sep 17 00:00:00 2001 From: AnthonyMikh Date: Fri, 18 Oct 2024 02:47:24 +0400 Subject: [PATCH 09/14] remove outdated documentation for `repeat_n` After rust/#106943 the part about `ExactSizeIterator` is no longer valid --- library/core/src/iter/sources/repeat_n.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/library/core/src/iter/sources/repeat_n.rs b/library/core/src/iter/sources/repeat_n.rs index 7e162ff387baf..cc089c617c0e3 100644 --- a/library/core/src/iter/sources/repeat_n.rs +++ b/library/core/src/iter/sources/repeat_n.rs @@ -8,9 +8,7 @@ use crate::num::NonZero; /// The `repeat_n()` function repeats a single value exactly `n` times. /// /// This is very similar to using [`repeat()`] with [`Iterator::take()`], -/// but there are two differences: -/// - `repeat_n()` can return the original value, rather than always cloning. -/// - `repeat_n()` produces an [`ExactSizeIterator`]. +/// but `repeat_n()` can return the original value, rather than always cloning. /// /// [`repeat()`]: crate::iter::repeat /// From 7b2320c3df9e57e8920a8eeec94e907e3d3e6347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Fri, 18 Oct 2024 10:12:11 +0800 Subject: [PATCH 10/14] Avoid shadowing user provided types or type aliases in `thread_local!` By using qualified imports, i.e. `$crate::...::LocalKey`. --- .../std/src/sys/thread_local/native/mod.rs | 31 +++++++++---------- library/std/src/sys/thread_local/os.rs | 17 ++++++---- library/std/tests/thread.rs | 23 ++++++++++++++ 3 files changed, 49 insertions(+), 22 deletions(-) diff --git a/library/std/src/sys/thread_local/native/mod.rs b/library/std/src/sys/thread_local/native/mod.rs index f498dee0899f9..a5dffe3c45883 100644 --- a/library/std/src/sys/thread_local/native/mod.rs +++ b/library/std/src/sys/thread_local/native/mod.rs @@ -49,20 +49,21 @@ pub use lazy::Storage as LazyStorage; #[unstable(feature = "thread_local_internals", issue = "none")] #[rustc_macro_transparency = "semitransparent"] pub macro thread_local_inner { - // used to generate the `LocalKey` value for const-initialized thread locals + // NOTE: we cannot import `LocalKey`, `LazyStorage` or `EagerStorage` with a `use` because that + // can shadow user provided type or type alias with a matching name. Please update the shadowing + // test in `tests/thread.rs` if these types are renamed. + + // Used to generate the `LocalKey` value for const-initialized thread locals. (@key $t:ty, const $init:expr) => {{ const __INIT: $t = $init; unsafe { - use $crate::mem::needs_drop; - use $crate::thread::LocalKey; - use $crate::thread::local_impl::EagerStorage; - - LocalKey::new(const { - if needs_drop::<$t>() { + $crate::thread::LocalKey::new(const { + if $crate::mem::needs_drop::<$t>() { |_| { #[thread_local] - static VAL: EagerStorage<$t> = EagerStorage::new(__INIT); + static VAL: $crate::thread::local_impl::EagerStorage<$t> + = $crate::thread::local_impl::EagerStorage::new(__INIT); VAL.get() } } else { @@ -84,21 +85,19 @@ pub macro thread_local_inner { } unsafe { - use $crate::mem::needs_drop; - use $crate::thread::LocalKey; - use $crate::thread::local_impl::LazyStorage; - - LocalKey::new(const { - if needs_drop::<$t>() { + $crate::thread::LocalKey::new(const { + if $crate::mem::needs_drop::<$t>() { |init| { #[thread_local] - static VAL: LazyStorage<$t, ()> = LazyStorage::new(); + static VAL: $crate::thread::local_impl::LazyStorage<$t, ()> + = $crate::thread::local_impl::LazyStorage::new(); VAL.get_or_init(init, __init) } } else { |init| { #[thread_local] - static VAL: LazyStorage<$t, !> = LazyStorage::new(); + static VAL: $crate::thread::local_impl::LazyStorage<$t, !> + = $crate::thread::local_impl::LazyStorage::new(); VAL.get_or_init(init, __init) } } diff --git a/library/std/src/sys/thread_local/os.rs b/library/std/src/sys/thread_local/os.rs index 26ce3322a16e3..f5a2aaa6c6a3f 100644 --- a/library/std/src/sys/thread_local/os.rs +++ b/library/std/src/sys/thread_local/os.rs @@ -15,19 +15,24 @@ pub macro thread_local_inner { $crate::thread::local_impl::thread_local_inner!(@key $t, { const INIT_EXPR: $t = $init; INIT_EXPR }) }, - // used to generate the `LocalKey` value for `thread_local!` + // NOTE: we cannot import `Storage` or `LocalKey` with a `use` because that can shadow user + // provided type or type alias with a matching name. Please update the shadowing test in + // `tests/thread.rs` if these types are renamed. + + // used to generate the `LocalKey` value for `thread_local!`. (@key $t:ty, $init:expr) => {{ #[inline] fn __init() -> $t { $init } + // NOTE: this cannot import `LocalKey` or `Storage` with a `use` because that can shadow + // user provided type or type alias with a matching name. Please update the shadowing test + // in `tests/thread.rs` if these types are renamed. unsafe { - use $crate::thread::LocalKey; - use $crate::thread::local_impl::Storage; - // Inlining does not work on windows-gnu due to linking errors around // dllimports. See https://github.com/rust-lang/rust/issues/109797. - LocalKey::new(#[cfg_attr(windows, inline(never))] |init| { - static VAL: Storage<$t> = Storage::new(); + $crate::thread::LocalKey::new(#[cfg_attr(windows, inline(never))] |init| { + static VAL: $crate::thread::local_impl::Storage<$t> + = $crate::thread::local_impl::Storage::new(); VAL.get(init, __init) }) } diff --git a/library/std/tests/thread.rs b/library/std/tests/thread.rs index 83574176186a4..1bb17d149fa10 100644 --- a/library/std/tests/thread.rs +++ b/library/std/tests/thread.rs @@ -38,6 +38,29 @@ fn thread_local_containing_const_statements() { assert_eq!(REFCELL.take(), 1); } +#[test] +fn thread_local_hygeiene() { + // Previously `thread_local_inner!` had use imports for `LocalKey`, `Storage`, `EagerStorage` + // and `LazyStorage`. The use imports will shadow a user-provided type or type alias if the + // user-provided type or type alias has the same name. Make sure that this does not happen. See + // . + // + // NOTE: if the internal implementation details change (i.e. get renamed), this test should be + // updated. + + #![allow(dead_code)] + type LocalKey = (); + type Storage = (); + type LazyStorage = (); + type EagerStorage = (); + thread_local! { + static A: LocalKey = const { () }; + static B: Storage = const { () }; + static C: LazyStorage = const { () }; + static D: EagerStorage = const { () }; + } +} + #[test] // Include an ignore list on purpose, so that new platforms don't miss it #[cfg_attr( From 275ec06900b757dc6b3d184715f84b9ed4c92d97 Mon Sep 17 00:00:00 2001 From: WANG Rui Date: Fri, 18 Oct 2024 14:14:42 +0800 Subject: [PATCH 11/14] Default to the medium code model on OpenHarmony LoongArch target The context for this is #130266: setting the medium code model for the 'loongarch64-linux-ohos' target. --- .../src/spec/targets/loongarch64_unknown_linux_ohos.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_ohos.rs b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_ohos.rs index 785c58f3ab737..12e026294cf4f 100644 --- a/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_ohos.rs +++ b/compiler/rustc_target/src/spec/targets/loongarch64_unknown_linux_ohos.rs @@ -1,4 +1,4 @@ -use crate::spec::{SanitizerSet, Target, TargetOptions, base}; +use crate::spec::{CodeModel, SanitizerSet, Target, TargetOptions, base}; pub(crate) fn target() -> Target { Target { @@ -13,6 +13,7 @@ pub(crate) fn target() -> Target { data_layout: "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128".into(), arch: "loongarch64".into(), options: TargetOptions { + code_model: Some(CodeModel::Medium), cpu: "generic".into(), features: "+f,+d".into(), llvm_abiname: "lp64d".into(), From e20636a78621d823334f3114d5d49ab8a442dfa3 Mon Sep 17 00:00:00 2001 From: Jan Sommer Date: Wed, 16 Oct 2024 10:48:07 +0200 Subject: [PATCH 12/14] Add entropy source for RTEMS --- library/std/src/random.rs | 1 + library/std/src/sys/random/arc4random.rs | 2 ++ library/std/src/sys/random/mod.rs | 1 + 3 files changed, 4 insertions(+) diff --git a/library/std/src/random.rs b/library/std/src/random.rs index 604fa4df11066..cdb88c795bfd4 100644 --- a/library/std/src/random.rs +++ b/library/std/src/random.rs @@ -40,6 +40,7 @@ use crate::sys::random as sys; /// Horizon | `getrandom` shim /// Hurd, L4Re, QNX | `/dev/urandom` /// Redox | `/scheme/rand` +/// RTEMS | [`arc4random_buf`](https://docs.rtems.org/branches/master/bsp-howto/getentropy.html) /// SGX | [`rdrand`](https://en.wikipedia.org/wiki/RDRAND) /// SOLID | `SOLID_RNG_SampleRandomBytes` /// TEEOS | `TEE_GenerateRandom` diff --git a/library/std/src/sys/random/arc4random.rs b/library/std/src/sys/random/arc4random.rs index 32467e9ebaa64..ffabaafbee803 100644 --- a/library/std/src/sys/random/arc4random.rs +++ b/library/std/src/sys/random/arc4random.rs @@ -12,6 +12,7 @@ #[cfg(not(any( target_os = "haiku", target_os = "illumos", + target_os = "rtems", target_os = "solaris", target_os = "vita", )))] @@ -21,6 +22,7 @@ use libc::arc4random_buf; #[cfg(any( target_os = "haiku", // See https://git.haiku-os.org/haiku/tree/headers/compatibility/bsd/stdlib.h target_os = "illumos", // See https://www.illumos.org/man/3C/arc4random + target_os = "rtems", // See https://docs.rtems.org/branches/master/bsp-howto/getentropy.html target_os = "solaris", // See https://docs.oracle.com/cd/E88353_01/html/E37843/arc4random-3c.html target_os = "vita", // See https://github.com/vitasdk/newlib/blob/b89e5bc183b516945f9ee07eef483ecb916e45ff/newlib/libc/include/stdlib.h#L74 ))] diff --git a/library/std/src/sys/random/mod.rs b/library/std/src/sys/random/mod.rs index d625814d15b39..edc2cacdfd874 100644 --- a/library/std/src/sys/random/mod.rs +++ b/library/std/src/sys/random/mod.rs @@ -17,6 +17,7 @@ cfg_if::cfg_if! { target_os = "illumos", target_os = "netbsd", target_os = "openbsd", + target_os = "rtems", target_os = "solaris", target_os = "vita", ))] { From 4277160f610b7a6e754bd7c51e8ec26f02d6dd6a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 18 Oct 2024 10:30:22 +0200 Subject: [PATCH 13/14] checktools.sh: add link to issue for more context about disabled Miri tests --- src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh | 2 ++ src/ci/github-actions/jobs.yml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh b/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh index 303a2f26c0f82..8324d1ec58624 100755 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/checktools.sh @@ -59,6 +59,8 @@ case $HOST_TARGET in # "error: cannot produce cdylib for ... as the target ... does not support these crate types". # Only run "pass" tests, which is quite a bit faster. #FIXME: Re-enable this once CI issues are fixed + # See + # For now, these tests are moved to `x86_64-msvc-ext2` in `src/ci/github-actions/jobs.yml`. #python3 "$X_PY" test --stage 2 src/tools/miri --target aarch64-apple-darwin --test-args pass #python3 "$X_PY" test --stage 2 src/tools/miri --target i686-pc-windows-gnu --test-args pass ;; diff --git a/src/ci/github-actions/jobs.yml b/src/ci/github-actions/jobs.yml index 8f49f623afaa4..21a073c937f6f 100644 --- a/src/ci/github-actions/jobs.yml +++ b/src/ci/github-actions/jobs.yml @@ -381,6 +381,8 @@ auto: <<: *job-windows-8c # Temporary builder to workaround CI issues + # See + #FIXME: Remove this, and re-enable the same tests in `checktools.sh`, once CI issues are fixed. - image: x86_64-msvc-ext2 env: SCRIPT: > From cdbf28af7694e0037d2ed8f78c40f60e5aef6de2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 16 Oct 2024 16:09:41 -0400 Subject: [PATCH 14/14] Dont ICE when computing coverage of synthetic async closure body --- compiler/rustc_middle/src/query/mod.rs | 1 + .../src/coroutine/by_move_body.rs | 1 + .../rustc_mir_transform/src/coverage/mod.rs | 5 ++ tests/coverage/async_closure.cov-map | 56 +++++++++++++++++++ tests/coverage/async_closure.coverage | 24 ++++++++ tests/coverage/async_closure.rs | 15 +++++ tests/crashes/131190.rs | 19 ------- 7 files changed, 102 insertions(+), 19 deletions(-) create mode 100644 tests/coverage/async_closure.cov-map create mode 100644 tests/coverage/async_closure.coverage create mode 100644 tests/coverage/async_closure.rs delete mode 100644 tests/crashes/131190.rs diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index f0be70e00dfca..97fc347d48281 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -569,6 +569,7 @@ rustc_queries! { /// either `#[coverage(on)]` or no coverage attribute was found. query coverage_attr_on(key: LocalDefId) -> bool { desc { |tcx| "checking for `#[coverage(..)]` on `{}`", tcx.def_path_str(key) } + feedable } /// Summarizes coverage IDs inserted by the `InstrumentCoverage` MIR pass diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs index cc4b7689d407c..2c622b1927e52 100644 --- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs +++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs @@ -223,6 +223,7 @@ pub(crate) fn coroutine_by_move_body_def_id<'tcx>( // Inherited from the by-ref coroutine. body_def.codegen_fn_attrs(tcx.codegen_fn_attrs(coroutine_def_id).clone()); + body_def.coverage_attr_on(tcx.coverage_attr_on(coroutine_def_id)); body_def.constness(tcx.constness(coroutine_def_id)); body_def.coroutine_kind(tcx.coroutine_kind(coroutine_def_id)); body_def.def_ident_span(tcx.def_ident_span(coroutine_def_id)); diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index d0f30314e79a9..2e4c503f3ce5b 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -524,6 +524,11 @@ fn extract_hir_info<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> ExtractedHir // FIXME(#79625): Consider improving MIR to provide the information needed, to avoid going back // to HIR for it. + // HACK: For synthetic MIR bodies (async closures), use the def id of the HIR body. + if tcx.is_synthetic_mir(def_id) { + return extract_hir_info(tcx, tcx.local_parent(def_id)); + } + let hir_node = tcx.hir_node_by_def_id(def_id); let fn_body_id = hir_node.body_id().expect("HIR node is a function with body"); let hir_body = tcx.hir().body(fn_body_id); diff --git a/tests/coverage/async_closure.cov-map b/tests/coverage/async_closure.cov-map new file mode 100644 index 0000000000000..4d00f0d9b3375 --- /dev/null +++ b/tests/coverage/async_closure.cov-map @@ -0,0 +1,56 @@ +Function name: async_closure::call_once:: +Raw bytes (9): 0x[01, 01, 00, 01, 01, 07, 01, 00, 2c] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 7, 1) to (start + 0, 44) +Highest counter ID seen: c0 + +Function name: async_closure::call_once::::{closure#0} +Raw bytes (14): 0x[01, 01, 00, 02, 01, 07, 2c, 01, 0e, 05, 02, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 2 +- Code(Counter(0)) at (prev + 7, 44) to (start + 1, 14) +- Code(Counter(1)) at (prev + 2, 1) to (start + 0, 2) +Highest counter ID seen: c1 + +Function name: async_closure::main +Raw bytes (14): 0x[01, 01, 00, 02, 01, 0b, 01, 01, 16, 01, 02, 05, 02, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 2 +- Code(Counter(0)) at (prev + 11, 1) to (start + 1, 22) +- Code(Counter(0)) at (prev + 2, 5) to (start + 2, 2) +Highest counter ID seen: c0 + +Function name: async_closure::main::{closure#0} +Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 23, 00, 24] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 12, 35) to (start + 0, 36) +Highest counter ID seen: c0 + +Function name: async_closure::main::{closure#0}::{closure#0}:: +Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 22, 00, 24] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 12, 34) to (start + 0, 36) +Highest counter ID seen: c0 + +Function name: async_closure::main::{closure#0}::{closure#1}:: +Raw bytes (9): 0x[01, 01, 00, 01, 01, 0c, 23, 00, 24] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Counter(0)) at (prev + 12, 35) to (start + 0, 36) +Highest counter ID seen: c0 + diff --git a/tests/coverage/async_closure.coverage b/tests/coverage/async_closure.coverage new file mode 100644 index 0000000000000..fd6edf7c29e3d --- /dev/null +++ b/tests/coverage/async_closure.coverage @@ -0,0 +1,24 @@ + LL| |#![feature(async_closure)] + LL| |//@ edition: 2021 + LL| | + LL| |//@ aux-build: executor.rs + LL| |extern crate executor; + LL| | + LL| 1|async fn call_once(f: impl async FnOnce()) { + LL| 1| f().await; + LL| 1|} + LL| | + LL| 1|pub fn main() { + LL| 2| let async_closure = async || {}; + ^1 + ------------------ + | async_closure::main::{closure#0}: + | LL| 1| let async_closure = async || {}; + ------------------ + | async_closure::main::{closure#0}::{closure#1}::: + | LL| 1| let async_closure = async || {}; + ------------------ + LL| 1| executor::block_on(async_closure()); + LL| 1| executor::block_on(call_once(async_closure)); + LL| 1|} + diff --git a/tests/coverage/async_closure.rs b/tests/coverage/async_closure.rs new file mode 100644 index 0000000000000..c076d03eef431 --- /dev/null +++ b/tests/coverage/async_closure.rs @@ -0,0 +1,15 @@ +#![feature(async_closure)] +//@ edition: 2021 + +//@ aux-build: executor.rs +extern crate executor; + +async fn call_once(f: impl async FnOnce()) { + f().await; +} + +pub fn main() { + let async_closure = async || {}; + executor::block_on(async_closure()); + executor::block_on(call_once(async_closure)); +} diff --git a/tests/crashes/131190.rs b/tests/crashes/131190.rs deleted file mode 100644 index 3a0e64c69d5b5..0000000000000 --- a/tests/crashes/131190.rs +++ /dev/null @@ -1,19 +0,0 @@ -//@ known-bug: #131190 -//@ compile-flags: -Cinstrument-coverage --edition=2018 - -use std::future::Future; - -pub fn block_on(fut: impl Future) -> T {} - -async fn call_once(f: impl async FnOnce(DropMe)) { - f(DropMe("world")).await; -} - -struct DropMe(&'static str); - -pub fn main() { - block_on(async { - let async_closure = async move |a: DropMe| {}; - call_once(async_closure).await; - }); -}