From de560c30659281e8c91a48579f907097efe22adf Mon Sep 17 00:00:00 2001 From: Urgau Date: Tue, 20 Feb 2024 21:40:25 +0100 Subject: [PATCH 01/10] Implement `ambiguous_negative_literals` lint --- compiler/rustc_lint/messages.ftl | 5 + compiler/rustc_lint/src/lib.rs | 3 + compiler/rustc_lint/src/lints.rs | 29 ++++ compiler/rustc_lint/src/precedence.rs | 70 ++++++++++ tests/ui/lint/negative_literals.rs | 35 +++++ tests/ui/lint/negative_literals.stderr | 179 +++++++++++++++++++++++++ 6 files changed, 321 insertions(+) create mode 100644 compiler/rustc_lint/src/precedence.rs create mode 100644 tests/ui/lint/negative_literals.rs create mode 100644 tests/ui/lint/negative_literals.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index f048f6fe8ad4b..4f7f01e56b63d 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -5,6 +5,11 @@ lint_ambiguous_glob_reexport = ambiguous glob re-exports .label_first_reexport = the name `{$name}` in the {$namespace} namespace is first re-exported here .label_duplicate_reexport = but the name `{$name}` in the {$namespace} namespace is also re-exported here +lint_ambiguous_negative_literals = `-` has lower precedence than method calls, which might be unexpected + .example = e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` + .negative_literal = add parentheses around the `-` and the literal to call the method on a negative literal + .current_behavior = add parentheses around the literal and the method call to keep the current behavior + lint_ambiguous_wide_pointer_comparisons = ambiguous wide pointer comparison, the comparison includes metadata which may not be expected .addr_metadata_suggestion = use explicit `std::ptr::eq` method to compare metadata and addresses .addr_suggestion = use `std::ptr::addr_eq` or untyped pointers to only compare their addresses diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 868a44a980a0f..0a058b675ffa5 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -72,6 +72,7 @@ mod noop_method_call; mod opaque_hidden_inferred_bound; mod pass_by_value; mod passes; +mod precedence; mod ptr_nulls; mod redundant_semicolon; mod reference_casting; @@ -109,6 +110,7 @@ use nonstandard_style::*; use noop_method_call::*; use opaque_hidden_inferred_bound::*; use pass_by_value::*; +use precedence::*; use ptr_nulls::*; use redundant_semicolon::*; use reference_casting::*; @@ -173,6 +175,7 @@ early_lint_methods!( RedundantSemicolons: RedundantSemicolons, UnusedDocComment: UnusedDocComment, Expr2024: Expr2024, + Precedence: Precedence, ] ] ); diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 54c73710eca6f..873322c13b46d 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -1489,6 +1489,35 @@ pub struct NonLocalDefinitionsCargoUpdateNote { pub crate_name: Symbol, } +// precedence.rs +#[derive(LintDiagnostic)] +#[diag(lint_ambiguous_negative_literals)] +#[note(lint_example)] +pub struct AmbiguousNegativeLiteralsDiag { + #[subdiagnostic] + pub negative_literal: AmbiguousNegativeLiteralsNegativeLiteralSuggestion, + #[subdiagnostic] + pub current_behavior: AmbiguousNegativeLiteralsCurrentBehaviorSuggestion, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(lint_negative_literal, applicability = "maybe-incorrect")] +pub struct AmbiguousNegativeLiteralsNegativeLiteralSuggestion { + #[suggestion_part(code = "(")] + pub start_span: Span, + #[suggestion_part(code = ")")] + pub end_span: Span, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(lint_current_behavior, applicability = "maybe-incorrect")] +pub struct AmbiguousNegativeLiteralsCurrentBehaviorSuggestion { + #[suggestion_part(code = "(")] + pub start_span: Span, + #[suggestion_part(code = ")")] + pub end_span: Span, +} + // pass_by_value.rs #[derive(LintDiagnostic)] #[diag(lint_pass_by_value)] diff --git a/compiler/rustc_lint/src/precedence.rs b/compiler/rustc_lint/src/precedence.rs new file mode 100644 index 0000000000000..eb2ba39727797 --- /dev/null +++ b/compiler/rustc_lint/src/precedence.rs @@ -0,0 +1,70 @@ +use rustc_ast::token::LitKind; +use rustc_ast::{Expr, ExprKind, MethodCall, UnOp}; +use rustc_session::{declare_lint, declare_lint_pass}; + +use crate::lints::{ + AmbiguousNegativeLiteralsCurrentBehaviorSuggestion, AmbiguousNegativeLiteralsDiag, + AmbiguousNegativeLiteralsNegativeLiteralSuggestion, +}; +use crate::{EarlyContext, EarlyLintPass, LintContext}; + +declare_lint! { + /// The `ambiguous_negative_literals` lint checks for cases that are + /// confusing between a negative literal and a negation that's not part + /// of the literal. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// # #![allow(unused)] + /// -1i32.abs(); // equals -1, while `(-1i32).abs()` equals 1 + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Method calls take precedence over unary precedence. Setting the + /// precedence explicitly makes the code clearer and avoid potential bugs. + pub AMBIGUOUS_NEGATIVE_LITERALS, + Deny, + "ambiguous negative literals operations", + report_in_external_macro +} + +declare_lint_pass!(Precedence => [AMBIGUOUS_NEGATIVE_LITERALS]); + +impl EarlyLintPass for Precedence { + fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { + let ExprKind::Unary(UnOp::Neg, operand) = &expr.kind else { + return; + }; + + let mut arg = operand; + let mut at_least_one = false; + while let ExprKind::MethodCall(box MethodCall { receiver, .. }) = &arg.kind { + at_least_one = true; + arg = receiver; + } + + if at_least_one + && let ExprKind::Lit(lit) = &arg.kind + && let LitKind::Integer | LitKind::Float = &lit.kind + { + cx.emit_span_lint( + AMBIGUOUS_NEGATIVE_LITERALS, + expr.span, + AmbiguousNegativeLiteralsDiag { + negative_literal: AmbiguousNegativeLiteralsNegativeLiteralSuggestion { + start_span: expr.span.shrink_to_lo(), + end_span: arg.span.shrink_to_hi(), + }, + current_behavior: AmbiguousNegativeLiteralsCurrentBehaviorSuggestion { + start_span: operand.span.shrink_to_lo(), + end_span: operand.span.shrink_to_hi(), + }, + }, + ); + } + } +} diff --git a/tests/ui/lint/negative_literals.rs b/tests/ui/lint/negative_literals.rs new file mode 100644 index 0000000000000..048fcd6ff57bc --- /dev/null +++ b/tests/ui/lint/negative_literals.rs @@ -0,0 +1,35 @@ +//@ check-fail + +fn main() { + let _ = -1i32.abs(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1f32.abs(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1f64.asin(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1f64.asinh(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1f64.tan(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1f64.tanh(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1.0_f64.cos().cos(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1.0_f64.cos().sin(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1.0_f64.sin().cos(); + //~^ ERROR `-` has lower precedence than method calls + let _ = -1f64.sin().sin(); + //~^ ERROR `-` has lower precedence than method calls + + dbg!( -1.0_f32.cos() ); + //~^ ERROR `-` has lower precedence than method calls + + // should not warn + let _ = (-1i32).abs(); + let _ = (-1f32).abs(); + let _ = -(1i32).abs(); + let _ = -(1f32).abs(); + let _ = -(1i32.abs()); + let _ = -(1f32.abs()); +} diff --git a/tests/ui/lint/negative_literals.stderr b/tests/ui/lint/negative_literals.stderr new file mode 100644 index 0000000000000..df000a7188255 --- /dev/null +++ b/tests/ui/lint/negative_literals.stderr @@ -0,0 +1,179 @@ +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:4:13 + | +LL | let _ = -1i32.abs(); + | ^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` + = note: `#[deny(ambiguous_negative_literals)]` on by default +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1i32).abs(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1i32.abs()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:6:13 + | +LL | let _ = -1f32.abs(); + | ^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1f32).abs(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1f32.abs()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:8:13 + | +LL | let _ = -1f64.asin(); + | ^^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1f64).asin(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1f64.asin()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:10:13 + | +LL | let _ = -1f64.asinh(); + | ^^^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1f64).asinh(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1f64.asinh()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:12:13 + | +LL | let _ = -1f64.tan(); + | ^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1f64).tan(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1f64.tan()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:14:13 + | +LL | let _ = -1f64.tanh(); + | ^^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1f64).tanh(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1f64.tanh()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:16:13 + | +LL | let _ = -1.0_f64.cos().cos(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1.0_f64).cos().cos(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1.0_f64.cos().cos()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:18:13 + | +LL | let _ = -1.0_f64.cos().sin(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1.0_f64).cos().sin(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1.0_f64.cos().sin()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:20:13 + | +LL | let _ = -1.0_f64.sin().cos(); + | ^^^^^^^^^^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1.0_f64).sin().cos(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1.0_f64.sin().cos()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:22:13 + | +LL | let _ = -1f64.sin().sin(); + | ^^^^^^^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | let _ = (-1f64).sin().sin(); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | let _ = -(1f64.sin().sin()); + | + + + +error: `-` has lower precedence than method calls, which might be unexpected + --> $DIR/negative_literals.rs:25:11 + | +LL | dbg!( -1.0_f32.cos() ); + | ^^^^^^^^^^^^^^ + | + = note: e.g. `-4.abs()` equals `-4`; while `(-4).abs()` equals `4` +help: add parentheses around the `-` and the literal to call the method on a negative literal + | +LL | dbg!( (-1.0_f32).cos() ); + | + + +help: add parentheses around the literal and the method call to keep the current behavior + | +LL | dbg!( -(1.0_f32.cos()) ); + | + + + +error: aborting due to 11 previous errors + From c5e1a12efc32b4e487a8899b143909e17d864adf Mon Sep 17 00:00:00 2001 From: Urgau Date: Tue, 20 Feb 2024 22:19:26 +0100 Subject: [PATCH 02/10] Remove unary neg from `clippy::precedence` lint --- .../clippy/clippy_lints/src/precedence.rs | 56 +------------------ src/tools/clippy/tests/ui/precedence.fixed | 34 ----------- src/tools/clippy/tests/ui/precedence.rs | 34 ----------- src/tools/clippy/tests/ui/precedence.stderr | 32 +---------- .../clippy/tests/ui/unnecessary_cast.fixed | 2 +- src/tools/clippy/tests/ui/unnecessary_cast.rs | 2 +- 6 files changed, 4 insertions(+), 156 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/precedence.rs b/src/tools/clippy/clippy_lints/src/precedence.rs index ff83725da6913..37f5dd5583bfb 100644 --- a/src/tools/clippy/clippy_lints/src/precedence.rs +++ b/src/tools/clippy/clippy_lints/src/precedence.rs @@ -1,38 +1,17 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use rustc_ast::ast::{BinOpKind, Expr, ExprKind, MethodCall, UnOp}; -use rustc_ast::token; +use rustc_ast::ast::{BinOpKind, Expr, ExprKind}; use rustc_errors::Applicability; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::declare_lint_pass; use rustc_span::source_map::Spanned; -const ALLOWED_ODD_FUNCTIONS: [&str; 14] = [ - "asin", - "asinh", - "atan", - "atanh", - "cbrt", - "fract", - "round", - "signum", - "sin", - "sinh", - "tan", - "tanh", - "to_degrees", - "to_radians", -]; - declare_clippy_lint! { /// ### What it does /// Checks for operations where precedence may be unclear /// and suggests to add parentheses. Currently it catches the following: /// * mixed usage of arithmetic and bit shifting/combining operators without /// parentheses - /// * a "negative" numeric literal (which is really a unary `-` followed by a - /// numeric literal) - /// followed by a method call /// /// ### Why is this bad? /// Not everyone knows the precedence of those operators by @@ -41,7 +20,6 @@ declare_clippy_lint! { /// /// ### Example /// * `1 << 2 + 3` equals 32, while `(1 << 2) + 3` equals 7 - /// * `-1i32.abs()` equals -1, while `(-1i32).abs()` equals 1 #[clippy::version = "pre 1.29.0"] pub PRECEDENCE, complexity, @@ -104,38 +82,6 @@ impl EarlyLintPass for Precedence { (false, false) => (), } } - - if let ExprKind::Unary(UnOp::Neg, operand) = &expr.kind { - let mut arg = operand; - - let mut all_odd = true; - while let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &arg.kind { - let seg_str = seg.ident.name.as_str(); - all_odd &= ALLOWED_ODD_FUNCTIONS - .iter() - .any(|odd_function| **odd_function == *seg_str); - arg = receiver; - } - - if !all_odd - && let ExprKind::Lit(lit) = &arg.kind - && let token::LitKind::Integer | token::LitKind::Float = &lit.kind - { - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - PRECEDENCE, - expr.span, - "unary minus has lower precedence than method call", - "consider adding parentheses to clarify your intent", - format!( - "-({})", - snippet_with_applicability(cx, operand.span, "..", &mut applicability) - ), - applicability, - ); - } - } } } diff --git a/src/tools/clippy/tests/ui/precedence.fixed b/src/tools/clippy/tests/ui/precedence.fixed index cc87de0d90f18..c25c2062aceba 100644 --- a/src/tools/clippy/tests/ui/precedence.fixed +++ b/src/tools/clippy/tests/ui/precedence.fixed @@ -20,40 +20,6 @@ fn main() { 1 ^ (1 - 1); 3 | (2 - 1); 3 & (5 - 2); - -(1i32.abs()); - -(1f32.abs()); - - // These should not trigger an error - let _ = (-1i32).abs(); - let _ = (-1f32).abs(); - let _ = -(1i32).abs(); - let _ = -(1f32).abs(); - let _ = -(1i32.abs()); - let _ = -(1f32.abs()); - - // Odd functions should not trigger an error - let _ = -1f64.asin(); - let _ = -1f64.asinh(); - let _ = -1f64.atan(); - let _ = -1f64.atanh(); - let _ = -1f64.cbrt(); - let _ = -1f64.fract(); - let _ = -1f64.round(); - let _ = -1f64.signum(); - let _ = -1f64.sin(); - let _ = -1f64.sinh(); - let _ = -1f64.tan(); - let _ = -1f64.tanh(); - let _ = -1f64.to_degrees(); - let _ = -1f64.to_radians(); - - // Chains containing any non-odd function should trigger (issue #5924) - let _ = -(1.0_f64.cos().cos()); - let _ = -(1.0_f64.cos().sin()); - let _ = -(1.0_f64.sin().cos()); - - // Chains of odd functions shouldn't trigger - let _ = -1f64.sin().sin(); let b = 3; trip!(b * 8); diff --git a/src/tools/clippy/tests/ui/precedence.rs b/src/tools/clippy/tests/ui/precedence.rs index 00c18d92b5fb6..dc242ecf4c72e 100644 --- a/src/tools/clippy/tests/ui/precedence.rs +++ b/src/tools/clippy/tests/ui/precedence.rs @@ -20,40 +20,6 @@ fn main() { 1 ^ 1 - 1; 3 | 2 - 1; 3 & 5 - 2; - -1i32.abs(); - -1f32.abs(); - - // These should not trigger an error - let _ = (-1i32).abs(); - let _ = (-1f32).abs(); - let _ = -(1i32).abs(); - let _ = -(1f32).abs(); - let _ = -(1i32.abs()); - let _ = -(1f32.abs()); - - // Odd functions should not trigger an error - let _ = -1f64.asin(); - let _ = -1f64.asinh(); - let _ = -1f64.atan(); - let _ = -1f64.atanh(); - let _ = -1f64.cbrt(); - let _ = -1f64.fract(); - let _ = -1f64.round(); - let _ = -1f64.signum(); - let _ = -1f64.sin(); - let _ = -1f64.sinh(); - let _ = -1f64.tan(); - let _ = -1f64.tanh(); - let _ = -1f64.to_degrees(); - let _ = -1f64.to_radians(); - - // Chains containing any non-odd function should trigger (issue #5924) - let _ = -1.0_f64.cos().cos(); - let _ = -1.0_f64.cos().sin(); - let _ = -1.0_f64.sin().cos(); - - // Chains of odd functions shouldn't trigger - let _ = -1f64.sin().sin(); let b = 3; trip!(b * 8); diff --git a/src/tools/clippy/tests/ui/precedence.stderr b/src/tools/clippy/tests/ui/precedence.stderr index 47e61326219d9..8057c25a5e499 100644 --- a/src/tools/clippy/tests/ui/precedence.stderr +++ b/src/tools/clippy/tests/ui/precedence.stderr @@ -43,35 +43,5 @@ error: operator precedence can trip the unwary LL | 3 & 5 - 2; | ^^^^^^^^^ help: consider parenthesizing your expression: `3 & (5 - 2)` -error: unary minus has lower precedence than method call - --> tests/ui/precedence.rs:23:5 - | -LL | -1i32.abs(); - | ^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1i32.abs())` - -error: unary minus has lower precedence than method call - --> tests/ui/precedence.rs:24:5 - | -LL | -1f32.abs(); - | ^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1f32.abs())` - -error: unary minus has lower precedence than method call - --> tests/ui/precedence.rs:51:13 - | -LL | let _ = -1.0_f64.cos().cos(); - | ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.cos().cos())` - -error: unary minus has lower precedence than method call - --> tests/ui/precedence.rs:52:13 - | -LL | let _ = -1.0_f64.cos().sin(); - | ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.cos().sin())` - -error: unary minus has lower precedence than method call - --> tests/ui/precedence.rs:53:13 - | -LL | let _ = -1.0_f64.sin().cos(); - | ^^^^^^^^^^^^^^^^^^^^ help: consider adding parentheses to clarify your intent: `-(1.0_f64.sin().cos())` - -error: aborting due to 12 previous errors +error: aborting due to 7 previous errors diff --git a/src/tools/clippy/tests/ui/unnecessary_cast.fixed b/src/tools/clippy/tests/ui/unnecessary_cast.fixed index 288541362cddb..c43e50761bd52 100644 --- a/src/tools/clippy/tests/ui/unnecessary_cast.fixed +++ b/src/tools/clippy/tests/ui/unnecessary_cast.fixed @@ -206,7 +206,7 @@ mod fixable { fn issue_9563() { let _: f64 = (-8.0_f64).exp(); - #[allow(clippy::precedence)] + #[allow(ambiguous_negative_literals)] let _: f64 = -8.0_f64.exp(); // should suggest `-8.0_f64.exp()` here not to change code behavior } diff --git a/src/tools/clippy/tests/ui/unnecessary_cast.rs b/src/tools/clippy/tests/ui/unnecessary_cast.rs index eef3a42e35149..4a5ca231315eb 100644 --- a/src/tools/clippy/tests/ui/unnecessary_cast.rs +++ b/src/tools/clippy/tests/ui/unnecessary_cast.rs @@ -206,7 +206,7 @@ mod fixable { fn issue_9563() { let _: f64 = (-8.0 as f64).exp(); - #[allow(clippy::precedence)] + #[allow(ambiguous_negative_literals)] let _: f64 = -(8.0 as f64).exp(); // should suggest `-8.0_f64.exp()` here not to change code behavior } From c31ff97bf1f9fbbd37a501cef0b019faa43b0b7f Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 24 Jul 2024 15:04:43 +0200 Subject: [PATCH 03/10] centralize turning asm flags into human readable names --- compiler/rustc_ast/src/ast.rs | 36 +++++++++++++++++++ compiler/rustc_ast_pretty/src/pprust/state.rs | 30 +--------------- compiler/rustc_hir_pretty/src/lib.rs | 30 +--------------- 3 files changed, 38 insertions(+), 58 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 75c656973f963..411fc8311a08f 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -2264,6 +2264,42 @@ bitflags::bitflags! { } } +impl InlineAsmOptions { + pub fn human_readable_names(&self) -> Vec<&'static str> { + let mut options = vec![]; + + if self.contains(InlineAsmOptions::PURE) { + options.push("pure"); + } + if self.contains(InlineAsmOptions::NOMEM) { + options.push("nomem"); + } + if self.contains(InlineAsmOptions::READONLY) { + options.push("readonly"); + } + if self.contains(InlineAsmOptions::PRESERVES_FLAGS) { + options.push("preserves_flags"); + } + if self.contains(InlineAsmOptions::NORETURN) { + options.push("noreturn"); + } + if self.contains(InlineAsmOptions::NOSTACK) { + options.push("nostack"); + } + if self.contains(InlineAsmOptions::ATT_SYNTAX) { + options.push("att_syntax"); + } + if self.contains(InlineAsmOptions::RAW) { + options.push("raw"); + } + if self.contains(InlineAsmOptions::MAY_UNWIND) { + options.push("may_unwind"); + } + + options + } +} + impl std::fmt::Debug for InlineAsmOptions { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { bitflags::parser::to_writer(self, f) diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 0568d368d8c4c..b463d1f36ce51 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1505,35 +1505,7 @@ impl<'a> State<'a> { AsmArg::Options(opts) => { s.word("options"); s.popen(); - let mut options = vec![]; - if opts.contains(InlineAsmOptions::PURE) { - options.push("pure"); - } - if opts.contains(InlineAsmOptions::NOMEM) { - options.push("nomem"); - } - if opts.contains(InlineAsmOptions::READONLY) { - options.push("readonly"); - } - if opts.contains(InlineAsmOptions::PRESERVES_FLAGS) { - options.push("preserves_flags"); - } - if opts.contains(InlineAsmOptions::NORETURN) { - options.push("noreturn"); - } - if opts.contains(InlineAsmOptions::NOSTACK) { - options.push("nostack"); - } - if opts.contains(InlineAsmOptions::ATT_SYNTAX) { - options.push("att_syntax"); - } - if opts.contains(InlineAsmOptions::RAW) { - options.push("raw"); - } - if opts.contains(InlineAsmOptions::MAY_UNWIND) { - options.push("may_unwind"); - } - s.commasep(Inconsistent, &options, |s, &opt| { + s.commasep(Inconsistent, &opts.human_readable_names(), |s, &opt| { s.word(opt); }); s.pclose(); diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index 5105d60ae188c..433be32604769 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -1289,35 +1289,7 @@ impl<'a> State<'a> { AsmArg::Options(opts) => { s.word("options"); s.popen(); - let mut options = vec![]; - if opts.contains(ast::InlineAsmOptions::PURE) { - options.push("pure"); - } - if opts.contains(ast::InlineAsmOptions::NOMEM) { - options.push("nomem"); - } - if opts.contains(ast::InlineAsmOptions::READONLY) { - options.push("readonly"); - } - if opts.contains(ast::InlineAsmOptions::PRESERVES_FLAGS) { - options.push("preserves_flags"); - } - if opts.contains(ast::InlineAsmOptions::NORETURN) { - options.push("noreturn"); - } - if opts.contains(ast::InlineAsmOptions::NOSTACK) { - options.push("nostack"); - } - if opts.contains(ast::InlineAsmOptions::ATT_SYNTAX) { - options.push("att_syntax"); - } - if opts.contains(ast::InlineAsmOptions::RAW) { - options.push("raw"); - } - if opts.contains(ast::InlineAsmOptions::MAY_UNWIND) { - options.push("may_unwind"); - } - s.commasep(Inconsistent, &options, |s, &opt| { + s.commasep(Inconsistent, &opts.human_readable_names(), |s, &opt| { s.word(opt); }); s.pclose(); From 4b7a87de1094522c5c6e5a7bda04e0e0b281e049 Mon Sep 17 00:00:00 2001 From: Folkert Date: Wed, 24 Jul 2024 15:27:56 +0200 Subject: [PATCH 04/10] use an allow list for allowed asm options in naked functions --- compiler/rustc_passes/src/naked_functions.rs | 21 +++++++++----------- tests/ui/asm/naked-functions.rs | 2 +- tests/ui/asm/naked-functions.stderr | 2 +- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs index d45ee32a624de..dbbf802c920d4 100644 --- a/compiler/rustc_passes/src/naked_functions.rs +++ b/compiler/rustc_passes/src/naked_functions.rs @@ -244,22 +244,19 @@ impl<'tcx> CheckInlineAssembly<'tcx> { self.tcx.dcx().emit_err(NakedFunctionsOperands { unsupported_operands }); } - let unsupported_options: Vec<&'static str> = [ - (InlineAsmOptions::MAY_UNWIND, "`may_unwind`"), - (InlineAsmOptions::NOMEM, "`nomem`"), - (InlineAsmOptions::NOSTACK, "`nostack`"), - (InlineAsmOptions::PRESERVES_FLAGS, "`preserves_flags`"), - (InlineAsmOptions::PURE, "`pure`"), - (InlineAsmOptions::READONLY, "`readonly`"), - ] - .iter() - .filter_map(|&(option, name)| if asm.options.contains(option) { Some(name) } else { None }) - .collect(); + let supported_options = + InlineAsmOptions::RAW | InlineAsmOptions::NORETURN | InlineAsmOptions::ATT_SYNTAX; + let unsupported_options = asm.options.difference(supported_options); if !unsupported_options.is_empty() { self.tcx.dcx().emit_err(NakedFunctionsAsmOptions { span, - unsupported_options: unsupported_options.join(", "), + unsupported_options: unsupported_options + .human_readable_names() + .into_iter() + .map(|name| format!("`{name}`")) + .collect::>() + .join(", "), }); } diff --git a/tests/ui/asm/naked-functions.rs b/tests/ui/asm/naked-functions.rs index 1619ebfcf39f0..6b2d2aea238da 100644 --- a/tests/ui/asm/naked-functions.rs +++ b/tests/ui/asm/naked-functions.rs @@ -111,7 +111,7 @@ unsafe extern "C" fn invalid_options() { unsafe extern "C" fn invalid_options_continued() { asm!("", options(readonly, nostack), options(pure)); //~^ ERROR asm with the `pure` option must have at least one output - //~| ERROR asm options unsupported in naked functions: `nostack`, `pure`, `readonly` + //~| ERROR asm options unsupported in naked functions: `pure`, `readonly`, `nostack` //~| ERROR asm in naked functions must use `noreturn` option } diff --git a/tests/ui/asm/naked-functions.stderr b/tests/ui/asm/naked-functions.stderr index 77bc80a101f02..4dd9e29bdc6f8 100644 --- a/tests/ui/asm/naked-functions.stderr +++ b/tests/ui/asm/naked-functions.stderr @@ -212,7 +212,7 @@ error[E0787]: asm options unsupported in naked functions: `nomem`, `preserves_fl LL | asm!("", options(nomem, preserves_flags, noreturn)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error[E0787]: asm options unsupported in naked functions: `nostack`, `pure`, `readonly` +error[E0787]: asm options unsupported in naked functions: `pure`, `readonly`, `nostack` --> $DIR/naked-functions.rs:112:5 | LL | asm!("", options(readonly, nostack), options(pure)); From 2c7ae388b34a2cd8a38a27320c7a5ed2654cd632 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 24 Jul 2024 11:42:31 -0700 Subject: [PATCH 05/10] std: unsafe-wrap gcc::rust_eh_personality and impl --- library/std/src/sys/personality/gcc.rs | 279 +++++++++++++------------ 1 file changed, 148 insertions(+), 131 deletions(-) diff --git a/library/std/src/sys/personality/gcc.rs b/library/std/src/sys/personality/gcc.rs index 0dc53550ca943..5542cf03d20c1 100644 --- a/library/std/src/sys/personality/gcc.rs +++ b/library/std/src/sys/personality/gcc.rs @@ -35,6 +35,7 @@ //! //! Once stack has been unwound down to the handler frame level, unwinding stops //! and the last personality routine transfers control to the catch block. +#![forbid(unsafe_op_in_unsafe_fn)] use super::dwarf::eh::{self, EHAction, EHContext}; use crate::ffi::c_int; @@ -92,7 +93,11 @@ const UNWIND_DATA_REG: (i32, i32) = (4, 5); // a0, a1 // https://github.com/gcc-mirror/gcc/blob/trunk/libgcc/unwind-c.c cfg_if::cfg_if! { - if #[cfg(all(not(all(target_vendor = "apple", not(target_os = "watchos"))), target_arch = "arm", not(target_os = "netbsd")))] { + if #[cfg(all( + target_arch = "arm", + not(all(target_vendor = "apple", not(target_os = "watchos"))), + not(target_os = "netbsd"), + ))] { // ARM EHABI personality routine. // https://web.archive.org/web/20190728160938/https://infocenter.arm.com/help/topic/com.arm.doc.ihi0038b/IHI0038B_ehabi.pdf // @@ -104,90 +109,94 @@ cfg_if::cfg_if! { exception_object: *mut uw::_Unwind_Exception, context: *mut uw::_Unwind_Context, ) -> uw::_Unwind_Reason_Code { - let state = state as c_int; - let action = state & uw::_US_ACTION_MASK as c_int; - let search_phase = if action == uw::_US_VIRTUAL_UNWIND_FRAME as c_int { - // Backtraces on ARM will call the personality routine with - // state == _US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND. In those cases - // we want to continue unwinding the stack, otherwise all our backtraces - // would end at __rust_try - if state & uw::_US_FORCE_UNWIND as c_int != 0 { + unsafe { + let state = state as c_int; + let action = state & uw::_US_ACTION_MASK as c_int; + let search_phase = if action == uw::_US_VIRTUAL_UNWIND_FRAME as c_int { + // Backtraces on ARM will call the personality routine with + // state == _US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND. In those cases + // we want to continue unwinding the stack, otherwise all our backtraces + // would end at __rust_try + if state & uw::_US_FORCE_UNWIND as c_int != 0 { + return continue_unwind(exception_object, context); + } + true + } else if action == uw::_US_UNWIND_FRAME_STARTING as c_int { + false + } else if action == uw::_US_UNWIND_FRAME_RESUME as c_int { return continue_unwind(exception_object, context); - } - true - } else if action == uw::_US_UNWIND_FRAME_STARTING as c_int { - false - } else if action == uw::_US_UNWIND_FRAME_RESUME as c_int { - return continue_unwind(exception_object, context); - } else { - return uw::_URC_FAILURE; - }; + } else { + return uw::_URC_FAILURE; + }; - // The DWARF unwinder assumes that _Unwind_Context holds things like the function - // and LSDA pointers, however ARM EHABI places them into the exception object. - // To preserve signatures of functions like _Unwind_GetLanguageSpecificData(), which - // take only the context pointer, GCC personality routines stash a pointer to - // exception_object in the context, using location reserved for ARM's - // "scratch register" (r12). - uw::_Unwind_SetGR(context, uw::UNWIND_POINTER_REG, exception_object as uw::_Unwind_Ptr); - // ...A more principled approach would be to provide the full definition of ARM's - // _Unwind_Context in our libunwind bindings and fetch the required data from there - // directly, bypassing DWARF compatibility functions. + // The DWARF unwinder assumes that _Unwind_Context holds things like the function + // and LSDA pointers, however ARM EHABI places them into the exception object. + // To preserve signatures of functions like _Unwind_GetLanguageSpecificData(), which + // take only the context pointer, GCC personality routines stash a pointer to + // exception_object in the context, using location reserved for ARM's + // "scratch register" (r12). + uw::_Unwind_SetGR(context, uw::UNWIND_POINTER_REG, exception_object as uw::_Unwind_Ptr); + // ...A more principled approach would be to provide the full definition of ARM's + // _Unwind_Context in our libunwind bindings and fetch the required data from there + // directly, bypassing DWARF compatibility functions. - let eh_action = match find_eh_action(context) { - Ok(action) => action, - Err(_) => return uw::_URC_FAILURE, - }; - if search_phase { - match eh_action { - EHAction::None | EHAction::Cleanup(_) => { - return continue_unwind(exception_object, context); + let eh_action = match find_eh_action(context) { + Ok(action) => action, + Err(_) => return uw::_URC_FAILURE, + }; + if search_phase { + match eh_action { + EHAction::None | EHAction::Cleanup(_) => { + return continue_unwind(exception_object, context); + } + EHAction::Catch(_) | EHAction::Filter(_) => { + // EHABI requires the personality routine to update the + // SP value in the barrier cache of the exception object. + (*exception_object).private[5] = + uw::_Unwind_GetGR(context, uw::UNWIND_SP_REG); + return uw::_URC_HANDLER_FOUND; + } + EHAction::Terminate => return uw::_URC_FAILURE, } - EHAction::Catch(_) | EHAction::Filter(_) => { - // EHABI requires the personality routine to update the - // SP value in the barrier cache of the exception object. - (*exception_object).private[5] = - uw::_Unwind_GetGR(context, uw::UNWIND_SP_REG); - return uw::_URC_HANDLER_FOUND; - } - EHAction::Terminate => return uw::_URC_FAILURE, - } - } else { - match eh_action { - EHAction::None => return continue_unwind(exception_object, context), - EHAction::Filter(_) if state & uw::_US_FORCE_UNWIND as c_int != 0 => return continue_unwind(exception_object, context), - EHAction::Cleanup(lpad) | EHAction::Catch(lpad) | EHAction::Filter(lpad) => { - uw::_Unwind_SetGR( - context, - UNWIND_DATA_REG.0, - exception_object as uw::_Unwind_Ptr, - ); - uw::_Unwind_SetGR(context, UNWIND_DATA_REG.1, core::ptr::null()); - uw::_Unwind_SetIP(context, lpad); - return uw::_URC_INSTALL_CONTEXT; + } else { + match eh_action { + EHAction::None => return continue_unwind(exception_object, context), + EHAction::Filter(_) if state & uw::_US_FORCE_UNWIND as c_int != 0 => return continue_unwind(exception_object, context), + EHAction::Cleanup(lpad) | EHAction::Catch(lpad) | EHAction::Filter(lpad) => { + uw::_Unwind_SetGR( + context, + UNWIND_DATA_REG.0, + exception_object as uw::_Unwind_Ptr, + ); + uw::_Unwind_SetGR(context, UNWIND_DATA_REG.1, core::ptr::null()); + uw::_Unwind_SetIP(context, lpad); + return uw::_URC_INSTALL_CONTEXT; + } + EHAction::Terminate => return uw::_URC_FAILURE, } - EHAction::Terminate => return uw::_URC_FAILURE, } - } - // On ARM EHABI the personality routine is responsible for actually - // unwinding a single stack frame before returning (ARM EHABI Sec. 6.1). - unsafe fn continue_unwind( - exception_object: *mut uw::_Unwind_Exception, - context: *mut uw::_Unwind_Context, - ) -> uw::_Unwind_Reason_Code { - if __gnu_unwind_frame(exception_object, context) == uw::_URC_NO_REASON { - uw::_URC_CONTINUE_UNWIND - } else { - uw::_URC_FAILURE - } - } - // defined in libgcc - extern "C" { - fn __gnu_unwind_frame( + // On ARM EHABI the personality routine is responsible for actually + // unwinding a single stack frame before returning (ARM EHABI Sec. 6.1). + unsafe fn continue_unwind( exception_object: *mut uw::_Unwind_Exception, context: *mut uw::_Unwind_Context, - ) -> uw::_Unwind_Reason_Code; + ) -> uw::_Unwind_Reason_Code { + unsafe { + if __gnu_unwind_frame(exception_object, context) == uw::_URC_NO_REASON { + uw::_URC_CONTINUE_UNWIND + } else { + uw::_URC_FAILURE + } + } + } + // defined in libgcc + extern "C" { + fn __gnu_unwind_frame( + exception_object: *mut uw::_Unwind_Exception, + context: *mut uw::_Unwind_Context, + ) -> uw::_Unwind_Reason_Code; + } } } } else { @@ -200,35 +209,37 @@ cfg_if::cfg_if! { exception_object: *mut uw::_Unwind_Exception, context: *mut uw::_Unwind_Context, ) -> uw::_Unwind_Reason_Code { - if version != 1 { - return uw::_URC_FATAL_PHASE1_ERROR; - } - let eh_action = match find_eh_action(context) { - Ok(action) => action, - Err(_) => return uw::_URC_FATAL_PHASE1_ERROR, - }; - if actions as i32 & uw::_UA_SEARCH_PHASE as i32 != 0 { - match eh_action { - EHAction::None | EHAction::Cleanup(_) => uw::_URC_CONTINUE_UNWIND, - EHAction::Catch(_) | EHAction::Filter(_) => uw::_URC_HANDLER_FOUND, - EHAction::Terminate => uw::_URC_FATAL_PHASE1_ERROR, + unsafe { + if version != 1 { + return uw::_URC_FATAL_PHASE1_ERROR; } - } else { - match eh_action { - EHAction::None => uw::_URC_CONTINUE_UNWIND, - // Forced unwinding hits a terminate action. - EHAction::Filter(_) if actions as i32 & uw::_UA_FORCE_UNWIND as i32 != 0 => uw::_URC_CONTINUE_UNWIND, - EHAction::Cleanup(lpad) | EHAction::Catch(lpad) | EHAction::Filter(lpad) => { - uw::_Unwind_SetGR( - context, - UNWIND_DATA_REG.0, - exception_object.cast(), - ); - uw::_Unwind_SetGR(context, UNWIND_DATA_REG.1, core::ptr::null()); - uw::_Unwind_SetIP(context, lpad); - uw::_URC_INSTALL_CONTEXT + let eh_action = match find_eh_action(context) { + Ok(action) => action, + Err(_) => return uw::_URC_FATAL_PHASE1_ERROR, + }; + if actions as i32 & uw::_UA_SEARCH_PHASE as i32 != 0 { + match eh_action { + EHAction::None | EHAction::Cleanup(_) => uw::_URC_CONTINUE_UNWIND, + EHAction::Catch(_) | EHAction::Filter(_) => uw::_URC_HANDLER_FOUND, + EHAction::Terminate => uw::_URC_FATAL_PHASE1_ERROR, + } + } else { + match eh_action { + EHAction::None => uw::_URC_CONTINUE_UNWIND, + // Forced unwinding hits a terminate action. + EHAction::Filter(_) if actions as i32 & uw::_UA_FORCE_UNWIND as i32 != 0 => uw::_URC_CONTINUE_UNWIND, + EHAction::Cleanup(lpad) | EHAction::Catch(lpad) | EHAction::Filter(lpad) => { + uw::_Unwind_SetGR( + context, + UNWIND_DATA_REG.0, + exception_object.cast(), + ); + uw::_Unwind_SetGR(context, UNWIND_DATA_REG.1, core::ptr::null()); + uw::_Unwind_SetIP(context, lpad); + uw::_URC_INSTALL_CONTEXT + } + EHAction::Terminate => uw::_URC_FATAL_PHASE2_ERROR, } - EHAction::Terminate => uw::_URC_FATAL_PHASE2_ERROR, } } } @@ -245,13 +256,15 @@ cfg_if::cfg_if! { contextRecord: *mut uw::CONTEXT, dispatcherContext: *mut uw::DISPATCHER_CONTEXT, ) -> uw::EXCEPTION_DISPOSITION { - uw::_GCC_specific_handler( - exceptionRecord, - establisherFrame, - contextRecord, - dispatcherContext, - rust_eh_personality_impl, - ) + unsafe { + uw::_GCC_specific_handler( + exceptionRecord, + establisherFrame, + contextRecord, + dispatcherContext, + rust_eh_personality_impl, + ) + } } } else { // The personality routine for most of our targets. @@ -263,13 +276,15 @@ cfg_if::cfg_if! { exception_object: *mut uw::_Unwind_Exception, context: *mut uw::_Unwind_Context, ) -> uw::_Unwind_Reason_Code { - rust_eh_personality_impl( - version, - actions, - exception_class, - exception_object, - context, - ) + unsafe { + rust_eh_personality_impl( + version, + actions, + exception_class, + exception_object, + context, + ) + } } } } @@ -277,18 +292,20 @@ cfg_if::cfg_if! { } unsafe fn find_eh_action(context: *mut uw::_Unwind_Context) -> Result { - let lsda = uw::_Unwind_GetLanguageSpecificData(context) as *const u8; - let mut ip_before_instr: c_int = 0; - let ip = uw::_Unwind_GetIPInfo(context, &mut ip_before_instr); - let eh_context = EHContext { - // The return address points 1 byte past the call instruction, - // which could be in the next IP range in LSDA range table. - // - // `ip = -1` has special meaning, so use wrapping sub to allow for that - ip: if ip_before_instr != 0 { ip } else { ip.wrapping_sub(1) }, - func_start: uw::_Unwind_GetRegionStart(context), - get_text_start: &|| uw::_Unwind_GetTextRelBase(context), - get_data_start: &|| uw::_Unwind_GetDataRelBase(context), - }; - eh::find_eh_action(lsda, &eh_context) + unsafe { + let lsda = uw::_Unwind_GetLanguageSpecificData(context) as *const u8; + let mut ip_before_instr: c_int = 0; + let ip = uw::_Unwind_GetIPInfo(context, &mut ip_before_instr); + let eh_context = EHContext { + // The return address points 1 byte past the call instruction, + // which could be in the next IP range in LSDA range table. + // + // `ip = -1` has special meaning, so use wrapping sub to allow for that + ip: if ip_before_instr != 0 { ip } else { ip.wrapping_sub(1) }, + func_start: uw::_Unwind_GetRegionStart(context), + get_text_start: &|| uw::_Unwind_GetTextRelBase(context), + get_data_start: &|| uw::_Unwind_GetDataRelBase(context), + }; + eh::find_eh_action(lsda, &eh_context) + } } From c9cd4a68532fb963cc8b9074242848baa3bd73cc Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 24 Jul 2024 11:53:56 -0700 Subject: [PATCH 06/10] std: update comments on gcc personality fn --- library/std/src/sys/personality/gcc.rs | 42 ++++++++++++++++++++------ 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/library/std/src/sys/personality/gcc.rs b/library/std/src/sys/personality/gcc.rs index 5542cf03d20c1..1fb85a10179c5 100644 --- a/library/std/src/sys/personality/gcc.rs +++ b/library/std/src/sys/personality/gcc.rs @@ -98,11 +98,12 @@ cfg_if::cfg_if! { not(all(target_vendor = "apple", not(target_os = "watchos"))), not(target_os = "netbsd"), ))] { - // ARM EHABI personality routine. - // https://web.archive.org/web/20190728160938/https://infocenter.arm.com/help/topic/com.arm.doc.ihi0038b/IHI0038B_ehabi.pdf - // - // Apple 32-bit ARM (but not watchOS) uses the default routine instead - // since it uses SjLj unwinding. + /// personality fn called by [ARM EHABI][armeabi-eh] + /// + /// Apple 32-bit ARM (but not watchOS) uses the default routine instead + /// since it uses "setjmp-longjmp" unwinding. + /// + /// [armeabi-eh]: https://web.archive.org/web/20190728160938/https://infocenter.arm.com/help/topic/com.arm.doc.ihi0038b/IHI0038B_ehabi.pdf #[lang = "eh_personality"] unsafe extern "C" fn rust_eh_personality( state: uw::_Unwind_State, @@ -200,8 +201,8 @@ cfg_if::cfg_if! { } } } else { - // Default personality routine, which is used directly on most targets - // and indirectly on Windows x86_64 via SEH. + /// Default personality routine, which is used directly on most targets + /// and indirectly on Windows x86_64 and AArch64 via SEH. unsafe extern "C" fn rust_eh_personality_impl( version: c_int, actions: uw::_Unwind_Action, @@ -246,8 +247,12 @@ cfg_if::cfg_if! { cfg_if::cfg_if! { if #[cfg(all(windows, any(target_arch = "aarch64", target_arch = "x86_64"), target_env = "gnu"))] { - // On x86_64 MinGW targets, the unwinding mechanism is SEH however the unwind - // handler data (aka LSDA) uses GCC-compatible encoding. + /// personality fn called by [Windows Structured Exception Handling][windows-eh] + /// + /// On x86_64 and AArch64 MinGW targets, the unwinding mechanism is SEH, + /// however the unwind handler data (aka LSDA) uses GCC-compatible encoding + /// + /// [windows-eh]: https://learn.microsoft.com/en-us/cpp/cpp/structured-exception-handling-c-cpp?view=msvc-170 #[lang = "eh_personality"] #[allow(nonstandard_style)] unsafe extern "C" fn rust_eh_personality( @@ -256,6 +261,9 @@ cfg_if::cfg_if! { contextRecord: *mut uw::CONTEXT, dispatcherContext: *mut uw::DISPATCHER_CONTEXT, ) -> uw::EXCEPTION_DISPOSITION { + // SAFETY: the cfg is still target_os = "windows" and target_env = "gnu", + // which means that this is the correct function to call, passing our impl fn + // as the callback which gets actually used unsafe { uw::_GCC_specific_handler( exceptionRecord, @@ -267,7 +275,19 @@ cfg_if::cfg_if! { } } } else { - // The personality routine for most of our targets. + /// personality fn called by [Itanium C++ ABI Exception Handling][itanium-eh] + /// + /// The personality routine for most non-Windows targets. This will be called by + /// the unwinding library: + /// - "In the search phase, the framework repeatedly calls the personality routine, + /// with the _UA_SEARCH_PHASE flag as described below, first for the current PC + /// and register state, and then unwinding a frame to a new PC at each step..." + /// - "If the search phase reports success, the framework restarts in the cleanup + /// phase. Again, it repeatedly calls the personality routine, with the + /// _UA_CLEANUP_PHASE flag as described below, first for the current PC and + /// register state, and then unwinding a frame to a new PC at each step..."i + /// + /// [itanium-eh]: https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html #[lang = "eh_personality"] unsafe extern "C" fn rust_eh_personality( version: c_int, @@ -276,6 +296,8 @@ cfg_if::cfg_if! { exception_object: *mut uw::_Unwind_Exception, context: *mut uw::_Unwind_Context, ) -> uw::_Unwind_Reason_Code { + // SAFETY: the platform support must modify the cfg for the inner fn + // if it needs something different than what is currently invoked. unsafe { rust_eh_personality_impl( version, From 40d132f0f826b0f0ae203219ab30d23da8f55573 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 24 Jul 2024 22:59:38 -0400 Subject: [PATCH 07/10] Make sure that args are compatible in resolve_associated_item --- compiler/rustc_ty_utils/src/instance.rs | 15 +++++++-- tests/crashes/120792.rs | 25 --------------- tests/crashes/120793-2.rs | 22 ------------- tests/crashes/120793.rs | 21 ------------ tests/crashes/121063.rs | 20 ------------ tests/crashes/121957-1.rs | 20 ------------ tests/crashes/121957-2.rs | 20 ------------ .../inline-incorrect-early-bound.rs | 27 ++++++++++++++++ .../inline-incorrect-early-bound.stderr | 15 +++++++++ .../inline-incorrect-early-bound-in-ctfe.rs | 32 +++++++++++++++++++ ...nline-incorrect-early-bound-in-ctfe.stderr | 21 ++++++++++++ 11 files changed, 108 insertions(+), 130 deletions(-) delete mode 100644 tests/crashes/120792.rs delete mode 100644 tests/crashes/120793-2.rs delete mode 100644 tests/crashes/120793.rs delete mode 100644 tests/crashes/121063.rs delete mode 100644 tests/crashes/121957-1.rs delete mode 100644 tests/crashes/121957-2.rs create mode 100644 tests/ui/polymorphization/inline-incorrect-early-bound.rs create mode 100644 tests/ui/polymorphization/inline-incorrect-early-bound.stderr create mode 100644 tests/ui/rfcs/rfc-2632-const-trait-impl/inline-incorrect-early-bound-in-ctfe.rs create mode 100644 tests/ui/rfcs/rfc-2632-const-trait-impl/inline-incorrect-early-bound-in-ctfe.stderr diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 7b6d86d22a578..a2bed61a7ae1a 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -191,11 +191,22 @@ fn resolve_associated_item<'tcx>( // Any final impl is required to define all associated items. if !leaf_def.item.defaultness(tcx).has_value() { - let guard = tcx.dcx().span_delayed_bug( + let guar = tcx.dcx().span_delayed_bug( tcx.def_span(leaf_def.item.def_id), "missing value for assoc item in impl", ); - return Err(guard); + return Err(guar); + } + + // Make sure that we're projecting to an item that has compatible args. + // This may happen if we are resolving an instance before codegen, such + // as during inlining. This check is also done in projection. + if !tcx.check_args_compatible(leaf_def.item.def_id, args) { + let guar = tcx.dcx().span_delayed_bug( + tcx.def_span(leaf_def.item.def_id), + "missing value for assoc item in impl", + ); + return Err(guar); } let args = tcx.erase_regions(args); diff --git a/tests/crashes/120792.rs b/tests/crashes/120792.rs deleted file mode 100644 index 51889251787d4..0000000000000 --- a/tests/crashes/120792.rs +++ /dev/null @@ -1,25 +0,0 @@ -//@ known-bug: #120792 -//@ compile-flags: -Zpolymorphize=on -Zinline-mir=yes - -impl Trait<()> for () { - fn foo<'a, K>(self, _: (), _: K) { - todo!(); - } -} - -trait Foo {} - -impl Foo for F { - fn main() { - ().foo((), ()); - } -} - -trait Trait { - fn foo<'a, K>(self, _: T, _: K) - where - T: 'a, - K: 'a; -} - -pub fn main() {} diff --git a/tests/crashes/120793-2.rs b/tests/crashes/120793-2.rs deleted file mode 100644 index 0ce5e4df2247c..0000000000000 --- a/tests/crashes/120793-2.rs +++ /dev/null @@ -1,22 +0,0 @@ -//@ known-bug: #120793 -// can't use build-fail, because this also fails check-fail, but -// the ICE from #120787 only reproduces on build-fail. -//@ compile-flags: --emit=mir - -#![feature(effects)] - -trait Dim { - fn dim() -> usize; -} - -enum Dim3 {} - -impl Dim for Dim3 { - fn dim(x: impl Sized) -> usize { - 3 - } -} - -fn main() { - [0; Dim3::dim()]; -} diff --git a/tests/crashes/120793.rs b/tests/crashes/120793.rs deleted file mode 100644 index 7e9166a50e553..0000000000000 --- a/tests/crashes/120793.rs +++ /dev/null @@ -1,21 +0,0 @@ -//@ known-bug: #120793 -#![feature(effects)] - -trait Dim { - fn dim() -> usize; -} - -enum Dim3 {} - -impl Dim for Dim3 { - fn dim(mut x: impl Iterator) -> usize { - 3 - } -} - -fn main() { - let array: [usize; Dim3::dim()] - //~^ ERROR E0015 - = [0; Dim3::dim()]; - //~^ ERROR E0015 -} diff --git a/tests/crashes/121063.rs b/tests/crashes/121063.rs deleted file mode 100644 index cb9db2853c2ac..0000000000000 --- a/tests/crashes/121063.rs +++ /dev/null @@ -1,20 +0,0 @@ -//@ known-bug: #121063 -//@ compile-flags: -Zpolymorphize=on --edition=2021 -Zinline-mir=yes - -use std::{ - fmt, ops, - path::{Component, Path, PathBuf}, -}; - -pub struct AbsPathBuf(PathBuf); - -impl TryFrom for AbsPathBuf { - type Error = PathBuf; - fn try_from(path: impl AsRef) -> Result {} -} - -impl TryFrom<&str> for AbsPathBuf { - fn try_from(path: &str) -> Result { - AbsPathBuf::try_from(PathBuf::from(path)) - } -} diff --git a/tests/crashes/121957-1.rs b/tests/crashes/121957-1.rs deleted file mode 100644 index 74b4649cc9d95..0000000000000 --- a/tests/crashes/121957-1.rs +++ /dev/null @@ -1,20 +0,0 @@ -//@ known-bug: #121957 -#![feature(const_trait_impl, effects)] - -#[const_trait] -trait Main { - fn compute() -> u32; -} - -impl const Main for () { - fn compute<'x, 'y, 'z: 'x>() -> u32 {} -} - -#[const_trait] -trait Aux {} - -impl const Aux for () {} - -fn main() { - const _: u32 = <()>::compute::<()>(); -} diff --git a/tests/crashes/121957-2.rs b/tests/crashes/121957-2.rs deleted file mode 100644 index 74b4649cc9d95..0000000000000 --- a/tests/crashes/121957-2.rs +++ /dev/null @@ -1,20 +0,0 @@ -//@ known-bug: #121957 -#![feature(const_trait_impl, effects)] - -#[const_trait] -trait Main { - fn compute() -> u32; -} - -impl const Main for () { - fn compute<'x, 'y, 'z: 'x>() -> u32 {} -} - -#[const_trait] -trait Aux {} - -impl const Aux for () {} - -fn main() { - const _: u32 = <()>::compute::<()>(); -} diff --git a/tests/ui/polymorphization/inline-incorrect-early-bound.rs b/tests/ui/polymorphization/inline-incorrect-early-bound.rs new file mode 100644 index 0000000000000..e69e4a4faa05d --- /dev/null +++ b/tests/ui/polymorphization/inline-incorrect-early-bound.rs @@ -0,0 +1,27 @@ +// This test demonstrates an ICE that may occur when we try to resolve the instance +// of a impl that has different generics than the trait it's implementing. This ensures +// we first check that the args are compatible before resolving the body, just like +// we do in projection before substituting a GAT. +// +// When polymorphization is enabled, we check the optimized MIR for unused parameters. +// This will invoke the inliner, leading to this ICE. + +//@ compile-flags: -Zpolymorphize=on -Zinline-mir=yes + +trait Trait { + fn foo<'a, K: 'a>(self, _: K); +} + +impl Trait for () { + #[inline] + fn foo(self, _: K) { + //~^ ERROR lifetime parameters or bounds on method `foo` do not match the trait declaration + todo!(); + } +} + +pub fn qux() { + ().foo(()); +} + +fn main() {} diff --git a/tests/ui/polymorphization/inline-incorrect-early-bound.stderr b/tests/ui/polymorphization/inline-incorrect-early-bound.stderr new file mode 100644 index 0000000000000..3a1d05e8a3613 --- /dev/null +++ b/tests/ui/polymorphization/inline-incorrect-early-bound.stderr @@ -0,0 +1,15 @@ +error[E0195]: lifetime parameters or bounds on method `foo` do not match the trait declaration + --> $DIR/inline-incorrect-early-bound.rs:17:11 + | +LL | fn foo<'a, K: 'a>(self, _: K); + | ----------- + | | | + | | this bound might be missing in the impl + | lifetimes in impl do not match this method in trait +... +LL | fn foo(self, _: K) { + | ^^^ lifetimes do not match method in trait + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0195`. diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/inline-incorrect-early-bound-in-ctfe.rs b/tests/ui/rfcs/rfc-2632-const-trait-impl/inline-incorrect-early-bound-in-ctfe.rs new file mode 100644 index 0000000000000..e3adcce17b42c --- /dev/null +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/inline-incorrect-early-bound-in-ctfe.rs @@ -0,0 +1,32 @@ +// This test demonstrates an ICE that may occur when we try to resolve the instance +// of a impl that has different generics than the trait it's implementing. This ensures +// we first check that the args are compatible before resolving the body, just like +// we do in projection before substituting a GAT. +// +// Const traits aren't the only way to achieve this ICE, but it's a convenient way +// to ensure the inliner is called. + +//@ compile-flags: -Znext-solver -Zinline-mir=yes + +#![feature(const_trait_impl, effects)] +//~^ WARN the feature `effects` is incomplete + +trait Trait { + fn foo(self); +} + +impl Trait for () { + #[inline] + fn foo(self) { + //~^ ERROR method `foo` has 1 type parameter but its trait declaration has 0 type parameters + todo!(); + } +} + +const fn foo() { + ().foo(); +} + +const UWU: () = foo(); + +fn main() {} diff --git a/tests/ui/rfcs/rfc-2632-const-trait-impl/inline-incorrect-early-bound-in-ctfe.stderr b/tests/ui/rfcs/rfc-2632-const-trait-impl/inline-incorrect-early-bound-in-ctfe.stderr new file mode 100644 index 0000000000000..2e7801c0b8acd --- /dev/null +++ b/tests/ui/rfcs/rfc-2632-const-trait-impl/inline-incorrect-early-bound-in-ctfe.stderr @@ -0,0 +1,21 @@ +warning: the feature `effects` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/inline-incorrect-early-bound-in-ctfe.rs:11:30 + | +LL | #![feature(const_trait_impl, effects)] + | ^^^^^^^ + | + = note: see issue #102090 for more information + = note: `#[warn(incomplete_features)]` on by default + +error[E0049]: method `foo` has 1 type parameter but its trait declaration has 0 type parameters + --> $DIR/inline-incorrect-early-bound-in-ctfe.rs:20:12 + | +LL | fn foo(self); + | - expected 0 type parameters +... +LL | fn foo(self) { + | ^ found 1 type parameter + +error: aborting due to 1 previous error; 1 warning emitted + +For more information about this error, try `rustc --explain E0049`. From d004edf311eef38e91a6cd490629c60b55d16e09 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 24 Jul 2024 23:36:42 -0400 Subject: [PATCH 08/10] Don't ICE if HIR and middle types disagree in borrowck error reporting --- .../src/diagnostics/conflict_errors.rs | 34 +++++++++++++----- tests/crashes/121816.rs | 12 ------- tests/ui/borrowck/ice-on-non-ref-sig-ty.rs | 19 ++++++++++ .../ui/borrowck/ice-on-non-ref-sig-ty.stderr | 36 +++++++++++++++++++ 4 files changed, 81 insertions(+), 20 deletions(-) delete mode 100644 tests/crashes/121816.rs create mode 100644 tests/ui/borrowck/ice-on-non-ref-sig-ty.rs create mode 100644 tests/ui/borrowck/ice-on-non-ref-sig-ty.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index b147567001db1..2d9bc45ebc823 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -4304,17 +4304,35 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> { // search for relevant arguments. let mut arguments = Vec::new(); for (index, argument) in sig.inputs().skip_binder().iter().enumerate() { - if let ty::Ref(argument_region, _, _) = argument.kind() { - if argument_region == return_region { - // Need to use the `rustc_middle::ty` types to compare against the - // `return_region`. Then use the `rustc_hir` type to get only - // the lifetime span. - if let hir::TyKind::Ref(lifetime, _) = &fn_decl.inputs[index].kind { + if let ty::Ref(argument_region, _, _) = argument.kind() + && argument_region == return_region + { + // Need to use the `rustc_middle::ty` types to compare against the + // `return_region`. Then use the `rustc_hir` type to get only + // the lifetime span. + match &fn_decl.inputs[index].kind { + hir::TyKind::Ref(lifetime, _) => { // With access to the lifetime, we can get // the span of it. arguments.push((*argument, lifetime.ident.span)); - } else { - bug!("ty type is a ref but hir type is not"); + } + // Resolve `self` whose self type is `&T`. + hir::TyKind::Path(hir::QPath::Resolved(None, path)) => { + if let Res::SelfTyAlias { alias_to, .. } = path.res + && let Some(alias_to) = alias_to.as_local() + && let hir::Impl { self_ty, .. } = self + .infcx + .tcx + .hir_node_by_def_id(alias_to) + .expect_item() + .expect_impl() + && let hir::TyKind::Ref(lifetime, _) = self_ty.kind + { + arguments.push((*argument, lifetime.ident.span)); + } + } + _ => { + // Don't ICE though. It might be a type alias. } } } diff --git a/tests/crashes/121816.rs b/tests/crashes/121816.rs deleted file mode 100644 index a5569ea19d3ea..0000000000000 --- a/tests/crashes/121816.rs +++ /dev/null @@ -1,12 +0,0 @@ -//@ known-bug: #121816 -fn f<'a, T>(_: &'static &'a (), x: &'a T) -> &'static T { - x -} -trait W<'a> { - fn g(self, x: &'a T) -> &'static T; -} -impl<'a> W<'a> for &'static () { - fn g(self, x: &'a T) -> &'static T { - f(&self, x) - } -} diff --git a/tests/ui/borrowck/ice-on-non-ref-sig-ty.rs b/tests/ui/borrowck/ice-on-non-ref-sig-ty.rs new file mode 100644 index 0000000000000..1c867bd2378d0 --- /dev/null +++ b/tests/ui/borrowck/ice-on-non-ref-sig-ty.rs @@ -0,0 +1,19 @@ +// Don't ICE when trying to annotate signature and we see `&()` + +fn f<'a, T>(_: &'static &'a (), x: &'a T) -> &'static T { + x +} +trait W<'a> { + fn g(self, x: &'a T) -> &'static T; +} + +// Frankly this error message is impossible to parse, but :shrug:. +impl<'a> W<'a> for &'static () { + fn g(self, x: &'a T) -> &'static T { + f(&self, x) + //~^ ERROR borrowed data escapes outside of method + //~| ERROR `self` does not live long enough + } +} + +fn main() {} diff --git a/tests/ui/borrowck/ice-on-non-ref-sig-ty.stderr b/tests/ui/borrowck/ice-on-non-ref-sig-ty.stderr new file mode 100644 index 0000000000000..2b900a8e68a7b --- /dev/null +++ b/tests/ui/borrowck/ice-on-non-ref-sig-ty.stderr @@ -0,0 +1,36 @@ +error[E0521]: borrowed data escapes outside of method + --> $DIR/ice-on-non-ref-sig-ty.rs:13:9 + | +LL | impl<'a> W<'a> for &'static () { + | -- lifetime `'a` defined here +LL | fn g(self, x: &'a T) -> &'static T { + | ---- - `x` is a reference that is only valid in the method body + | | + | `self` declared here, outside of the method body +LL | f(&self, x) + | ^^^^^^^^^^^ + | | + | `x` escapes the method body here + | argument requires that `'a` must outlive `'static` + +error[E0597]: `self` does not live long enough + --> $DIR/ice-on-non-ref-sig-ty.rs:13:11 + | +LL | impl<'a> W<'a> for &'static () { + | ------- has lifetime `'static` +LL | fn g(self, x: &'a T) -> &'static T { + | ------- also has lifetime `'static` +LL | f(&self, x) + | ^^^^^ `self` would have to be valid for `'static`... +... +LL | } + | - ...but `self` will be dropped here, when the function `g` returns + | + = help: use data from the highlighted arguments which match the `'static` lifetime of the return type + = note: functions cannot return a borrow to data owned within the function's scope, functions can only return borrows to data passed as arguments + = note: to learn more, visit + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0521, E0597. +For more information about an error, try `rustc --explain E0521`. From 34819b72983af230c3e413371a096e94c684a5e5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 24 Jul 2024 23:49:21 -0400 Subject: [PATCH 09/10] Don't add crashes for misuses of intrinsics --- tests/crashes/101962.rs | 11 ----------- tests/crashes/111699.rs | 14 -------------- tests/crashes/97501.rs | 22 ---------------------- 3 files changed, 47 deletions(-) delete mode 100644 tests/crashes/101962.rs delete mode 100644 tests/crashes/111699.rs delete mode 100644 tests/crashes/97501.rs diff --git a/tests/crashes/101962.rs b/tests/crashes/101962.rs deleted file mode 100644 index b6a78ce053af0..0000000000000 --- a/tests/crashes/101962.rs +++ /dev/null @@ -1,11 +0,0 @@ -//@ known-bug: #101962 - -#![feature(core_intrinsics)] - -pub fn wrapping(a: T, b: T) { - let _z = core::intrinsics::wrapping_mul(a, b); -} - -fn main() { - wrapping(1,2); -} diff --git a/tests/crashes/111699.rs b/tests/crashes/111699.rs deleted file mode 100644 index 5ba17c2aa1afc..0000000000000 --- a/tests/crashes/111699.rs +++ /dev/null @@ -1,14 +0,0 @@ -//@ known-bug: #111699 -//@ edition:2021 -//@ compile-flags: -Copt-level=0 -#![feature(core_intrinsics)] -use std::intrinsics::offset; - -fn main() { - let a = [1u8, 2, 3]; - let ptr: *const u8 = a.as_ptr(); - - unsafe { - assert_eq!(*offset(ptr, 0), 1); - } -} diff --git a/tests/crashes/97501.rs b/tests/crashes/97501.rs deleted file mode 100644 index 51a83d8be169d..0000000000000 --- a/tests/crashes/97501.rs +++ /dev/null @@ -1,22 +0,0 @@ -//@ known-bug: #97501 - -#![feature(core_intrinsics)] -use std::intrinsics::wrapping_add; - -#[derive(Clone, Copy)] -struct WrapInt8 { - value: u8 -} - -impl std::ops::Add for WrapInt8 { - type Output = WrapInt8; - fn add(self, other: WrapInt8) -> WrapInt8 { - wrapping_add(self, other) - } -} - -fn main() { - let p = WrapInt8 { value: 123 }; - let q = WrapInt8 { value: 234 }; - println!("{}", (p + q).value); -} From 17b4fbc388ec11a3c1cdf7e33ff00a0322edaf98 Mon Sep 17 00:00:00 2001 From: B I Mohammed Abbas Date: Thu, 25 Jul 2024 15:11:26 +0530 Subject: [PATCH 10/10] In connect timeout, read readiness of socket for vxworks. Check pollhup or pollerr for refused connections in linux --- library/std/src/sys/pal/unix/net.rs | 29 +++++++++++++++++++---------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/library/std/src/sys/pal/unix/net.rs b/library/std/src/sys/pal/unix/net.rs index b8dc1538a6378..092c3ee594d00 100644 --- a/library/std/src/sys/pal/unix/net.rs +++ b/library/std/src/sys/pal/unix/net.rs @@ -213,16 +213,25 @@ impl Socket { } 0 => {} _ => { - // linux returns POLLOUT|POLLERR|POLLHUP for refused connections (!), so look - // for POLLHUP rather than read readiness - if pollfd.revents & libc::POLLHUP != 0 { - let e = self.take_error()?.unwrap_or_else(|| { - io::const_io_error!( - io::ErrorKind::Uncategorized, - "no error set after POLLHUP", - ) - }); - return Err(e); + if cfg!(target_os = "vxworks") { + // VxWorks poll does not return POLLHUP or POLLERR in revents. Check if the + // connnection actually succeeded and return ok only when the socket is + // ready and no errors were found. + if let Some(e) = self.take_error()? { + return Err(e); + } + } else { + // linux returns POLLOUT|POLLERR|POLLHUP for refused connections (!), so look + // for POLLHUP or POLLERR rather than read readiness + if pollfd.revents & (libc::POLLHUP | libc::POLLERR) != 0 { + let e = self.take_error()?.unwrap_or_else(|| { + io::const_io_error!( + io::ErrorKind::Uncategorized, + "no error set after POLLHUP", + ) + }); + return Err(e); + } } return Ok(());