From 752c939dfee797008da1af044786346adb2d7b4b Mon Sep 17 00:00:00 2001 From: Simonas Kazlauskas Date: Sat, 5 Jun 2021 14:35:19 +0300 Subject: [PATCH 1/7] Disable the machine outliner by default This addresses a codegen-issue that needs to be fixed upstream in LLVM. While we wait for the fix, we can disable it. Verified manually that the outliner is no longer run when `-Copt-level=z` is specified, and also that you can override this with `-Cllvm-args=-enable-machine-outliner` if you need it anyway. A regression test is not really feasible in this instance, given that we do not have any minimal reproducers. Fixes #85351 --- compiler/rustc_codegen_llvm/src/llvm_util.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/llvm_util.rs b/compiler/rustc_codegen_llvm/src/llvm_util.rs index b44553e4f6d3b..97684ca6c1a31 100644 --- a/compiler/rustc_codegen_llvm/src/llvm_util.rs +++ b/compiler/rustc_codegen_llvm/src/llvm_util.rs @@ -1,5 +1,5 @@ use crate::back::write::create_informational_target_machine; -use crate::llvm; +use crate::{llvm, llvm_util}; use libc::c_int; use rustc_codegen_ssa::target_features::supported_target_features; use rustc_data_structures::fx::FxHashSet; @@ -84,6 +84,17 @@ unsafe fn configure_llvm(sess: &Session) { if !sess.opts.debugging_opts.no_generate_arange_section { add("-generate-arange-section", false); } + + // FIXME(nagisa): disable the machine outliner by default in LLVM versions 11, where it was + // introduced and up. + // + // This should remain in place until https://reviews.llvm.org/D103167 is fixed. If LLVM + // has been upgraded since, consider adjusting the version check below to contain an upper + // bound. + if llvm_util::get_version() >= (11, 0, 0) { + add("-enable-machine-outliner=never", false); + } + match sess.opts.debugging_opts.merge_functions.unwrap_or(sess.target.merge_functions) { MergeFunctions::Disabled | MergeFunctions::Trampolines => {} MergeFunctions::Aliases => { From c05bbb72371534ce02f76097280deca6518df69a Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 18 May 2021 14:14:20 -0400 Subject: [PATCH 2/7] Fix incorrect gating of nonterminals in key-value attributes Fixes #85432 When processing a `#[derive]` or `#[cfg_eval]` attribute, we need to re-parse our attribute target, which requires flattenting all `Nonterminals`. However, this caused us to incorrectly gate on a (flattented) nonterminal in a key-value attribute, which is supposed to be allowed. Since we already perform this gating during the initial parse, we suppress it in `capture_cfg` mode. --- compiler/rustc_parse/src/parser/mod.rs | 7 +++-- .../macros/issue-85432-ungated-attr-macro.rs | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/macros/issue-85432-ungated-attr-macro.rs diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index ed95a5661b18e..a55eb3cf7526d 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -1077,8 +1077,11 @@ impl<'a> Parser<'a> { let span = expr.span; match &expr.kind { - // Not gated to supporte things like `doc = $expr` that work on stable. - _ if is_interpolated_expr => {} + // Not gated to support things like `doc = $expr` that work on stable. + // Do not gate in `capture_cfg` mode, since we flatten all nontemrinals + // before parsing. `capture_cfg` mode is only used to reparse existing + // tokens, so the gating will be performed by the initial parse + _ if is_interpolated_expr || self.capture_cfg => {} ExprKind::Lit(lit) if lit.kind.is_unsuffixed() => {} _ => self.sess.gated_spans.gate(sym::extended_key_value_attributes, span), } diff --git a/src/test/ui/macros/issue-85432-ungated-attr-macro.rs b/src/test/ui/macros/issue-85432-ungated-attr-macro.rs new file mode 100644 index 0000000000000..bac124c6f4c3c --- /dev/null +++ b/src/test/ui/macros/issue-85432-ungated-attr-macro.rs @@ -0,0 +1,30 @@ +// check-pass +// Regression test for issue #85432 +// Ensures that we don't incorrectly gate nonterminals +// in key-value macros when we need to reparse due to +// the presence of `#[derive]` + +macro_rules! with_doc_comment { + ($comment:expr, $item:item) => { + #[doc = $comment] + $item + }; +} + +macro_rules! database_table_doc { + () => { + "" + }; +} + +with_doc_comment! { + database_table_doc!(), + #[derive(Debug)] + struct Image { + #[cfg(FALSE)] + _f: (), + } + +} + +fn main() {} From 8b1a5a74299e609a0e542b3c826dbe03da872efd Mon Sep 17 00:00:00 2001 From: 12101111 Date: Mon, 17 May 2021 11:22:07 +0800 Subject: [PATCH 3/7] Build crtbengin.o/crtend.o from source code --- src/bootstrap/builder.rs | 3 +- src/bootstrap/compile.rs | 3 +- src/bootstrap/native.rs | 66 +++++++++++++++++++ .../host-x86_64/dist-various-1/Dockerfile | 6 +- .../run-make-fulldeps/issue-47551/Makefile | 9 +++ .../issue-47551/eh_frame-terminator.rs | 23 +++++++ 6 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 src/test/run-make-fulldeps/issue-47551/Makefile create mode 100644 src/test/run-make-fulldeps/issue-47551/eh_frame-terminator.rs diff --git a/src/bootstrap/builder.rs b/src/bootstrap/builder.rs index 62a3a87eeb850..cff1ec843ff71 100644 --- a/src/bootstrap/builder.rs +++ b/src/bootstrap/builder.rs @@ -369,7 +369,8 @@ impl<'a> Builder<'a> { tool::Rustfmt, tool::Miri, tool::CargoMiri, - native::Lld + native::Lld, + native::CrtBeginEnd ), Kind::Check | Kind::Clippy { .. } | Kind::Fix | Kind::Format => describe!( check::Std, diff --git a/src/bootstrap/compile.rs b/src/bootstrap/compile.rs index 66a88e85fea39..ee3527b6bf197 100644 --- a/src/bootstrap/compile.rs +++ b/src/bootstrap/compile.rs @@ -199,8 +199,9 @@ fn copy_self_contained_objects( DependencyType::TargetSelfContained, ); } + let crt_path = builder.ensure(native::CrtBeginEnd { target }); for &obj in &["crtbegin.o", "crtbeginS.o", "crtend.o", "crtendS.o"] { - let src = compiler_file(builder, builder.cc(target), target, obj); + let src = crt_path.join(obj); let target = libdir_self_contained.join(obj); builder.copy(&src, &target); target_deps.push((target, DependencyType::TargetSelfContained)); diff --git a/src/bootstrap/native.rs b/src/bootstrap/native.rs index bde0a96f03013..156e8dd53d50d 100644 --- a/src/bootstrap/native.rs +++ b/src/bootstrap/native.rs @@ -858,3 +858,69 @@ impl HashStamp { fs::write(&self.path, self.hash.as_deref().unwrap_or(b"")) } } + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] +pub struct CrtBeginEnd { + pub target: TargetSelection, +} + +impl Step for CrtBeginEnd { + type Output = PathBuf; + + fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + run.path("src/llvm-project/compiler-rt/lib/crt") + } + + fn make_run(run: RunConfig<'_>) { + run.builder.ensure(CrtBeginEnd { target: run.target }); + } + + /// Build crtbegin.o/crtend.o for musl target. + fn run(self, builder: &Builder<'_>) -> Self::Output { + let out_dir = builder.native_dir(self.target).join("crt"); + + if builder.config.dry_run { + return out_dir; + } + + let crtbegin_src = builder.src.join("src/llvm-project/compiler-rt/lib/crt/crtbegin.c"); + let crtend_src = builder.src.join("src/llvm-project/compiler-rt/lib/crt/crtend.c"); + if up_to_date(&crtbegin_src, &out_dir.join("crtbegin.o")) + && up_to_date(&crtend_src, &out_dir.join("crtendS.o")) + { + return out_dir; + } + + builder.info("Building crtbegin.o and crtend.o"); + t!(fs::create_dir_all(&out_dir)); + + let mut cfg = cc::Build::new(); + + if let Some(ar) = builder.ar(self.target) { + cfg.archiver(ar); + } + cfg.compiler(builder.cc(self.target)); + cfg.cargo_metadata(false) + .out_dir(&out_dir) + .target(&self.target.triple) + .host(&builder.config.build.triple) + .warnings(false) + .debug(false) + .opt_level(3) + .file(crtbegin_src) + .file(crtend_src); + + // Those flags are defined in src/llvm-project/compiler-rt/lib/crt/CMakeLists.txt + // Currently only consumer of those objects is musl, which use .init_array/.fini_array + // instead of .ctors/.dtors + cfg.flag("-std=c11") + .define("CRT_HAS_INITFINI_ARRAY", None) + .define("EH_USE_FRAME_REGISTRY", None); + + cfg.compile("crt"); + + t!(fs::copy(out_dir.join("crtbegin.o"), out_dir.join("crtbeginS.o"))); + t!(fs::copy(out_dir.join("crtend.o"), out_dir.join("crtendS.o"))); + out_dir + } +} diff --git a/src/ci/docker/host-x86_64/dist-various-1/Dockerfile b/src/ci/docker/host-x86_64/dist-various-1/Dockerfile index 1f8f9fc518ac1..cd0f01faa1bfa 100644 --- a/src/ci/docker/host-x86_64/dist-various-1/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-various-1/Dockerfile @@ -144,7 +144,11 @@ ENV TARGETS=$TARGETS,armv7a-none-eabi # riscv targets currently do not need a C compiler, as compiler_builtins # doesn't currently have it enabled, and the riscv gcc compiler is not # installed. -ENV CC_mipsel_unknown_linux_musl=mipsel-openwrt-linux-gcc \ +ENV CFLAGS_armv5te_unknown_linux_musleabi="-march=armv5te -marm -mfloat-abi=soft" \ + CFLAGS_arm_unknown_linux_musleabi="-march=armv6 -marm" \ + CFLAGS_arm_unknown_linux_musleabihf="-march=armv6 -marm -mfpu=vfp" \ + CFLAGS_armv7_unknown_linux_musleabihf="-march=armv7-a" \ + CC_mipsel_unknown_linux_musl=mipsel-openwrt-linux-gcc \ CC_mips_unknown_linux_musl=mips-openwrt-linux-gcc \ CC_mips64el_unknown_linux_muslabi64=mips64el-linux-gnuabi64-gcc \ CC_mips64_unknown_linux_muslabi64=mips64-linux-gnuabi64-gcc \ diff --git a/src/test/run-make-fulldeps/issue-47551/Makefile b/src/test/run-make-fulldeps/issue-47551/Makefile new file mode 100644 index 0000000000000..f4495e6b671d7 --- /dev/null +++ b/src/test/run-make-fulldeps/issue-47551/Makefile @@ -0,0 +1,9 @@ +# only-linux +# ignore-32bit + +-include ../tools.mk + +all: + $(RUSTC) eh_frame-terminator.rs + $(call RUN,eh_frame-terminator) | $(CGREP) '1122334455667788' + objdump --dwarf=frames $(TMPDIR)/eh_frame-terminator | $(CGREP) 'ZERO terminator' diff --git a/src/test/run-make-fulldeps/issue-47551/eh_frame-terminator.rs b/src/test/run-make-fulldeps/issue-47551/eh_frame-terminator.rs new file mode 100644 index 0000000000000..2f740dc4fac38 --- /dev/null +++ b/src/test/run-make-fulldeps/issue-47551/eh_frame-terminator.rs @@ -0,0 +1,23 @@ +// run-pass + +#![feature(backtrace)] +#[derive(Clone, Copy)] +struct Foo { + array: [u64; 10240], +} + +impl Foo { + const fn new() -> Self { + Self { + array: [0x1122_3344_5566_7788; 10240] + } + } +} + +static BAR: [Foo; 10240] = [Foo::new(); 10240]; + +fn main() { + let bt = std::backtrace::Backtrace::force_capture(); + println!("Hello, world! {:?}", bt); + println!("{:x}", BAR[0].array[0]); +} From bf28f680b72251626dde1000a60479d2b8e0214a Mon Sep 17 00:00:00 2001 From: Jakub Kulik Date: Thu, 13 May 2021 12:40:06 +0200 Subject: [PATCH 4/7] Update Docker to build the deprecated target alongside the new one --- .../docker/host-x86_64/dist-various-2/Dockerfile | 10 ++++++++-- .../dist-various-2/build-solaris-toolchain.sh | 14 ++------------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/ci/docker/host-x86_64/dist-various-2/Dockerfile b/src/ci/docker/host-x86_64/dist-various-2/Dockerfile index de3a99f34fcfa..357ac7a2becb9 100644 --- a/src/ci/docker/host-x86_64/dist-various-2/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-various-2/Dockerfile @@ -42,6 +42,9 @@ ENV \ AR_x86_64_pc_solaris=x86_64-pc-solaris2.10-ar \ CC_x86_64_pc_solaris=x86_64-pc-solaris2.10-gcc \ CXX_x86_64_pc_solaris=x86_64-pc-solaris2.10-g++ \ + AR_x86_64_sun_solaris=x86_64-sun-solaris2.10-ar \ + CC_x86_64_sun_solaris=x86_64-sun-solaris2.10-gcc \ + CXX_x86_64_sun_solaris=x86_64-sun-solaris2.10-g++ \ CC_armv7_unknown_linux_gnueabi=arm-linux-gnueabi-gcc-8 \ CXX_armv7_unknown_linux_gnueabi=arm-linux-gnueabi-g++-8 \ AR_x86_64_fortanix_unknown_sgx=ar \ @@ -68,8 +71,10 @@ COPY host-x86_64/dist-various-2/shared.sh /tmp/ COPY host-x86_64/dist-various-2/build-fuchsia-toolchain.sh /tmp/ RUN /tmp/build-fuchsia-toolchain.sh COPY host-x86_64/dist-various-2/build-solaris-toolchain.sh /tmp/ -RUN /tmp/build-solaris-toolchain.sh x86_64 amd64 solaris-i386 -RUN /tmp/build-solaris-toolchain.sh sparcv9 sparcv9 solaris-sparc +RUN /tmp/build-solaris-toolchain.sh x86_64 amd64 solaris-i386 pc +# Build now deprecated Solaris target as well +RUN /tmp/build-solaris-toolchain.sh x86_64 amd64 solaris-i386 sun +RUN /tmp/build-solaris-toolchain.sh sparcv9 sparcv9 solaris-sparc sun COPY host-x86_64/dist-various-2/build-x86_64-fortanix-unknown-sgx-toolchain.sh /tmp/ RUN /tmp/build-x86_64-fortanix-unknown-sgx-toolchain.sh @@ -99,6 +104,7 @@ ENV TARGETS=$TARGETS,wasm32-unknown-unknown ENV TARGETS=$TARGETS,wasm32-wasi ENV TARGETS=$TARGETS,sparcv9-sun-solaris ENV TARGETS=$TARGETS,x86_64-pc-solaris +ENV TARGETS=$TARGETS,x86_64-sun-solaris ENV TARGETS=$TARGETS,x86_64-unknown-linux-gnux32 ENV TARGETS=$TARGETS,x86_64-fortanix-unknown-sgx ENV TARGETS=$TARGETS,nvptx64-nvidia-cuda diff --git a/src/ci/docker/host-x86_64/dist-various-2/build-solaris-toolchain.sh b/src/ci/docker/host-x86_64/dist-various-2/build-solaris-toolchain.sh index ee76fafb1f948..4cbc2ccfe3814 100755 --- a/src/ci/docker/host-x86_64/dist-various-2/build-solaris-toolchain.sh +++ b/src/ci/docker/host-x86_64/dist-various-2/build-solaris-toolchain.sh @@ -6,21 +6,11 @@ source shared.sh ARCH=$1 LIB_ARCH=$2 APT_ARCH=$3 +MANUFACTURER=$4 BINUTILS=2.28.1 GCC=6.5.0 -# Choose correct target based on the $ARCH -case "$ARCH" in -x86_64) - TARGET=x86_64-pc-solaris2.10 - ;; -sparcv9) - TARGET=sparcv9-sun-solaris2.10 - ;; -*) - printf 'ERROR: unknown architecture: %s\n' "$ARCH" - exit 1 -esac +TARGET=${ARCH}-${MANUFACTURER}-solaris2.10 # First up, build binutils mkdir binutils From f724ee4233d72f14d58ecc3c56064c465dcbcfb8 Mon Sep 17 00:00:00 2001 From: Jakub Kulik Date: Thu, 13 May 2021 12:43:46 +0200 Subject: [PATCH 5/7] Improve comment --- src/ci/docker/host-x86_64/dist-various-2/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ci/docker/host-x86_64/dist-various-2/Dockerfile b/src/ci/docker/host-x86_64/dist-various-2/Dockerfile index 357ac7a2becb9..b7f181adf2a09 100644 --- a/src/ci/docker/host-x86_64/dist-various-2/Dockerfile +++ b/src/ci/docker/host-x86_64/dist-various-2/Dockerfile @@ -72,7 +72,7 @@ COPY host-x86_64/dist-various-2/build-fuchsia-toolchain.sh /tmp/ RUN /tmp/build-fuchsia-toolchain.sh COPY host-x86_64/dist-various-2/build-solaris-toolchain.sh /tmp/ RUN /tmp/build-solaris-toolchain.sh x86_64 amd64 solaris-i386 pc -# Build now deprecated Solaris target as well +# Build deprecated target 'x86_64-sun-solaris2.10' until removed RUN /tmp/build-solaris-toolchain.sh x86_64 amd64 solaris-i386 sun RUN /tmp/build-solaris-toolchain.sh sparcv9 sparcv9 solaris-sparc sun COPY host-x86_64/dist-various-2/build-x86_64-fortanix-unknown-sgx-toolchain.sh /tmp/ From 9529cbd0ba0ec29308589dbc8899305bd629f72a Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 12 May 2021 00:10:41 -0400 Subject: [PATCH 6/7] Preserve `SyntaxContext` for invalid/dummy spans in crate metadata Fixes #85197 We already preserved the `SyntaxContext` for invalid/dummy spans in the incremental cache, but we weren't doing the same for crate metadata. If an invalid (lo/hi from different files) span is written to the incremental cache, we will decode it with a 'dummy' location, but keep the original `SyntaxContext`. Since the crate metadata encoder was only checking for `DUMMY_SP` (dummy location + root `SyntaxContext`), the metadata encoder would treat it as a normal span, encoding the `SyntaxContext`. As a result, the final span encoded to the metadata would change across sessions, even if the crate itself was unchanged. This PR updates our encoding of spans in the crate metadata to mirror the encoding of spans into the incremental cache. We now always encode a `SyntaxContext`, and encode location information for spans with a non-dummy location. --- compiler/rustc_metadata/src/rmeta/decoder.rs | 6 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 82 +++++++++---------- compiler/rustc_metadata/src/rmeta/mod.rs | 2 +- .../auxiliary/invalid-span-helper-lib.rs | 11 +++ .../auxiliary/invalid-span-helper-mod.rs | 14 ++++ .../auxiliary/respan.rs | 19 +++++ .../invalid_span_main.rs | 24 ++++++ 7 files changed, 113 insertions(+), 45 deletions(-) create mode 100644 src/test/incremental/issue-85197-invalid-span/auxiliary/invalid-span-helper-lib.rs create mode 100644 src/test/incremental/issue-85197-invalid-span/auxiliary/invalid-span-helper-mod.rs create mode 100644 src/test/incremental/issue-85197-invalid-span/auxiliary/respan.rs create mode 100644 src/test/incremental/issue-85197-invalid-span/invalid_span_main.rs diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 2ade1bb4f95de..6671cf6fea48f 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -406,17 +406,17 @@ impl<'a, 'tcx> Decodable> for ExpnId { impl<'a, 'tcx> Decodable> for Span { fn decode(decoder: &mut DecodeContext<'a, 'tcx>) -> Result { + let ctxt = SyntaxContext::decode(decoder)?; let tag = u8::decode(decoder)?; - if tag == TAG_INVALID_SPAN { - return Ok(DUMMY_SP); + if tag == TAG_PARTIAL_SPAN { + return Ok(DUMMY_SP.with_ctxt(ctxt)); } debug_assert!(tag == TAG_VALID_SPAN_LOCAL || tag == TAG_VALID_SPAN_FOREIGN); let lo = BytePos::decode(decoder)?; let len = BytePos::decode(decoder)?; - let ctxt = SyntaxContext::decode(decoder)?; let hi = lo + len; let sess = if let Some(sess) = decoder.sess { diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index e8f02b8e66f0a..02fa6905a08ce 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -184,11 +184,48 @@ impl<'a, 'tcx> Encodable> for ExpnId { impl<'a, 'tcx> Encodable> for Span { fn encode(&self, s: &mut EncodeContext<'a, 'tcx>) -> opaque::EncodeResult { - if *self == rustc_span::DUMMY_SP { - return TAG_INVALID_SPAN.encode(s); + let span = self.data(); + + // Don't serialize any `SyntaxContext`s from a proc-macro crate, + // since we don't load proc-macro dependencies during serialization. + // This means that any hygiene information from macros used *within* + // a proc-macro crate (e.g. invoking a macro that expands to a proc-macro + // definition) will be lost. + // + // This can show up in two ways: + // + // 1. Any hygiene information associated with identifier of + // a proc macro (e.g. `#[proc_macro] pub fn $name`) will be lost. + // Since proc-macros can only be invoked from a different crate, + // real code should never need to care about this. + // + // 2. Using `Span::def_site` or `Span::mixed_site` will not + // include any hygiene information associated with the definition + // site. This means that a proc-macro cannot emit a `$crate` + // identifier which resolves to one of its dependencies, + // which also should never come up in practice. + // + // Additionally, this affects `Span::parent`, and any other + // span inspection APIs that would otherwise allow traversing + // the `SyntaxContexts` associated with a span. + // + // None of these user-visible effects should result in any + // cross-crate inconsistencies (getting one behavior in the same + // crate, and a different behavior in another crate) due to the + // limited surface that proc-macros can expose. + // + // IMPORTANT: If this is ever changed, be sure to update + // `rustc_span::hygiene::raw_encode_expn_id` to handle + // encoding `ExpnData` for proc-macro crates. + if s.is_proc_macro { + SyntaxContext::root().encode(s)?; + } else { + span.ctxt.encode(s)?; } - let span = self.data(); + if self.is_dummy() { + return TAG_PARTIAL_SPAN.encode(s); + } // The Span infrastructure should make sure that this invariant holds: debug_assert!(span.lo <= span.hi); @@ -203,7 +240,7 @@ impl<'a, 'tcx> Encodable> for Span { if !s.source_file_cache.0.contains(span.hi) { // Unfortunately, macro expansion still sometimes generates Spans // that malformed in this way. - return TAG_INVALID_SPAN.encode(s); + return TAG_PARTIAL_SPAN.encode(s); } let source_files = s.required_source_files.as_mut().expect("Already encoded SourceMap!"); @@ -259,43 +296,6 @@ impl<'a, 'tcx> Encodable> for Span { let len = hi - lo; len.encode(s)?; - // Don't serialize any `SyntaxContext`s from a proc-macro crate, - // since we don't load proc-macro dependencies during serialization. - // This means that any hygiene information from macros used *within* - // a proc-macro crate (e.g. invoking a macro that expands to a proc-macro - // definition) will be lost. - // - // This can show up in two ways: - // - // 1. Any hygiene information associated with identifier of - // a proc macro (e.g. `#[proc_macro] pub fn $name`) will be lost. - // Since proc-macros can only be invoked from a different crate, - // real code should never need to care about this. - // - // 2. Using `Span::def_site` or `Span::mixed_site` will not - // include any hygiene information associated with the definition - // site. This means that a proc-macro cannot emit a `$crate` - // identifier which resolves to one of its dependencies, - // which also should never come up in practice. - // - // Additionally, this affects `Span::parent`, and any other - // span inspection APIs that would otherwise allow traversing - // the `SyntaxContexts` associated with a span. - // - // None of these user-visible effects should result in any - // cross-crate inconsistencies (getting one behavior in the same - // crate, and a different behavior in another crate) due to the - // limited surface that proc-macros can expose. - // - // IMPORTANT: If this is ever changed, be sure to update - // `rustc_span::hygiene::raw_encode_expn_id` to handle - // encoding `ExpnData` for proc-macro crates. - if s.is_proc_macro { - SyntaxContext::root().encode(s)?; - } else { - span.ctxt.encode(s)?; - } - if tag == TAG_VALID_SPAN_FOREIGN { // This needs to be two lines to avoid holding the `s.source_file_cache` // while calling `cnum.encode(s)` diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 9f665d5daaa03..959aa1719aef5 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -450,4 +450,4 @@ struct GeneratorData<'tcx> { // Tags used for encoding Spans: const TAG_VALID_SPAN_LOCAL: u8 = 0; const TAG_VALID_SPAN_FOREIGN: u8 = 1; -const TAG_INVALID_SPAN: u8 = 2; +const TAG_PARTIAL_SPAN: u8 = 2; diff --git a/src/test/incremental/issue-85197-invalid-span/auxiliary/invalid-span-helper-lib.rs b/src/test/incremental/issue-85197-invalid-span/auxiliary/invalid-span-helper-lib.rs new file mode 100644 index 0000000000000..2453af5b6b47b --- /dev/null +++ b/src/test/incremental/issue-85197-invalid-span/auxiliary/invalid-span-helper-lib.rs @@ -0,0 +1,11 @@ +// revisions: rpass1 rpass2 + +extern crate respan; + +#[macro_use] +#[path = "invalid-span-helper-mod.rs"] +mod invalid_span_helper_mod; + +// Invoke a macro from a different file - this +// allows us to get tokens with spans from different files +helper!(1); diff --git a/src/test/incremental/issue-85197-invalid-span/auxiliary/invalid-span-helper-mod.rs b/src/test/incremental/issue-85197-invalid-span/auxiliary/invalid-span-helper-mod.rs new file mode 100644 index 0000000000000..747174b1ebf15 --- /dev/null +++ b/src/test/incremental/issue-85197-invalid-span/auxiliary/invalid-span-helper-mod.rs @@ -0,0 +1,14 @@ +#[macro_export] +macro_rules! helper { + // Use `:tt` instead of `:ident` so that we don't get a `None`-delimited group + ($first:tt) => { + pub fn foo() { + // The span of `$first` comes from another file, + // so the expression `1 + $first` ends up with an + // 'invalid' span that starts and ends in different files. + // We use the `respan!` macro to give all tokens the same + // `SyntaxContext`, so that the parser will try to merge the spans. + respan::respan!(let a = 1 + $first;); + } + } +} diff --git a/src/test/incremental/issue-85197-invalid-span/auxiliary/respan.rs b/src/test/incremental/issue-85197-invalid-span/auxiliary/respan.rs new file mode 100644 index 0000000000000..5088eab62946f --- /dev/null +++ b/src/test/incremental/issue-85197-invalid-span/auxiliary/respan.rs @@ -0,0 +1,19 @@ +// force-host +// no-prefer-dynamic + +#![crate_type = "proc-macro"] + +extern crate proc_macro; +use proc_macro::TokenStream; + + +/// Copies the resolution information (the `SyntaxContext`) of the first +/// token to all other tokens in the stream. Does not recurse into groups. +#[proc_macro] +pub fn respan(input: TokenStream) -> TokenStream { + let first_span = input.clone().into_iter().next().unwrap().span(); + input.into_iter().map(|mut tree| { + tree.set_span(tree.span().resolved_at(first_span)); + tree + }).collect() +} diff --git a/src/test/incremental/issue-85197-invalid-span/invalid_span_main.rs b/src/test/incremental/issue-85197-invalid-span/invalid_span_main.rs new file mode 100644 index 0000000000000..f358460b33825 --- /dev/null +++ b/src/test/incremental/issue-85197-invalid-span/invalid_span_main.rs @@ -0,0 +1,24 @@ +// revisions: rpass1 rpass2 +// aux-build:respan.rs +// aux-build:invalid-span-helper-lib.rs + +// This issue has several different parts. The high level idea is: +// 1. We create an 'invalid' span with the help of the `respan` proc-macro, +// The compiler attempts to prevent the creation of invalid spans by +// refusing to join spans with different `SyntaxContext`s. We work around +// this by applying the same `SyntaxContext` to the span of every token, +// using `Span::resolved_at` +// 2. We using this invalid span in the body of a function, causing it to get +// encoded into the `optimized_mir` +// 3. We call the function from a different crate - since the function is generic, +// monomorphization runs, causing `optimized_mir` to get called. +// 4. We re-run compilation using our populated incremental cache, but without +// making any changes. When we recompile the crate containing our generic function +// (`invalid_span_helper_lib`), we load the span from the incremental cache, and +// write it into the crate metadata. + +extern crate invalid_span_helper_lib; + +fn main() { + invalid_span_helper_lib::foo::(); +} From 28ea3582642e9b41c7e047e007f68cd7bf20ba45 Mon Sep 17 00:00:00 2001 From: Frank Steffahn Date: Tue, 1 Jun 2021 02:21:26 +0200 Subject: [PATCH 7/7] Remove unsound TrustedRandomAccess implementations Removes the implementations that depend on the user-definable trait `Copy`. Beta backport: Does not modify `vec::IntoIter`. --- .../src/collections/vec_deque/into_iter.rs | 29 +------------------ library/core/src/array/iter.rs | 25 +--------------- 2 files changed, 2 insertions(+), 52 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/into_iter.rs b/library/alloc/src/collections/vec_deque/into_iter.rs index 1c635dd4f27fa..612f7e6eb4da8 100644 --- a/library/alloc/src/collections/vec_deque/into_iter.rs +++ b/library/alloc/src/collections/vec_deque/into_iter.rs @@ -1,5 +1,5 @@ use core::fmt; -use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess}; +use core::iter::{FusedIterator, TrustedLen}; use super::VecDeque; @@ -36,22 +36,6 @@ impl Iterator for IntoIter { let len = self.inner.len(); (len, Some(len)) } - - #[inline] - unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item - where - Self: TrustedRandomAccess, - { - // Safety: The TrustedRandomAccess contract requires that callers only pass an index - // that is in bounds. - // Additionally Self: TrustedRandomAccess is only implemented for T: Copy which means even - // multiple repeated reads of the same index would be safe and the - // values are !Drop, thus won't suffer from double drops. - unsafe { - let idx = self.inner.wrap_add(self.inner.tail, idx); - self.inner.buffer_read(idx) - } - } } #[stable(feature = "rust1", since = "1.0.0")] @@ -74,14 +58,3 @@ impl FusedIterator for IntoIter {} #[unstable(feature = "trusted_len", issue = "37572")] unsafe impl TrustedLen for IntoIter {} - -#[doc(hidden)] -#[unstable(feature = "trusted_random_access", issue = "none")] -// T: Copy as approximation for !Drop since get_unchecked does not update the pointers -// and thus we can't implement drop-handling -unsafe impl TrustedRandomAccess for IntoIter -where - T: Copy, -{ - const MAY_HAVE_SIDE_EFFECT: bool = false; -} diff --git a/library/core/src/array/iter.rs b/library/core/src/array/iter.rs index c36542f631488..61ab1b1faff89 100644 --- a/library/core/src/array/iter.rs +++ b/library/core/src/array/iter.rs @@ -2,7 +2,7 @@ use crate::{ fmt, - iter::{self, ExactSizeIterator, FusedIterator, TrustedLen, TrustedRandomAccess}, + iter::{self, ExactSizeIterator, FusedIterator, TrustedLen}, mem::{self, MaybeUninit}, ops::Range, ptr, @@ -130,18 +130,6 @@ impl Iterator for IntoIter { fn last(mut self) -> Option { self.next_back() } - - #[inline] - unsafe fn __iterator_get_unchecked(&mut self, idx: usize) -> Self::Item - where - Self: TrustedRandomAccess, - { - // SAFETY: Callers are only allowed to pass an index that is in bounds - // Additionally Self: TrustedRandomAccess is only implemented for T: Copy which means even - // multiple repeated reads of the same index would be safe and the - // values aree !Drop, thus won't suffer from double drops. - unsafe { self.data.get_unchecked(self.alive.start + idx).assume_init_read() } - } } #[stable(feature = "array_value_iter_impls", since = "1.40.0")] @@ -196,17 +184,6 @@ impl FusedIterator for IntoIter {} #[stable(feature = "array_value_iter_impls", since = "1.40.0")] unsafe impl TrustedLen for IntoIter {} -#[doc(hidden)] -#[unstable(feature = "trusted_random_access", issue = "none")] -// T: Copy as approximation for !Drop since get_unchecked does not update the pointers -// and thus we can't implement drop-handling -unsafe impl TrustedRandomAccess for IntoIter -where - T: Copy, -{ - const MAY_HAVE_SIDE_EFFECT: bool = false; -} - #[stable(feature = "array_value_iter_impls", since = "1.40.0")] impl Clone for IntoIter { fn clone(&self) -> Self {