From 0b87af9d4f7c6faa9e89496609f016dc3e3977e1 Mon Sep 17 00:00:00 2001 From: Mrmaxmeier Date: Sat, 27 Apr 2024 23:14:36 +0200 Subject: [PATCH 01/23] Add `-Z embed-source=yes` to embed source code in DWARF debug info --- .../src/debuginfo/metadata.rs | 9 +++++++ compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 2 ++ compiler/rustc_interface/src/tests.rs | 1 + .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 9 +++++-- compiler/rustc_session/messages.ftl | 6 +++++ compiler/rustc_session/src/errors.rs | 14 +++++++++++ compiler/rustc_session/src/options.rs | 2 ++ compiler/rustc_session/src/session.rs | 25 +++++++++++++++++-- .../src/compiler-flags/embed-source.md | 12 +++++++++ 9 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 src/doc/unstable-book/src/compiler-flags/embed-source.md diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs index ad63858861261..701ea62b21a7d 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs @@ -629,6 +629,9 @@ pub fn file_metadata<'ll>(cx: &CodegenCx<'ll, '_>, source_file: &SourceFile) -> }; let hash_value = hex_encode(source_file.src_hash.hash_bytes()); + let source = + cx.sess().opts.unstable_opts.embed_source.then_some(()).and(source_file.src.as_ref()); + unsafe { llvm::LLVMRustDIBuilderCreateFile( DIB(cx), @@ -639,6 +642,8 @@ pub fn file_metadata<'ll>(cx: &CodegenCx<'ll, '_>, source_file: &SourceFile) -> hash_kind, hash_value.as_ptr().cast(), hash_value.len(), + source.map_or(ptr::null(), |x| x.as_ptr().cast()), + source.map_or(0, |x| x.len()), ) } } @@ -659,6 +664,8 @@ pub fn unknown_file_metadata<'ll>(cx: &CodegenCx<'ll, '_>) -> &'ll DIFile { llvm::ChecksumKind::None, hash_value.as_ptr().cast(), hash_value.len(), + ptr::null(), + 0, ) }) } @@ -943,6 +950,8 @@ pub fn build_compile_unit_di_node<'ll, 'tcx>( llvm::ChecksumKind::None, ptr::null(), 0, + ptr::null(), + 0, ); let unit_metadata = llvm::LLVMRustDIBuilderCreateCompileUnit( diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index c8e0e075eeabc..faa675b66c8a1 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -1853,6 +1853,8 @@ extern "C" { CSKind: ChecksumKind, Checksum: *const c_char, ChecksumLen: size_t, + Source: *const c_char, + SourceLen: size_t, ) -> &'a DIFile; pub fn LLVMRustDIBuilderCreateSubroutineType<'a>( diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index ce3b2f77f210a..c4704e38ce6fa 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -773,6 +773,7 @@ fn test_unstable_options_tracking_hash() { tracked!(direct_access_external_data, Some(true)); tracked!(dual_proc_macros, true); tracked!(dwarf_version, Some(5)); + tracked!(embed_source, true); tracked!(emit_thin_lto, false); tracked!(export_executable_symbols, true); tracked!(fewer_names, Some(true)); diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 4cdd8af1008c0..6e700c31e6763 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -901,14 +901,19 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateFile(LLVMRustDIBuilderRef Builder, const char *Filename, size_t FilenameLen, const char *Directory, size_t DirectoryLen, LLVMRustChecksumKind CSKind, - const char *Checksum, size_t ChecksumLen) { + const char *Checksum, size_t ChecksumLen, + const char *Source, size_t SourceLen) { std::optional llvmCSKind = fromRust(CSKind); std::optional> CSInfo{}; if (llvmCSKind) CSInfo.emplace(*llvmCSKind, StringRef{Checksum, ChecksumLen}); + std::optional oSource{}; + if (Source) + oSource = StringRef(Source, SourceLen); return wrap(Builder->createFile(StringRef(Filename, FilenameLen), - StringRef(Directory, DirectoryLen), CSInfo)); + StringRef(Directory, DirectoryLen), CSInfo, + oSource)); } extern "C" LLVMMetadataRef diff --git a/compiler/rustc_session/messages.ftl b/compiler/rustc_session/messages.ftl index b84280a3ccf3f..afd5360c81194 100644 --- a/compiler/rustc_session/messages.ftl +++ b/compiler/rustc_session/messages.ftl @@ -14,6 +14,12 @@ session_crate_name_empty = crate name must not be empty session_crate_name_invalid = crate names cannot start with a `-`, but `{$s}` has a leading hyphen +session_embed_source_insufficient_dwarf_version = `-Zembed-source=y` requires at least `-Z dwarf-version=5` but DWARF version is {$dwarf_version} + +session_embed_source_requires_debug_info = `-Zembed-source=y` requires debug information to be enabled + +session_embed_source_requires_llvm_backend = `-Zembed-source=y` is only supported on the LLVM codegen backend + session_expr_parentheses_needed = parentheses are required to parse this as an expression session_failed_to_create_profiler = failed to create profiler: {$err} diff --git a/compiler/rustc_session/src/errors.rs b/compiler/rustc_session/src/errors.rs index 5cc54a5855bbe..f708109b87a0c 100644 --- a/compiler/rustc_session/src/errors.rs +++ b/compiler/rustc_session/src/errors.rs @@ -165,6 +165,20 @@ pub(crate) struct UnsupportedDwarfVersion { pub(crate) dwarf_version: u32, } +#[derive(Diagnostic)] +#[diag(session_embed_source_insufficient_dwarf_version)] +pub(crate) struct EmbedSourceInsufficientDwarfVersion { + pub(crate) dwarf_version: u32, +} + +#[derive(Diagnostic)] +#[diag(session_embed_source_requires_debug_info)] +pub(crate) struct EmbedSourceRequiresDebugInfo; + +#[derive(Diagnostic)] +#[diag(session_embed_source_requires_llvm_backend)] +pub(crate) struct EmbedSourceRequiresLLVMBackend; + #[derive(Diagnostic)] #[diag(session_target_stack_protector_not_supported)] pub(crate) struct StackProtectorNotSupportedForTarget<'a> { diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index bf54aae1cfeb0..13aac6669fe4f 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -1708,6 +1708,8 @@ options! { them only if an error has not been emitted"), ehcont_guard: bool = (false, parse_bool, [TRACKED], "generate Windows EHCont Guard tables"), + embed_source: bool = (false, parse_bool, [TRACKED], + "embed source text in DWARF debug sections (default: no)"), emit_stack_sizes: bool = (false, parse_bool, [UNTRACKED], "emit a section containing stack size metadata (default: no)"), emit_thin_lto: bool = (true, parse_bool, [TRACKED], diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index be67baf57f6dc..634f3684b51aa 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -37,8 +37,9 @@ use rustc_target::spec::{ use crate::code_stats::CodeStats; pub use crate::code_stats::{DataTypeKind, FieldInfo, FieldKind, SizeKind, VariantInfo}; use crate::config::{ - self, CoverageLevel, CrateType, ErrorOutputType, FunctionReturn, Input, InstrumentCoverage, - OptLevel, OutFileName, OutputType, RemapPathScopeComponents, SwitchWithOptPath, + self, CoverageLevel, CrateType, DebugInfo, ErrorOutputType, FunctionReturn, Input, + InstrumentCoverage, OptLevel, OutFileName, OutputType, RemapPathScopeComponents, + SwitchWithOptPath, }; use crate::parse::{add_feature_diagnostics, ParseSess}; use crate::search_paths::{PathKind, SearchPath}; @@ -1300,6 +1301,26 @@ fn validate_commandline_args_with_session_available(sess: &Session) { .emit_err(errors::SplitDebugInfoUnstablePlatform { debuginfo: sess.split_debuginfo() }); } + if sess.opts.unstable_opts.embed_source { + let dwarf_version = + sess.opts.unstable_opts.dwarf_version.unwrap_or(sess.target.default_dwarf_version); + + let uses_llvm_backend = + matches!(sess.opts.unstable_opts.codegen_backend.as_deref(), None | Some("llvm")); + + if dwarf_version < 5 { + sess.dcx().emit_warn(errors::EmbedSourceInsufficientDwarfVersion { dwarf_version }); + } + + if sess.opts.debuginfo == DebugInfo::None { + sess.dcx().emit_warn(errors::EmbedSourceRequiresDebugInfo); + } + + if !uses_llvm_backend { + sess.dcx().emit_warn(errors::EmbedSourceRequiresLLVMBackend); + } + } + if sess.opts.unstable_opts.instrument_xray.is_some() && !sess.target.options.supports_xray { sess.dcx().emit_err(errors::InstrumentationNotSupported { us: "XRay".to_string() }); } diff --git a/src/doc/unstable-book/src/compiler-flags/embed-source.md b/src/doc/unstable-book/src/compiler-flags/embed-source.md new file mode 100644 index 0000000000000..01a11e3779712 --- /dev/null +++ b/src/doc/unstable-book/src/compiler-flags/embed-source.md @@ -0,0 +1,12 @@ +# `embed-source` + +This flag controls whether the compiler embeds the program source code text into +the object debug information section. It takes one of the following values: + +* `y`, `yes`, `on` or `true`: put source code in debug info. +* `n`, `no`, `off`, `false` or no value: omit source code from debug info (the default). + +This flag is ignored in configurations that don't emit DWARF debug information +and is ignored on non-LLVM backends. `-Z embed-source` requires DWARFv5. Use +`-Z dwarf-version=5` to control the compiler's DWARF target version and `-g` to +enable debug info generation. From 608901b9c07d7d2f3e2803378c4f0cc07c61bc36 Mon Sep 17 00:00:00 2001 From: Mrmaxmeier Date: Tue, 16 Jul 2024 20:50:28 +0200 Subject: [PATCH 02/23] Add run-make test for -Zembed-source=yes --- tests/run-make/embed-source-dwarf/main.rs | 2 + tests/run-make/embed-source-dwarf/rmake.rs | 70 ++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 tests/run-make/embed-source-dwarf/main.rs create mode 100644 tests/run-make/embed-source-dwarf/rmake.rs diff --git a/tests/run-make/embed-source-dwarf/main.rs b/tests/run-make/embed-source-dwarf/main.rs new file mode 100644 index 0000000000000..c80af84f41415 --- /dev/null +++ b/tests/run-make/embed-source-dwarf/main.rs @@ -0,0 +1,2 @@ +// hello +fn main() {} diff --git a/tests/run-make/embed-source-dwarf/rmake.rs b/tests/run-make/embed-source-dwarf/rmake.rs new file mode 100644 index 0000000000000..06d550121b0de --- /dev/null +++ b/tests/run-make/embed-source-dwarf/rmake.rs @@ -0,0 +1,70 @@ +//@ ignore-windows +//@ ignore-apple + +// LLVM 17's embed-source implementation requires that source code is attached +// for all files in the output DWARF debug info. This restriction was lifted in +// LLVM 18 (87e22bdd2bd6d77d782f9d64b3e3ae5bdcd5080d). +//@ min-llvm-version: 18 + +// This test should be replaced with one in tests/debuginfo once we can easily +// tell via GDB or LLDB if debuginfo contains source code. Cheap tricks in LLDB +// like setting an invalid source map path don't appear to work, maybe this'll +// become easier once GDB supports DWARFv6? + +use std::collections::HashMap; +use std::path::PathBuf; +use std::rc::Rc; + +use gimli::{AttributeValue, EndianRcSlice, Reader, RunTimeEndian}; +use object::{Object, ObjectSection}; +use run_make_support::{gimli, object, rfs, rustc}; + +fn main() { + let output = PathBuf::from("embed-source-main"); + rustc() + .input("main.rs") + .output(&output) + .arg("-g") + .arg("-Zembed-source=yes") + .arg("-Zdwarf-version=5") + .run(); + let output = rfs::read(output); + let obj = object::File::parse(output.as_slice()).unwrap(); + let endian = if obj.is_little_endian() { RunTimeEndian::Little } else { RunTimeEndian::Big }; + let dwarf = gimli::Dwarf::load(|section| -> Result<_, ()> { + let data = obj.section_by_name(section.name()).map(|s| s.uncompressed_data().unwrap()); + Ok(EndianRcSlice::new(Rc::from(data.unwrap_or_default().as_ref()), endian)) + }) + .unwrap(); + + let mut sources = HashMap::new(); + + let mut iter = dwarf.units(); + while let Some(header) = iter.next().unwrap() { + let unit = dwarf.unit(header).unwrap(); + let unit = unit.unit_ref(&dwarf); + + if let Some(program) = &unit.line_program { + let header = program.header(); + for file in header.file_names() { + if let Some(source) = file.source() { + let path = unit + .attr_string(file.path_name()) + .unwrap() + .to_string_lossy() + .unwrap() + .to_string(); + let source = + unit.attr_string(source).unwrap().to_string_lossy().unwrap().to_string(); + if !source.is_empty() { + sources.insert(path, source); + } + } + } + } + } + + dbg!(&sources); + assert_eq!(sources.len(), 1); + assert_eq!(sources.get("main.rs").unwrap(), "// hello\nfn main() {}\n"); +} From 6899f5a8e12986ee16e028f1597963d0de668aca Mon Sep 17 00:00:00 2001 From: Mrmaxmeier Date: Tue, 6 Aug 2024 20:31:12 +0200 Subject: [PATCH 03/23] -Zembed-source: Don't try to warn about incompatible codegen backends --- compiler/rustc_session/messages.ftl | 2 -- compiler/rustc_session/src/errors.rs | 4 ---- compiler/rustc_session/src/session.rs | 7 ------- 3 files changed, 13 deletions(-) diff --git a/compiler/rustc_session/messages.ftl b/compiler/rustc_session/messages.ftl index afd5360c81194..01c371ee49884 100644 --- a/compiler/rustc_session/messages.ftl +++ b/compiler/rustc_session/messages.ftl @@ -18,8 +18,6 @@ session_embed_source_insufficient_dwarf_version = `-Zembed-source=y` requires at session_embed_source_requires_debug_info = `-Zembed-source=y` requires debug information to be enabled -session_embed_source_requires_llvm_backend = `-Zembed-source=y` is only supported on the LLVM codegen backend - session_expr_parentheses_needed = parentheses are required to parse this as an expression session_failed_to_create_profiler = failed to create profiler: {$err} diff --git a/compiler/rustc_session/src/errors.rs b/compiler/rustc_session/src/errors.rs index f708109b87a0c..15bbd4ff7bf4b 100644 --- a/compiler/rustc_session/src/errors.rs +++ b/compiler/rustc_session/src/errors.rs @@ -175,10 +175,6 @@ pub(crate) struct EmbedSourceInsufficientDwarfVersion { #[diag(session_embed_source_requires_debug_info)] pub(crate) struct EmbedSourceRequiresDebugInfo; -#[derive(Diagnostic)] -#[diag(session_embed_source_requires_llvm_backend)] -pub(crate) struct EmbedSourceRequiresLLVMBackend; - #[derive(Diagnostic)] #[diag(session_target_stack_protector_not_supported)] pub(crate) struct StackProtectorNotSupportedForTarget<'a> { diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 634f3684b51aa..e2ef144e732a4 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1305,9 +1305,6 @@ fn validate_commandline_args_with_session_available(sess: &Session) { let dwarf_version = sess.opts.unstable_opts.dwarf_version.unwrap_or(sess.target.default_dwarf_version); - let uses_llvm_backend = - matches!(sess.opts.unstable_opts.codegen_backend.as_deref(), None | Some("llvm")); - if dwarf_version < 5 { sess.dcx().emit_warn(errors::EmbedSourceInsufficientDwarfVersion { dwarf_version }); } @@ -1315,10 +1312,6 @@ fn validate_commandline_args_with_session_available(sess: &Session) { if sess.opts.debuginfo == DebugInfo::None { sess.dcx().emit_warn(errors::EmbedSourceRequiresDebugInfo); } - - if !uses_llvm_backend { - sess.dcx().emit_warn(errors::EmbedSourceRequiresLLVMBackend); - } } if sess.opts.unstable_opts.instrument_xray.is_some() && !sess.target.options.supports_xray { From 8b18c6bdd3d61dda5fe8c4ed378286abec2a9bc4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 10 Aug 2024 21:54:44 +0200 Subject: [PATCH 04/23] miri weak memory emulation: initialize store buffer only on atomic writes; pre-fill with previous value --- .../rustc_const_eval/src/interpret/memory.rs | 2 +- src/tools/miri/src/concurrency/data_race.rs | 19 +- src/tools/miri/src/concurrency/weak_memory.rs | 164 ++++++++---------- .../tests/fail/weak_memory/weak_uninit.rs | 43 +++++ .../tests/fail/weak_memory/weak_uninit.stderr | 15 ++ src/tools/miri/tests/pass/weak_memory/weak.rs | 19 +- 6 files changed, 153 insertions(+), 109 deletions(-) create mode 100644 src/tools/miri/tests/fail/weak_memory/weak_uninit.rs create mode 100644 src/tools/miri/tests/fail/weak_memory/weak_uninit.stderr diff --git a/compiler/rustc_const_eval/src/interpret/memory.rs b/compiler/rustc_const_eval/src/interpret/memory.rs index 2e5d0ae773654..b0a583f136fd7 100644 --- a/compiler/rustc_const_eval/src/interpret/memory.rs +++ b/compiler/rustc_const_eval/src/interpret/memory.rs @@ -1011,7 +1011,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { /// /// We do this so Miri's allocation access tracking does not show the validation /// reads as spurious accesses. - pub(super) fn run_for_validation(&self, f: impl FnOnce() -> R) -> R { + pub fn run_for_validation(&self, f: impl FnOnce() -> R) -> R { // This deliberately uses `==` on `bool` to follow the pattern // `assert!(val.replace(new) == old)`. assert!( diff --git a/src/tools/miri/src/concurrency/data_race.rs b/src/tools/miri/src/concurrency/data_race.rs index 9df0d95f1f275..6fd207c92b937 100644 --- a/src/tools/miri/src/concurrency/data_race.rs +++ b/src/tools/miri/src/concurrency/data_race.rs @@ -617,9 +617,10 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> { // the *value* (including the associated provenance if this is an AtomicPtr) at this location. // Only metadata on the location itself is used. let scalar = this.allow_data_races_ref(move |this| this.read_scalar(place))?; - this.buffered_atomic_read(place, atomic, scalar, || { + let buffered_scalar = this.buffered_atomic_read(place, atomic, scalar, || { this.validate_atomic_load(place, atomic) - }) + })?; + Ok(buffered_scalar.ok_or_else(|| err_ub!(InvalidUninitBytes(None)))?) } /// Perform an atomic write operation at the memory location. @@ -632,14 +633,14 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> { let this = self.eval_context_mut(); this.atomic_access_check(dest, AtomicAccessType::Store)?; + // Read the previous value so we can put it in the store buffer later. + // The program didn't actually do a read, so suppress the memory access hooks. + // This is also a very special exception where we just ignore an error -- if this read + // was UB e.g. because the memory is uninitialized, we don't want to know! + let old_val = this.run_for_validation(|| this.read_scalar(dest)).ok(); this.allow_data_races_mut(move |this| this.write_scalar(val, dest))?; this.validate_atomic_store(dest, atomic)?; - // FIXME: it's not possible to get the value before write_scalar. A read_scalar will cause - // side effects from a read the program did not perform. So we have to initialise - // the store buffer with the value currently being written - // ONCE this is fixed please remove the hack in buffered_atomic_write() in weak_memory.rs - // https://github.com/rust-lang/miri/issues/2164 - this.buffered_atomic_write(val, dest, atomic, val) + this.buffered_atomic_write(val, dest, atomic, old_val) } /// Perform an atomic RMW operation on a memory location. @@ -768,7 +769,7 @@ pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> { // in the modification order. // Since `old` is only a value and not the store element, we need to separately // find it in our store buffer and perform load_impl on it. - this.perform_read_on_buffered_latest(place, fail, old.to_scalar())?; + this.perform_read_on_buffered_latest(place, fail)?; } // Return the old value. diff --git a/src/tools/miri/src/concurrency/weak_memory.rs b/src/tools/miri/src/concurrency/weak_memory.rs index 6f4171584a8f7..0605b744e6afd 100644 --- a/src/tools/miri/src/concurrency/weak_memory.rs +++ b/src/tools/miri/src/concurrency/weak_memory.rs @@ -39,11 +39,10 @@ //! to attach store buffers to atomic objects. However, Rust follows LLVM in that it only has //! 'atomic accesses'. Therefore Miri cannot know when and where atomic 'objects' are being //! created or destroyed, to manage its store buffers. Instead, we hence lazily create an -//! atomic object on the first atomic access to a given region, and we destroy that object -//! on the next non-atomic or imperfectly overlapping atomic access to that region. +//! atomic object on the first atomic write to a given region, and we destroy that object +//! on the next non-atomic or imperfectly overlapping atomic write to that region. //! These lazy (de)allocations happen in memory_accessed() on non-atomic accesses, and -//! get_or_create_store_buffer() on atomic accesses. This mostly works well, but it does -//! lead to some issues (). +//! get_or_create_store_buffer_mut() on atomic writes. //! //! One consequence of this difference is that safe/sound Rust allows for more operations on atomic locations //! than the C++20 atomic API was intended to allow, such as non-atomically accessing @@ -144,11 +143,9 @@ struct StoreElement { /// The timestamp of the storing thread when it performed the store timestamp: VTimestamp, - /// The value of this store - // FIXME: this means the store must be fully initialized; - // we will have to change this if we want to support atomics on - // (partially) uninitialized data. - val: Scalar, + /// The value of this store. `None` means uninitialized. + // FIXME: Currently, we cannot represent partial initialization. + val: Option, /// Metadata about loads from this store element, /// behind a RefCell to keep load op take &self @@ -170,7 +167,7 @@ impl StoreBufferAlloc { /// When a non-atomic access happens on a location that has been atomically accessed /// before without data race, we can determine that the non-atomic access fully happens - /// after all the prior atomic accesses so the location no longer needs to exhibit + /// after all the prior atomic writes so the location no longer needs to exhibit /// any weak memory behaviours until further atomic accesses. pub fn memory_accessed(&self, range: AllocRange, global: &DataRaceState) { if !global.ongoing_action_data_race_free() { @@ -192,37 +189,29 @@ impl StoreBufferAlloc { } } - /// Gets a store buffer associated with an atomic object in this allocation, - /// or creates one with the specified initial value if no atomic object exists yet. - fn get_or_create_store_buffer<'tcx>( + /// Gets a store buffer associated with an atomic object in this allocation. + /// Returns `None` if there is no store buffer. + fn get_store_buffer<'tcx>( &self, range: AllocRange, - init: Scalar, - ) -> InterpResult<'tcx, Ref<'_, StoreBuffer>> { + ) -> InterpResult<'tcx, Option>> { let access_type = self.store_buffers.borrow().access_type(range); let pos = match access_type { AccessType::PerfectlyOverlapping(pos) => pos, - AccessType::Empty(pos) => { - let mut buffers = self.store_buffers.borrow_mut(); - buffers.insert_at_pos(pos, range, StoreBuffer::new(init)); - pos - } - AccessType::ImperfectlyOverlapping(pos_range) => { - // Once we reach here we would've already checked that this access is not racy. - let mut buffers = self.store_buffers.borrow_mut(); - buffers.remove_pos_range(pos_range.clone()); - buffers.insert_at_pos(pos_range.start, range, StoreBuffer::new(init)); - pos_range.start - } + // If there is nothing here yet, that means there wasn't an atomic write yet so + // we can't return anything outdated. + _ => return Ok(None), }; - Ok(Ref::map(self.store_buffers.borrow(), |buffer| &buffer[pos])) + let store_buffer = Ref::map(self.store_buffers.borrow(), |buffer| &buffer[pos]); + Ok(Some(store_buffer)) } - /// Gets a mutable store buffer associated with an atomic object in this allocation + /// Gets a mutable store buffer associated with an atomic object in this allocation, + /// or creates one with the specified initial value if no atomic object exists yet. fn get_or_create_store_buffer_mut<'tcx>( &mut self, range: AllocRange, - init: Scalar, + init: Option, ) -> InterpResult<'tcx, &mut StoreBuffer> { let buffers = self.store_buffers.get_mut(); let access_type = buffers.access_type(range); @@ -244,10 +233,8 @@ impl StoreBufferAlloc { } impl<'tcx> StoreBuffer { - fn new(init: Scalar) -> Self { + fn new(init: Option) -> Self { let mut buffer = VecDeque::new(); - buffer.reserve(STORE_BUFFER_LIMIT); - let mut ret = Self { buffer }; let store_elem = StoreElement { // The thread index and timestamp of the initialisation write // are never meaningfully used, so it's fine to leave them as 0 @@ -257,11 +244,11 @@ impl<'tcx> StoreBuffer { is_seqcst: false, load_info: RefCell::new(LoadInfo::default()), }; - ret.buffer.push_back(store_elem); - ret + buffer.push_back(store_elem); + Self { buffer } } - /// Reads from the last store in modification order + /// Reads from the last store in modification order, if any. fn read_from_last_store( &self, global: &DataRaceState, @@ -282,7 +269,7 @@ impl<'tcx> StoreBuffer { is_seqcst: bool, rng: &mut (impl rand::Rng + ?Sized), validate: impl FnOnce() -> InterpResult<'tcx>, - ) -> InterpResult<'tcx, (Scalar, LoadRecency)> { + ) -> InterpResult<'tcx, (Option, LoadRecency)> { // Having a live borrow to store_buffer while calling validate_atomic_load is fine // because the race detector doesn't touch store_buffer @@ -419,15 +406,15 @@ impl<'tcx> StoreBuffer { // In the language provided in the paper, an atomic store takes the value from a // non-atomic memory location. // But we already have the immediate value here so we don't need to do the memory - // access - val, + // access. + val: Some(val), is_seqcst, load_info: RefCell::new(LoadInfo::default()), }; - self.buffer.push_back(store_elem); - if self.buffer.len() > STORE_BUFFER_LIMIT { + if self.buffer.len() >= STORE_BUFFER_LIMIT { self.buffer.pop_front(); } + self.buffer.push_back(store_elem); if is_seqcst { // Every store that happens before this needs to be marked as SC // so that in a later SC load, only the last SC store (i.e. this one) or stores that @@ -450,7 +437,12 @@ impl StoreElement { /// buffer regardless of subsequent loads by the same thread; if the earliest load of another /// thread doesn't happen before the current one, then no subsequent load by the other thread /// can happen before the current one. - fn load_impl(&self, index: VectorIdx, clocks: &ThreadClockSet, is_seqcst: bool) -> Scalar { + fn load_impl( + &self, + index: VectorIdx, + clocks: &ThreadClockSet, + is_seqcst: bool, + ) -> Option { let mut load_info = self.load_info.borrow_mut(); load_info.sc_loaded |= is_seqcst; let _ = load_info.timestamps.try_insert(index, clocks.clock[index]); @@ -479,7 +471,7 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { global.sc_write(threads); } let range = alloc_range(base_offset, place.layout.size); - let buffer = alloc_buffers.get_or_create_store_buffer_mut(range, init)?; + let buffer = alloc_buffers.get_or_create_store_buffer_mut(range, Some(init))?; buffer.read_from_last_store(global, threads, atomic == AtomicRwOrd::SeqCst); buffer.buffered_write(new_val, global, threads, atomic == AtomicRwOrd::SeqCst)?; } @@ -492,47 +484,55 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { atomic: AtomicReadOrd, latest_in_mo: Scalar, validate: impl FnOnce() -> InterpResult<'tcx>, - ) -> InterpResult<'tcx, Scalar> { + ) -> InterpResult<'tcx, Option> { let this = self.eval_context_ref(); - if let Some(global) = &this.machine.data_race { - let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr(), 0)?; - if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() { - if atomic == AtomicReadOrd::SeqCst { - global.sc_read(&this.machine.threads); - } - let mut rng = this.machine.rng.borrow_mut(); - let buffer = alloc_buffers.get_or_create_store_buffer( - alloc_range(base_offset, place.layout.size), - latest_in_mo, - )?; - let (loaded, recency) = buffer.buffered_read( - global, - &this.machine.threads, - atomic == AtomicReadOrd::SeqCst, - &mut *rng, - validate, - )?; - if global.track_outdated_loads && recency == LoadRecency::Outdated { - this.emit_diagnostic(NonHaltingDiagnostic::WeakMemoryOutdatedLoad { - ptr: place.ptr(), - }); + 'fallback: { + if let Some(global) = &this.machine.data_race { + let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr(), 0)?; + if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() { + if atomic == AtomicReadOrd::SeqCst { + global.sc_read(&this.machine.threads); + } + let mut rng = this.machine.rng.borrow_mut(); + let Some(buffer) = alloc_buffers + .get_store_buffer(alloc_range(base_offset, place.layout.size))? + else { + // No old writes available, fall back to base case. + break 'fallback; + }; + let (loaded, recency) = buffer.buffered_read( + global, + &this.machine.threads, + atomic == AtomicReadOrd::SeqCst, + &mut *rng, + validate, + )?; + if global.track_outdated_loads && recency == LoadRecency::Outdated { + this.emit_diagnostic(NonHaltingDiagnostic::WeakMemoryOutdatedLoad { + ptr: place.ptr(), + }); + } + + return Ok(loaded); } - - return Ok(loaded); } } // Race detector or weak memory disabled, simply read the latest value validate()?; - Ok(latest_in_mo) + Ok(Some(latest_in_mo)) } + /// Add the given write to the store buffer. (Does not change machine memory.) + /// + /// `init` says with which value to initialize the store buffer in case there wasn't a store + /// buffer for this memory range before. fn buffered_atomic_write( &mut self, val: Scalar, dest: &MPlaceTy<'tcx>, atomic: AtomicWriteOrd, - init: Scalar, + init: Option, ) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(dest.ptr(), 0)?; @@ -545,23 +545,8 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { global.sc_write(threads); } - // UGLY HACK: in write_scalar_atomic() we don't know the value before our write, - // so init == val always. If the buffer is fresh then we would've duplicated an entry, - // so we need to remove it. - // See https://github.com/rust-lang/miri/issues/2164 - let was_empty = matches!( - alloc_buffers - .store_buffers - .borrow() - .access_type(alloc_range(base_offset, dest.layout.size)), - AccessType::Empty(_) - ); let buffer = alloc_buffers .get_or_create_store_buffer_mut(alloc_range(base_offset, dest.layout.size), init)?; - if was_empty { - buffer.buffer.pop_front(); - } - buffer.buffered_write(val, global, threads, atomic == AtomicWriteOrd::SeqCst)?; } @@ -576,7 +561,6 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { &self, place: &MPlaceTy<'tcx>, atomic: AtomicReadOrd, - init: Scalar, ) -> InterpResult<'tcx> { let this = self.eval_context_ref(); @@ -587,8 +571,12 @@ pub(super) trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> { let size = place.layout.size; let (alloc_id, base_offset, ..) = this.ptr_get_alloc_id(place.ptr(), 0)?; if let Some(alloc_buffers) = this.get_alloc_extra(alloc_id)?.weak_memory.as_ref() { - let buffer = alloc_buffers - .get_or_create_store_buffer(alloc_range(base_offset, size), init)?; + let Some(buffer) = + alloc_buffers.get_store_buffer(alloc_range(base_offset, size))? + else { + // No store buffer, nothing to do. + return Ok(()); + }; buffer.read_from_last_store( global, &this.machine.threads, diff --git a/src/tools/miri/tests/fail/weak_memory/weak_uninit.rs b/src/tools/miri/tests/fail/weak_memory/weak_uninit.rs new file mode 100644 index 0000000000000..54bea6c6908e3 --- /dev/null +++ b/src/tools/miri/tests/fail/weak_memory/weak_uninit.rs @@ -0,0 +1,43 @@ +//@compile-flags: -Zmiri-ignore-leaks -Zmiri-preemption-rate=0 + +// Tests showing weak memory behaviours are exhibited. All tests +// return true when the desired behaviour is seen. +// This is scheduler and pseudo-RNG dependent, so each test is +// run multiple times until one try returns true. +// Spurious failure is possible, if you are really unlucky with +// the RNG and always read the latest value from the store buffer. +#![feature(new_uninit)] + +use std::sync::atomic::*; +use std::thread::spawn; + +#[allow(dead_code)] +#[derive(Copy, Clone)] +struct EvilSend(pub T); + +unsafe impl Send for EvilSend {} +unsafe impl Sync for EvilSend {} + +// We can't create static items because we need to run each test multiple times. +fn static_uninit_atomic() -> &'static AtomicUsize { + unsafe { Box::leak(Box::new_uninit()).assume_init_ref() } +} + +fn relaxed() { + let x = static_uninit_atomic(); + let j1 = spawn(move || { + x.store(1, Ordering::Relaxed); + }); + + let j2 = spawn(move || x.load(Ordering::Relaxed)); //~ERROR: using uninitialized data + + j1.join().unwrap(); + j2.join().unwrap(); +} + +pub fn main() { + // If we try often enough, we should hit UB. + for _ in 0..100 { + relaxed(); + } +} diff --git a/src/tools/miri/tests/fail/weak_memory/weak_uninit.stderr b/src/tools/miri/tests/fail/weak_memory/weak_uninit.stderr new file mode 100644 index 0000000000000..9aa5bc2fa76df --- /dev/null +++ b/src/tools/miri/tests/fail/weak_memory/weak_uninit.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory + --> $DIR/weak_uninit.rs:LL:CC + | +LL | let j2 = spawn(move || x.load(Ordering::Relaxed)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE on thread `unnamed-ID`: + = note: inside closure at $DIR/weak_uninit.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/src/tools/miri/tests/pass/weak_memory/weak.rs b/src/tools/miri/tests/pass/weak_memory/weak.rs index dac63eeeb0b24..1b5c98cd51826 100644 --- a/src/tools/miri/tests/pass/weak_memory/weak.rs +++ b/src/tools/miri/tests/pass/weak_memory/weak.rs @@ -18,11 +18,9 @@ struct EvilSend(pub T); unsafe impl Send for EvilSend {} unsafe impl Sync for EvilSend {} -// We can't create static items because we need to run each test -// multiple times +// We can't create static items because we need to run each test multiple times. fn static_atomic(val: usize) -> &'static AtomicUsize { - let ret = Box::leak(Box::new(AtomicUsize::new(val))); - ret + Box::leak(Box::new(AtomicUsize::new(val))) } // Spins until it reads the given value @@ -33,7 +31,7 @@ fn reads_value(loc: &AtomicUsize, val: usize) -> usize { val } -fn relaxed() -> bool { +fn relaxed(initial_read: bool) -> bool { let x = static_atomic(0); let j1 = spawn(move || { x.store(1, Relaxed); @@ -47,7 +45,9 @@ fn relaxed() -> bool { j1.join().unwrap(); let r2 = j2.join().unwrap(); - r2 == 1 + // There are three possible values here: 0 (from the initial read), 1 (from the first relaxed + // read), and 2 (the last read). The last case is boring and we cover the other two. + r2 == if initial_read { 0 } else { 1 } } // https://www.doc.ic.ac.uk/~afd/homepages/papers/pdfs/2017/POPL.pdf Figure 8 @@ -74,7 +74,6 @@ fn seq_cst() -> bool { fn initialization_write(add_fence: bool) -> bool { let x = static_atomic(11); - assert_eq!(x.load(Relaxed), 11); // work around https://github.com/rust-lang/miri/issues/2164 let wait = static_atomic(0); @@ -112,11 +111,8 @@ fn faa_replaced_by_load() -> bool { } let x = static_atomic(0); - assert_eq!(x.load(Relaxed), 0); // work around https://github.com/rust-lang/miri/issues/2164 let y = static_atomic(0); - assert_eq!(y.load(Relaxed), 0); // work around https://github.com/rust-lang/miri/issues/2164 let z = static_atomic(0); - assert_eq!(z.load(Relaxed), 0); // work around https://github.com/rust-lang/miri/issues/2164 // Since each thread is so short, we need to make sure that they truely run at the same time // Otherwise t1 will finish before t2 even starts @@ -146,7 +142,8 @@ fn assert_once(f: fn() -> bool) { } pub fn main() { - assert_once(relaxed); + assert_once(|| relaxed(false)); + assert_once(|| relaxed(true)); assert_once(seq_cst); assert_once(|| initialization_write(false)); assert_once(|| initialization_write(true)); From 194baa820d36926cfa9128211bbd61866d49d501 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Aug 2024 12:25:37 +0200 Subject: [PATCH 05/23] simd_shuffle intrinsic: allow argument to be passed as vector (not just as array) --- .../src/intrinsics/simd.rs | 10 +++++++ .../rustc_codegen_gcc/src/intrinsic/simd.rs | 19 ++++++++----- compiler/rustc_codegen_llvm/src/intrinsic.rs | 19 ++++++++----- library/core/src/intrinsics/simd.rs | 2 +- tests/ui/simd/shuffle.rs | 27 +++++++++++++++++-- 5 files changed, 60 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs b/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs index ca910dccb0d06..604a88393fd95 100644 --- a/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs +++ b/compiler/rustc_codegen_cranelift/src/intrinsics/simd.rs @@ -191,6 +191,14 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>( }) .try_into() .unwrap(), + _ if idx_ty.is_simd() + && matches!( + idx_ty.simd_size_and_type(fx.tcx).1.kind(), + ty::Uint(ty::UintTy::U32) + ) => + { + idx_ty.simd_size_and_type(fx.tcx).0.try_into().unwrap() + } _ => { fx.tcx.dcx().span_err( span, @@ -213,6 +221,8 @@ pub(super) fn codegen_simd_intrinsic_call<'tcx>( let total_len = lane_count * 2; + // FIXME: this is a terrible abstraction-breaking hack. + // Find a way to reuse `immediate_const_vector` from `codegen_ssa` instead. let indexes = { use rustc_middle::mir::interpret::*; let idx_const = match &idx.node { diff --git a/compiler/rustc_codegen_gcc/src/intrinsic/simd.rs b/compiler/rustc_codegen_gcc/src/intrinsic/simd.rs index 8da1df3be1534..96a833ccaf2b6 100644 --- a/compiler/rustc_codegen_gcc/src/intrinsic/simd.rs +++ b/compiler/rustc_codegen_gcc/src/intrinsic/simd.rs @@ -353,19 +353,24 @@ pub fn generic_simd_intrinsic<'a, 'gcc, 'tcx>( } if name == sym::simd_shuffle { - // Make sure this is actually an array, since typeck only checks the length-suffixed + // Make sure this is actually an array or SIMD vector, since typeck only checks the length-suffixed // version of this intrinsic. - let n: u64 = match *args[2].layout.ty.kind() { + let idx_ty = args[2].layout.ty; + let n: u64 = match idx_ty.kind() { ty::Array(ty, len) if matches!(*ty.kind(), ty::Uint(ty::UintTy::U32)) => { len.try_eval_target_usize(bx.cx.tcx, ty::ParamEnv::reveal_all()).unwrap_or_else( || span_bug!(span, "could not evaluate shuffle index array length"), ) } - _ => return_error!(InvalidMonomorphization::SimdShuffle { - span, - name, - ty: args[2].layout.ty - }), + _ if idx_ty.is_simd() + && matches!( + idx_ty.simd_size_and_type(bx.cx.tcx).1.kind(), + ty::Uint(ty::UintTy::U32) + ) => + { + idx_ty.simd_size_and_type(bx.cx.tcx).0 + } + _ => return_error!(InvalidMonomorphization::SimdShuffle { span, name, ty: idx_ty }), }; require_simd!(ret_ty, InvalidMonomorphization::SimdReturn { span, name, ty: ret_ty }); diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index f5558723d11bf..5d32ef0d9d65f 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -1279,19 +1279,24 @@ fn generic_simd_intrinsic<'ll, 'tcx>( } if name == sym::simd_shuffle { - // Make sure this is actually an array, since typeck only checks the length-suffixed + // Make sure this is actually an array or SIMD vector, since typeck only checks the length-suffixed // version of this intrinsic. - let n: u64 = match args[2].layout.ty.kind() { + let idx_ty = args[2].layout.ty; + let n: u64 = match idx_ty.kind() { ty::Array(ty, len) if matches!(ty.kind(), ty::Uint(ty::UintTy::U32)) => { len.try_eval_target_usize(bx.cx.tcx, ty::ParamEnv::reveal_all()).unwrap_or_else( || span_bug!(span, "could not evaluate shuffle index array length"), ) } - _ => return_error!(InvalidMonomorphization::SimdShuffle { - span, - name, - ty: args[2].layout.ty - }), + _ if idx_ty.is_simd() + && matches!( + idx_ty.simd_size_and_type(bx.cx.tcx).1.kind(), + ty::Uint(ty::UintTy::U32) + ) => + { + idx_ty.simd_size_and_type(bx.cx.tcx).0 + } + _ => return_error!(InvalidMonomorphization::SimdShuffle { span, name, ty: idx_ty }), }; let (out_len, out_ty) = require_simd!(ret_ty, SimdReturn); diff --git a/library/core/src/intrinsics/simd.rs b/library/core/src/intrinsics/simd.rs index 221724d7b4ae9..5982819809937 100644 --- a/library/core/src/intrinsics/simd.rs +++ b/library/core/src/intrinsics/simd.rs @@ -232,7 +232,7 @@ extern "rust-intrinsic" { /// /// `T` must be a vector. /// - /// `U` must be a **const** array of `i32`s. This means it must either refer to a named + /// `U` must be a **const** array or vector of `u32`s. This means it must either refer to a named /// const or be given as an inline const expression (`const { ... }`). /// /// `V` must be a vector with the same element type as `T` and the same length as `U`. diff --git a/tests/ui/simd/shuffle.rs b/tests/ui/simd/shuffle.rs index 09926d95557cd..dc0d688284e3c 100644 --- a/tests/ui/simd/shuffle.rs +++ b/tests/ui/simd/shuffle.rs @@ -6,15 +6,20 @@ #![allow(incomplete_features)] #![feature(adt_const_params)] +use std::marker::ConstParamTy; + extern "rust-intrinsic" { fn simd_shuffle(a: T, b: T, i: I) -> U; } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, ConstParamTy, PartialEq, Eq)] #[repr(simd)] struct Simd([T; N]); -pub unsafe fn __shuffle_vector16(x: T, y: T) -> U { +unsafe fn __shuffle_vector16(x: T, y: T) -> U { + simd_shuffle(x, y, IDX) +} +unsafe fn __shuffle_vector16_v2, T, U>(x: T, y: T) -> U { simd_shuffle(x, y, IDX) } @@ -30,6 +35,17 @@ fn main() { let y: Simd = simd_shuffle(a, b, I2); assert_eq!(y.0, [1, 5]); } + // Test that we can also use a SIMD vector instead of a normal array for the shuffle. + const I1_SIMD: Simd = Simd([0, 2, 4, 6]); + const I2_SIMD: Simd = Simd([1, 5]); + unsafe { + let x: Simd = simd_shuffle(a, b, I1_SIMD); + assert_eq!(x.0, [0, 2, 4, 6]); + + let y: Simd = simd_shuffle(a, b, I2_SIMD); + assert_eq!(y.0, [1, 5]); + } + // Test that an indirection (via an unnamed constant) // through a const generic parameter also works. // See https://github.com/rust-lang/rust/issues/113500 for details. @@ -42,4 +58,11 @@ fn main() { Simd, >(a, b); } + unsafe { + __shuffle_vector16_v2::< + { Simd([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]) }, + Simd, + Simd, + >(a, b); + } } From daedbd4d7abb9132638cb420acc549d198c46c48 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 6 Aug 2024 17:08:50 +0200 Subject: [PATCH 06/23] make the GCC backend compatible with vector shuffle indices --- compiler/rustc_codegen_gcc/src/builder.rs | 44 +++++++++++++++-------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_codegen_gcc/src/builder.rs b/compiler/rustc_codegen_gcc/src/builder.rs index 47b378cc1cd82..6ba678e2e7c65 100644 --- a/compiler/rustc_codegen_gcc/src/builder.rs +++ b/compiler/rustc_codegen_gcc/src/builder.rs @@ -1923,15 +1923,11 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> { v2: RValue<'gcc>, mask: RValue<'gcc>, ) -> RValue<'gcc> { - let struct_type = mask.get_type().is_struct().expect("mask should be of struct type"); - // TODO(antoyo): use a recursive unqualified() here. let vector_type = v1.get_type().unqualified().dyncast_vector().expect("vector type"); let element_type = vector_type.get_element_type(); let vec_num_units = vector_type.get_num_units(); - let mask_num_units = struct_type.get_field_count(); - let mut vector_elements = vec![]; let mask_element_type = if element_type.is_integral() { element_type } else { @@ -1942,19 +1938,39 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> { #[cfg(not(feature = "master"))] self.int_type }; - for i in 0..mask_num_units { - let field = struct_type.get_field(i as i32); - vector_elements.push(self.context.new_cast( - self.location, - mask.access_field(self.location, field).to_rvalue(), - mask_element_type, - )); - } + + let mut mask_elements = if let Some(vector_type) = mask.get_type().dyncast_vector() { + let mask_num_units = vector_type.get_num_units(); + let mut mask_elements = vec![]; + for i in 0..mask_num_units { + let index = self.context.new_rvalue_from_long(self.cx.type_u32(), i as _); + mask_elements.push(self.context.new_cast( + self.location, + self.extract_element(mask, index).to_rvalue(), + mask_element_type, + )); + } + mask_elements + } else { + let struct_type = mask.get_type().is_struct().expect("mask should be of struct type"); + let mask_num_units = struct_type.get_field_count(); + let mut mask_elements = vec![]; + for i in 0..mask_num_units { + let field = struct_type.get_field(i as i32); + mask_elements.push(self.context.new_cast( + self.location, + mask.access_field(self.location, field).to_rvalue(), + mask_element_type, + )); + } + mask_elements + }; + let mask_num_units = mask_elements.len(); // NOTE: the mask needs to be the same length as the input vectors, so add the missing // elements in the mask if needed. for _ in mask_num_units..vec_num_units { - vector_elements.push(self.context.new_rvalue_zero(mask_element_type)); + mask_elements.push(self.context.new_rvalue_zero(mask_element_type)); } let result_type = self.context.new_vector_type(element_type, mask_num_units as u64); @@ -1998,7 +2014,7 @@ impl<'a, 'gcc, 'tcx> Builder<'a, 'gcc, 'tcx> { let new_mask_num_units = std::cmp::max(mask_num_units, vec_num_units); let mask_type = self.context.new_vector_type(mask_element_type, new_mask_num_units as u64); - let mask = self.context.new_rvalue_from_vector(self.location, mask_type, &vector_elements); + let mask = self.context.new_rvalue_from_vector(self.location, mask_type, &mask_elements); let result = self.context.new_rvalue_vector_perm(self.location, v1, v2, mask); if vec_num_units != mask_num_units { From f62b9e0179434218040f42ee58bb99c1bf27c6d7 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 30 Jul 2024 20:27:17 +0300 Subject: [PATCH 07/23] rustc: Simplify getting sysroot library directory --- compiler/rustc_codegen_ssa/src/back/link.rs | 27 +++++++++------------ compiler/rustc_session/src/filesearch.rs | 16 +----------- compiler/rustc_session/src/session.rs | 16 ++---------- 3 files changed, 14 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 7bad9d33e7d31..ca439460adc11 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -1317,11 +1317,9 @@ fn link_sanitizer_runtime( name: &str, ) { fn find_sanitizer_runtime(sess: &Session, filename: &str) -> PathBuf { - let session_tlib = - filesearch::make_target_lib_path(&sess.sysroot, sess.opts.target_triple.triple()); - let path = session_tlib.join(filename); + let path = sess.target_tlib_path.dir.join(filename); if path.exists() { - return session_tlib; + return sess.target_tlib_path.dir.clone(); } else { let default_sysroot = filesearch::get_or_default_sysroot().expect("Failed finding sysroot"); @@ -1612,19 +1610,18 @@ fn print_native_static_libs( } fn get_object_file_path(sess: &Session, name: &str, self_contained: bool) -> PathBuf { - let fs = sess.target_filesearch(PathKind::Native); - let file_path = fs.get_lib_path().join(name); + let file_path = sess.target_tlib_path.dir.join(name); if file_path.exists() { return file_path; } // Special directory with objects used only in self-contained linkage mode if self_contained { - let file_path = fs.get_self_contained_lib_path().join(name); + let file_path = sess.target_tlib_path.dir.join("self-contained").join(name); if file_path.exists() { return file_path; } } - for search_path in fs.search_paths() { + for search_path in sess.target_filesearch(PathKind::Native).search_paths() { let file_path = search_path.dir.join(name); if file_path.exists() { return file_path; @@ -2131,7 +2128,7 @@ fn add_library_search_dirs( | LinkSelfContainedComponents::UNWIND | LinkSelfContainedComponents::MINGW, ) { - let lib_path = sess.target_filesearch(PathKind::Native).get_self_contained_lib_path(); + let lib_path = sess.target_tlib_path.dir.join("self-contained"); cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); } @@ -2146,8 +2143,7 @@ fn add_library_search_dirs( || sess.target.os == "fuchsia" || sess.target.is_like_osx && !sess.opts.unstable_opts.sanitizer.is_empty() { - let lib_path = sess.target_filesearch(PathKind::Native).get_lib_path(); - cmd.include_path(&fix_windows_verbatim_for_gcc(&lib_path)); + cmd.include_path(&fix_windows_verbatim_for_gcc(&sess.target_tlib_path.dir)); } // Mac Catalyst uses the macOS SDK, but to link to iOS-specific frameworks @@ -2859,15 +2855,14 @@ fn add_upstream_native_libraries( // // The returned path will always have `fix_windows_verbatim_for_gcc()` applied to it. fn rehome_sysroot_lib_dir(sess: &Session, lib_dir: &Path) -> PathBuf { - let sysroot_lib_path = sess.target_filesearch(PathKind::All).get_lib_path(); + let sysroot_lib_path = &sess.target_tlib_path.dir; let canonical_sysroot_lib_path = - { try_canonicalize(&sysroot_lib_path).unwrap_or_else(|_| sysroot_lib_path.clone()) }; + { try_canonicalize(sysroot_lib_path).unwrap_or_else(|_| sysroot_lib_path.clone()) }; let canonical_lib_dir = try_canonicalize(lib_dir).unwrap_or_else(|_| lib_dir.to_path_buf()); if canonical_lib_dir == canonical_sysroot_lib_path { - // This path, returned by `target_filesearch().get_lib_path()`, has - // already had `fix_windows_verbatim_for_gcc()` applied if needed. - sysroot_lib_path + // This path already had `fix_windows_verbatim_for_gcc()` applied if needed. + sysroot_lib_path.clone() } else { fix_windows_verbatim_for_gcc(lib_dir) } diff --git a/compiler/rustc_session/src/filesearch.rs b/compiler/rustc_session/src/filesearch.rs index 63ca5fefd9faf..d78f4a78de732 100644 --- a/compiler/rustc_session/src/filesearch.rs +++ b/compiler/rustc_session/src/filesearch.rs @@ -5,14 +5,11 @@ use std::{env, fs}; use rustc_fs_util::{fix_windows_verbatim_for_gcc, try_canonicalize}; use smallvec::{smallvec, SmallVec}; -use tracing::debug; use crate::search_paths::{PathKind, SearchPath}; #[derive(Clone)] pub struct FileSearch<'a> { - sysroot: &'a Path, - triple: &'a str, cli_search_paths: &'a [SearchPath], tlib_path: &'a SearchPath, kind: PathKind, @@ -32,23 +29,12 @@ impl<'a> FileSearch<'a> { .chain(std::iter::once(self.tlib_path)) } - pub fn get_lib_path(&self) -> PathBuf { - make_target_lib_path(self.sysroot, self.triple) - } - - pub fn get_self_contained_lib_path(&self) -> PathBuf { - self.get_lib_path().join("self-contained") - } - pub fn new( - sysroot: &'a Path, - triple: &'a str, cli_search_paths: &'a [SearchPath], tlib_path: &'a SearchPath, kind: PathKind, ) -> FileSearch<'a> { - debug!("using sysroot = {}, triple = {}", sysroot.display(), triple); - FileSearch { sysroot, triple, cli_search_paths, tlib_path, kind } + FileSearch { cli_search_paths, tlib_path, kind } } } diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 693867c3853da..b073950a750b4 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -439,22 +439,10 @@ impl Session { } pub fn target_filesearch(&self, kind: PathKind) -> filesearch::FileSearch<'_> { - filesearch::FileSearch::new( - &self.sysroot, - self.opts.target_triple.triple(), - &self.opts.search_paths, - &self.target_tlib_path, - kind, - ) + filesearch::FileSearch::new(&self.opts.search_paths, &self.target_tlib_path, kind) } pub fn host_filesearch(&self, kind: PathKind) -> filesearch::FileSearch<'_> { - filesearch::FileSearch::new( - &self.sysroot, - config::host_triple(), - &self.opts.search_paths, - &self.host_tlib_path, - kind, - ) + filesearch::FileSearch::new(&self.opts.search_paths, &self.host_tlib_path, kind) } /// Returns a list of directories where target-specific tool binaries are located. Some fallback From 681a866067610390f76e493e10f805d84a5ff459 Mon Sep 17 00:00:00 2001 From: Nicole LeGare Date: Fri, 23 Aug 2024 16:09:56 -0700 Subject: [PATCH 08/23] Add Trusty OS as tier 3 target --- compiler/rustc_target/src/spec/mod.rs | 3 ++ .../spec/targets/aarch64_unknown_trusty.rs | 34 ++++++++++++ .../src/spec/targets/armv7_unknown_trusty.rs | 37 +++++++++++++ src/doc/rustc/src/SUMMARY.md | 1 + src/doc/rustc/src/platform-support.md | 2 + src/doc/rustc/src/platform-support/trusty.md | 53 +++++++++++++++++++ tests/assembly/targets/targets-elf.rs | 6 +++ tests/ui/check-cfg/well-known-values.stderr | 4 +- 8 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 compiler/rustc_target/src/spec/targets/aarch64_unknown_trusty.rs create mode 100644 compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs create mode 100644 src/doc/rustc/src/platform-support/trusty.md diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 80f89a0ab2be0..d5f227a84a4ca 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -1750,6 +1750,9 @@ supported_targets! { ("x86_64-unikraft-linux-musl", x86_64_unikraft_linux_musl), + ("armv7-unknown-trusty", armv7_unknown_trusty), + ("aarch64-unknown-trusty", aarch64_unknown_trusty), + ("riscv32i-unknown-none-elf", riscv32i_unknown_none_elf), ("riscv32im-risc0-zkvm-elf", riscv32im_risc0_zkvm_elf), ("riscv32im-unknown-none-elf", riscv32im_unknown_none_elf), diff --git a/compiler/rustc_target/src/spec/targets/aarch64_unknown_trusty.rs b/compiler/rustc_target/src/spec/targets/aarch64_unknown_trusty.rs new file mode 100644 index 0000000000000..1525faf9b7e1d --- /dev/null +++ b/compiler/rustc_target/src/spec/targets/aarch64_unknown_trusty.rs @@ -0,0 +1,34 @@ +// Trusty OS target for AArch64. + +use crate::spec::{LinkSelfContainedDefault, PanicStrategy, RelroLevel, Target, TargetOptions}; + +pub fn target() -> Target { + Target { + llvm_target: "aarch64-unknown-unknown-musl".into(), + metadata: crate::spec::TargetMetadata { + description: Some("ARM64 Trusty".into()), + tier: Some(2), + host_tools: Some(false), + std: Some(false), + }, + pointer_width: 64, + data_layout: "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128-Fn32".into(), + arch: "aarch64".into(), + options: TargetOptions { + features: "+neon,+fp-armv8,+reserve-x18".into(), + executables: true, + max_atomic_width: Some(128), + panic_strategy: PanicStrategy::Abort, + os: "trusty".into(), + position_independent_executables: true, + static_position_independent_executables: true, + crt_static_default: true, + crt_static_respected: true, + dynamic_linking: false, + link_self_contained: LinkSelfContainedDefault::InferredForMusl, + relro_level: RelroLevel::Full, + mcount: "\u{1}_mcount".into(), + ..Default::default() + }, + } +} diff --git a/compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs b/compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs new file mode 100644 index 0000000000000..11784788651ee --- /dev/null +++ b/compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs @@ -0,0 +1,37 @@ +use crate::spec::{LinkSelfContainedDefault, PanicStrategy, RelroLevel, Target, TargetOptions}; + +pub fn target() -> Target { + Target { + // It's important we use "gnueabi" and not "musleabi" here. LLVM uses it + // to determine the calling convention and float ABI, and it doesn't + // support the "musleabi" value. + llvm_target: "armv7-unknown-unknown-gnueabi".into(), + metadata: crate::spec::TargetMetadata { + description: Some("Armv7-A Trusty".into()), + tier: Some(2), + host_tools: Some(false), + std: Some(false), + }, + pointer_width: 32, + data_layout: "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64".into(), + arch: "arm".into(), + options: TargetOptions { + abi: "eabi".into(), + features: "+v7,+thumb2,+soft-float,-neon,+reserve-x18".into(), + max_atomic_width: Some(64), + mcount: "\u{1}mcount".into(), + os: "trusty".into(), + link_self_contained: LinkSelfContainedDefault::InferredForMusl, + dynamic_linking: false, + executables: true, + crt_static_default: true, + crt_static_respected: true, + relro_level: RelroLevel::Full, + panic_strategy: PanicStrategy::Abort, + position_independent_executables: true, + static_position_independent_executables: true, + + ..Default::default() + }, + } +} diff --git a/src/doc/rustc/src/SUMMARY.md b/src/doc/rustc/src/SUMMARY.md index 4e34e2667cfc0..b3a74a7716bb3 100644 --- a/src/doc/rustc/src/SUMMARY.md +++ b/src/doc/rustc/src/SUMMARY.md @@ -49,6 +49,7 @@ - [aarch64-unknown-teeos](platform-support/aarch64-unknown-teeos.md) - [\*-espidf](platform-support/esp-idf.md) - [\*-unknown-fuchsia](platform-support/fuchsia.md) + - [\*-unknown-trusty](platform-support/trusty.md) - [\*-kmc-solid_\*](platform-support/kmc-solid.md) - [csky-unknown-linux-gnuabiv2\*](platform-support/csky-unknown-linux-gnuabiv2.md) - [hexagon-unknown-linux-musl](platform-support/hexagon-unknown-linux-musl.md) diff --git a/src/doc/rustc/src/platform-support.md b/src/doc/rustc/src/platform-support.md index dd478e9df379e..0d79e5f4114e3 100644 --- a/src/doc/rustc/src/platform-support.md +++ b/src/doc/rustc/src/platform-support.md @@ -264,6 +264,7 @@ target | std | host | notes [`aarch64-unknown-netbsd`](platform-support/netbsd.md) | ✓ | ✓ | ARM64 NetBSD [`aarch64-unknown-openbsd`](platform-support/openbsd.md) | ✓ | ✓ | ARM64 OpenBSD [`aarch64-unknown-redox`](platform-support/redox.md) | ✓ | | ARM64 Redox OS +[`aarch64-unknown-trusty`](platform-support/trusty.md) | ? | | `aarch64-uwp-windows-msvc` | ✓ | | [`aarch64-wrs-vxworks`](platform-support/vxworks.md) | ✓ | | ARM64 VxWorks OS `aarch64_be-unknown-linux-gnu_ilp32` | ✓ | ✓ | ARM64 Linux (big-endian, ILP32 ABI) @@ -283,6 +284,7 @@ target | std | host | notes [`armv7-unknown-linux-uclibceabihf`](platform-support/armv7-unknown-linux-uclibceabihf.md) | ✓ | ? | Armv7-A Linux with uClibc, hardfloat `armv7-unknown-freebsd` | ✓ | ✓ | Armv7-A FreeBSD [`armv7-unknown-netbsd-eabihf`](platform-support/netbsd.md) | ✓ | ✓ | Armv7-A NetBSD w/hard-float +[`armv7-unknown-trusty`](platform-support/trusty.md) | ? | | [`armv7-wrs-vxworks-eabihf`](platform-support/vxworks.md) | ✓ | | Armv7-A for VxWorks [`armv7a-kmc-solid_asp3-eabi`](platform-support/kmc-solid.md) | ✓ | | ARM SOLID with TOPPERS/ASP3 [`armv7a-kmc-solid_asp3-eabihf`](platform-support/kmc-solid.md) | ✓ | | ARM SOLID with TOPPERS/ASP3, hardfloat diff --git a/src/doc/rustc/src/platform-support/trusty.md b/src/doc/rustc/src/platform-support/trusty.md new file mode 100644 index 0000000000000..97a03d3de04c2 --- /dev/null +++ b/src/doc/rustc/src/platform-support/trusty.md @@ -0,0 +1,53 @@ +# `aarch64-unknown-trusty` and `armv7-unknown-trusty` + +**Tier: 3** + +[Trusty] is a secure Operating System that provides a Trusted Execution +Environment (TEE) for Android. + +## Target maintainers + +- Nicole LeGare (@randomPoison) +- Stephen Crane (@rinon) +- As a fallback trusty-dev-team@google.com can be contacted + +## Requirements + +This target is cross-compiled. It has no special requirements for the host. + +It fully supports alloc with the default allocator, and partially supports std. +Notably, most I/O functionality is not supported, e.g. filesystem support and +networking support are not present and any APIs that rely on them will panic at +runtime. + +Trusty uses the ELF file format. + +## Building the target + +The targets can be built by enabling them for a `rustc` build, for example: + +```toml +[build] +build-stage = 1 +target = ["aarch64-unknown-trusty", "armv7-unknown-trusty"] +``` + +## Building Rust programs + +There is currently no supported way to build a Trusty app with Cargo. You can +follow the [Trusty build instructions] to build the Trusty kernel along with any +Rust apps that are setup in the project. + +## Testing + +See the [Trusty build instructions] for information on how to build Rust code +within the main Trusty project. The main project also includes infrastructure +for testing Rust applications within a QEMU emulator. + +## Cross-compilation toolchains and C code + +See the [Trusty build instructions] for information on how C code is built +within Trusty. + +[Trusty]: https://source.android.com/docs/security/features/trusty +[Trusty build instructions]: https://source.android.com/docs/security/features/trusty/download-and-build diff --git a/tests/assembly/targets/targets-elf.rs b/tests/assembly/targets/targets-elf.rs index c3a083321e229..6f0a200a08cf3 100644 --- a/tests/assembly/targets/targets-elf.rs +++ b/tests/assembly/targets/targets-elf.rs @@ -66,6 +66,9 @@ //@ revisions: aarch64_unknown_teeos //@ [aarch64_unknown_teeos] compile-flags: --target aarch64-unknown-teeos //@ [aarch64_unknown_teeos] needs-llvm-components: aarch64 +//@ revisions: aarch64_unknown_trusty +//@ [aarch64_unknown_trusty] compile-flags: --target aarch64-unknown-trusty +//@ [aarch64_unknown_trusty] needs-llvm-components: aarch64 //@ revisions: aarch64_wrs_vxworks //@ [aarch64_wrs_vxworks] compile-flags: --target aarch64-wrs-vxworks //@ [aarch64_wrs_vxworks] needs-llvm-components: aarch64 @@ -153,6 +156,9 @@ //@ revisions: armv7_unknown_netbsd_eabihf //@ [armv7_unknown_netbsd_eabihf] compile-flags: --target armv7-unknown-netbsd-eabihf //@ [armv7_unknown_netbsd_eabihf] needs-llvm-components: arm +//@ revisions: armv7_unknown_trusty +//@ [armv7_unknown_trusty] compile-flags: --target armv7-unknown-trusty +//@ [armv7_unknown_trusty] needs-llvm-components: arm //@ revisions: armv7_wrs_vxworks_eabihf //@ [armv7_wrs_vxworks_eabihf] compile-flags: --target armv7-wrs-vxworks-eabihf //@ [armv7_wrs_vxworks_eabihf] needs-llvm-components: arm diff --git a/tests/ui/check-cfg/well-known-values.stderr b/tests/ui/check-cfg/well-known-values.stderr index d780e04e729a2..103a7564a0f87 100644 --- a/tests/ui/check-cfg/well-known-values.stderr +++ b/tests/ui/check-cfg/well-known-values.stderr @@ -201,7 +201,7 @@ warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` LL | target_os = "_UNEXPECTED_VALUE", | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: expected values for `target_os` are: `aix`, `android`, `cuda`, `dragonfly`, `emscripten`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `macos`, `netbsd`, `none`, `nto`, `nuttx`, `openbsd`, `psp`, `redox`, `solaris`, `solid_asp3`, `teeos`, `tvos`, `uefi`, `unknown`, `visionos`, `vita`, `vxworks`, `wasi`, `watchos`, `windows`, `xous`, and `zkvm` + = note: expected values for `target_os` are: `aix`, `android`, `cuda`, `dragonfly`, `emscripten`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `macos`, `netbsd`, `none`, `nto`, `nuttx`, `openbsd`, `psp`, `redox`, `solaris`, `solid_asp3`, `teeos`, `trusty`, `tvos`, `uefi`, `unknown`, `visionos`, `vita`, `vxworks`, `wasi`, `watchos`, `windows`, `xous`, and `zkvm` = note: see for more information about checking conditional configuration warning: unexpected `cfg` condition value: `_UNEXPECTED_VALUE` @@ -285,7 +285,7 @@ LL | #[cfg(target_os = "linuz")] // testing that we suggest `linux` | | | help: there is a expected value with a similar name: `"linux"` | - = note: expected values for `target_os` are: `aix`, `android`, `cuda`, `dragonfly`, `emscripten`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `macos`, `netbsd`, `none`, `nto`, `nuttx`, `openbsd`, `psp`, `redox`, `solaris`, `solid_asp3`, `teeos`, `tvos`, `uefi`, `unknown`, `visionos`, `vita`, `vxworks`, `wasi`, `watchos`, `windows`, `xous`, and `zkvm` + = note: expected values for `target_os` are: `aix`, `android`, `cuda`, `dragonfly`, `emscripten`, `espidf`, `freebsd`, `fuchsia`, `haiku`, `hermit`, `horizon`, `hurd`, `illumos`, `ios`, `l4re`, `linux`, `macos`, `netbsd`, `none`, `nto`, `nuttx`, `openbsd`, `psp`, `redox`, `solaris`, `solid_asp3`, `teeos`, `trusty`, `tvos`, `uefi`, `unknown`, `visionos`, `vita`, `vxworks`, `wasi`, `watchos`, `windows`, `xous`, and `zkvm` = note: see for more information about checking conditional configuration warning: 29 warnings emitted From 1ed9ee7b035553dc8e719e959e1f3d03d5678a00 Mon Sep 17 00:00:00 2001 From: Nicole LeGare Date: Fri, 23 Aug 2024 16:26:20 -0700 Subject: [PATCH 09/23] Remove `reserve-x18` from armv7-unknown-trusty --- compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs b/compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs index 11784788651ee..ae73de5e64dc5 100644 --- a/compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs +++ b/compiler/rustc_target/src/spec/targets/armv7_unknown_trusty.rs @@ -17,7 +17,7 @@ pub fn target() -> Target { arch: "arm".into(), options: TargetOptions { abi: "eabi".into(), - features: "+v7,+thumb2,+soft-float,-neon,+reserve-x18".into(), + features: "+v7,+thumb2,+soft-float,-neon".into(), max_atomic_width: Some(64), mcount: "\u{1}mcount".into(), os: "trusty".into(), From 5b374631df79f879560f3a4d041290e152c05ab0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 15:02:13 +0000 Subject: [PATCH 10/23] compiletest: implement `needs-lvm-zstd` directive --- src/tools/compiletest/src/command-list.rs | 1 + src/tools/compiletest/src/header.rs | 101 ++++++++++++++++++++++ src/tools/compiletest/src/header/needs.rs | 10 ++- 3 files changed, 111 insertions(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/command-list.rs b/src/tools/compiletest/src/command-list.rs index 7f8080235c870..a559d6f81a240 100644 --- a/src/tools/compiletest/src/command-list.rs +++ b/src/tools/compiletest/src/command-list.rs @@ -141,6 +141,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "needs-force-clang-based-tests", "needs-git-hash", "needs-llvm-components", + "needs-llvm-zstd", "needs-profiler-support", "needs-relocation-model-pic", "needs-run-enabled", diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 1fc24301c85e6..933913eb47c34 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -1203,6 +1203,107 @@ pub fn extract_llvm_version_from_binary(binary_path: &str) -> Option { None } +/// For tests using the `needs-llvm-zstd` directive: +/// - for local LLVM builds, try to find the static zstd library in the llvm-config system libs. +/// - for `download-ci-llvm`, see if `lld` was built with zstd support. +pub fn llvm_has_libzstd(config: &Config) -> bool { + // Strategy 1: works for local builds but not with `download-ci-llvm`. + // + // We check whether `llvm-config` returns the zstd library. Bootstrap's `llvm.libzstd` will only + // ask to statically link it when building LLVM, so we only check if the list of system libs + // contains a path to that static lib, and that it exists. + // + // See compiler/rustc_llvm/build.rs for more details and similar expectations. + fn is_zstd_in_config(llvm_bin_dir: &Path) -> Option<()> { + let llvm_config_path = llvm_bin_dir.join("llvm-config"); + let output = Command::new(llvm_config_path).arg("--system-libs").output().ok()?; + assert!(output.status.success(), "running llvm-config --system-libs failed"); + + let libs = String::from_utf8(output.stdout).ok()?; + for lib in libs.split_whitespace() { + if lib.ends_with("libzstd.a") && Path::new(lib).exists() { + return Some(()); + } + } + + None + } + + // Strategy 2: `download-ci-llvm`'s `llvm-config --system-libs` will not return any libs to + // use. + // + // The CI artifacts also don't contain the bootstrap config used to build them: otherwise we + // could have looked at the `llvm.libzstd` config. + // + // We infer whether `LLVM_ENABLE_ZSTD` was used to build LLVM as a byproduct of testing whether + // `lld` supports it. If not, an error will be emitted: "LLVM was not built with + // LLVM_ENABLE_ZSTD or did not find zstd at build time". + #[cfg(unix)] + fn is_lld_built_with_zstd(llvm_bin_dir: &Path) -> Option<()> { + let lld_path = llvm_bin_dir.join("lld"); + if lld_path.exists() { + // We can't call `lld` as-is, it expects to be invoked by a compiler driver using a + // different name. Prepare a temporary symlink to do that. + let lld_symlink_path = llvm_bin_dir.join("ld.lld"); + if !lld_symlink_path.exists() { + std::os::unix::fs::symlink(lld_path, &lld_symlink_path).ok()?; + } + + // Run `lld` with a zstd flag. We expect this command to always error here, we don't + // want to link actual files and don't pass any. + let output = Command::new(&lld_symlink_path) + .arg("--compress-debug-sections=zstd") + .output() + .ok()?; + assert!(!output.status.success()); + + // Look for a specific error caused by LLVM not being built with zstd support. We could + // also look for the "no input files" message, indicating the zstd flag was accepted. + let stderr = String::from_utf8(output.stderr).ok()?; + let zstd_available = !stderr.contains("LLVM was not built with LLVM_ENABLE_ZSTD"); + + // We don't particularly need to clean the link up (so the previous commands could fail + // in theory but won't in practice), but we can try. + std::fs::remove_file(lld_symlink_path).ok()?; + + if zstd_available { + return Some(()); + } + } + + None + } + + #[cfg(not(unix))] + fn is_lld_built_with_zstd(_llvm_bin_dir: &Path) -> Option<()> { + None + } + + if let Some(llvm_bin_dir) = &config.llvm_bin_dir { + // Strategy 1: for local LLVM builds. + if is_zstd_in_config(llvm_bin_dir).is_some() { + return true; + } + + // Strategy 2: for LLVM artifacts built on CI via `download-ci-llvm`. + // + // It doesn't work for cases where the artifacts don't contain the linker, but it's + // best-effort: CI has `llvm.libzstd` and `lld` enabled on the x64 linux artifacts, so it + // will at least work there. + // + // If this can be improved and expanded to less common cases in the future, it should. + if config.target == "x86_64-unknown-linux-gnu" + && config.host == config.target + && is_lld_built_with_zstd(llvm_bin_dir).is_some() + { + return true; + } + } + + // Otherwise, all hope is lost. + false +} + /// Takes a directive of the form " [- ]", /// returns the numeric representation of and as /// tuple: ( as u32, as u32) diff --git a/src/tools/compiletest/src/header/needs.rs b/src/tools/compiletest/src/header/needs.rs index 8f935d5b74441..e04abea0bf987 100644 --- a/src/tools/compiletest/src/header/needs.rs +++ b/src/tools/compiletest/src/header/needs.rs @@ -1,5 +1,5 @@ use crate::common::{Config, Sanitizer}; -use crate::header::IgnoreDecision; +use crate::header::{llvm_has_libzstd, IgnoreDecision}; pub(super) fn handle_needs( cache: &CachedNeedsConditions, @@ -144,6 +144,11 @@ pub(super) fn handle_needs( condition: cache.symlinks, ignore_reason: "ignored if symlinks are unavailable", }, + Need { + name: "needs-llvm-zstd", + condition: cache.llvm_zstd, + ignore_reason: "ignored if LLVM wasn't build with zstd for ELF section compression", + }, ]; let (name, comment) = match ln.split_once([':', ' ']) { @@ -210,6 +215,8 @@ pub(super) struct CachedNeedsConditions { rust_lld: bool, dlltool: bool, symlinks: bool, + /// Whether LLVM built with zstd, for the `needs-llvm-zstd` directive. + llvm_zstd: bool, } impl CachedNeedsConditions { @@ -253,6 +260,7 @@ impl CachedNeedsConditions { .join(if config.host.contains("windows") { "rust-lld.exe" } else { "rust-lld" }) .exists(), + llvm_zstd: llvm_has_libzstd(&config), dlltool: find_dlltool(&config), symlinks: has_symlinks(), } From 79f3c51a01693b264419f74b5527c146b4d52e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 15:09:20 +0000 Subject: [PATCH 11/23] mark `rust-lld-compress-debug-sections` test as needing zstd also make it fail if there's a compression issue --- .../rust-lld-compress-debug-sections/rmake.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/tests/run-make/rust-lld-compress-debug-sections/rmake.rs b/tests/run-make/rust-lld-compress-debug-sections/rmake.rs index ea4997fab8099..9889659046ea7 100644 --- a/tests/run-make/rust-lld-compress-debug-sections/rmake.rs +++ b/tests/run-make/rust-lld-compress-debug-sections/rmake.rs @@ -1,12 +1,13 @@ // Checks the `compress-debug-sections` option on rust-lld. //@ needs-rust-lld +//@ needs-llvm-zstd //@ only-linux //@ ignore-cross-compile // FIXME: This test isn't comprehensive and isn't covering all possible combinations. -use run_make_support::{assert_contains, llvm_readobj, run_in_tmpdir, rustc}; +use run_make_support::{llvm_readobj, run_in_tmpdir, rustc}; fn check_compression(compression: &str, to_find: &str) { run_in_tmpdir(|| { @@ -17,19 +18,8 @@ fn check_compression(compression: &str, to_find: &str) { .arg("-Cdebuginfo=full") .link_arg(&format!("-Wl,--compress-debug-sections={compression}")) .input("main.rs") - .run_unchecked(); - let stderr = out.stderr_utf8(); - if stderr.is_empty() { - llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find); - } else { - assert_contains( - stderr, - format!( - "LLVM was not built with LLVM_ENABLE_{to_find} \ - or did not find {compression} at build time" - ), - ); - } + .run(); + llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find); }); } From 802222fefca2e374a254877c54beb051843cd974 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 16:22:04 +0000 Subject: [PATCH 12/23] prepare test for expanding scope --- .../main.rs | 0 .../rmake.rs | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tests/run-make/{rust-lld-compress-debug-sections => compressed-debuginfo-zstd}/main.rs (100%) rename tests/run-make/{rust-lld-compress-debug-sections => compressed-debuginfo-zstd}/rmake.rs (100%) diff --git a/tests/run-make/rust-lld-compress-debug-sections/main.rs b/tests/run-make/compressed-debuginfo-zstd/main.rs similarity index 100% rename from tests/run-make/rust-lld-compress-debug-sections/main.rs rename to tests/run-make/compressed-debuginfo-zstd/main.rs diff --git a/tests/run-make/rust-lld-compress-debug-sections/rmake.rs b/tests/run-make/compressed-debuginfo-zstd/rmake.rs similarity index 100% rename from tests/run-make/rust-lld-compress-debug-sections/rmake.rs rename to tests/run-make/compressed-debuginfo-zstd/rmake.rs From 1935e21029d28af3022064f57b36fff85b8db9d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 16:59:00 +0000 Subject: [PATCH 13/23] expand zstd debuginfo compression test it now checks zlib and zstd, via rustc and rust-lld --- .../compressed-debuginfo-zstd/rmake.rs | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/tests/run-make/compressed-debuginfo-zstd/rmake.rs b/tests/run-make/compressed-debuginfo-zstd/rmake.rs index 9889659046ea7..8356373e949aa 100644 --- a/tests/run-make/compressed-debuginfo-zstd/rmake.rs +++ b/tests/run-make/compressed-debuginfo-zstd/rmake.rs @@ -1,24 +1,37 @@ -// Checks the `compress-debug-sections` option on rust-lld. +// Checks debuginfo compression both for the always-enabled zlib, and when the optional zstd is +// enabled: +// - via rustc's `debuginfo-compression`, +// - and via rust-lld's `compress-debug-sections` -//@ needs-rust-lld -//@ needs-llvm-zstd +//@ needs-llvm-zstd: we want LLVM/LLD to be built with zstd support +//@ needs-rust-lld: the system linker will most likely not support zstd //@ only-linux //@ ignore-cross-compile -// FIXME: This test isn't comprehensive and isn't covering all possible combinations. - -use run_make_support::{llvm_readobj, run_in_tmpdir, rustc}; +use run_make_support::{llvm_readobj, run_in_tmpdir, Rustc}; fn check_compression(compression: &str, to_find: &str) { + // check compressed debug sections via rustc flag + prepare_and_check(to_find, |rustc| { + rustc.arg(&format!("-Zdebuginfo-compression={compression}")) + }); + + // check compressed debug sections via rust-lld flag + prepare_and_check(to_find, |rustc| { + rustc.link_arg(&format!("-Wl,--compress-debug-sections={compression}")) + }); +} + +fn prepare_and_check &mut Rustc>(to_find: &str, prepare_rustc: F) { run_in_tmpdir(|| { - let out = rustc() + let mut rustc = Rustc::new(); + rustc .arg("-Zlinker-features=+lld") .arg("-Clink-self-contained=+linker") .arg("-Zunstable-options") .arg("-Cdebuginfo=full") - .link_arg(&format!("-Wl,--compress-debug-sections={compression}")) - .input("main.rs") - .run(); + .input("main.rs"); + prepare_rustc(&mut rustc).run(); llvm_readobj().arg("-t").arg("main").run().assert_stdout_contains(to_find); }); } From 62c8c693bdda3932dbb049f8a2707f2f616d29eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sat, 10 Aug 2024 22:25:22 +0000 Subject: [PATCH 14/23] move and rename zstd script move it where it's used, and name it like the other scripts --- src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile | 6 +++--- .../zstd.sh => host-x86_64/dist-x86_64-linux/build-zstd.sh} | 0 2 files changed, 3 insertions(+), 3 deletions(-) rename src/ci/docker/{scripts/zstd.sh => host-x86_64/dist-x86_64-linux/build-zstd.sh} (100%) diff --git a/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile index 61e9694f1e2ae..e857f38e68a85 100644 --- a/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile @@ -62,9 +62,9 @@ COPY host-x86_64/dist-x86_64-linux/build-clang.sh /tmp/ RUN ./build-clang.sh ENV CC=clang CXX=clang++ -# rustc's LLVM needs zstd. -COPY scripts/zstd.sh /tmp/ -RUN ./zstd.sh +# Build zstd to enable `llvm.libzstd`. +COPY host-x86_64/dist-x86_64-linux/build-zstd.sh /tmp/ +RUN ./build-zstd.sh COPY scripts/sccache.sh /scripts/ RUN sh /scripts/sccache.sh diff --git a/src/ci/docker/scripts/zstd.sh b/src/ci/docker/host-x86_64/dist-x86_64-linux/build-zstd.sh similarity index 100% rename from src/ci/docker/scripts/zstd.sh rename to src/ci/docker/host-x86_64/dist-x86_64-linux/build-zstd.sh From 7963beda2481deb5bd01286b61386d599536a3b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sun, 11 Aug 2024 15:02:13 +0000 Subject: [PATCH 15/23] strip whitespace for ignored tests reason comments --- src/tools/compiletest/src/header/needs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/header/needs.rs b/src/tools/compiletest/src/header/needs.rs index e04abea0bf987..72b1b9c6d480a 100644 --- a/src/tools/compiletest/src/header/needs.rs +++ b/src/tools/compiletest/src/header/needs.rs @@ -174,7 +174,7 @@ pub(super) fn handle_needs( } else { return IgnoreDecision::Ignore { reason: if let Some(comment) = comment { - format!("{} ({comment})", need.ignore_reason) + format!("{} ({})", need.ignore_reason, comment.trim()) } else { need.ignore_reason.into() }, From 650ba7fdf001353664a60be43efb2d96c3c0295b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Sun, 11 Aug 2024 19:05:30 +0000 Subject: [PATCH 16/23] enable `llvm.libzstd` on test x64 linux builder --- src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile index 19683317126ab..83c2aa8cfb3b7 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu/Dockerfile @@ -28,5 +28,6 @@ ENV RUST_CONFIGURE_ARGS \ --build=x86_64-unknown-linux-gnu \ --enable-sanitizers \ --enable-profiler \ - --enable-compiler-docs + --enable-compiler-docs \ + --set llvm.libzstd=true ENV SCRIPT python3 ../x.py --stage 2 test From 5d83cb27c886137887c087fbce77032069f5ec8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Rakic?= Date: Tue, 20 Aug 2024 14:04:44 +0000 Subject: [PATCH 17/23] allow `llvm.libzstd` with `download-ci-llvm = true` but warn about it --- src/bootstrap/src/core/config/config.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/config/config.rs b/src/bootstrap/src/core/config/config.rs index ce23b7735f8bd..bdfee55d8d183 100644 --- a/src/bootstrap/src/core/config/config.rs +++ b/src/bootstrap/src/core/config/config.rs @@ -1885,6 +1885,22 @@ impl Config { warn("link-shared"); } + // FIXME(#129153): instead of all the ad-hoc `download-ci-llvm` checks that follow, + // use the `builder-config` present in tarballs since #128822 to compare the local + // config to the ones used to build the LLVM artifacts on CI, and only notify users + // if they've chosen a different value. + + if libzstd.is_some() { + println!( + "WARNING: when using `download-ci-llvm`, the local `llvm.libzstd` option, \ + like almost all `llvm.*` options, will be ignored and set by the LLVM CI \ + artifacts builder config." + ); + println!( + "HELP: To use `llvm.libzstd` for LLVM/LLD builds, set `download-ci-llvm` option to false." + ); + } + // None of the LLVM options, except assertions, are supported // when using downloaded LLVM. We could just ignore these but // that's potentially confusing, so force them to not be @@ -1894,7 +1910,6 @@ impl Config { check_ci_llvm!(optimize_toml); check_ci_llvm!(thin_lto); check_ci_llvm!(release_debuginfo); - check_ci_llvm!(libzstd); check_ci_llvm!(targets); check_ci_llvm!(experimental_targets); check_ci_llvm!(clang_cl); From fe2975076f1f84ff892cf95655268985e3550948 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 25 Aug 2024 16:03:29 +0200 Subject: [PATCH 18/23] float types: document NaN bit pattern guarantees --- library/core/src/num/f128.rs | 30 ++++++++----- library/core/src/num/f16.rs | 30 ++++++++----- library/core/src/num/f32.rs | 14 +++--- library/core/src/num/f64.rs | 14 +++--- library/core/src/primitive_docs.rs | 71 ++++++++++++++++++++++++++++++ 5 files changed, 123 insertions(+), 36 deletions(-) diff --git a/library/core/src/num/f128.rs b/library/core/src/num/f128.rs index 38e69e7641ab4..d4236e47bfe3b 100644 --- a/library/core/src/num/f128.rs +++ b/library/core/src/num/f128.rs @@ -454,11 +454,14 @@ impl f128 { } /// Returns `true` if `self` has a positive sign, including `+0.0`, NaNs with - /// positive sign bit and positive infinity. Note that IEEE 754 doesn't assign any - /// meaning to the sign bit in case of a NaN, and as Rust doesn't guarantee that - /// the bit pattern of NaNs are conserved over arithmetic operations, the result of - /// `is_sign_positive` on a NaN might produce an unexpected result in some cases. - /// See [explanation of NaN as a special value](f128) for more info. + /// positive sign bit and positive infinity. + /// + /// Note that IEEE 754 doesn't assign any meaning to the sign bit in case of + /// a NaN, and as Rust doesn't guarantee that the bit pattern of NaNs are + /// conserved over arithmetic operations, the result of `is_sign_positive` on + /// a NaN might produce an unexpected or non-portable result. See the [specification + /// of NaN bit patterns](f32#nan-bit-patterns) for more info. Use `self.signum() == 1.0` + /// if you need fully portable behavior (will return `false` for all NaNs). /// /// ``` /// #![feature(f128)] @@ -477,11 +480,14 @@ impl f128 { } /// Returns `true` if `self` has a negative sign, including `-0.0`, NaNs with - /// negative sign bit and negative infinity. Note that IEEE 754 doesn't assign any - /// meaning to the sign bit in case of a NaN, and as Rust doesn't guarantee that - /// the bit pattern of NaNs are conserved over arithmetic operations, the result of - /// `is_sign_negative` on a NaN might produce an unexpected result in some cases. - /// See [explanation of NaN as a special value](f128) for more info. + /// negative sign bit and negative infinity. + /// + /// Note that IEEE 754 doesn't assign any meaning to the sign bit in case of + /// a NaN, and as Rust doesn't guarantee that the bit pattern of NaNs are + /// conserved over arithmetic operations, the result of `is_sign_negative` on + /// a NaN might produce an unexpected or non-portable result. See the [specification + /// of NaN bit patterns](f32#nan-bit-patterns) for more info. Use `self.signum() == -1.0` + /// if you need fully portable behavior (will return `false` for all NaNs). /// /// ``` /// #![feature(f128)] @@ -750,7 +756,7 @@ impl f128 { /// Note that this follows the semantics specified in IEEE 754-2019. /// /// Also note that "propagation" of NaNs here doesn't necessarily mean that the bitpattern of a NaN - /// operand is conserved; see [explanation of NaN as a special value](f128) for more info. + /// operand is conserved; see the [specification of NaN bit patterns](f32#nan-bit-patterns) for more info. #[inline] #[unstable(feature = "f128", issue = "116909")] // #[unstable(feature = "float_minimum_maximum", issue = "91079")] @@ -791,7 +797,7 @@ impl f128 { /// Note that this follows the semantics specified in IEEE 754-2019. /// /// Also note that "propagation" of NaNs here doesn't necessarily mean that the bitpattern of a NaN - /// operand is conserved; see [explanation of NaN as a special value](f128) for more info. + /// operand is conserved; see the [specification of NaN bit patterns](f32#nan-bit-patterns) for more info. #[inline] #[unstable(feature = "f128", issue = "116909")] // #[unstable(feature = "float_minimum_maximum", issue = "91079")] diff --git a/library/core/src/num/f16.rs b/library/core/src/num/f16.rs index bb0cc1c60ba5a..1e2f841aca733 100644 --- a/library/core/src/num/f16.rs +++ b/library/core/src/num/f16.rs @@ -464,11 +464,14 @@ impl f16 { } /// Returns `true` if `self` has a positive sign, including `+0.0`, NaNs with - /// positive sign bit and positive infinity. Note that IEEE 754 doesn't assign any - /// meaning to the sign bit in case of a NaN, and as Rust doesn't guarantee that - /// the bit pattern of NaNs are conserved over arithmetic operations, the result of - /// `is_sign_positive` on a NaN might produce an unexpected result in some cases. - /// See [explanation of NaN as a special value](f16) for more info. + /// positive sign bit and positive infinity. + /// + /// Note that IEEE 754 doesn't assign any meaning to the sign bit in case of + /// a NaN, and as Rust doesn't guarantee that the bit pattern of NaNs are + /// conserved over arithmetic operations, the result of `is_sign_positive` on + /// a NaN might produce an unexpected or non-portable result. See the [specification + /// of NaN bit patterns](f32#nan-bit-patterns) for more info. Use `self.signum() == 1.0` + /// if you need fully portable behavior (will return `false` for all NaNs). /// /// ``` /// #![feature(f16)] @@ -490,11 +493,14 @@ impl f16 { } /// Returns `true` if `self` has a negative sign, including `-0.0`, NaNs with - /// negative sign bit and negative infinity. Note that IEEE 754 doesn't assign any - /// meaning to the sign bit in case of a NaN, and as Rust doesn't guarantee that - /// the bit pattern of NaNs are conserved over arithmetic operations, the result of - /// `is_sign_negative` on a NaN might produce an unexpected result in some cases. - /// See [explanation of NaN as a special value](f16) for more info. + /// negative sign bit and negative infinity. + /// + /// Note that IEEE 754 doesn't assign any meaning to the sign bit in case of + /// a NaN, and as Rust doesn't guarantee that the bit pattern of NaNs are + /// conserved over arithmetic operations, the result of `is_sign_negative` on + /// a NaN might produce an unexpected or non-portable result. See the [specification + /// of NaN bit patterns](f32#nan-bit-patterns) for more info. Use `self.signum() == -1.0` + /// if you need fully portable behavior (will return `false` for all NaNs). /// /// ``` /// #![feature(f16)] @@ -762,7 +768,7 @@ impl f16 { /// Note that this follows the semantics specified in IEEE 754-2019. /// /// Also note that "propagation" of NaNs here doesn't necessarily mean that the bitpattern of a NaN - /// operand is conserved; see [explanation of NaN as a special value](f16) for more info. + /// operand is conserved; see the [specification of NaN bit patterns](f32#nan-bit-patterns) for more info. #[inline] #[unstable(feature = "f16", issue = "116909")] // #[unstable(feature = "float_minimum_maximum", issue = "91079")] @@ -802,7 +808,7 @@ impl f16 { /// Note that this follows the semantics specified in IEEE 754-2019. /// /// Also note that "propagation" of NaNs here doesn't necessarily mean that the bitpattern of a NaN - /// operand is conserved; see [explanation of NaN as a special value](f16) for more info. + /// operand is conserved; see the [specification of NaN bit patterns](f32#nan-bit-patterns) for more info. #[inline] #[unstable(feature = "f16", issue = "116909")] // #[unstable(feature = "float_minimum_maximum", issue = "91079")] diff --git a/library/core/src/num/f32.rs b/library/core/src/num/f32.rs index 719727e2f1e0a..c1adcc753f2e5 100644 --- a/library/core/src/num/f32.rs +++ b/library/core/src/num/f32.rs @@ -700,8 +700,9 @@ impl f32 { /// Note that IEEE 754 doesn't assign any meaning to the sign bit in case of /// a NaN, and as Rust doesn't guarantee that the bit pattern of NaNs are /// conserved over arithmetic operations, the result of `is_sign_positive` on - /// a NaN might produce an unexpected result in some cases. See [explanation - /// of NaN as a special value](f32) for more info. + /// a NaN might produce an unexpected or non-portable result. See the [specification + /// of NaN bit patterns](f32#nan-bit-patterns) for more info. Use `self.signum() == 1.0` + /// if you need fully portable behavior (will return `false` for all NaNs). /// /// ``` /// let f = 7.0_f32; @@ -724,8 +725,9 @@ impl f32 { /// Note that IEEE 754 doesn't assign any meaning to the sign bit in case of /// a NaN, and as Rust doesn't guarantee that the bit pattern of NaNs are /// conserved over arithmetic operations, the result of `is_sign_negative` on - /// a NaN might produce an unexpected result in some cases. See [explanation - /// of NaN as a special value](f32) for more info. + /// a NaN might produce an unexpected or non-portable result. See the [specification + /// of NaN bit patterns](f32#nan-bit-patterns) for more info. Use `self.signum() == -1.0` + /// if you need fully portable behavior (will return `false` for all NaNs). /// /// ``` /// let f = 7.0f32; @@ -954,7 +956,7 @@ impl f32 { /// Note that this follows the semantics specified in IEEE 754-2019. /// /// Also note that "propagation" of NaNs here doesn't necessarily mean that the bitpattern of a NaN - /// operand is conserved; see [explanation of NaN as a special value](f32) for more info. + /// operand is conserved; see the [specification of NaN bit patterns](f32#nan-bit-patterns) for more info. #[must_use = "this returns the result of the comparison, without modifying either input"] #[unstable(feature = "float_minimum_maximum", issue = "91079")] #[inline] @@ -989,7 +991,7 @@ impl f32 { /// Note that this follows the semantics specified in IEEE 754-2019. /// /// Also note that "propagation" of NaNs here doesn't necessarily mean that the bitpattern of a NaN - /// operand is conserved; see [explanation of NaN as a special value](f32) for more info. + /// operand is conserved; see the [specification of NaN bit patterns](f32#nan-bit-patterns) for more info. #[must_use = "this returns the result of the comparison, without modifying either input"] #[unstable(feature = "float_minimum_maximum", issue = "91079")] #[inline] diff --git a/library/core/src/num/f64.rs b/library/core/src/num/f64.rs index 85eb152ad1f19..e6406771ad333 100644 --- a/library/core/src/num/f64.rs +++ b/library/core/src/num/f64.rs @@ -695,8 +695,9 @@ impl f64 { /// Note that IEEE 754 doesn't assign any meaning to the sign bit in case of /// a NaN, and as Rust doesn't guarantee that the bit pattern of NaNs are /// conserved over arithmetic operations, the result of `is_sign_positive` on - /// a NaN might produce an unexpected result in some cases. See [explanation - /// of NaN as a special value](f32) for more info. + /// a NaN might produce an unexpected or non-portable result. See the [specification + /// of NaN bit patterns](f32#nan-bit-patterns) for more info. Use `self.signum() == 1.0` + /// if you need fully portable behavior (will return `false` for all NaNs). /// /// ``` /// let f = 7.0_f64; @@ -728,8 +729,9 @@ impl f64 { /// Note that IEEE 754 doesn't assign any meaning to the sign bit in case of /// a NaN, and as Rust doesn't guarantee that the bit pattern of NaNs are /// conserved over arithmetic operations, the result of `is_sign_negative` on - /// a NaN might produce an unexpected result in some cases. See [explanation - /// of NaN as a special value](f32) for more info. + /// a NaN might produce an unexpected or non-portable result. See the [specification + /// of NaN bit patterns](f32#nan-bit-patterns) for more info. Use `self.signum() == -1.0` + /// if you need fully portable behavior (will return `false` for all NaNs). /// /// ``` /// let f = 7.0_f64; @@ -968,7 +970,7 @@ impl f64 { /// Note that this follows the semantics specified in IEEE 754-2019. /// /// Also note that "propagation" of NaNs here doesn't necessarily mean that the bitpattern of a NaN - /// operand is conserved; see [explanation of NaN as a special value](f32) for more info. + /// operand is conserved; see the [specification of NaN bit patterns](f32#nan-bit-patterns) for more info. #[must_use = "this returns the result of the comparison, without modifying either input"] #[unstable(feature = "float_minimum_maximum", issue = "91079")] #[inline] @@ -1003,7 +1005,7 @@ impl f64 { /// Note that this follows the semantics specified in IEEE 754-2019. /// /// Also note that "propagation" of NaNs here doesn't necessarily mean that the bitpattern of a NaN - /// operand is conserved; see [explanation of NaN as a special value](f32) for more info. + /// operand is conserved; see the [specification of NaN bit patterns](f32#nan-bit-patterns) for more info. #[must_use = "this returns the result of the comparison, without modifying either input"] #[unstable(feature = "float_minimum_maximum", issue = "91079")] #[inline] diff --git a/library/core/src/primitive_docs.rs b/library/core/src/primitive_docs.rs index 09ebef89fb0c2..e0593b1c0e75d 100644 --- a/library/core/src/primitive_docs.rs +++ b/library/core/src/primitive_docs.rs @@ -1190,6 +1190,11 @@ mod prim_f16 {} /// portable or even fully deterministic! This means that there may be some /// surprising results upon inspecting the bit patterns, /// as the same calculations might produce NaNs with different bit patterns. +/// This also affects the sign of the NaN: checking `is_sign_positive` or `is_sign_negative` on +/// a NaN is the most common way to run into these surprising results. +/// (Checking `x >= 0.0` or `x <= 0.0` avoids those surprises, but also how negative/positive +/// zero are treated.) +/// See the section below for what exactly is guaranteed about the bit pattern of a NaN. /// /// When a primitive operation (addition, subtraction, multiplication, or /// division) is performed on this type, the result is rounded according to the @@ -1211,6 +1216,72 @@ mod prim_f16 {} /// *[See also the `std::f32::consts` module](crate::f32::consts).* /// /// [wikipedia]: https://en.wikipedia.org/wiki/Single-precision_floating-point_format +/// +/// # NaN bit patterns +/// +/// This section defines the possible NaN bit patterns returned by non-"bitwise" floating point +/// operations. The bitwise operations are unary `-`, `abs`, `copysign`; those are guaranteed to +/// exactly preserve the bit pattern of their input except for possibly changing the sign bit. +/// +/// A floating-point NaN value consists of: +/// - a sign bit +/// - a quiet/signaling bit +/// - a payload, which makes up the rest of the significand (i.e., the mantissa) except for the +/// quiet/signaling bit. +/// +/// Rust assumes that the quiet/signaling bit being set to `1` indicates a quiet NaN (QNaN), and a +/// value of `0` indicates a signaling NaN (SNaN). In the following we will hence just call it the +/// "quiet bit". +/// +/// The following rules apply when a NaN value is returned: the result has a non-deterministic sign. +/// The quiet bit and payload are non-deterministically chosen from the following set of options: +/// +/// - **Preferred NaN**: The quiet bit is set and the payload is all-zero. +/// - **Quieting NaN propagation**: The quiet bit is set and the payload is copied from any input +/// operand that is a NaN. If the inputs and outputs do not have the same payload size (i.e., for +/// `as` casts), then +/// - If the output is smaller than the input, low-order bits of the payload get dropped. +/// - If the output is larger than the input, the payload gets filled up with 0s in the low-order +/// bits. +/// - **Unchanged NaN propagation**: The quiet bit and payload are copied from any input operand +/// that is a NaN. If the inputs and outputs do not have the same size (i.e., for `as` casts), the +/// same rules as for "quieting NaN propagation" apply, with one caveat: if the output is smaller +/// than the input, droppig the low-order bits may result in a payload of 0; a payload of 0 is not +/// possible with a signaling NaN (the all-0 significand encodes an infinity) so unchanged NaN +/// propagation cannot occur with some inputs. +/// - **Target-specific NaN**: The quiet bit is set and the payload is picked from a target-specific +/// set of "extra" possible NaN payloads. The set can depend on the input operand values. This set +/// is empty on x86, ARM, and RISC-V (32bit and 64bit), but can be non-empty on other +/// architectures. Targets where this set is non-empty should document this in a suitable +/// location, e.g. their platform support page. (For instance, on wasm, if any input NaN does not +/// have the preferred all-zero payload or any input NaN is an SNaN, then this set contains all +/// possible payloads; otherwise, it is empty. On SPARC, this set consists of the all-one +/// payload.) +/// +/// In particular, if all input NaNs are quiet (or if there are no input NaNs), then the output NaN +/// is definitely quiet. Signaling NaN outputs can only occur if they are provided as an input +/// value. Similarly, if all input NaNs are preferred (or if there are no input NaNs) and the target +/// does not have any "extra" NaN payloads, then the output NaN is guaranteed to be preferred. +/// +/// The non-deterministic choice happens when the operation is executed; i.e., the result of a +/// NaN-producing floating point operation is a stable bit pattern (looking at these bits multiple +/// times will yield consistent results), but running the same operation twice with the same inputs +/// can produce different results. +/// +/// These guarantees are neither stronger nor weaker than those of IEEE 754: IEEE 754 guarantees +/// that an operation never returns a signaling NaN, whereas it is possible for operations like +/// `SNAN * 1.0` to return a signaling NaN in Rust. Conversely, IEEE 754 makes no statement at all +/// about which quiet NaN is returned, whereas Rust restricts the set of possible results to the +/// ones listed above. +/// +/// Unless noted otherwise, the same rules also apply to NaNs returned by other library functions +/// (e.g. `min`, `minimum`, `max`, `maximum`); other aspects of their semantics and which IEEE 754 +/// operation they correspond to are documented with the respective functions. +/// +/// When a floating-point operation is executed in `const` context, the same rules apply: no +/// guarantee is made about which of the NaN bit patterns described above will be returned. The +/// result does not have to match what happens when executing the same code at runtime, and the +/// result can vary depending on factors such as compiler version and flags. #[stable(feature = "rust1", since = "1.0.0")] mod prim_f32 {} From 53d4544628c0134a6b4d7edd971539dd8e655269 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 26 Aug 2024 17:16:53 +0200 Subject: [PATCH 19/23] move per-target NaN info into a table --- library/core/src/primitive_docs.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/library/core/src/primitive_docs.rs b/library/core/src/primitive_docs.rs index e0593b1c0e75d..7f35c7789e281 100644 --- a/library/core/src/primitive_docs.rs +++ b/library/core/src/primitive_docs.rs @@ -1250,13 +1250,8 @@ mod prim_f16 {} /// possible with a signaling NaN (the all-0 significand encodes an infinity) so unchanged NaN /// propagation cannot occur with some inputs. /// - **Target-specific NaN**: The quiet bit is set and the payload is picked from a target-specific -/// set of "extra" possible NaN payloads. The set can depend on the input operand values. This set -/// is empty on x86, ARM, and RISC-V (32bit and 64bit), but can be non-empty on other -/// architectures. Targets where this set is non-empty should document this in a suitable -/// location, e.g. their platform support page. (For instance, on wasm, if any input NaN does not -/// have the preferred all-zero payload or any input NaN is an SNaN, then this set contains all -/// possible payloads; otherwise, it is empty. On SPARC, this set consists of the all-one -/// payload.) +/// set of "extra" possible NaN payloads. The set can depend on the input operand values. +/// See the table below for the concrete NaNs this set contains on various targets. /// /// In particular, if all input NaNs are quiet (or if there are no input NaNs), then the output NaN /// is definitely quiet. Signaling NaN outputs can only occur if they are provided as an input @@ -1282,6 +1277,18 @@ mod prim_f16 {} /// guarantee is made about which of the NaN bit patterns described above will be returned. The /// result does not have to match what happens when executing the same code at runtime, and the /// result can vary depending on factors such as compiler version and flags. +/// +/// ### Target-specific "extra" NaN values +// FIXME: Is there a better place to put this? +/// +/// | `target_arch` | Extra payloads possible on this platform | +/// |---------------|---------| +/// | `x86`, `x86_64`, `arm`, `aarch64`, `riscv32`, `riscv64` | None | +/// | `sparc`, `sparc64` | The all-one payload | +/// | `wasm32`, `wasm64` | If all input NaNs are quiet with all-zero payload: None.
Otherwise: all possible payloads. | +/// +/// For targets not in this table, all payloads are possible. + #[stable(feature = "rust1", since = "1.0.0")] mod prim_f32 {} From 0c7d6c45e6b664fa4875741e78d185845d374f07 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 26 Aug 2024 17:25:24 +0200 Subject: [PATCH 20/23] also update copysign docs --- library/std/src/f128.rs | 10 +++++----- library/std/src/f16.rs | 10 +++++----- library/std/src/f32.rs | 10 +++++----- library/std/src/f64.rs | 10 +++++----- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/library/std/src/f128.rs b/library/std/src/f128.rs index f6df6259137bf..506d708d0d2eb 100644 --- a/library/std/src/f128.rs +++ b/library/std/src/f128.rs @@ -248,11 +248,11 @@ impl f128 { /// Returns a number composed of the magnitude of `self` and the sign of /// `sign`. /// - /// Equal to `self` if the sign of `self` and `sign` are the same, otherwise - /// equal to `-self`. If `self` is a NaN, then a NaN with the sign bit of - /// `sign` is returned. Note, however, that conserving the sign bit on NaN - /// across arithmetical operations is not generally guaranteed. - /// See [explanation of NaN as a special value](primitive@f128) for more info. + /// Equal to `self` if the sign of `self` and `sign` are the same, otherwise equal to `-self`. + /// If `self` is a NaN, then a NaN with the same payload as `self` and the sign bit of `sign` is + /// returned. Note, however, that conserving the sign bit on NaN across arithmetical operations + /// is not generally guaranteed. See [specification of NaN bit + /// patterns](primitive@f32#nan-bit-patterns) for more info. /// /// # Examples /// diff --git a/library/std/src/f16.rs b/library/std/src/f16.rs index 10908332762d5..033a3d4500932 100644 --- a/library/std/src/f16.rs +++ b/library/std/src/f16.rs @@ -247,11 +247,11 @@ impl f16 { /// Returns a number composed of the magnitude of `self` and the sign of /// `sign`. /// - /// Equal to `self` if the sign of `self` and `sign` are the same, otherwise - /// equal to `-self`. If `self` is a NaN, then a NaN with the sign bit of - /// `sign` is returned. Note, however, that conserving the sign bit on NaN - /// across arithmetical operations is not generally guaranteed. - /// See [explanation of NaN as a special value](primitive@f16) for more info. + /// Equal to `self` if the sign of `self` and `sign` are the same, otherwise equal to `-self`. + /// If `self` is a NaN, then a NaN with the same payload as `self` and the sign bit of `sign` is + /// returned. Note, however, that conserving the sign bit on NaN across arithmetical operations + /// is not generally guaranteed. See [specification of NaN bit + /// patterns](primitive@f32#nan-bit-patterns) for more info. /// /// # Examples /// diff --git a/library/std/src/f32.rs b/library/std/src/f32.rs index 12433d25bfa45..35c2a77b5338d 100644 --- a/library/std/src/f32.rs +++ b/library/std/src/f32.rs @@ -226,11 +226,11 @@ impl f32 { /// Returns a number composed of the magnitude of `self` and the sign of /// `sign`. /// - /// Equal to `self` if the sign of `self` and `sign` are the same, otherwise - /// equal to `-self`. If `self` is a NaN, then a NaN with the sign bit of - /// `sign` is returned. Note, however, that conserving the sign bit on NaN - /// across arithmetical operations is not generally guaranteed. - /// See [explanation of NaN as a special value](primitive@f32) for more info. + /// Equal to `self` if the sign of `self` and `sign` are the same, otherwise equal to `-self`. + /// If `self` is a NaN, then a NaN with the same payload as `self` and the sign bit of `sign` is + /// returned. Note, however, that conserving the sign bit on NaN across arithmetical operations + /// is not generally guaranteed. See [specification of NaN bit + /// patterns](primitive@f32#nan-bit-patterns) for more info. /// /// # Examples /// diff --git a/library/std/src/f64.rs b/library/std/src/f64.rs index a343e19173e59..c177f74a97e15 100644 --- a/library/std/src/f64.rs +++ b/library/std/src/f64.rs @@ -226,11 +226,11 @@ impl f64 { /// Returns a number composed of the magnitude of `self` and the sign of /// `sign`. /// - /// Equal to `self` if the sign of `self` and `sign` are the same, otherwise - /// equal to `-self`. If `self` is a NaN, then a NaN with the sign bit of - /// `sign` is returned. Note, however, that conserving the sign bit on NaN - /// across arithmetical operations is not generally guaranteed. - /// See [explanation of NaN as a special value](primitive@f32) for more info. + /// Equal to `self` if the sign of `self` and `sign` are the same, otherwise equal to `-self`. + /// If `self` is a NaN, then a NaN with the same payload as `self` and the sign bit of `sign` is + /// returned. Note, however, that conserving the sign bit on NaN across arithmetical operations + /// is not generally guaranteed. See [specification of NaN bit + /// patterns](primitive@f32#nan-bit-patterns) for more info. /// /// # Examples /// From ebf46f760774f63a8025f1ead3cf359e88aaa103 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 18 Jul 2024 12:44:59 -0300 Subject: [PATCH 21/23] Add unsafe to extern blocks in style guide --- src/doc/style-guide/src/items.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/doc/style-guide/src/items.md b/src/doc/style-guide/src/items.md index 8e1b1d80fb644..5ea8b6cd54294 100644 --- a/src/doc/style-guide/src/items.md +++ b/src/doc/style-guide/src/items.md @@ -449,8 +449,8 @@ entries, format it across multiple lines as with a type alias. ## extern items When writing extern items (such as `extern "C" fn`), always specify the ABI. -For example, write `extern "C" fn foo ...`, not `extern fn foo ...`, or -`extern "C" { ... }`. +For example, write `extern "C" fn foo ...` or `unsafe extern "C" { ...}` +and avoid `extern fn foo ...` and `unsafe extern { ... }`. ## Imports (`use` statements) From 5d0ce4c3918b1d5cd925c945ecf37f49bf11d4e4 Mon Sep 17 00:00:00 2001 From: Nicole LeGare Date: Mon, 26 Aug 2024 12:22:17 -0700 Subject: [PATCH 22/23] Note that std support is WIP --- src/doc/rustc/src/platform-support/trusty.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/doc/rustc/src/platform-support/trusty.md b/src/doc/rustc/src/platform-support/trusty.md index 97a03d3de04c2..dec3b96ff6449 100644 --- a/src/doc/rustc/src/platform-support/trusty.md +++ b/src/doc/rustc/src/platform-support/trusty.md @@ -13,12 +13,10 @@ Environment (TEE) for Android. ## Requirements -This target is cross-compiled. It has no special requirements for the host. +These targets are cross-compiled. They have no special requirements for the host. -It fully supports alloc with the default allocator, and partially supports std. -Notably, most I/O functionality is not supported, e.g. filesystem support and -networking support are not present and any APIs that rely on them will panic at -runtime. +Support for the standard library is work-in-progress. It is expected that +they will support alloc with the default allocator, and partially support std. Trusty uses the ELF file format. From 0763a3afc8f7d71cfe3b62c76fd40912f6a6c9ed Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 26 Aug 2024 20:11:58 -0700 Subject: [PATCH 23/23] Bump backtrace to rust-lang/backtrace@fc37b22 It should be 0.3.74~ish. This should help with backtraces on Android, QNX NTO 7.0, and Windows. --- library/backtrace | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/backtrace b/library/backtrace index 72265bea21089..fc37b22dc8a38 160000 --- a/library/backtrace +++ b/library/backtrace @@ -1 +1 @@ -Subproject commit 72265bea210891ae47bbe6d4f17b493ef0606619 +Subproject commit fc37b22dc8a384d93f6b7b4817266eec6433875e