Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow setting some built-in cfg via the command-line #126158

Merged
merged 2 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ fn make_format_args(
};
let arg_name = args.explicit_args()[index].kind.ident().unwrap();
ecx.buffered_early_lint.push(BufferedEarlyLint {
span: arg_name.span.into(),
span: Some(arg_name.span.into()),
node_id: rustc_ast::CRATE_NODE_ID,
lint_id: LintId::of(NAMED_ARGUMENTS_USED_POSITIONALLY),
diagnostic: BuiltinLintDiag::NamedArgumentUsedPositionally {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,10 @@ lint_undropped_manually_drops = calls to `std::mem::drop` with `std::mem::Manual
.label = argument has type `{$arg_ty}`
.suggestion = use `std::mem::ManuallyDrop::into_inner` to get the inner value

lint_unexpected_builtin_cfg = unexpected `--cfg {$cfg}` flag
.controlled_by = config `{$cfg_name}` is only supposed to be controlled by `{$controlled_by}`
.incoherent = manually setting a built-in cfg can and does create incoherent behaviors

lint_unexpected_cfg_add_build_rs_println = or consider adding `{$build_rs_println}` to the top of the `build.rs`
lint_unexpected_cfg_add_cargo_feature = consider using a Cargo feature instead
lint_unexpected_cfg_add_cargo_toml_lint_cfg = or consider adding in `Cargo.toml` the `check-cfg` lint config for the lint:{$cargo_toml_lint_cfg}
Expand Down
18 changes: 16 additions & 2 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ pub struct EarlyContext<'a> {
}

impl EarlyContext<'_> {
/// Emit a lint at the appropriate level, with an optional associated span and an existing
/// Emit a lint at the appropriate level, with an associated span and an existing
/// diagnostic.
///
/// [`lint_level`]: rustc_middle::lint::lint_level#decorate-signature
Expand All @@ -544,7 +544,21 @@ impl EarlyContext<'_> {
span: MultiSpan,
diagnostic: BuiltinLintDiag,
) {
self.opt_span_lint(lint, Some(span), |diag| {
self.opt_span_lint_with_diagnostics(lint, Some(span), diagnostic);
}

/// Emit a lint at the appropriate level, with an optional associated span and an existing
/// diagnostic.
///
/// [`lint_level`]: rustc_middle::lint::lint_level#decorate-signature
#[rustc_lint_diagnostics]
pub fn opt_span_lint_with_diagnostics(
&self,
lint: &'static Lint,
span: Option<MultiSpan>,
diagnostic: BuiltinLintDiag,
) {
self.opt_span_lint(lint, span, |diag| {
diagnostics::decorate_lint(self.sess(), diagnostic, diag);
});
}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/context/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -438,5 +438,8 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &
BuiltinLintDiag::OutOfScopeMacroCalls { path } => {
lints::OutOfScopeMacroCalls { path }.decorate_lint(diag)
}
BuiltinLintDiag::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by } => {
lints::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by }.decorate_lint(diag)
}
}
}
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ impl<'a, T: EarlyLintPass> EarlyContextAndPass<'a, T> {
fn inlined_check_id(&mut self, id: ast::NodeId) {
for early_lint in self.context.buffered.take(id) {
let BufferedEarlyLint { span, node_id: _, lint_id, diagnostic } = early_lint;
self.context.span_lint_with_diagnostics(lint_id.lint, span, diagnostic);
self.context.opt_span_lint_with_diagnostics(lint_id.lint, span, diagnostic);
}
}

Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2379,6 +2379,16 @@ pub mod unexpected_cfg_value {
}
}

#[derive(LintDiagnostic)]
#[diag(lint_unexpected_builtin_cfg)]
#[note(lint_controlled_by)]
#[note(lint_incoherent)]
pub struct UnexpectedBuiltinCfg {
pub(crate) cfg: String,
pub(crate) cfg_name: Symbol,
pub(crate) controlled_by: &'static str,
}

#[derive(LintDiagnostic)]
#[diag(lint_macro_use_deprecated)]
#[help]
Expand Down
34 changes: 34 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ declare_lint_pass! {
DUPLICATE_MACRO_ATTRIBUTES,
ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
ELIDED_LIFETIMES_IN_PATHS,
EXPLICIT_BUILTIN_CFGS_IN_FLAGS,
EXPORTED_PRIVATE_DEPENDENCIES,
FFI_UNWIND_CALLS,
FORBIDDEN_LINT_GROUPS,
Expand Down Expand Up @@ -3288,6 +3289,39 @@ declare_lint! {
"detects unexpected names and values in `#[cfg]` conditions",
}

declare_lint! {
/// The `explicit_builtin_cfgs_in_flags` lint detects builtin cfgs set via the `--cfg` flag.
///
/// ### Example
///
/// ```text
/// rustc --cfg unix
/// ```
///
/// ```rust,ignore (needs command line option)
/// fn main() {}
/// ```
///
/// This will produce:
///
/// ```text
/// error: unexpected `--cfg unix` flag
/// |
/// = note: config `unix` is only supposed to be controlled by `--target`
/// = note: manually setting a built-in cfg can and does create incoherent behaviors
/// = note: `#[deny(explicit_builtin_cfgs_in_flags)]` on by default
/// ```
///
/// ### Explanation
///
/// Setting builtin cfgs can and does produce incoherent behavior, it's better to the use
/// the appropriate `rustc` flag that controls the config. For example setting the `windows`
/// cfg but on Linux based target.
pub EXPLICIT_BUILTIN_CFGS_IN_FLAGS,
Deny,
"detects builtin cfgs set via the `--cfg`"
}

declare_lint! {
/// The `repr_transparent_external_private_fields` lint
/// detects types marked `#[repr(transparent)]` that (transitively)
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,14 +746,19 @@ pub enum BuiltinLintDiag {
OutOfScopeMacroCalls {
path: String,
},
UnexpectedBuiltinCfg {
cfg: String,
cfg_name: Symbol,
controlled_by: &'static str,
},
}

/// Lints that are buffered up early on in the `Session` before the
/// `LintLevels` is calculated.
#[derive(Debug)]
pub struct BufferedEarlyLint {
/// The span of code that we are linting on.
pub span: MultiSpan,
pub span: Option<MultiSpan>,
lcnr marked this conversation as resolved.
Show resolved Hide resolved

/// The `NodeId` of the AST node that generated the lint.
pub node_id: NodeId,
Expand Down Expand Up @@ -791,7 +796,7 @@ impl LintBuffer {
self.add_early_lint(BufferedEarlyLint {
lint_id: LintId::of(lint),
node_id,
span: span.into(),
span: Some(span.into()),
diagnostic,
});
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1304,7 +1304,10 @@ pub(crate) const fn default_lib_output() -> CrateType {
}

pub fn build_configuration(sess: &Session, mut user_cfg: Cfg) -> Cfg {
// Combine the configuration requested by the session (command line) with
// First disallow some configuration given on the command line
cfg::disallow_cfgs(sess, &user_cfg);

// Then combine the configuration requested by the session (command line) with
// some default and generated configuration items.
user_cfg.extend(cfg::default_configuration(sess));
user_cfg
Expand Down
65 changes: 65 additions & 0 deletions compiler/rustc_session/src/config/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@
//! - Add the activation logic in [`default_configuration`]
//! - Add the cfg to [`CheckCfg::fill_well_known`] (and related files),
//! so that the compiler can know the cfg is expected
//! - Add the cfg in [`disallow_cfgs`] to disallow users from setting it via `--cfg`
//! - Add the feature gating in `compiler/rustc_feature/src/builtin_attrs.rs`

use std::hash::Hash;
use std::iter;

use rustc_ast::ast;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_lint_defs::builtin::EXPLICIT_BUILTIN_CFGS_IN_FLAGS;
use rustc_lint_defs::BuiltinLintDiag;
use rustc_span::symbol::{sym, Symbol};
use rustc_target::abi::Align;
use rustc_target::spec::{PanicStrategy, RelocModel, SanitizerSet, Target, TargetTriple, TARGETS};
Expand Down Expand Up @@ -82,6 +86,67 @@ impl<'a, T: Eq + Hash + Copy + 'a> Extend<&'a T> for ExpectedValues<T> {
}
}

/// Disallow builtin cfgs from the CLI.
pub(crate) fn disallow_cfgs(sess: &Session, user_cfgs: &Cfg) {
let disallow = |cfg: &(Symbol, Option<Symbol>), controlled_by| {
let cfg_name = cfg.0;
let cfg = if let Some(value) = cfg.1 {
format!(r#"{}="{}""#, cfg_name, value)
} else {
format!("{}", cfg_name)
};
sess.psess.opt_span_buffer_lint(
EXPLICIT_BUILTIN_CFGS_IN_FLAGS,
None,
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
ast::CRATE_NODE_ID,
BuiltinLintDiag::UnexpectedBuiltinCfg { cfg, cfg_name, controlled_by },
)
};

// We want to restrict setting builtin cfgs that will produce incoherent behavior
// between the cfg and the rustc cli flag that sets it.
//
// The tests are in tests/ui/cfg/disallowed-cli-cfgs.rs.

// By-default all builtin cfgs are disallowed, only those are allowed:
// - test: as it makes sense to the have the `test` cfg active without the builtin
// test harness. See Cargo `harness = false` config.
//
// Cargo `--cfg test`: https://github.com/rust-lang/cargo/blob/bc89bffa5987d4af8f71011c7557119b39e44a65/src/cargo/core/compiler/mod.rs#L1124

for cfg in user_cfgs {
match cfg {
(sym::overflow_checks, None) => disallow(cfg, "-C overflow-checks"),
(sym::debug_assertions, None) => disallow(cfg, "-C debug-assertions"),
(sym::ub_checks, None) => disallow(cfg, "-Z ub-checks"),
(sym::sanitize, None | Some(_)) => disallow(cfg, "-Z sanitizer"),
(
sym::sanitizer_cfi_generalize_pointers | sym::sanitizer_cfi_normalize_integers,
None | Some(_),
) => disallow(cfg, "-Z sanitizer=cfi"),
(sym::proc_macro, None) => disallow(cfg, "--crate-type proc-macro"),
(sym::panic, Some(sym::abort | sym::unwind)) => disallow(cfg, "-C panic"),
(sym::target_feature, Some(_)) => disallow(cfg, "-C target-feature"),
(sym::unix, None)
| (sym::windows, None)
| (sym::relocation_model, Some(_))
| (sym::target_abi, None | Some(_))
| (sym::target_arch, Some(_))
| (sym::target_endian, Some(_))
| (sym::target_env, None | Some(_))
| (sym::target_family, Some(_))
| (sym::target_os, Some(_))
| (sym::target_pointer_width, Some(_))
| (sym::target_vendor, None | Some(_))
| (sym::target_has_atomic, Some(_))
| (sym::target_has_atomic_equal_alignment, Some(_))
| (sym::target_has_atomic_load_store, Some(_))
| (sym::target_thread_local, None) => disallow(cfg, "--target"),
_ => {}
}
}
}

/// Generate the default configs for a given session
pub(crate) fn default_configuration(sess: &Session) -> Cfg {
let mut ret = Cfg::default();
Expand Down
12 changes: 11 additions & 1 deletion compiler/rustc_session/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,20 @@ impl ParseSess {
span: impl Into<MultiSpan>,
node_id: NodeId,
diagnostic: BuiltinLintDiag,
) {
self.opt_span_buffer_lint(lint, Some(span.into()), node_id, diagnostic)
}

pub fn opt_span_buffer_lint(
&self,
lint: &'static Lint,
span: Option<MultiSpan>,
node_id: NodeId,
diagnostic: BuiltinLintDiag,
) {
self.buffered_lints.with_lock(|buffered_lints| {
buffered_lints.push(BufferedEarlyLint {
span: span.into(),
span,
node_id,
lint_id: LintId::of(lint),
diagnostic,
Expand Down
54 changes: 27 additions & 27 deletions tests/assembly/x86-return-float.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
// Force frame pointers to make ASM more consistent between targets
//@ compile-flags: -O -C force-frame-pointers
//@ filecheck-flags: --implicit-check-not fld --implicit-check-not fst
//@ revisions: unix windows
//@[unix] ignore-windows
//@[windows] only-windows
//@ revisions: normal win
//@[normal] ignore-windows
//@[win] only-windows

#![crate_type = "lib"]
#![feature(f16, f128)]
Expand Down Expand Up @@ -190,10 +190,10 @@ pub unsafe fn call_f64_f64(x: &mut (f64, f64)) {
}
// CHECK: movl {{.*}}(%ebp), %[[PTR:.*]]
// CHECK: calll {{()|_}}get_f64_f64
// unix: movsd [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// unix-NEXT: movsd [[#%d,OFFSET+8]](%ebp), %[[VAL2:.*]]
// windows: movsd (%esp), %[[VAL1:.*]]
// windows-NEXT: movsd 8(%esp), %[[VAL2:.*]]
// normal: movsd [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// normal-NEXT: movsd [[#%d,OFFSET+8]](%ebp), %[[VAL2:.*]]
// win: movsd (%esp), %[[VAL1:.*]]
// win-NEXT: movsd 8(%esp), %[[VAL2:.*]]
// CHECK-NEXT: movsd %[[VAL1]], (%[[PTR]])
// CHECK-NEXT: movsd %[[VAL2]], 8(%[[PTR]])
*x = get_f64_f64();
Expand All @@ -207,13 +207,13 @@ pub unsafe fn call_f32_f64(x: &mut (f32, f64)) {
}
// CHECK: movl {{.*}}(%ebp), %[[PTR:.*]]
// CHECK: calll {{()|_}}get_f32_f64
// unix: movss [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// unix-NEXT: movsd [[#%d,OFFSET+4]](%ebp), %[[VAL2:.*]]
// windows: movss (%esp), %[[VAL1:.*]]
// windows-NEXT: movsd 8(%esp), %[[VAL2:.*]]
// normal: movss [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// normal-NEXT: movsd [[#%d,OFFSET+4]](%ebp), %[[VAL2:.*]]
// win: movss (%esp), %[[VAL1:.*]]
// win-NEXT: movsd 8(%esp), %[[VAL2:.*]]
// CHECK-NEXT: movss %[[VAL1]], (%[[PTR]])
// unix-NEXT: movsd %[[VAL2]], 4(%[[PTR]])
// windows-NEXT: movsd %[[VAL2]], 8(%[[PTR]])
// normal-NEXT: movsd %[[VAL2]], 4(%[[PTR]])
// win-NEXT: movsd %[[VAL2]], 8(%[[PTR]])
*x = get_f32_f64();
}

Expand All @@ -225,10 +225,10 @@ pub unsafe fn call_f64_f32(x: &mut (f64, f32)) {
}
// CHECK: movl {{.*}}(%ebp), %[[PTR:.*]]
// CHECK: calll {{()|_}}get_f64_f32
// unix: movsd [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// unix-NEXT: movss [[#%d,OFFSET+8]](%ebp), %[[VAL2:.*]]
// windows: movsd (%esp), %[[VAL1:.*]]
// windows-NEXT: movss 8(%esp), %[[VAL2:.*]]
// normal: movsd [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// normal-NEXT: movss [[#%d,OFFSET+8]](%ebp), %[[VAL2:.*]]
// win: movsd (%esp), %[[VAL1:.*]]
// win-NEXT: movss 8(%esp), %[[VAL2:.*]]
// CHECK-NEXT: movsd %[[VAL1]], (%[[PTR]])
// CHECK-NEXT: movss %[[VAL2]], 8(%[[PTR]])
*x = get_f64_f32();
Expand Down Expand Up @@ -257,10 +257,10 @@ pub unsafe fn call_f64_other(x: &mut (f64, usize)) {
}
// CHECK: movl {{.*}}(%ebp), %[[PTR:.*]]
// CHECK: calll {{()|_}}get_f64_other
// unix: movsd [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// unix-NEXT: movl [[#%d,OFFSET+8]](%ebp), %[[VAL2:.*]]
// windows: movsd (%esp), %[[VAL1:.*]]
// windows-NEXT: movl 8(%esp), %[[VAL2:.*]]
// normal: movsd [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// normal-NEXT: movl [[#%d,OFFSET+8]](%ebp), %[[VAL2:.*]]
// win: movsd (%esp), %[[VAL1:.*]]
// win-NEXT: movl 8(%esp), %[[VAL2:.*]]
// CHECK-NEXT: movsd %[[VAL1]], (%[[PTR]])
// CHECK-NEXT: movl %[[VAL2]], 8(%[[PTR]])
*x = get_f64_other();
Expand Down Expand Up @@ -289,13 +289,13 @@ pub unsafe fn call_other_f64(x: &mut (usize, f64)) {
}
// CHECK: movl {{.*}}(%ebp), %[[PTR:.*]]
// CHECK: calll {{()|_}}get_other_f64
// unix: movl [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// unix-NEXT: movsd [[#%d,OFFSET+4]](%ebp), %[[VAL2:.*]]
// windows: movl (%esp), %[[VAL1:.*]]
// windows-NEXT: movsd 8(%esp), %[[VAL2:.*]]
// normal: movl [[#%d,OFFSET:]](%ebp), %[[VAL1:.*]]
// normal-NEXT: movsd [[#%d,OFFSET+4]](%ebp), %[[VAL2:.*]]
// win: movl (%esp), %[[VAL1:.*]]
// win-NEXT: movsd 8(%esp), %[[VAL2:.*]]
// CHECK-NEXT: movl %[[VAL1]], (%[[PTR]])
// unix-NEXT: movsd %[[VAL2]], 4(%[[PTR]])
// windows-NEXT: movsd %[[VAL2]], 8(%[[PTR]])
// normal-NEXT: movsd %[[VAL2]], 4(%[[PTR]])
// win-NEXT: movsd %[[VAL2]], 8(%[[PTR]])
*x = get_other_f64();
}

Expand Down
Loading
Loading