diff --git a/compiler/rustc_codegen_ssa/src/back/metadata.rs b/compiler/rustc_codegen_ssa/src/back/metadata.rs index a43bf72b6a27d..32b80b621038b 100644 --- a/compiler/rustc_codegen_ssa/src/back/metadata.rs +++ b/compiler/rustc_codegen_ssa/src/back/metadata.rs @@ -129,13 +129,10 @@ pub(super) fn search_for_section<'a>( fn add_gnu_property_note( file: &mut write::Object<'static>, architecture: Architecture, - binary_format: BinaryFormat, endianness: Endianness, ) { - // check bti protection - if binary_format != BinaryFormat::Elf - || !matches!(architecture, Architecture::X86_64 | Architecture::Aarch64) - { + // Only X86_64 and Aarch64 require a GNU property note. + if !matches!(architecture, Architecture::X86_64 | Architecture::Aarch64) { return; } @@ -253,12 +250,14 @@ pub(crate) fn create_object_file(sess: &Session) -> Option u32 { } } Architecture::PowerPc64 => { - const EF_PPC64_ABI_UNKNOWN: u32 = 0; const EF_PPC64_ABI_ELF_V1: u32 = 1; const EF_PPC64_ABI_ELF_V2: u32 = 2; @@ -392,11 +390,7 @@ pub(super) fn elf_e_flags(architecture: Architecture, sess: &Session) -> u32 { // which leads to broken binaries if ELFv1 is used for the object files. LlvmAbi::ElfV1 => EF_PPC64_ABI_ELF_V1, LlvmAbi::ElfV2 => EF_PPC64_ABI_ELF_V2, - _ if sess.target.options.binary_format.to_object() == BinaryFormat::Elf => { - bug!("invalid ABI specified for this PPC64 ELF target"); - } - // Fall back - _ => EF_PPC64_ABI_UNKNOWN, + _ => bug!("invalid ABI specified for this PPC64 ELF target"), } } Architecture::Sparc32Plus => elf::EF_SPARC_32PLUS, diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index e8b25eb4359d0..60ac8a2663b90 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -12,7 +12,7 @@ use rustc_data_structures::profiling::{SelfProfilerRef, VerboseTimingGuard}; use rustc_errors::emitter::Emitter; use rustc_errors::{ Diag, DiagArgMap, DiagCtxt, DiagCtxtHandle, DiagMessage, ErrCode, FatalError, FatalErrorMarker, - Level, MultiSpan, Style, Suggestions, catch_fatal_errors, + Level, MultiSpan, Style, Sublevel, Suggestions, catch_fatal_errors, }; use rustc_fs_util::link_or_copy; use rustc_incremental::{copy_cgu_workproduct_to_incr_comp_cache_dir, in_incr_comp_dir_sess}; @@ -1217,7 +1217,7 @@ struct Diagnostic { // missing the following fields from `rustc_errors::Subdiag`. // - `span`: it doesn't impl `Send`. struct Subdiagnostic { - level: Level, + level: Sublevel, messages: Vec<(DiagMessage, Style)>, } diff --git a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs index 7e7f72943c7cd..f075ff21bc7bc 100644 --- a/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs +++ b/compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs @@ -5,7 +5,6 @@ //! //! [annotate_snippets]: https://docs.rs/crate/annotate-snippets/ -use std::borrow::Cow; use std::fmt::Debug; use std::io; use std::io::Write; @@ -29,7 +28,7 @@ use crate::emitter::{ use crate::formatting::{format_diag_message, format_diag_messages}; use crate::{ CodeSuggestion, DiagInner, DiagMessage, Emitter, ErrCode, Level, MultiSpan, Style, Subdiag, - SuggestionStyle, TerminalUrl, + Sublevel, SuggestionStyle, TerminalUrl, }; /// Generates diagnostics using annotate-snippet @@ -126,14 +125,23 @@ fn annotation_level_for_level(level: Level) -> annotate_snippets::level::Level<' } Level::Fatal | Level::Error => annotate_snippets::level::ERROR, Level::ForceWarning | Level::Warning => annotate_snippets::Level::WARNING, - Level::Note | Level::OnceNote => annotate_snippets::Level::NOTE, - Level::Help | Level::OnceHelp => annotate_snippets::Level::HELP, + Level::Note => annotate_snippets::Level::NOTE, + Level::Help => annotate_snippets::Level::HELP, Level::FailureNote => annotate_snippets::Level::NOTE.no_name(), Level::Allow => panic!("Should not call with Allow"), Level::Expect => panic!("Should not call with Expect"), } } +fn annotation_level_for_sublevel(level: Sublevel) -> annotate_snippets::level::Level<'static> { + match level { + Sublevel::Error => annotate_snippets::Level::ERROR, + Sublevel::Warning => annotate_snippets::Level::WARNING, + Sublevel::Note | Sublevel::OnceNote => annotate_snippets::Level::NOTE, + Sublevel::Help | Sublevel::OnceHelp => annotate_snippets::Level::HELP, + } +} + impl AnnotateSnippetEmitter { pub fn new(dst: Destination) -> Self { Self { @@ -166,9 +174,7 @@ impl AnnotateSnippetEmitter { // If at least one portion of the message is styled, we need to // "pre-style" the message let mut title = if msgs.iter().any(|(_, style)| style != &crate::Style::NoStyle) { - annotation_level - .clone() - .secondary_title(Cow::Owned(self.pre_style_msgs(msgs, *level, args))) + annotation_level.clone().secondary_title(self.pre_style_msgs(msgs, args)) } else { annotation_level.clone().primary_title(format_diag_messages(msgs, args)) }; @@ -186,8 +192,8 @@ impl AnnotateSnippetEmitter { // If we don't have span information, emit and exit let Some(sm) = self.sm.as_ref() else { group = group.elements(children.iter().map(|c| { - let msg = format_diag_messages(&c.messages, args).to_string(); - let level = annotation_level_for_level(c.level); + let msg = format_diag_messages(&c.messages, args); + let level = annotation_level_for_sublevel(c.level); level.message(msg) })); @@ -250,12 +256,12 @@ impl AnnotateSnippetEmitter { } for c in children { - let level = annotation_level_for_level(c.level); + let level = annotation_level_for_sublevel(c.level); // If at least one portion of the message is styled, we need to // "pre-style" the message let msg = if c.messages.iter().any(|(_, style)| style != &crate::Style::NoStyle) { - Cow::Owned(self.pre_style_msgs(&c.messages, c.level, args)) + self.pre_style_msgs(&c.messages, args) } else { format_diag_messages(&c.messages, args) }; @@ -309,10 +315,7 @@ impl AnnotateSnippetEmitter { // do not display this suggestion, it is meant only for tools } SuggestionStyle::HideCodeAlways => { - let msg = format_diag_messages( - &[(suggestion.msg.to_owned(), Style::HeaderMsg)], - args, - ); + let msg = format_diag_message(&suggestion.msg, args).into_owned(); group = group.element(annotate_snippets::Level::HELP.message(msg)); } SuggestionStyle::HideCodeInline @@ -544,16 +547,11 @@ impl AnnotateSnippetEmitter { .short_message(self.short_message) } - fn pre_style_msgs( - &self, - msgs: &[(DiagMessage, Style)], - level: Level, - args: &DiagArgMap, - ) -> String { + fn pre_style_msgs(&self, msgs: &[(DiagMessage, Style)], args: &DiagArgMap) -> String { msgs.iter() .filter_map(|(m, style)| { let text = format_diag_message(m, args); - let style = style.anstyle(level); + let style = style.anstyle(); if text.is_empty() { None } else { Some(format!("{style}{text}{style:#}")) } }) .collect() diff --git a/compiler/rustc_errors/src/diagnostic.rs b/compiler/rustc_errors/src/diagnostic.rs index 2874b85e9b67a..461e310d67896 100644 --- a/compiler/rustc_errors/src/diagnostic.rs +++ b/compiler/rustc_errors/src/diagnostic.rs @@ -16,7 +16,8 @@ use tracing::debug; use crate::{ CodeSuggestion, DiagCtxtHandle, DiagMessage, ErrCode, ErrorGuaranteed, ExplicitBug, Level, - MultiSpan, StashKey, Style, Substitution, SubstitutionPart, SuggestionStyle, Suggestions, + MultiSpan, StashKey, Style, Sublevel, Substitution, SubstitutionPart, SuggestionStyle, + Suggestions, }; /// Trait for types that `Diag::emit` can return as a "guarantee" (or "proof") @@ -321,9 +322,7 @@ impl DiagInner { Level::ForceWarning | Level::Warning | Level::Note - | Level::OnceNote | Level::Help - | Level::OnceHelp | Level::FailureNote | Level::Allow | Level::Expect => false, @@ -350,7 +349,12 @@ impl DiagInner { } } - pub(crate) fn sub(&mut self, level: Level, message: impl Into, span: MultiSpan) { + pub(crate) fn sub( + &mut self, + level: Sublevel, + message: impl Into, + span: MultiSpan, + ) { let sub = Subdiag { level, messages: vec![(message.into(), Style::NoStyle)], span }; self.children.push(sub); } @@ -374,7 +378,7 @@ impl DiagInner { pub fn emitted_at_sub_diag(&self) -> Subdiag { let track = format!("-Ztrack-diagnostics: created at {}", self.emitted_at); Subdiag { - level: crate::Level::Note, + level: crate::Sublevel::Note, messages: vec![(DiagMessage::Str(Cow::Owned(track)), Style::NoStyle)], span: MultiSpan::new(), } @@ -427,7 +431,7 @@ impl PartialEq for DiagInner { /// For example, a note attached to an error. #[derive(Clone, Debug, PartialEq, Hash, Encodable, Decodable)] pub struct Subdiag { - pub level: Level, + pub level: Sublevel, pub messages: Vec<(DiagMessage, Style)>, pub span: MultiSpan, } @@ -708,12 +712,12 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { with_fn! { with_note, /// Add a note attached to this diagnostic. pub fn note(&mut self, msg: impl Into) -> &mut Self { - self.sub(Level::Note, msg, MultiSpan::new()); + self.sub(Sublevel::Note, msg, MultiSpan::new()); self } } pub fn highlighted_note(&mut self, msg: Vec) -> &mut Self { - self.sub_with_highlights(Level::Note, msg, MultiSpan::new()); + self.sub_with_highlights(Sublevel::Note, msg, MultiSpan::new()); self } @@ -722,13 +726,13 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { span: impl Into, msg: Vec, ) -> &mut Self { - self.sub_with_highlights(Level::Note, msg, span.into()); + self.sub_with_highlights(Sublevel::Note, msg, span.into()); self } /// This is like [`Diag::note()`], but it's only printed once. pub fn note_once(&mut self, msg: impl Into) -> &mut Self { - self.sub(Level::OnceNote, msg, MultiSpan::new()); + self.sub(Sublevel::OnceNote, msg, MultiSpan::new()); self } @@ -740,7 +744,7 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { sp: impl Into, msg: impl Into, ) -> &mut Self { - self.sub(Level::Note, msg, sp.into()); + self.sub(Sublevel::Note, msg, sp.into()); self } } @@ -751,14 +755,14 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { sp: S, msg: impl Into, ) -> &mut Self { - self.sub(Level::OnceNote, msg, sp.into()); + self.sub(Sublevel::OnceNote, msg, sp.into()); self } with_fn! { with_warn, /// Add a warning attached to this diagnostic. pub fn warn(&mut self, msg: impl Into) -> &mut Self { - self.sub(Level::Warning, msg, MultiSpan::new()); + self.sub(Sublevel::Warning, msg, MultiSpan::new()); self } } @@ -769,26 +773,26 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { sp: S, msg: impl Into, ) -> &mut Self { - self.sub(Level::Warning, msg, sp.into()); + self.sub(Sublevel::Warning, msg, sp.into()); self } with_fn! { with_help, /// Add a help message attached to this diagnostic. pub fn help(&mut self, msg: impl Into) -> &mut Self { - self.sub(Level::Help, msg, MultiSpan::new()); + self.sub(Sublevel::Help, msg, MultiSpan::new()); self } } /// This is like [`Diag::help()`], but it's only printed once. pub fn help_once(&mut self, msg: impl Into) -> &mut Self { - self.sub(Level::OnceHelp, msg, MultiSpan::new()); + self.sub(Sublevel::OnceHelp, msg, MultiSpan::new()); self } /// Add a help message attached to this diagnostic with a customizable highlighted message. pub fn highlighted_help(&mut self, msg: Vec) -> &mut Self { - self.sub_with_highlights(Level::Help, msg, MultiSpan::new()); + self.sub_with_highlights(Sublevel::Help, msg, MultiSpan::new()); self } @@ -798,7 +802,7 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { span: impl Into, msg: Vec, ) -> &mut Self { - self.sub_with_highlights(Level::Help, msg, span.into()); + self.sub_with_highlights(Sublevel::Help, msg, span.into()); self } @@ -810,7 +814,7 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { sp: impl Into, msg: impl Into, ) -> &mut Self { - self.sub(Level::Help, msg, sp.into()); + self.sub(Sublevel::Help, msg, sp.into()); self } } @@ -1233,13 +1237,13 @@ impl<'a, G: EmissionGuarantee> Diag<'a, G> { /// public methods above. /// /// Used by `proc_macro_server` for implementing `server::Diagnostic`. - pub fn sub(&mut self, level: Level, message: impl Into, span: MultiSpan) { + pub fn sub(&mut self, level: Sublevel, message: impl Into, span: MultiSpan) { self.deref_mut().sub(level, message, span); } /// Convenience function for internal use, clients should use one of the /// public methods above. - fn sub_with_highlights(&mut self, level: Level, messages: Vec, span: MultiSpan) { + fn sub_with_highlights(&mut self, level: Sublevel, messages: Vec, span: MultiSpan) { let messages = messages.into_iter().map(|m| (m.content.into(), m.style)).collect(); let sub = Subdiag { level, messages, span }; self.children.push(sub); diff --git a/compiler/rustc_errors/src/emitter.rs b/compiler/rustc_errors/src/emitter.rs index 6d5f8462ff496..749b58e5d4b82 100644 --- a/compiler/rustc_errors/src/emitter.rs +++ b/compiler/rustc_errors/src/emitter.rs @@ -26,7 +26,8 @@ use tracing::{debug, warn}; use crate::formatting::format_diag_message; use crate::timings::TimingRecord; use crate::{ - CodeSuggestion, DiagInner, DiagMessage, Level, MultiSpan, Style, Subdiag, SuggestionStyle, + CodeSuggestion, DiagInner, DiagMessage, Level, MultiSpan, Style, Subdiag, Sublevel, + SuggestionStyle, }; /// Describes the way the content of the `rendered` field of the json output is generated @@ -209,7 +210,7 @@ pub trait Emitter { ); children.push(Subdiag { - level: Level::Note, + level: Sublevel::Note, messages: vec![(DiagMessage::from(msg), Style::NoStyle)], span: MultiSpan::new(), }); @@ -379,7 +380,7 @@ impl Emitter for EmitterWithNote { } fn emit_diagnostic(&mut self, mut diag: DiagInner) { - diag.sub(Level::Note, self.note.clone(), MultiSpan::new()); + diag.sub(Sublevel::Note, self.note.clone(), MultiSpan::new()); self.emitter.emit_diagnostic(diag); } } @@ -555,33 +556,10 @@ pub fn get_stderr_color_choice(color: ColorConfig, stderr: &std::io::Stderr) -> if matches!(choice, ColorChoice::Auto) { AutoStream::choice(stderr) } else { choice } } -/// On Windows, BRIGHT_BLUE is hard to read on black. Use cyan instead. -/// -/// See #36178. -const BRIGHT_BLUE: anstyle::Style = if cfg!(windows) { - AnsiColor::BrightCyan.on_default() -} else { - AnsiColor::BrightBlue.on_default() -}; - impl Style { - pub(crate) fn anstyle(&self, lvl: Level) -> anstyle::Style { + pub(crate) fn anstyle(&self) -> anstyle::Style { match self { - Style::Addition => AnsiColor::BrightGreen.on_default(), - Style::Removal => AnsiColor::BrightRed.on_default(), - Style::LineAndColumn => anstyle::Style::new(), - Style::LineNumber => BRIGHT_BLUE.effects(Effects::BOLD), - Style::Quotation => anstyle::Style::new(), - Style::MainHeaderMsg => if cfg!(windows) { - AnsiColor::BrightWhite.on_default() - } else { - anstyle::Style::new() - } - .effects(Effects::BOLD), - Style::UnderlinePrimary | Style::LabelPrimary => lvl.color().effects(Effects::BOLD), - Style::UnderlineSecondary | Style::LabelSecondary => BRIGHT_BLUE.effects(Effects::BOLD), - Style::HeaderMsg | Style::NoStyle => anstyle::Style::new(), - Style::Level(lvl) => lvl.color().effects(Effects::BOLD), + Style::NoStyle => anstyle::Style::new(), Style::Highlight => AnsiColor::Magenta.on_default().effects(Effects::BOLD), } } diff --git a/compiler/rustc_errors/src/formatting.rs b/compiler/rustc_errors/src/formatting.rs index 7b617031d6c8e..a56b45c729887 100644 --- a/compiler/rustc_errors/src/formatting.rs +++ b/compiler/rustc_errors/src/formatting.rs @@ -24,11 +24,8 @@ fn to_fluent_args<'iter>(iter: impl Iterator>) -> FluentAr } /// Convert `DiagMessage`s to a string -pub fn format_diag_messages( - messages: &[(DiagMessage, Style)], - args: &DiagArgMap, -) -> Cow<'static, str> { - Cow::Owned(messages.iter().map(|(m, _)| format_diag_message(m, args)).collect::()) +pub fn format_diag_messages(messages: &[(DiagMessage, Style)], args: &DiagArgMap) -> String { + messages.iter().map(|(m, _)| format_diag_message(m, args)).collect::() } /// Convert a `DiagMessage` to a string diff --git a/compiler/rustc_errors/src/json.rs b/compiler/rustc_errors/src/json.rs index 04ac140f33261..1f5a8c2fe94a6 100644 --- a/compiler/rustc_errors/src/json.rs +++ b/compiler/rustc_errors/src/json.rs @@ -379,20 +379,13 @@ impl Diagnostic { let buf = Arc::try_unwrap(buf.0).unwrap().into_inner().unwrap(); let buf = String::from_utf8(buf).unwrap(); - Diagnostic { - message: formatted_message.to_string(), - code, - level, - spans, - children, - rendered: Some(buf), - } + Diagnostic { message: formatted_message, code, level, spans, children, rendered: Some(buf) } } fn from_sub_diagnostic(subdiag: &Subdiag, args: &DiagArgMap, je: &JsonEmitter) -> Diagnostic { let formatted_message = format_diag_messages(&subdiag.messages, args); Diagnostic { - message: formatted_message.to_string(), + message: formatted_message, code: None, level: subdiag.level.to_str(), spans: DiagnosticSpan::from_multispan(&subdiag.span, args, je), diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 98a5b32e5d902..0951189dd3256 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -617,8 +617,7 @@ impl<'a> DiagCtxtHandle<'a> { DelayedBug => { return self.dcx.inner.borrow_mut().emit_diagnostic(diag, self.tainted_with_errors); } - ForceWarning | Warning | Note | OnceNote | Help | OnceHelp | FailureNote | Allow - | Expect => None, + ForceWarning | Warning | Note | Help | FailureNote | Allow | Expect => None, }; // FIXME(Centril, #69537): Consider reintroducing panic on overwriting a stashed diagnostic @@ -1300,7 +1299,6 @@ impl DiagCtxtInner { } } Note | Help | FailureNote => {} - OnceNote | OnceHelp => panic!("bad level: {:?}", diagnostic.level), Allow => { // Nothing emitted for allowed lints. if diagnostic.has_future_breakage() { @@ -1359,8 +1357,11 @@ impl DiagCtxtInner { let not_yet_emitted = |sub: &mut Subdiag| { debug!(?sub); - if sub.level != OnceNote && sub.level != OnceHelp { - return true; + match sub.level { + Sublevel::Error | Sublevel::Warning | Sublevel::Note | Sublevel::Help => { + return true; + } + Sublevel::OnceNote | Sublevel::OnceHelp => {} } let mut hasher = StableHasher::new(); sub.hash(&mut hasher); @@ -1371,7 +1372,7 @@ impl DiagCtxtInner { diagnostic.children.retain_mut(not_yet_emitted); if already_emitted { let msg = "duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`"; - diagnostic.sub(Note, msg, MultiSpan::new()); + diagnostic.sub(Sublevel::Note, msg, MultiSpan::new()); } if is_error { @@ -1521,7 +1522,7 @@ impl DiagCtxtInner { let msg = msg!( "`flushed_delayed` got diagnostic with level {$level}, instead of the expected `DelayedBug`" ).arg("level", bug.level).format(); - bug.sub(Note, msg, bug.span.primary_span().unwrap().into()); + bug.sub(Sublevel::Note, msg, bug.span.primary_span().unwrap().into()); } bug.level = Bug; @@ -1572,26 +1573,24 @@ impl DelayedDiagInner { .arg("emitted_at", diag.emitted_at.clone()) .arg("note", self.note) .format(); - diag.sub(Note, msg, diag.span.primary_span().unwrap_or(DUMMY_SP).into()); + diag.sub(Sublevel::Note, msg, diag.span.primary_span().unwrap_or(DUMMY_SP).into()); diag } } -/// | Level | is_error | EmissionGuarantee | Top-level | Sub | Used in lints? -/// | ----- | -------- | ----------------- | --------- | --- | -------------- -/// | Bug | yes | BugAbort | yes | - | - -/// | Fatal | yes | FatalAbort/FatalError[^star] | yes | - | - -/// | Error | yes | ErrorGuaranteed | yes | - | yes -/// | DelayedBug | yes | ErrorGuaranteed | yes | - | - -/// | ForceWarning | - | () | yes | - | lint-only -/// | Warning | - | () | yes | yes | yes -/// | Note | - | () | rare | yes | - -/// | OnceNote | - | () | - | yes | lint-only -/// | Help | - | () | rare | yes | - -/// | OnceHelp | - | () | - | yes | lint-only -/// | FailureNote | - | () | rare | - | - -/// | Allow | - | () | yes | - | lint-only -/// | Expect | - | () | yes | - | lint-only +/// | Level | is_error | EmissionGuarantee | Top-level | Used in lints? +/// | ----- | -------- | ----------------- | --------- | -------------- +/// | Bug | yes | BugAbort | yes | - +/// | Fatal | yes | FatalAbort/FatalError[^star] | yes | - +/// | Error | yes | ErrorGuaranteed | yes | yes +/// | DelayedBug | yes | ErrorGuaranteed | yes | - +/// | ForceWarning | - | () | yes | lint-only +/// | Warning | - | () | yes | yes +/// | Note | - | () | rare | - +/// | Help | - | () | rare | - +/// | FailureNote | - | () | rare | - +/// | Allow | - | () | yes | lint-only +/// | Expect | - | () | yes | lint-only /// /// [^star]: `FatalAbort` normally, `FatalError` in the non-aborting "almost fatal" case that is /// occasionally used. @@ -1629,15 +1628,9 @@ pub enum Level { /// A message giving additional context. Note, - /// A note that is only emitted once. - OnceNote, - /// A message suggesting how to fix something. Help, - /// A help that is only emitted once. - OnceHelp, - /// Similar to `Note`, but used in cases where compilation has failed. When printed for human /// consumption, it doesn't have any kind of `note:` label. FailureNote, @@ -1656,30 +1649,13 @@ impl fmt::Display for Level { } impl Level { - fn color(self) -> anstyle::Style { - match self { - Bug | Fatal | Error | DelayedBug => AnsiColor::BrightRed.on_default(), - ForceWarning | Warning => { - if cfg!(windows) { - AnsiColor::BrightYellow.on_default() - } else { - AnsiColor::Yellow.on_default() - } - } - Note | OnceNote => AnsiColor::BrightGreen.on_default(), - Help | OnceHelp => AnsiColor::BrightCyan.on_default(), - FailureNote => anstyle::Style::new(), - Allow | Expect => unreachable!(), - } - } - pub fn to_str(self) -> &'static str { match self { Bug | DelayedBug => "error: internal compiler error", Fatal | Error => "error", ForceWarning | Warning => "warning", - Note | OnceNote => "note", - Help | OnceHelp => "help", + Note => "note", + Help => "help", FailureNote => "failure-note", Allow | Expect => unreachable!(), } @@ -1696,22 +1672,46 @@ impl IntoDiagArg for Level { } } +/// The level for a subdiagnostic. +#[derive(Copy, PartialEq, Eq, Clone, Hash, Debug, Encodable, Decodable)] +pub enum Sublevel { + /// See `Level::Error`. + /// + /// The compiler never uses this level in a subdiagnostic, but it can be produced by proc + /// macros. See tests/ui/proc-macro/sub-error-diag.rs for details. + Error, + + /// See `Level::Warning`. + Warning, + + /// See `Level::Note`. + Note, + + /// A note that is only emitted once. + OnceNote, + + /// See `Level::Help`. + Help, + + /// A help that is only emitted once. + OnceHelp, +} + +impl Sublevel { + pub fn to_str(self) -> &'static str { + match self { + Sublevel::Error => "error", + Sublevel::Warning => "warning", + Sublevel::Note | Sublevel::OnceNote => "note", + Sublevel::Help | Sublevel::OnceHelp => "help", + } + } +} + #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, Encodable, Decodable)] pub enum Style { - MainHeaderMsg, - HeaderMsg, - LineAndColumn, - LineNumber, - Quotation, - UnderlinePrimary, - UnderlineSecondary, - LabelPrimary, - LabelSecondary, NoStyle, - Level(Level), Highlight, - Addition, - Removal, } // FIXME(eddyb) this doesn't belong here AFAICT, should be moved to callsite. diff --git a/compiler/rustc_expand/src/module.rs b/compiler/rustc_expand/src/module.rs index febe5e16cf446..d49eae9830b3f 100644 --- a/compiler/rustc_expand/src/module.rs +++ b/compiler/rustc_expand/src/module.rs @@ -4,12 +4,11 @@ use std::path::{self, Path, PathBuf}; use rustc_ast::{AttrVec, Attribute, Inline, Item, ModSpans}; use rustc_attr_parsing::template; use rustc_attr_parsing::validate_attr::emit_malformed_attribute; -use rustc_errors::{Diag, ErrorGuaranteed}; +use rustc_errors::{Diag, ErrorGuaranteed, FatalError}; use rustc_parse::lexer::StripTokens; use rustc_parse::{exp, new_parser_from_file, unwrap_or_emit_fatal}; use rustc_session::Session; use rustc_session::parse::ParseSess; -use rustc_span::fatal_error::FatalError; use rustc_span::{Ident, Span, sym}; use thin_vec::ThinVec; diff --git a/compiler/rustc_expand/src/proc_macro_server.rs b/compiler/rustc_expand/src/proc_macro_server.rs index c522626b39562..c0a9a43c64cdf 100644 --- a/compiler/rustc_expand/src/proc_macro_server.rs +++ b/compiler/rustc_expand/src/proc_macro_server.rs @@ -414,6 +414,18 @@ impl ToInternal for Level { } } +impl ToInternal for Level { + fn to_internal(self) -> rustc_errors::Sublevel { + match self { + Level::Error => rustc_errors::Sublevel::Error, + Level::Warning => rustc_errors::Sublevel::Warning, + Level::Note => rustc_errors::Sublevel::Note, + Level::Help => rustc_errors::Sublevel::Help, + _ => unreachable!("unknown proc_macro::Level variant: {:?}", self), + } + } +} + fn cancel_diags_into_string(diags: Vec>) -> String { let mut messages = diags.into_iter().flat_map(Diag::cancel_into_message); let msg = messages.next().expect("no diagnostic has a message"); diff --git a/compiler/rustc_mir_build/src/builder/custom/parse/instruction.rs b/compiler/rustc_mir_build/src/builder/custom/parse/instruction.rs index 84203c5caefea..5454a7bef67a7 100644 --- a/compiler/rustc_mir_build/src/builder/custom/parse/instruction.rs +++ b/compiler/rustc_mir_build/src/builder/custom/parse/instruction.rs @@ -1,8 +1,10 @@ use rustc_abi::{FieldIdx, VariantIdx}; +use rustc_hir::Safety; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::*; use rustc_middle::thir::*; use rustc_middle::ty; +use rustc_middle::ty::adjustment::PointerCoercion; use rustc_middle::ty::cast::mir_cast_kind; use rustc_span::{Span, Spanned}; @@ -205,6 +207,97 @@ impl<'a, 'tcx> ParseCtxt<'a, 'tcx> { ) } + fn parse_cast_fn_ptr_safety(&self, expr_id: ExprId) -> PResult { + parse_by_kind!(self, expr_id, _, "function pointer safety", + @variant(mir_cast_fn_ptr_safety, Safe) => { + Ok(Safety::Safe) + }, + @variant(mir_cast_fn_ptr_safety, Unsafe) => { + Ok(Safety::Unsafe) + }, + ) + } + + fn parse_cast_pointer_coercion(&self, expr_id: ExprId) -> PResult { + parse_by_kind!(self, expr_id, expr, "pointer coercion kind", + @variant(mir_cast_ptr_coercion, ReifyFnPointer) => { + let ExprKind::Adt(AdtExpr { fields, .. }) = &expr.kind else { + unreachable!("already matched") + }; + Ok(PointerCoercion::ReifyFnPointer( + self.parse_cast_fn_ptr_safety(fields[0].expr)?, + )) + }, + @variant(mir_cast_ptr_coercion, UnsafeFnPointer) => { + Ok(PointerCoercion::UnsafeFnPointer) + }, + @variant(mir_cast_ptr_coercion, ClosureFnPointer) => { + let ExprKind::Adt(AdtExpr { fields, .. }) = &expr.kind else { + unreachable!("already matched") + }; + Ok(PointerCoercion::ClosureFnPointer( + self.parse_cast_fn_ptr_safety(fields[0].expr)?, + )) + }, + @variant(mir_cast_ptr_coercion, MutToConstPointer) => { + Ok(PointerCoercion::MutToConstPointer) + }, + @variant(mir_cast_ptr_coercion, ArrayToPointer) => { + Ok(PointerCoercion::ArrayToPointer) + }, + @variant(mir_cast_ptr_coercion, UnsizePointee) => { + Ok(PointerCoercion::Unsize) + }, + ) + } + + fn parse_cast_kind(&self, expr_id: ExprId) -> PResult { + parse_by_kind!(self, expr_id, expr, "cast kind", + @variant(mir_cast_kind, PointerExposeProvenance) => { + Ok(CastKind::PointerExposeProvenance) + }, + @variant(mir_cast_kind, PointerWithExposedProvenance) => { + Ok(CastKind::PointerWithExposedProvenance) + }, + @variant(mir_cast_kind, IntToInt) => { + Ok(CastKind::IntToInt) + }, + @variant(mir_cast_kind, FloatToInt) => { + Ok(CastKind::FloatToInt) + }, + @variant(mir_cast_kind, FloatToFloat) => { + Ok(CastKind::FloatToFloat) + }, + @variant(mir_cast_kind, IntToFloat) => { + Ok(CastKind::IntToFloat) + }, + @variant(mir_cast_kind, PtrToPtr) => { + Ok(CastKind::PtrToPtr) + }, + @variant(mir_cast_kind, FnPtrToPtr) => { + Ok(CastKind::FnPtrToPtr) + }, + @variant(mir_cast_kind, Transmute) => { + Ok(CastKind::Transmute) + }, + @variant(mir_cast_kind, BoxDerefTransmute) => { + Ok(CastKind::BoxDerefTransmute) + }, + @variant(mir_cast_kind, Subtype) => { + Ok(CastKind::Subtype) + }, + @variant(mir_cast_kind, PointerCoercion) => { + let ExprKind::Adt(AdtExpr { fields, .. }) = &expr.kind else { + unreachable!("already matched") + }; + Ok(CastKind::PointerCoercion( + self.parse_cast_pointer_coercion(fields[0].expr)?, + CoercionSource::AsCast, + )) + }, + ) + } + fn parse_rvalue(&self, expr_id: ExprId) -> PResult> { parse_by_kind!(self, expr_id, expr, "rvalue", @call(mir_discriminant, args) => self.parse_place(args[0]).map(Rvalue::Discriminant), @@ -221,6 +314,10 @@ impl<'a, 'tcx> ParseCtxt<'a, 'tcx> { let kind = CastKind::PointerCoercion(ty::adjustment::PointerCoercion::Unsize, CoercionSource::AsCast); Ok(Rvalue::Cast(kind, source, expr.ty)) }, + @call(mir_cast, args) => { + let source = self.parse_operand(args[0])?; + Ok(Rvalue::Cast(self.parse_cast_kind(args[1])?, source, expr.ty)) + }, @call(mir_checked, args) => { parse_by_kind!(self, args[0], _, "binary op", ExprKind::Binary { op, lhs, rhs } => { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 17ffb52f4b333..871d5aa97e94e 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -163,6 +163,7 @@ symbols! { Arc, ArcWeak, Array, + ArrayToPointer, AsMut, AsRef, AssertParamIsClone, @@ -176,6 +177,7 @@ symbols! { Bool, Borrow, BorrowMut, + BoxDerefTransmute, Break, BuildHasher, CStr, @@ -187,6 +189,7 @@ symbols! { Cleanup, Client, Clone, + ClosureFnPointer, CoercePointee, CoercePointeeValidated, CoerceShared, @@ -219,11 +222,14 @@ symbols! { ExternC, ExternRust, Float, + FloatToFloat, + FloatToInt, FmtArgumentsNew, Fn, FnMut, FnOnce, FnPtr, + FnPtrToPtr, Formatter, Forward, Found, @@ -239,6 +245,8 @@ symbols! { IndexOutput, Input, Int, + IntToFloat, + IntToInt, Into, IntoAsyncIterator, IntoFuture, @@ -255,6 +263,7 @@ symbols! { Lifetime, LintPass, LocalKey, + MutToConstPointer, Mutex, MutexGuard, Named, @@ -274,7 +283,11 @@ symbols! { PinDerefMutHelper, PinMacroHelper, Pointer, + PointerCoercion, + PointerExposeProvenance, + PointerWithExposedProvenance, Poll, + PtrToPtr, Range, RangeCopy, RangeFrom, @@ -294,6 +307,7 @@ symbols! { Reborrow, RefCell, Reference, + ReifyFnPointer, Relaxed, Release, Result, @@ -306,6 +320,8 @@ symbols! { RwLock, RwLockReadGuard, RwLockWriteGuard, + Safe, + Safety, SelfTy, Send, SeqCst, @@ -320,12 +336,14 @@ symbols! { String, Struct, StructuralPartialEq, + Subtype, SymbolIntern, Sync, SyncUnsafeCell, Target, This, TokenStream, + Transmute, TrivialClone, Try, TryCaptureGeneric, @@ -339,7 +357,10 @@ symbols! { Type, Union, Unresolved, + Unsafe, + UnsafeFnPointer, Unsize, + UnsizePointee, Vec, Wrapper, _DECLS, @@ -1321,6 +1342,10 @@ symbols! { mir_assume, mir_basic_block, mir_call, + mir_cast, + mir_cast_fn_ptr_safety, + mir_cast_kind, + mir_cast_ptr_coercion, mir_cast_ptr_to_ptr, mir_cast_transmute, mir_cast_unsize, diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index d8a22e745bcc2..5b029d6fad5d4 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -10,8 +10,8 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::unord::UnordSet; use rustc_errors::codes::*; use rustc_errors::{ - Applicability, Diag, ErrorGuaranteed, Level, MultiSpan, StashKey, StringPart, Suggestions, msg, - pluralize, struct_span_code_err, + Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey, StringPart, Sublevel, Suggestions, + msg, pluralize, struct_span_code_err, }; use rustc_hir::attrs::diagnostic::CustomDiagnostic; use rustc_hir::attrs::lang_items::LangItem; @@ -2286,7 +2286,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } if let [child, ..] = &err.children[..] - && child.level == Level::Help + && child.level == Sublevel::Help && let Some(line) = child.messages.get(0) && let Some(line) = line.0.as_str() && line.starts_with("the trait") diff --git a/library/core/src/intrinsics/mir.rs b/library/core/src/intrinsics/mir.rs index dce7bf681a7af..929e56872674d 100644 --- a/library/core/src/intrinsics/mir.rs +++ b/library/core/src/intrinsics/mir.rs @@ -508,6 +508,41 @@ define!( fn __debuginfo(name: &'static str, s: T) ); +#[rustc_diagnostic_item = "mir_cast_fn_ptr_safety"] +pub enum Safety { + Safe, + Unsafe, +} +#[rustc_diagnostic_item = "mir_cast_ptr_coercion"] +pub enum PointerCoercion { + ReifyFnPointer(Safety), + UnsafeFnPointer, + ClosureFnPointer(Safety), + MutToConstPointer, + ArrayToPointer, + UnsizePointee, +} +#[rustc_diagnostic_item = "mir_cast_kind"] +pub enum CastKind { + PointerExposeProvenance, + PointerWithExposedProvenance, + IntToInt, + FloatToInt, + FloatToFloat, + IntToFloat, + PtrToPtr, + FnPtrToPtr, + Transmute, + BoxDerefTransmute, + Subtype, + PointerCoercion(PointerCoercion), +} +define!( + "mir_cast", + /// Emits a cast of the specified kind. + fn Cast(operand: T, kind: CastKind) -> U +); + /// Macro for generating custom MIR. /// /// See the module documentation for syntax details. This macro is not magic - it only transforms diff --git a/library/std/src/sys/pal/sgx/abi/usercalls/mod.rs b/library/std/src/sys/pal/sgx/abi/usercalls/mod.rs index 2378028ccab92..236b918bfd063 100644 --- a/library/std/src/sys/pal/sgx/abi/usercalls/mod.rs +++ b/library/std/src/sys/pal/sgx/abi/usercalls/mod.rs @@ -1,6 +1,6 @@ +use crate::arch::x86_64::_rdrand64_step; use crate::cmp; use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut}; -use crate::random::random; use crate::time::{Duration, Instant}; pub(crate) mod alloc; @@ -167,6 +167,12 @@ pub fn exit(panic: bool) -> ! { /// Usercall `wait`. See the ABI documentation for more information. #[unstable(feature = "sgx_platform", issue = "56975")] pub fn wait(event_mask: u64, mut timeout: u64) -> io::Result { + fn try_rdrand() -> Option { + let mut val: u64 = 0; + // SAFETY: the rdrand feature is enabled on SGX targets + if unsafe { _rdrand64_step(&mut val) } == 1 { Some(val) } else { None } + } + if timeout != WAIT_NO && timeout != WAIT_INDEFINITE { // We don't want people to rely on accuracy of timeouts to make // security decisions in an SGX enclave. That's why we add a random @@ -175,9 +181,14 @@ pub fn wait(event_mask: u64, mut timeout: u64) -> io::Result { // to make things work in other cases. Note that in the SGX threat // model the enclave runner which is serving the wait usercall is not // trusted to ensure accurate timeouts. + // + // Since the random timeout is only intended as defense-in-depth + // protection at development/testing time, it's ok to continue if + // randomness generation fails. if let Ok(timeout_signed) = i64::try_from(timeout) { let tenth = timeout_signed / 10; - let deviation = random::(..).checked_rem(tenth).unwrap_or(0); + let deviation = + try_rdrand().and_then(|rnd| (rnd as i64).checked_rem(tenth)).unwrap_or(0); timeout = timeout_signed.saturating_add(deviation) as _; } } diff --git a/library/std/src/sys/pal/sgx/waitqueue/mod.rs b/library/std/src/sys/pal/sgx/waitqueue/mod.rs index 41d1413fcdee9..7f7f20116308d 100644 --- a/library/std/src/sys/pal/sgx/waitqueue/mod.rs +++ b/library/std/src/sys/pal/sgx/waitqueue/mod.rs @@ -14,16 +14,16 @@ mod tests; mod spin_mutex; -mod unsafe_list; use fortanix_sgx_abi::{EV_UNPARK, Tcs, WAIT_INDEFINITE}; -pub use self::spin_mutex::{SpinMutex, SpinMutexGuard, try_lock_or_false}; -use self::unsafe_list::{UnsafeList, UnsafeListEntry}; +pub use self::spin_mutex::{SpinMutex, SpinMutexGuard}; use super::abi::{thread, usercalls}; use crate::num::NonZero; use crate::ops::{Deref, DerefMut}; use crate::panic::{self, AssertUnwindSafe}; +use crate::pin::Pin; +use crate::sys::sync::unsafe_list::{UnsafeList, UnsafeListEntry}; use crate::time::Duration; /// An queue entry in a `WaitQueue`. @@ -38,24 +38,41 @@ struct WaitEntry { /// queue and the data are synchronized, since the type itself is not `Sync`. /// /// Consumers of this API should use a synchronization primitive for shared -/// access, such as `SpinMutex`. -#[derive(Default)] +/// access. `WaitVariable::new` is the only constructor and provides that +/// with `SpinMutex`. pub struct WaitVariable { queue: WaitQueue, lock: T, } impl WaitVariable { - pub const fn new(var: T) -> Self { - WaitVariable { queue: WaitQueue::new(), lock: var } - } - pub fn lock_var(&self) -> &T { &self.lock } - pub fn lock_var_mut(&mut self) -> &mut T { - &mut self.lock + pub fn lock_var_mut(self: Pin<&mut Self>) -> &mut T { + // SAFETY: `lock` is not structurally pinned: a pinned `WaitVariable` + // makes no promise that `T` is pinned. + unsafe { &mut self.get_unchecked_mut().lock } + } + + fn queue(self: Pin<&mut Self>) -> Pin<&mut WaitQueue> { + // SAFETY: `queue` is structurally pinned: a pinned `WaitVariable` + // pins it, and it is never moved out of it. + unsafe { self.map_unchecked_mut(|this| &mut this.queue) } + } + + /// Creates a mutex-protected `WaitVariable` on the heap, with its queue's + /// list initialized. Initialization makes the list self-referential and + /// happens before pinning: only the `Box` pointer is moved into the + /// `Pin`, the heap allocation itself never moves. + pub fn new(value: T) -> Pin>>> { + // SAFETY: `init` is called below, before the queue is otherwise used + // or dropped. + let queue = unsafe { WaitQueue::new() }; + let result = Box::new(SpinMutex::new(WaitVariable { queue, lock: value })); + result.lock().queue.inner.init(); + Box::into_pin(result) } } @@ -68,7 +85,7 @@ pub enum NotifiedTcs { /// An RAII guard that will notify a set of target threads as well as unlock /// a mutex on drop. pub struct WaitGuard<'a, T: 'a> { - mutex_guard: Option>>, + mutex_guard: Option>>>, notified_tcs: NotifiedTcs, } @@ -79,6 +96,27 @@ pub struct WaitGuard<'a, T: 'a> { /// safe because the waiting thread will not return from that stack frame until /// after it is notified. The notifying thread ensures to clean up any /// references to the list entries before sending the wakeup event. +// The safety requirements of `UnsafeList` are upheld as follows: +// +// * All list operations are performed while holding the lock of the +// `SpinMutex` around the `WaitVariable` containing the list. +// * A waiting thread pushes a stack-allocated entry and does not invalidate +// it while it is in the list: it only accesses the entry through the +// reference `push` returned, reading `wake` under the `WaitEntry`'s own +// `SpinMutex`. +// * `push` -> `pop`: a notifying thread pops the entry and sets `wake` under +// the `WaitEntry`'s `SpinMutex`; when that mutex is released, the thread +// will no longer access the entry (guaranteed by the mutex guard). The +// waiting thread only returns from the stack frame containing the entry +// once it observes `wake == true` under that same mutex, so the entry is +// only deallocated after the notifying thread's last access to it. +// * `push` -> `remove`: on a timeout, `wait_timeout` re-acquires the queue +// lock and checks `wake`: the entry is still in the list if and only if +// `wake` is not set, because notifying threads always `pop` an entry +// before setting its `wake`. Only if the entry is still in the list is it +// removed. +// * Besides as described, no other exclusive references to the entry are +// taken. pub struct WaitQueue { // We use an inner Mutex here to protect the data in the face of spurious // wakeups. @@ -86,14 +124,8 @@ pub struct WaitQueue { } unsafe impl Send for WaitQueue {} -impl Default for WaitQueue { - fn default() -> Self { - Self::new() - } -} - impl<'a, T> Deref for WaitGuard<'a, T> { - type Target = SpinMutexGuard<'a, WaitVariable>; + type Target = Pin>>; fn deref(&self) -> &Self::Target { self.mutex_guard.as_ref().unwrap() @@ -118,8 +150,24 @@ impl<'a, T> Drop for WaitGuard<'a, T> { } impl WaitQueue { - pub const fn new() -> Self { - WaitQueue { inner: UnsafeList::new() } + /// Creates a new queue. + /// + /// # Safety + /// + /// The caller must initialize the queue's list (`UnsafeList::init`) + /// before any other use of the queue, including dropping it. + /// `WaitVariable::new`, the sole constructor of the containing + /// structure, does this. + pub const unsafe fn new() -> Self { + // SAFETY: the caller upholds `UnsafeList::new`'s contract (see this + // function's safety requirements). + WaitQueue { inner: unsafe { UnsafeList::new() } } + } + + fn inner(self: Pin<&mut Self>) -> Pin<&mut UnsafeList>> { + // SAFETY: `inner` is structurally pinned: a pinned `WaitQueue` pins + // it, and it is never moved out of it. + unsafe { self.map_unchecked_mut(|this| &mut this.inner) } } /// Adds the calling thread to the `WaitVariable`'s wait queue, then wait @@ -127,14 +175,17 @@ impl WaitQueue { /// /// This function does not return until this thread has been awoken. When `before_wait` panics, /// this function will abort. - pub fn wait(mut guard: SpinMutexGuard<'_, WaitVariable>, before_wait: F) { + pub fn wait( + mut guard: Pin>>, + before_wait: F, + ) { // very unsafe: check requirements of UnsafeList::push unsafe { let mut entry = UnsafeListEntry::new(SpinMutex::new(WaitEntry { tcs: thread::current(), wake: false, })); - let entry = guard.queue.inner.push(&mut entry); + let entry = guard.as_mut().queue().inner().push(&mut entry); drop(guard); if let Err(_e) = panic::catch_unwind(AssertUnwindSafe(|| before_wait())) { rtabort!("Panic before wait on wakeup event") @@ -155,7 +206,7 @@ impl WaitQueue { /// If not, it will remove the calling thread from the wait queue. /// When `before_wait` panics, this function will abort. pub fn wait_timeout( - lock: &SpinMutex>, + lock: Pin<&SpinMutex>>, timeout: Duration, before_wait: F, ) -> bool { @@ -165,7 +216,7 @@ impl WaitQueue { tcs: thread::current(), wake: false, })); - let entry_lock = lock.lock().queue.inner.push(&mut entry); + let entry_lock = lock.lock_pinned().as_mut().queue().inner().push(&mut entry); if let Err(_e) = panic::catch_unwind(AssertUnwindSafe(|| before_wait())) { rtabort!("Panic before wait on wakeup event or timeout") } @@ -173,11 +224,11 @@ impl WaitQueue { // acquire the wait queue's lock first to avoid deadlock // and ensure no other function can simultaneously access the list // (e.g., `notify_one` or `notify_all`) - let mut guard = lock.lock(); + let mut guard = lock.lock_pinned(); let success = entry_lock.lock().wake; if !success { // nobody is waking us up, so remove our entry from the wait queue. - guard.queue.inner.remove(&mut entry); + guard.as_mut().queue().inner().remove(&mut entry); } success } @@ -189,14 +240,14 @@ impl WaitQueue { /// If a waiter is found, a `WaitGuard` is returned which will notify the /// waiter when it is dropped. pub fn notify_one( - mut guard: SpinMutexGuard<'_, WaitVariable>, - ) -> Result, SpinMutexGuard<'_, WaitVariable>> { + mut guard: Pin>>, + ) -> Result, Pin>>> { // SAFETY: lifetime of the pop() return value is limited to the map // closure (The closure return value is 'static). The underlying // stack frame won't be freed until after the lock on the queue is released // (i.e., `guard` is dropped). unsafe { - let tcs = guard.queue.inner.pop().map(|entry| -> Tcs { + let tcs = guard.as_mut().queue().inner().pop().map(|entry| -> Tcs { let mut entry_guard = entry.lock(); entry_guard.wake = true; entry_guard.tcs @@ -216,14 +267,14 @@ impl WaitQueue { /// If at least one waiter is found, a `WaitGuard` is returned which will /// notify all waiters when it is dropped. pub fn notify_all( - mut guard: SpinMutexGuard<'_, WaitVariable>, - ) -> Result, SpinMutexGuard<'_, WaitVariable>> { + mut guard: Pin>>, + ) -> Result, Pin>>> { // SAFETY: lifetime of the pop() return values are limited to the // while loop body. The underlying stack frames won't be freed until // after the lock on the queue is released (i.e., `guard` is dropped). unsafe { let mut count = 0; - while let Some(entry) = guard.queue.inner.pop() { + while let Some(entry) = guard.as_mut().queue().inner().pop() { count += 1; let mut entry_guard = entry.lock(); entry_guard.wake = true; diff --git a/library/std/src/sys/pal/sgx/waitqueue/spin_mutex.rs b/library/std/src/sys/pal/sgx/waitqueue/spin_mutex.rs index 73c7a101d601d..f052c73115015 100644 --- a/library/std/src/sys/pal/sgx/waitqueue/spin_mutex.rs +++ b/library/std/src/sys/pal/sgx/waitqueue/spin_mutex.rs @@ -7,6 +7,7 @@ mod tests; use crate::cell::UnsafeCell; use crate::hint; use crate::ops::{Deref, DerefMut}; +use crate::pin::Pin; use crate::sync::atomic::{Atomic, AtomicBool, Ordering}; #[derive(Default)] @@ -52,11 +53,19 @@ impl SpinMutex { None } } -} -/// Lock the Mutex or return false. -pub macro try_lock_or_false($e:expr) { - if let Some(v) = $e.try_lock() { v } else { return false } + #[inline(always)] + pub fn lock_pinned(self: Pin<&Self>) -> Pin> { + // SAFETY: `value` is structurally pinned: a pinned mutex pins its + // contents, and `SpinMutexGuard` never moves the value. + unsafe { Pin::new_unchecked(self.get_ref().lock()) } + } + + #[inline(always)] + pub fn try_lock_pinned(self: Pin<&Self>) -> Option>> { + // SAFETY: see `lock_pinned` + self.get_ref().try_lock().map(|guard| unsafe { Pin::new_unchecked(guard) }) + } } impl<'a, T> Deref for SpinMutexGuard<'a, T> { diff --git a/library/std/src/sys/pal/sgx/waitqueue/tests.rs b/library/std/src/sys/pal/sgx/waitqueue/tests.rs index bf91fdd08ed54..05ade6b0b5d17 100644 --- a/library/std/src/sys/pal/sgx/waitqueue/tests.rs +++ b/library/std/src/sys/pal/sgx/waitqueue/tests.rs @@ -4,14 +4,14 @@ use crate::thread; #[test] fn queue() { - let wq = Arc::new(SpinMutex::>::default()); + let wq = Arc::new(WaitVariable::new(())); let wq2 = wq.clone(); - let locked = wq.lock(); + let locked = (*wq).as_ref().lock_pinned(); let t1 = thread::spawn(move || { // if we obtain the lock, the main thread should be waiting - assert!(WaitQueue::notify_one(wq2.lock()).is_ok()); + assert!(WaitQueue::notify_one((*wq2).as_ref().lock_pinned()).is_ok()); }); WaitQueue::wait(locked, || {}); diff --git a/library/std/src/sys/pal/sgx/waitqueue/unsafe_list.rs b/library/std/src/sys/pal/sgx/waitqueue/unsafe_list.rs deleted file mode 100644 index c736cab576e4d..0000000000000 --- a/library/std/src/sys/pal/sgx/waitqueue/unsafe_list.rs +++ /dev/null @@ -1,156 +0,0 @@ -//! A doubly-linked list where callers are in charge of memory allocation -//! of the nodes in the list. - -#[cfg(test)] -mod tests; - -use crate::mem; -use crate::ptr::NonNull; - -pub struct UnsafeListEntry { - next: NonNull>, - prev: NonNull>, - value: Option, -} - -impl UnsafeListEntry { - fn dummy() -> Self { - UnsafeListEntry { next: NonNull::dangling(), prev: NonNull::dangling(), value: None } - } - - pub fn new(value: T) -> Self { - UnsafeListEntry { value: Some(value), ..Self::dummy() } - } -} - -// WARNING: self-referential struct! -pub struct UnsafeList { - head_tail: NonNull>, - head_tail_entry: Option>, -} - -impl UnsafeList { - pub const fn new() -> Self { - unsafe { UnsafeList { head_tail: NonNull::new_unchecked(1 as _), head_tail_entry: None } } - } - - /// # Safety - unsafe fn init(&mut self) { - if self.head_tail_entry.is_none() { - self.head_tail_entry = Some(UnsafeListEntry::dummy()); - // SAFETY: `head_tail_entry` must be non-null, which it is because we assign it above. - self.head_tail = - unsafe { NonNull::new_unchecked(self.head_tail_entry.as_mut().unwrap()) }; - // SAFETY: `self.head_tail` must meet all requirements for a mutable reference. - unsafe { self.head_tail.as_mut() }.next = self.head_tail; - unsafe { self.head_tail.as_mut() }.prev = self.head_tail; - } - } - - pub fn is_empty(&self) -> bool { - if self.head_tail_entry.is_some() { - let first = unsafe { self.head_tail.as_ref() }.next; - if first == self.head_tail { - // ,-------> /---------\ next ---, - // | |head_tail| | - // `--- prev \---------/ <-------` - // SAFETY: `self.head_tail` must meet all requirements for a reference. - unsafe { rtassert!(self.head_tail.as_ref().prev == first) }; - true - } else { - false - } - } else { - true - } - } - - /// Pushes an entry onto the back of the list. - /// - /// # Safety - /// - /// The entry must remain allocated until the entry is removed from the - /// list AND the caller who popped is done using the entry. Special - /// care must be taken in the caller of `push` to ensure unwinding does - /// not destroy the stack frame containing the entry. - pub unsafe fn push<'a>(&mut self, entry: &'a mut UnsafeListEntry) -> &'a T { - unsafe { self.init() }; - - // BEFORE: - // /---------\ next ---> /---------\ - // ... |prev_tail| |head_tail| ... - // \---------/ <--- prev \---------/ - // - // AFTER: - // /---------\ next ---> /-----\ next ---> /---------\ - // ... |prev_tail| |entry| |head_tail| ... - // \---------/ <--- prev \-----/ <--- prev \---------/ - let mut entry = unsafe { NonNull::new_unchecked(entry) }; - let mut prev_tail = mem::replace(&mut unsafe { self.head_tail.as_mut() }.prev, entry); - // SAFETY: `entry` must meet all requirements for a mutable reference. - unsafe { entry.as_mut() }.prev = prev_tail; - unsafe { entry.as_mut() }.next = self.head_tail; - // SAFETY: `prev_tail` must meet all requirements for a mutable reference. - unsafe { prev_tail.as_mut() }.next = entry; - // unwrap ok: always `Some` on non-dummy entries - unsafe { (*entry.as_ptr()).value.as_ref() }.unwrap() - } - - /// Pops an entry from the front of the list. - /// - /// # Safety - /// - /// The caller must make sure to synchronize ending the borrow of the - /// return value and deallocation of the containing entry. - pub unsafe fn pop<'a>(&mut self) -> Option<&'a T> { - unsafe { self.init() }; - - if self.is_empty() { - None - } else { - // BEFORE: - // /---------\ next ---> /-----\ next ---> /------\ - // ... |head_tail| |first| |second| ... - // \---------/ <--- prev \-----/ <--- prev \------/ - // - // AFTER: - // /---------\ next ---> /------\ - // ... |head_tail| |second| ... - // \---------/ <--- prev \------/ - let mut first = unsafe { self.head_tail.as_mut() }.next; - let mut second = unsafe { first.as_mut() }.next; - unsafe { self.head_tail.as_mut() }.next = second; - unsafe { second.as_mut() }.prev = self.head_tail; - unsafe { first.as_mut() }.next = NonNull::dangling(); - unsafe { first.as_mut() }.prev = NonNull::dangling(); - // unwrap ok: always `Some` on non-dummy entries - Some(unsafe { (*first.as_ptr()).value.as_ref() }.unwrap()) - } - } - - /// Removes an entry from the list. - /// - /// # Safety - /// - /// The caller must ensure that `entry` has been pushed onto `self` - /// prior to this call and has not moved since then. - pub unsafe fn remove(&mut self, entry: &mut UnsafeListEntry) { - rtassert!(!self.is_empty()); - // BEFORE: - // /----\ next ---> /-----\ next ---> /----\ - // ... |prev| |entry| |next| ... - // \----/ <--- prev \-----/ <--- prev \----/ - // - // AFTER: - // /----\ next ---> /----\ - // ... |prev| |next| ... - // \----/ <--- prev \----/ - let mut prev = entry.prev; - let mut next = entry.next; - // SAFETY: `prev` and `next` must meet all requirements for a mutable reference.entry - unsafe { prev.as_mut() }.next = next; - unsafe { next.as_mut() }.prev = prev; - entry.next = NonNull::dangling(); - entry.prev = NonNull::dangling(); - } -} diff --git a/library/std/src/sys/pal/sgx/waitqueue/unsafe_list/tests.rs b/library/std/src/sys/pal/sgx/waitqueue/unsafe_list/tests.rs deleted file mode 100644 index c653dee17bc36..0000000000000 --- a/library/std/src/sys/pal/sgx/waitqueue/unsafe_list/tests.rs +++ /dev/null @@ -1,105 +0,0 @@ -use super::*; -use crate::cell::Cell; - -/// # Safety -/// List must be valid. -unsafe fn assert_empty(list: &mut UnsafeList) { - assert!(unsafe { list.pop() }.is_none(), "assertion failed: list is not empty"); -} - -#[test] -fn init_empty() { - unsafe { - assert_empty(&mut UnsafeList::::new()); - } -} - -#[test] -fn push_pop() { - unsafe { - let mut node = UnsafeListEntry::new(1234); - let mut list = UnsafeList::new(); - assert_eq!(list.push(&mut node), &1234); - assert_eq!(list.pop().unwrap(), &1234); - assert_empty(&mut list); - } -} - -#[test] -fn push_remove() { - unsafe { - let mut node = UnsafeListEntry::new(1234); - let mut list = UnsafeList::new(); - assert_eq!(list.push(&mut node), &1234); - list.remove(&mut node); - assert_empty(&mut list); - } -} - -#[test] -fn push_remove_pop() { - unsafe { - let mut node1 = UnsafeListEntry::new(11); - let mut node2 = UnsafeListEntry::new(12); - let mut node3 = UnsafeListEntry::new(13); - let mut node4 = UnsafeListEntry::new(14); - let mut node5 = UnsafeListEntry::new(15); - let mut list = UnsafeList::new(); - assert_eq!(list.push(&mut node1), &11); - assert_eq!(list.push(&mut node2), &12); - assert_eq!(list.push(&mut node3), &13); - assert_eq!(list.push(&mut node4), &14); - assert_eq!(list.push(&mut node5), &15); - - list.remove(&mut node1); - assert_eq!(list.pop().unwrap(), &12); - list.remove(&mut node3); - assert_eq!(list.pop().unwrap(), &14); - list.remove(&mut node5); - assert_empty(&mut list); - - assert_eq!(list.push(&mut node1), &11); - assert_eq!(list.pop().unwrap(), &11); - assert_empty(&mut list); - - assert_eq!(list.push(&mut node3), &13); - assert_eq!(list.push(&mut node4), &14); - list.remove(&mut node3); - list.remove(&mut node4); - assert_empty(&mut list); - } -} - -#[test] -fn complex_pushes_pops() { - unsafe { - let mut node1 = UnsafeListEntry::new(1234); - let mut node2 = UnsafeListEntry::new(4567); - let mut node3 = UnsafeListEntry::new(9999); - let mut node4 = UnsafeListEntry::new(8642); - let mut list = UnsafeList::new(); - list.push(&mut node1); - list.push(&mut node2); - assert_eq!(list.pop().unwrap(), &1234); - list.push(&mut node3); - assert_eq!(list.pop().unwrap(), &4567); - assert_eq!(list.pop().unwrap(), &9999); - assert_empty(&mut list); - list.push(&mut node4); - assert_eq!(list.pop().unwrap(), &8642); - assert_empty(&mut list); - } -} - -#[test] -fn cell() { - unsafe { - let mut node = UnsafeListEntry::new(Cell::new(0)); - let mut list = UnsafeList::new(); - let noderef = list.push(&mut node); - assert_eq!(noderef.get(), 0); - list.pop().unwrap().set(1); - assert_empty(&mut list); - assert_eq!(noderef.get(), 1); - } -} diff --git a/library/std/src/sys/random/sgx.rs b/library/std/src/sys/random/sgx.rs index 462b19003fad2..5b834f5742615 100644 --- a/library/std/src/sys/random/sgx.rs +++ b/library/std/src/sys/random/sgx.rs @@ -3,46 +3,43 @@ use crate::arch::x86_64::{_rdrand16_step, _rdrand32_step, _rdrand64_step}; const RETRIES: u32 = 10; fn fail() -> ! { - panic!("failed to generate random data"); + rtabort!("failed to generate random data"); } fn rdrand64() -> u64 { - unsafe { - let mut ret: u64 = 0; - for _ in 0..RETRIES { - if _rdrand64_step(&mut ret) == 1 { - return ret; - } + let mut ret: u64 = 0; + for _ in 0..RETRIES { + // SAFETY: the rdrand feature is enabled on SGX targets + if unsafe { _rdrand64_step(&mut ret) } == 1 { + return ret; } - - fail(); } + + fail(); } fn rdrand32() -> u32 { - unsafe { - let mut ret: u32 = 0; - for _ in 0..RETRIES { - if _rdrand32_step(&mut ret) == 1 { - return ret; - } + let mut ret: u32 = 0; + for _ in 0..RETRIES { + // SAFETY: the rdrand feature is enabled on SGX targets + if unsafe { _rdrand32_step(&mut ret) } == 1 { + return ret; } - - fail(); } + + fail(); } fn rdrand16() -> u16 { - unsafe { - let mut ret: u16 = 0; - for _ in 0..RETRIES { - if _rdrand16_step(&mut ret) == 1 { - return ret; - } + let mut ret: u16 = 0; + for _ in 0..RETRIES { + // SAFETY: the rdrand feature is enabled on SGX targets + if unsafe { _rdrand16_step(&mut ret) } == 1 { + return ret; } - - fail(); } + + fail(); } pub fn fill_bytes(bytes: &mut [u8]) { diff --git a/library/std/src/sys/sync/condvar/sgx.rs b/library/std/src/sys/sync/condvar/sgx.rs index 2bde9d0694eda..77866bf773c65 100644 --- a/library/std/src/sys/sync/condvar/sgx.rs +++ b/library/std/src/sys/sync/condvar/sgx.rs @@ -1,3 +1,4 @@ +use crate::pin::Pin; use crate::sys::pal::waitqueue::{SpinMutex, WaitQueue, WaitVariable}; use crate::sys::sync::{Mutex, OnceBox}; use crate::time::Duration; @@ -12,24 +13,24 @@ impl Condvar { Condvar { inner: OnceBox::new() } } - fn get(&self) -> &SpinMutex> { - self.inner.get_or_init(|| Box::pin(SpinMutex::new(WaitVariable::new(())))).get_ref() + fn get(&self) -> Pin<&SpinMutex>> { + self.inner.get_or_init(|| WaitVariable::new(())) } #[inline] pub fn notify_one(&self) { - let guard = self.get().lock(); + let guard = self.get().lock_pinned(); let _ = WaitQueue::notify_one(guard); } #[inline] pub fn notify_all(&self) { - let guard = self.get().lock(); + let guard = self.get().lock_pinned(); let _ = WaitQueue::notify_all(guard); } pub unsafe fn wait(&self, mutex: &Mutex) { - let guard = self.get().lock(); + let guard = self.get().lock_pinned(); WaitQueue::wait(guard, || unsafe { mutex.unlock() }); mutex.lock() } diff --git a/library/std/src/sys/sync/mod.rs b/library/std/src/sys/sync/mod.rs index 8ee0b2649ed3d..ff675d22f1dc3 100644 --- a/library/std/src/sys/sync/mod.rs +++ b/library/std/src/sys/sync/mod.rs @@ -5,6 +5,9 @@ mod once; mod once_box; mod rwlock; mod thread_parking; +#[cfg(any(all(target_vendor = "fortanix", target_env = "sgx"), test))] +#[cfg_attr(not(all(target_vendor = "fortanix", target_env = "sgx")), allow(dead_code))] +pub(crate) mod unsafe_list; pub use condvar::Condvar; pub use mutex::Mutex; diff --git a/library/std/src/sys/sync/mutex/sgx.rs b/library/std/src/sys/sync/mutex/sgx.rs index 3eb981bc65af6..cd348a5f2e60e 100644 --- a/library/std/src/sys/sync/mutex/sgx.rs +++ b/library/std/src/sys/sync/mutex/sgx.rs @@ -1,4 +1,5 @@ -use crate::sys::pal::waitqueue::{SpinMutex, WaitQueue, WaitVariable, try_lock_or_false}; +use crate::pin::Pin; +use crate::sys::pal::waitqueue::{SpinMutex, WaitQueue, WaitVariable}; use crate::sys::sync::OnceBox; pub struct Mutex { @@ -12,20 +13,20 @@ impl Mutex { Mutex { inner: OnceBox::new() } } - fn get(&self) -> &SpinMutex> { - self.inner.get_or_init(|| Box::pin(SpinMutex::new(WaitVariable::new(false)))).get_ref() + fn get(&self) -> Pin<&SpinMutex>> { + self.inner.get_or_init(|| WaitVariable::new(false)) } #[inline] pub fn lock(&self) { - let mut guard = self.get().lock(); + let mut guard = self.get().lock_pinned(); if *guard.lock_var() { // Another thread has the lock, wait WaitQueue::wait(guard, || {}) // Another thread has passed the lock to us } else { // We are just now obtaining the lock - *guard.lock_var_mut() = true; + *guard.as_mut().lock_var_mut() = true; } } @@ -33,10 +34,10 @@ impl Mutex { pub unsafe fn unlock(&self) { // SAFETY: the mutex was locked by the current thread, so it has been // initialized already. - let guard = unsafe { self.inner.get_unchecked().get_ref().lock() }; + let guard = unsafe { self.inner.get_unchecked().lock_pinned() }; if let Err(mut guard) = WaitQueue::notify_one(guard) { // No other waiters, unlock - *guard.lock_var_mut() = false; + *guard.as_mut().lock_var_mut() = false; } else { // There was a thread waiting, just pass the lock } @@ -44,13 +45,13 @@ impl Mutex { #[inline] pub fn try_lock(&self) -> bool { - let mut guard = try_lock_or_false!(self.get()); + let Some(mut guard) = self.get().try_lock_pinned() else { return false }; if *guard.lock_var() { // Another thread has the lock false } else { // We are just now obtaining the lock - *guard.lock_var_mut() = true; + *guard.as_mut().lock_var_mut() = true; true } } diff --git a/library/std/src/sys/sync/unsafe_list.rs b/library/std/src/sys/sync/unsafe_list.rs new file mode 100644 index 0000000000000..c9d065279f294 --- /dev/null +++ b/library/std/src/sys/sync/unsafe_list.rs @@ -0,0 +1,298 @@ +//! A doubly-linked list where callers are in charge of memory allocation +//! of the nodes in the list. +//! +//! # Safety +//! +//! `UnsafeList` itself does not synchronize any of its memory accesses, so +//! callers must serialize all operations on a list, e.g. with a lock. +//! +//! While an entry passed to `push` is in the list, it must not be invalidated, +//! with one exception explained below. Invalidation of the entry, by creating a new +//! exclusive reference to it, would invalidate the pointers to the entry stored +//! in the list. The entry goes through one of two flows (see also each operation's +//! safety documentation): +//! +//! * `push` -> `pop`, usually with `pop` on another thread: the entry pointer +//! stored in the list keeps its `push`-time provenance. As mentioned, for it to +//! still be valid to dereference in `pop`, the pushing caller must not access +//! the entry in between. After `pop`, the references into `value` returned by +//! `push` and `pop` are held concurrently, possibly by two threads. This is +//! valid as they are shared references, but mutating `value` requires interior +//! mutability and synchronization. That synchronization must also ensure the +//! entry is only deallocated after the popping thread's last access to it. +//! * `push` -> `remove`, on the thread that pushed: the caller reclaims a pushed +//! entry by passing a reference to the entry to `remove`. The entry must still +//! be in the list. The caller of `remove` must create a new exclusive reference +//! to the entry, which invalidates the pointers to the entry stored in the list. +//! This is fine in this case because `remove` only overwrites those pointers, +//! and never dereferences them. + +// # Aliasing +// +// The list is self-referential: it stores pointers to its own `head_tail` +// field in the list entries' links. `UnsafePinned` is used to ensure pointer +// validity. +// +// Pointers to the other entries are derived from the exclusive reference passed +// to `push` and stay valid while the entry is in the list (see the safety +// requirements in the module documentation). Multiple immutable references may +// exist to values of entries in the list, while the links in the list may be +// mutated simultaneously. Creating mutable references to entries to update the +// links would invalidate any outstanding shared references. As such, all links +// are updated via raw-pointer place expressions instead, keeping the value +// references valid. +// +// # Pointer dereferencing +// +// All pointers stored in the list are valid to dereference: +// +// 1. The head/tail pointer is derived from `head_tail`'s `UnsafePinned` +// wherever it is needed. Because of the `UnsafePinned` wrapper, no +// exclusive reference to the list (or a structure containing it) makes an +// aliasing claim on `head_tail`, so every derived pointer and every copy +// of it stored in the links stay valid for the list's lifetime. +// 2. Pointers to other entries, stored in the links, are derived from the +// exclusive reference passed to `push` and stay valid while the entry is +// in the list, as ensured by the safety requirements in the module +// documentation. +// +// Both points rely on this code never creating references to entries, as +// those would make their own aliasing claims on the entries. +// +// # Pinning +// +// Once initialized, the list is self-referential, so it must not be moved. +// `UnsafeList` is `!Unpin` and the operations take `Pin<&mut Self>`, letting +// the compiler enforce this. Dropping the list while entries are still +// linked would leave those entries dangling; `Drop` checks this, and `Pin`'s +// drop guarantee ensures every path that invalidates the list's storage +// (including in-place replacement with `Pin::set`) runs the check. + +#[cfg(test)] +mod tests; + +use crate::pin::{Pin, UnsafePinned}; +use crate::ptr::{self, NonNull}; + +/// A caller-allocated list entry. +/// +/// While the entry is in a list, the list holds a pointer derived from the +/// exclusive reference passed to `UnsafeList::push`, so the caller must not +/// access the entry until it is removed from the list. `UnsafeList::push` +/// returns a reference borrowing the entry, and `UnsafeList::remove` +/// reborrows it exclusively, so the borrow checker enforces this for safe +/// accesses. +pub(crate) struct UnsafeListEntry { + next: NonNull>, + prev: NonNull>, + value: Option, +} + +impl UnsafeListEntry { + const fn dummy() -> Self { + UnsafeListEntry { next: NonNull::dangling(), prev: NonNull::dangling(), value: None } + } + + pub(crate) fn new(value: T) -> Self { + UnsafeListEntry { value: Some(value), ..Self::dummy() } + } +} + +// WARNING: self-referential struct! Must not be moved once initialized, see +// the `Pinning` explanation at the top of the file. +pub(crate) struct UnsafeList { + // UnsafePinned isn't required to implement this code, but it makes it a lot + // simpler. Without UnsafePinned, the provenance of each entry link pointer + // would need to be re-established prior to dereferencing, whenever it points + // to `head_tail`. + head_tail: UnsafePinned>, +} + +impl UnsafeList { + /// Creates a new list. + /// + /// Before use, the list must be placed in its final location and + /// initialized with `init`, making it self-referential; from then on + /// it must not be moved and can only be operated on through + /// `Pin<&mut Self>` (see the `Pinning` explanation at the top of the + /// file). `WaitVariable::new` performs this sequence. + /// + /// # Safety + /// + /// The caller must initialize the list with `init` before any other use, + /// including dropping it. + pub(crate) const unsafe fn new() -> Self { + UnsafeList { head_tail: UnsafePinned::new(UnsafeListEntry::dummy()) } + } + + fn head_tail(&mut self) -> NonNull> { + // SAFETY: `get_mut_unchecked` returns the address of `head_tail`, + // which is non-null. + unsafe { NonNull::new_unchecked(self.head_tail.get_mut_unchecked()) } + } + + /// Makes the list self-referential: the list must be in its final + /// location and must never be moved afterwards. Called exactly once per + /// list, during construction (`WaitVariable::new`), so lists + /// are always initialized before use. + pub(crate) fn init(&mut self) { + let head_tail = self.head_tail(); + // SAFETY: `head_tail` is valid to dereference (see point 1 of the + // `Pointer dereferencing` explanation at the top of the file). + unsafe { (*head_tail.as_ptr()).next = head_tail }; + unsafe { (*head_tail.as_ptr()).prev = head_tail }; + } + + pub(crate) fn is_empty(&self) -> bool { + // SAFETY: `get` returns the address of `head_tail`, which is + // non-null. + let head_tail = unsafe { NonNull::new_unchecked(self.head_tail.get()) }; + // SAFETY: `head_tail` is valid to dereference (see point 1 + // of the `Pointer dereferencing` explanation at the top of the + // file). + let first = unsafe { (*head_tail.as_ptr()).next }; + if first == head_tail { + // ,-------> /---------\ next ---, + // | |head_tail| | + // `--- prev \---------/ <-------` + // SAFETY: `head_tail` is valid to dereference. + unsafe { rtassert!((*head_tail.as_ptr()).prev == first) }; + true + } else { + false + } + } + + /// Pushes an entry onto the back of the list. + /// + /// # Safety + /// + /// The entry must remain allocated until the entry is removed from the + /// list AND the caller who popped is done using the entry. Special + /// care must be taken in the caller of `push` to ensure unwinding does + /// not destroy the stack frame containing the entry. While the entry is + /// in the list, it must not be accessed except through the reference + /// returned here or by passing the entry to `remove`. + pub(crate) unsafe fn push<'a>( + self: Pin<&mut Self>, + entry: &'a mut UnsafeListEntry, + ) -> &'a T { + // SAFETY: the list is not moved out of the pinned reference. + let this = unsafe { self.get_unchecked_mut() }; + + // BEFORE: + // /---------\ next ---> /---------\ + // ... |prev_tail| |head_tail| ... + // \---------/ <--- prev \---------/ + // + // AFTER: + // /---------\ next ---> /-----\ next ---> /---------\ + // ... |prev_tail| |entry| |head_tail| ... + // \---------/ <--- prev \-----/ <--- prev \---------/ + let entry = unsafe { NonNull::new_unchecked(entry) }; + let head_tail = this.head_tail(); + // SAFETY: `head_tail` is valid to dereference (see point 1 + // of the `Pointer dereferencing` explanation at the top of the + // file). + let prev_tail = unsafe { ptr::replace(&raw mut (*head_tail.as_ptr()).prev, entry) }; + // SAFETY: `entry` is valid to dereference: it was derived from an + // exclusive reference above. + unsafe { (*entry.as_ptr()).prev = prev_tail }; + unsafe { (*entry.as_ptr()).next = head_tail }; + // SAFETY: `prev_tail` was loaded from the list's links, so it is + // valid to dereference (see points 1 and 2 of the + // `Pointer dereferencing` explanation at the top of the file). + unsafe { (*prev_tail.as_ptr()).next = entry }; + // unwrap ok: always `Some` on non-dummy entries + unsafe { (*entry.as_ptr()).value.as_ref() }.unwrap() + } + + /// Pops an entry from the front of the list. + /// + /// # Safety + /// + /// The caller must make sure to synchronize ending the borrow of the + /// return value and deallocation of the containing entry. + pub(crate) unsafe fn pop<'a>(self: Pin<&mut Self>) -> Option<&'a T> { + if self.is_empty() { + None + } else { + // SAFETY: the list is not moved out of the pinned reference. + let this = unsafe { self.get_unchecked_mut() }; + + // BEFORE: + // /---------\ next ---> /-----\ next ---> /------\ + // ... |head_tail| |first| |second| ... + // \---------/ <--- prev \-----/ <--- prev \------/ + // + // AFTER: + // /---------\ next ---> /------\ + // ... |head_tail| |second| ... + // \---------/ <--- prev \------/ + + let head_tail = this.head_tail(); + // SAFETY: `head_tail` is valid to dereference (see point 1 + // of the `Pointer dereferencing` explanation at the top of the + // file). + let first = unsafe { (*head_tail.as_ptr()).next }; + // SAFETY: `first` was loaded from the list's links, so it is + // valid to dereference (see point 2 of the + // `Pointer dereferencing` explanation at the top of the file). + let second = unsafe { (*first.as_ptr()).next }; + unsafe { (*head_tail.as_ptr()).next = second }; + // SAFETY: `second` was loaded from the list's links, so it is + // valid to dereference (see points 1 and 2 of the + // `Pointer dereferencing` explanation at the top of the file). + unsafe { (*second.as_ptr()).prev = head_tail }; + unsafe { (*first.as_ptr()).next = NonNull::dangling() }; + unsafe { (*first.as_ptr()).prev = NonNull::dangling() }; + // unwrap ok: always `Some` on non-dummy entries + Some(unsafe { (*first.as_ptr()).value.as_ref() }.unwrap()) + } + } + + /// Removes an entry from the list. + /// + /// # Safety + /// + /// The caller must ensure that `entry` has been pushed onto `self` + /// prior to this call, has not been removed from the list since then + /// (by `pop` or `remove`), and has not moved since it was pushed. + pub(crate) unsafe fn remove(self: Pin<&mut Self>, entry: &mut UnsafeListEntry) { + rtassert!(!self.is_empty()); + + // BEFORE: + // /----\ next ---> /-----\ next ---> /----\ + // ... |prev| |entry| |next| ... + // \----/ <--- prev \-----/ <--- prev \----/ + // + // AFTER: + // /----\ next ---> /----\ + // ... |prev| |next| ... + // \----/ <--- prev \----/ + + // The exclusive reference `entry`, created by the caller, has + // invalidated the pointers to `entry` stored in its neighbors (see + // the module documentation); those are only overwritten below, + // never dereferenced. + let prev = entry.prev; + let next = entry.next; + // SAFETY: `prev` and `next` were loaded from `entry`'s links, so + // they are valid to dereference (see points 1 and 2 of the + // `Pointer dereferencing` explanation at the top of the file). + unsafe { (*prev.as_ptr()).next = next }; + unsafe { (*next.as_ptr()).prev = prev }; + entry.next = NonNull::dangling(); + entry.prev = NonNull::dangling(); + } +} + +impl Drop for UnsafeList { + fn drop(&mut self) { + // A non-empty list would leave its entries with dangling links. + // `Pin`'s drop guarantee routes every path that invalidates the + // list's storage (including in-place replacement via `Pin::set`) + // through this check. + rtassert!(self.is_empty()); + } +} diff --git a/library/std/src/sys/sync/unsafe_list/tests.rs b/library/std/src/sys/sync/unsafe_list/tests.rs new file mode 100644 index 0000000000000..4376b2870d426 --- /dev/null +++ b/library/std/src/sys/sync/unsafe_list/tests.rs @@ -0,0 +1,285 @@ +use super::*; +use crate::cell::Cell; +use crate::pin::Pin; + +/// All lists are constructed by `WaitVariable::new`; this test +/// stand-in likewise initializes the list before pinning it. +fn new_list() -> Pin>> { + // SAFETY: `init` is called below, before the list is otherwise used or + // dropped. + let mut list = Box::new(unsafe { UnsafeList::new() }); + list.init(); + Box::into_pin(list) +} + +/// # Safety +/// List must be valid. +unsafe fn assert_empty(list: Pin<&mut UnsafeList>) { + assert!(unsafe { list.pop() }.is_none(), "assertion failed: list is not empty"); +} + +#[test] +fn init_empty() { + unsafe { + assert_empty(new_list::().as_mut()); + } +} + +#[test] +fn push_pop() { + unsafe { + let mut node = UnsafeListEntry::new(1234); + let mut list = new_list(); + assert_eq!(list.as_mut().push(&mut node), &1234); + assert_eq!(list.as_mut().pop().unwrap(), &1234); + assert_empty(list.as_mut()); + } +} + +#[test] +fn push_remove() { + unsafe { + let mut node = UnsafeListEntry::new(1234); + let mut list = new_list(); + assert_eq!(list.as_mut().push(&mut node), &1234); + list.as_mut().remove(&mut node); + assert_empty(list.as_mut()); + } +} + +#[test] +fn push_remove_pop() { + unsafe { + let mut node1 = UnsafeListEntry::new(11); + let mut node2 = UnsafeListEntry::new(12); + let mut node3 = UnsafeListEntry::new(13); + let mut node4 = UnsafeListEntry::new(14); + let mut node5 = UnsafeListEntry::new(15); + let mut list = new_list(); + assert_eq!(list.as_mut().push(&mut node1), &11); + assert_eq!(list.as_mut().push(&mut node2), &12); + assert_eq!(list.as_mut().push(&mut node3), &13); + assert_eq!(list.as_mut().push(&mut node4), &14); + assert_eq!(list.as_mut().push(&mut node5), &15); + + list.as_mut().remove(&mut node1); + assert_eq!(list.as_mut().pop().unwrap(), &12); + list.as_mut().remove(&mut node3); + assert_eq!(list.as_mut().pop().unwrap(), &14); + list.as_mut().remove(&mut node5); + assert_empty(list.as_mut()); + + assert_eq!(list.as_mut().push(&mut node1), &11); + assert_eq!(list.as_mut().pop().unwrap(), &11); + assert_empty(list.as_mut()); + + assert_eq!(list.as_mut().push(&mut node3), &13); + assert_eq!(list.as_mut().push(&mut node4), &14); + list.as_mut().remove(&mut node3); + list.as_mut().remove(&mut node4); + assert_empty(list.as_mut()); + } +} + +#[test] +fn complex_pushes_pops() { + unsafe { + let mut node1 = UnsafeListEntry::new(1234); + let mut node2 = UnsafeListEntry::new(4567); + let mut node3 = UnsafeListEntry::new(9999); + let mut node4 = UnsafeListEntry::new(8642); + let mut list = new_list(); + list.as_mut().push(&mut node1); + list.as_mut().push(&mut node2); + assert_eq!(list.as_mut().pop().unwrap(), &1234); + list.as_mut().push(&mut node3); + assert_eq!(list.as_mut().pop().unwrap(), &4567); + assert_eq!(list.as_mut().pop().unwrap(), &9999); + assert_empty(list.as_mut()); + list.as_mut().push(&mut node4); + assert_eq!(list.as_mut().pop().unwrap(), &8642); + assert_empty(list.as_mut()); + } +} + +#[test] +fn cell() { + unsafe { + let mut node = UnsafeListEntry::new(Cell::new(0)); + let mut list = new_list(); + let noderef = list.as_mut().push(&mut node); + assert_eq!(noderef.get(), 0); + list.as_mut().pop().unwrap().set(1); + assert_empty(list.as_mut()); + assert_eq!(noderef.get(), 1); + } +} + +// Regression tests for the aliasing issues in rust-lang/rust#160603, +// exercising the usage patterns of the SGX `WaitQueue`. `hostile_reborrow` +// mirrors safe code reborrowing the structure containing the list between +// list operations (as `WaitVariable::lock_var_mut` and the pin projections +// do). + +struct Wrapper { + list: UnsafeList, + other: u32, +} + +impl Wrapper { + fn new() -> Pin>> { + // SAFETY: `init` is called below, before the list is otherwise used + // or dropped. + let mut wrapper = Box::new(Wrapper { list: unsafe { UnsafeList::new() }, other: 0 }); + wrapper.list.init(); + Box::into_pin(wrapper) + } + + fn list(self: Pin<&mut Self>) -> Pin<&mut UnsafeList> { + // SAFETY: `list` is structurally pinned: a pinned `Wrapper` pins it, + // and it is never moved out of it. + unsafe { self.map_unchecked_mut(|this| &mut this.list) } + } + + fn hostile_reborrow(self: Pin<&mut Self>) { + // SAFETY: nothing is moved; `other` is not structurally pinned. + let this = unsafe { self.get_unchecked_mut() }; + this.other = this.other.wrapping_add(1); + } +} + +// The `wait_timeout` fallback path: push an entry, use the returned +// reference, then remove the entry. +#[test] +fn wait_timeout_fallback() { + unsafe { + let mut w = Wrapper::new(); + let mut entry = UnsafeListEntry::new(1234); + let value = w.as_mut().list().push(&mut entry); + assert_eq!(*value, 1234); + + w.as_mut().hostile_reborrow(); + + // Not woken up: remove our own entry, as `wait_timeout` does. + w.as_mut().list().remove(&mut entry); + assert_empty(w.as_mut().list()); + } +} + +// Removing the first entry while others are present. +#[test] +fn remove_first_of_many() { + unsafe { + let mut w = Wrapper::new(); + let mut e1 = UnsafeListEntry::new(1); + let mut e2 = UnsafeListEntry::new(2); + let mut e3 = UnsafeListEntry::new(3); + w.as_mut().list().push(&mut e1); + w.as_mut().list().push(&mut e2); + w.as_mut().list().push(&mut e3); + w.as_mut().list().remove(&mut e1); + assert_eq!(w.as_mut().list().pop().unwrap(), &2); + assert_eq!(w.as_mut().list().pop().unwrap(), &3); + assert_empty(w.as_mut().list()); + } +} + +// Entries pushed from different "stack frames" and popped by a "notifier" +// (like `notify_all`), with hostile reborrows between every operation. +#[test] +fn notify_all_pattern() { + unsafe { + let mut w = Wrapper::new(); + let mut e1 = UnsafeListEntry::new(1); + let mut e2 = UnsafeListEntry::new(2); + w.as_mut().list().push(&mut e1); + w.as_mut().hostile_reborrow(); + w.as_mut().list().push(&mut e2); + w.as_mut().hostile_reborrow(); + + let mut count = 0; + while let Some(v) = w.as_mut().list().pop() { + count += *v; + w.as_mut().hostile_reborrow(); + } + assert_eq!(count, 3); + } +} + +// Empty-list churn: repeated push/pop cycles with reborrows in between. +#[test] +fn empty_churn() { + unsafe { + let mut w = Wrapper::new(); + for i in 0..4 { + let mut e = UnsafeListEntry::new(i); + w.as_mut().list().push(&mut e); + w.as_mut().hostile_reborrow(); + assert_eq!(w.as_mut().list().pop().unwrap(), &i); + w.as_mut().hostile_reborrow(); + assert!(w.list.is_empty()); + } + } +} + +// Cross-thread `wait`/`notify_one` pattern: the waiting thread pushes a +// stack-allocated entry and keeps reading through the reference returned by +// `push` while the notifying thread pops the entry and stores through the +// reference returned by `pop`. +#[test] +fn cross_thread_wait_notify() { + use crate::sync::atomic::{AtomicBool, Ordering}; + use crate::sync::{Arc, Mutex}; + use crate::thread; + + struct Queue { + list: UnsafeList, + } + // SAFETY: like the real `WaitQueue`, the list is only accessed while + // holding the mutex. + unsafe impl Send for Queue {} + + let queue = Arc::new(Mutex::new(Queue { + // SAFETY: `init` is called below, before the list is otherwise used + // or dropped. + list: unsafe { UnsafeList::new() }, + })); + queue.lock().unwrap().list.init(); + + for _ in 0..3 { + let waiter = { + let queue = Arc::clone(&queue); + thread::spawn(move || { + let mut entry = UnsafeListEntry::new(AtomicBool::new(false)); + let mut guard = queue.lock().unwrap(); + // SAFETY: the list lives in the heap allocation behind the + // `Arc` and is never moved. + let list = unsafe { Pin::new_unchecked(&mut guard.list) }; + // SAFETY: `entry` is only dropped after the notifier popped + // it and set the flag, and is not otherwise accessed while it + // is in the list. + let wake = unsafe { list.push(&mut entry) }; + drop(guard); + while !wake.load(Ordering::Acquire) { + thread::yield_now(); + } + }) + }; + loop { + let mut guard = queue.lock().unwrap(); + // SAFETY: the list lives in the heap allocation behind the `Arc` + // and is never moved. + let list = unsafe { Pin::new_unchecked(&mut guard.list) }; + // SAFETY: the entry is not deallocated until the waiting thread + // observes the flag, which is only set below. + if let Some(wake) = unsafe { list.pop() } { + // Set under the queue lock, like `notify_one`. + wake.store(true, Ordering::Release); + break; + } + drop(guard); + thread::yield_now(); + } + waiter.join().unwrap(); + } +} diff --git a/tests/mir-opt/building/custom/arbitrary_cast.rs b/tests/mir-opt/building/custom/arbitrary_cast.rs new file mode 100644 index 0000000000000..56e6373756b53 --- /dev/null +++ b/tests/mir-opt/building/custom/arbitrary_cast.rs @@ -0,0 +1,80 @@ +//@ skip-filecheck +#![feature(custom_mir, core_intrinsics)] + +extern crate core; +use core::intrinsics::mir::*; + +fn f(x: i32) -> i32 { + x +} + +#[custom_mir(dialect = "built")] +fn reify_fn_ptr() -> fn(i32) -> i32 { + mir! { + { + RET = Cast( + f, + CastKind::PointerCoercion(PointerCoercion::ReifyFnPointer(Safety::Safe)), + ); + Return() + } + } +} + +#[custom_mir(dialect = "built")] +fn fn_ptr_to_unsafe(f: fn()) -> unsafe fn() { + mir! { + { + RET = Cast( + f, + CastKind::PointerCoercion(PointerCoercion::UnsafeFnPointer), + ); + Return() + } + } +} + +#[custom_mir(dialect = "runtime")] +fn subtype_fn_ptr(f: fn(&i32)) -> fn(&'static i32) { + mir! { + { + RET = Cast::(f, CastKind::Subtype); + Return() + } + } +} + +#[custom_mir(dialect = "built")] +fn expose_ptr(p: *const i32) -> usize { + mir! { + { + RET = Cast(p, CastKind::PointerExposeProvenance); + Return() + } + } +} + +#[custom_mir(dialect = "built")] +fn ptr_from_exposed(p: usize) -> *const i32 { + mir! { + { + RET = Cast(p, CastKind::PointerWithExposedProvenance); + Return() + } + } +} + +fn main() { + assert_eq!(reify_fn_ptr(), f as fn(i32) -> i32); + + let fn_ptr: fn() = || {}; + assert_eq!(fn_ptr as unsafe fn(), fn_ptr_to_unsafe(fn_ptr)); + + let fn_ptr: fn(&i32) = |_| {}; + assert_eq!(fn_ptr as fn(&'static i32), subtype_fn_ptr(fn_ptr)); + + let p = &1; + assert_eq!(p as *const i32 as usize, expose_ptr(p)); + + assert_eq!(ptr_from_exposed(1), 1 as *const i32); +}