From 16db0e234b5bb5e99bb0bd2535bfe3861510e528 Mon Sep 17 00:00:00 2001 From: moxian Date: Thu, 8 May 2025 19:43:05 -0700 Subject: [PATCH 1/4] Remove obsolete and confusing comment This comment from 2017 says "every error should have an associated error code" However, a more recent rustc-dev-guide doc from 2020 suggests that this is not the case, and that errorcode-less errors are fine and sometimes even preferable to errorcode-full ones: https://github.com/rust-lang/rustc-dev-guide/pull/967 --- compiler/rustc_errors/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 6f37bad9bb462..a0bb9299dc699 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -1304,7 +1304,6 @@ impl<'a> DiagCtxtHandle<'a> { self.create_almost_fatal(fatal).emit() } - // FIXME: This method should be removed (every error should have an associated error code). #[rustc_lint_diagnostics] #[track_caller] pub fn struct_err(self, msg: impl Into) -> Diag<'a> { From 453fcbc9ef8e05ab48f4cd71a93dcd8f8fa6c6d2 Mon Sep 17 00:00:00 2001 From: moxian Date: Thu, 8 May 2025 20:34:19 -0700 Subject: [PATCH 2/4] disallow export_name starting with "llvm." --- compiler/rustc_codegen_ssa/messages.ftl | 3 +++ .../rustc_codegen_ssa/src/codegen_attrs.rs | 9 +++++++ compiler/rustc_codegen_ssa/src/errors.rs | 8 ++++++ tests/ui/attributes/export_name-checks.rs | 18 +++++++++++++ tests/ui/attributes/export_name-checks.stderr | 25 +++++++++++++++++++ 5 files changed, 63 insertions(+) create mode 100644 tests/ui/attributes/export_name-checks.rs create mode 100644 tests/ui/attributes/export_name-checks.stderr diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl index 2621935eecf94..e221f4893f244 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -52,6 +52,9 @@ codegen_ssa_expected_one_argument = expected one argument codegen_ssa_expected_used_symbol = expected `used`, `used(compiler)` or `used(linker)` +codegen_ssa_export_name_llvm_intrinsic = exported symbol name must not start with "llvm." + .note = symbols starting with "llvm." are reserved for LLVM intrinsics and cannot be defined in user code + codegen_ssa_extern_funcs_not_found = some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified codegen_ssa_extract_bundled_libs_archive_member = failed to get data from archive member '{$rlib}': {$error} diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 5d09e62f2742d..1de622070d8e1 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -258,6 +258,15 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { // so it may not contain any null characters. tcx.dcx().emit_err(errors::NullOnExport { span: attr.span() }); } + if s.as_str().starts_with("llvm.") { + // Symbols starting with "llvm." are reserved by LLVM + // trying to define those would produce invalid IR. + // LLVM complain about those *if* we enable LLVM verification checks + // but we often don't enable them by default due to perf reasons. + tcx.dcx().emit_err(errors::ExportNameLLVMIntrinsic { + span: attr.value_span().expect("attibute has value but not a span"), + }); + } codegen_fn_attrs.export_name = Some(s); mixed_export_name_no_mangle_lint_state.track_export_name(attr.span()); } diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs index c2064397855f0..fc5d4ba8b627d 100644 --- a/compiler/rustc_codegen_ssa/src/errors.rs +++ b/compiler/rustc_codegen_ssa/src/errors.rs @@ -147,6 +147,14 @@ pub(crate) struct NullOnExport { pub span: Span, } +#[derive(Diagnostic)] +#[diag(codegen_ssa_export_name_llvm_intrinsic)] +#[note] +pub(crate) struct ExportNameLLVMIntrinsic { + #[primary_span] + pub span: Span, +} + #[derive(Diagnostic)] #[diag(codegen_ssa_unsupported_instruction_set, code = E0779)] pub(crate) struct UnsuportedInstructionSet { diff --git a/tests/ui/attributes/export_name-checks.rs b/tests/ui/attributes/export_name-checks.rs new file mode 100644 index 0000000000000..5b484607ee612 --- /dev/null +++ b/tests/ui/attributes/export_name-checks.rs @@ -0,0 +1,18 @@ +#![crate_type = "lib"] + +// rdtsc is an existing LLVM intrinsic +#[export_name = "llvm.x86.rdtsc"] +//~^ ERROR: exported symbol name must not start with "llvm." +pub unsafe fn foo(a: u8) -> u8 { + 2 * a +} + +// qwerty is not a real llvm intrinsic +#[export_name = "llvm.x86.qwerty"] +//~^ ERROR: exported symbol name must not start with "llvm." +pub unsafe fn bar(a: u8) -> u8 { + 2 * a +} + +#[export_name="ab\0cd"] //~ ERROR `export_name` may not contain null characters +pub fn qux() {} diff --git a/tests/ui/attributes/export_name-checks.stderr b/tests/ui/attributes/export_name-checks.stderr new file mode 100644 index 0000000000000..1281cfa12084d --- /dev/null +++ b/tests/ui/attributes/export_name-checks.stderr @@ -0,0 +1,25 @@ +error: exported symbol name must not start with "llvm." + --> $DIR/export_name-checks.rs:4:17 + | +LL | #[export_name = "llvm.x86.rdtsc"] + | ^^^^^^^^^^^^^^^^ + | + = note: symbols starting with "llvm." are reserved for LLVM intrinsics and cannot be defined in user code + +error: exported symbol name must not start with "llvm." + --> $DIR/export_name-checks.rs:11:17 + | +LL | #[export_name = "llvm.x86.qwerty"] + | ^^^^^^^^^^^^^^^^^ + | + = note: symbols starting with "llvm." are reserved for LLVM intrinsics and cannot be defined in user code + +error[E0648]: `export_name` may not contain null characters + --> $DIR/export_name-checks.rs:17:1 + | +LL | #[export_name="ab\0cd"] + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0648`. From d443666c89a413446621318466ea3048512b8f7c Mon Sep 17 00:00:00 2001 From: moxian Date: Thu, 8 May 2025 21:51:04 -0700 Subject: [PATCH 3/4] Tighten the error span for explort_name null byte check Simply because it looks a little bit prettier that way, IMO --- compiler/rustc_codegen_ssa/src/codegen_attrs.rs | 15 +++++++-------- tests/ui/attributes/export_name-checks.stderr | 4 ++-- tests/ui/error-codes/E0648.stderr | 4 ++-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs index 1de622070d8e1..46054f2b66a53 100644 --- a/compiler/rustc_codegen_ssa/src/codegen_attrs.rs +++ b/compiler/rustc_codegen_ssa/src/codegen_attrs.rs @@ -252,22 +252,21 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, did: LocalDefId) -> CodegenFnAttrs { codegen_fn_attrs.flags |= CodegenFnAttrFlags::TRACK_CALLER } sym::export_name => { - if let Some(s) = attr.value_str() { - if s.as_str().contains('\0') { + if let Some(exported_name) = attr.value_str() { + let value_span = attr.value_span().expect("attibute has value but not a span"); + if exported_name.as_str().contains('\0') { // `#[export_name = ...]` will be converted to a null-terminated string, // so it may not contain any null characters. - tcx.dcx().emit_err(errors::NullOnExport { span: attr.span() }); + tcx.dcx().emit_err(errors::NullOnExport { span: value_span }); } - if s.as_str().starts_with("llvm.") { + if exported_name.as_str().starts_with("llvm.") { // Symbols starting with "llvm." are reserved by LLVM // trying to define those would produce invalid IR. // LLVM complain about those *if* we enable LLVM verification checks // but we often don't enable them by default due to perf reasons. - tcx.dcx().emit_err(errors::ExportNameLLVMIntrinsic { - span: attr.value_span().expect("attibute has value but not a span"), - }); + tcx.dcx().emit_err(errors::ExportNameLLVMIntrinsic { span: value_span }); } - codegen_fn_attrs.export_name = Some(s); + codegen_fn_attrs.export_name = Some(exported_name); mixed_export_name_no_mangle_lint_state.track_export_name(attr.span()); } } diff --git a/tests/ui/attributes/export_name-checks.stderr b/tests/ui/attributes/export_name-checks.stderr index 1281cfa12084d..c45fa8401f303 100644 --- a/tests/ui/attributes/export_name-checks.stderr +++ b/tests/ui/attributes/export_name-checks.stderr @@ -15,10 +15,10 @@ LL | #[export_name = "llvm.x86.qwerty"] = note: symbols starting with "llvm." are reserved for LLVM intrinsics and cannot be defined in user code error[E0648]: `export_name` may not contain null characters - --> $DIR/export_name-checks.rs:17:1 + --> $DIR/export_name-checks.rs:17:15 | LL | #[export_name="ab\0cd"] - | ^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^ error: aborting due to 3 previous errors diff --git a/tests/ui/error-codes/E0648.stderr b/tests/ui/error-codes/E0648.stderr index e995e53139511..480fb1eda5d9a 100644 --- a/tests/ui/error-codes/E0648.stderr +++ b/tests/ui/error-codes/E0648.stderr @@ -1,8 +1,8 @@ error[E0648]: `export_name` may not contain null characters - --> $DIR/E0648.rs:1:1 + --> $DIR/E0648.rs:1:15 | LL | #[export_name="\0foo"] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^ error: aborting due to 1 previous error From 67608b80f518c98719fa5c46f900787c7bff45a3 Mon Sep 17 00:00:00 2001 From: moxian Date: Fri, 9 May 2025 13:43:28 -0700 Subject: [PATCH 4/4] Minor wording tweaks --- compiler/rustc_codegen_ssa/messages.ftl | 4 ++-- tests/ui/attributes/export_name-checks.rs | 4 ++-- tests/ui/attributes/export_name-checks.stderr | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_codegen_ssa/messages.ftl b/compiler/rustc_codegen_ssa/messages.ftl index e221f4893f244..711ec4b0e8d91 100644 --- a/compiler/rustc_codegen_ssa/messages.ftl +++ b/compiler/rustc_codegen_ssa/messages.ftl @@ -52,8 +52,8 @@ codegen_ssa_expected_one_argument = expected one argument codegen_ssa_expected_used_symbol = expected `used`, `used(compiler)` or `used(linker)` -codegen_ssa_export_name_llvm_intrinsic = exported symbol name must not start with "llvm." - .note = symbols starting with "llvm." are reserved for LLVM intrinsics and cannot be defined in user code +codegen_ssa_export_name_llvm_intrinsic = exported symbol name must not start with `llvm.` + .note = symbols starting with `llvm.` are reserved for LLVM intrinsics codegen_ssa_extern_funcs_not_found = some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified diff --git a/tests/ui/attributes/export_name-checks.rs b/tests/ui/attributes/export_name-checks.rs index 5b484607ee612..ee5472279abf5 100644 --- a/tests/ui/attributes/export_name-checks.rs +++ b/tests/ui/attributes/export_name-checks.rs @@ -2,14 +2,14 @@ // rdtsc is an existing LLVM intrinsic #[export_name = "llvm.x86.rdtsc"] -//~^ ERROR: exported symbol name must not start with "llvm." +//~^ ERROR: exported symbol name must not start with `llvm.` pub unsafe fn foo(a: u8) -> u8 { 2 * a } // qwerty is not a real llvm intrinsic #[export_name = "llvm.x86.qwerty"] -//~^ ERROR: exported symbol name must not start with "llvm." +//~^ ERROR: exported symbol name must not start with `llvm.` pub unsafe fn bar(a: u8) -> u8 { 2 * a } diff --git a/tests/ui/attributes/export_name-checks.stderr b/tests/ui/attributes/export_name-checks.stderr index c45fa8401f303..f5a11170049a7 100644 --- a/tests/ui/attributes/export_name-checks.stderr +++ b/tests/ui/attributes/export_name-checks.stderr @@ -1,18 +1,18 @@ -error: exported symbol name must not start with "llvm." +error: exported symbol name must not start with `llvm.` --> $DIR/export_name-checks.rs:4:17 | LL | #[export_name = "llvm.x86.rdtsc"] | ^^^^^^^^^^^^^^^^ | - = note: symbols starting with "llvm." are reserved for LLVM intrinsics and cannot be defined in user code + = note: symbols starting with `llvm.` are reserved for LLVM intrinsics -error: exported symbol name must not start with "llvm." +error: exported symbol name must not start with `llvm.` --> $DIR/export_name-checks.rs:11:17 | LL | #[export_name = "llvm.x86.qwerty"] | ^^^^^^^^^^^^^^^^^ | - = note: symbols starting with "llvm." are reserved for LLVM intrinsics and cannot be defined in user code + = note: symbols starting with `llvm.` are reserved for LLVM intrinsics error[E0648]: `export_name` may not contain null characters --> $DIR/export_name-checks.rs:17:15