From ab62a352ba5eeae79ff3dab0788a2134eac3f674 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Wed, 2 Oct 2024 05:26:15 +0900 Subject: Stabilize s390x inline assembly --- tests/codegen/asm/s390x-clobbers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tests/codegen') diff --git a/tests/codegen/asm/s390x-clobbers.rs b/tests/codegen/asm/s390x-clobbers.rs index 45f72206bdf..56d82b4b044 100644 --- a/tests/codegen/asm/s390x-clobbers.rs +++ b/tests/codegen/asm/s390x-clobbers.rs @@ -3,7 +3,7 @@ //@[s390x] needs-llvm-components: systemz #![crate_type = "rlib"] -#![feature(no_core, rustc_attrs, lang_items, asm_experimental_arch)] +#![feature(no_core, rustc_attrs, lang_items)] #![no_core] #[lang = "sized"] -- cgit 1.4.1-3-g733a5 From 35a913b968eb9ad3a271f72fcc1bcd168bb572a4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 7 Nov 2024 21:44:28 +0100 Subject: pointee_info_at: fix logic for recursing into enums --- compiler/rustc_abi/src/lib.rs | 16 +++++++++---- compiler/rustc_middle/src/ty/layout.rs | 38 ++++++++++++++++++++++--------- compiler/rustc_target/src/callconv/mod.rs | 3 ++- tests/codegen/function-arguments.rs | 24 +++++++++++++++---- 4 files changed, 61 insertions(+), 20 deletions(-) (limited to 'tests/codegen') diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index ec758785173..7b6abdf1ea9 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1743,15 +1743,23 @@ pub enum PointerKind { Box { unpin: bool, global: bool }, } -/// Note that this information is advisory only, and backends are free to ignore it. -/// It can only be used to encode potential optimizations, but no critical information. +/// Encodes extra information we have about a pointer. +/// Note that this information is advisory only, and backends are free to ignore it: +/// if the information is wrong, that can cause UB, but if the information is absent, +/// that must always be okay. #[derive(Copy, Clone, Debug)] pub struct PointeeInfo { - pub size: Size, - pub align: Align, /// If this is `None`, then this is a raw pointer, so size and alignment are not guaranteed to /// be reliable. pub safe: Option, + /// If `safe` is `Some`, then the pointer is either null or dereferenceable for this many bytes. + /// On a function argument, "dereferenceable" here means "dereferenceable for the entire duration + /// of this function call", i.e. it is UB for the memory that this pointer points to to be freed + /// while this function is still running. + /// The size can be zero if the pointer is not dereferenceable. + pub size: Size, + /// If `safe` is `Some`, then the pointer is aligned as indicated. + pub align: Align, } impl LayoutData { diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 76e3183fcbb..a0eb9029319 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -1011,25 +1011,41 @@ where } _ => { - let mut data_variant = match this.variants { + let mut data_variant = match &this.variants { // Within the discriminant field, only the niche itself is // always initialized, so we only check for a pointer at its // offset. // - // If the niche is a pointer, it's either valid (according - // to its type), or null (which the niche field's scalar - // validity range encodes). This allows using - // `dereferenceable_or_null` for e.g., `Option<&T>`, and - // this will continue to work as long as we don't start - // using more niches than just null (e.g., the first page of - // the address space, or unaligned pointers). + // Our goal here is to check whether this represents a + // "dereferenceable or null" pointer, so we need to ensure + // that there is only one other variant, and it must be null. + // Below, we will then check whether the pointer is indeed + // dereferenceable. Variants::Multiple { - tag_encoding: TagEncoding::Niche { untagged_variant, .. }, + tag_encoding: + TagEncoding::Niche { untagged_variant, niche_variants, niche_start }, tag_field, + variants, .. - } if this.fields.offset(tag_field) == offset => { - Some(this.for_variant(cx, untagged_variant)) + } if variants.len() == 2 && this.fields.offset(*tag_field) == offset => { + let tagged_variant = if untagged_variant.as_u32() == 0 { + VariantIdx::from_u32(1) + } else { + VariantIdx::from_u32(0) + }; + assert_eq!(tagged_variant, *niche_variants.start()); + if *niche_start == 0 { + // The other variant is encoded as "null", so we can recurse searching for + // a pointer here. This relies on the fact that the codegen backend + // only adds "dereferenceable" if there's also a "nonnull" proof, + // and that null is aligned for all alignments so it's okay to forward + // the pointer's alignment. + Some(this.for_variant(cx, *untagged_variant)) + } else { + None + } } + Variants::Multiple { .. } => None, _ => Some(this), }; diff --git a/compiler/rustc_target/src/callconv/mod.rs b/compiler/rustc_target/src/callconv/mod.rs index 8c3df9c426b..aa639f1624f 100644 --- a/compiler/rustc_target/src/callconv/mod.rs +++ b/compiler/rustc_target/src/callconv/mod.rs @@ -143,7 +143,8 @@ pub struct ArgAttributes { pub regular: ArgAttribute, pub arg_ext: ArgExtension, /// The minimum size of the pointee, guaranteed to be valid for the duration of the whole call - /// (corresponding to LLVM's dereferenceable and dereferenceable_or_null attributes). + /// (corresponding to LLVM's dereferenceable_or_null attributes, i.e., it is okay for this to be + /// set on a null pointer, but all non-null pointers must be dereferenceable). pub pointee_size: Size, pub pointee_align: Option, } diff --git a/tests/codegen/function-arguments.rs b/tests/codegen/function-arguments.rs index 7fa1d659885..503799d3ed2 100644 --- a/tests/codegen/function-arguments.rs +++ b/tests/codegen/function-arguments.rs @@ -1,5 +1,6 @@ //@ compile-flags: -O -C no-prepopulate-passes #![crate_type = "lib"] +#![feature(rustc_attrs)] #![feature(dyn_star)] #![feature(allocator_api)] @@ -143,13 +144,28 @@ pub fn indirect_struct(_: S) {} #[no_mangle] pub fn borrowed_struct(_: &S) {} -// CHECK: @option_borrow(ptr noalias noundef readonly align 4 dereferenceable_or_null(4) %x) +// CHECK: @option_borrow(ptr noalias noundef readonly align 4 dereferenceable_or_null(4) %_x) #[no_mangle] -pub fn option_borrow(x: Option<&i32>) {} +pub fn option_borrow(_x: Option<&i32>) {} -// CHECK: @option_borrow_mut(ptr noalias noundef align 4 dereferenceable_or_null(4) %x) +// CHECK: @option_borrow_mut(ptr noalias noundef align 4 dereferenceable_or_null(4) %_x) #[no_mangle] -pub fn option_borrow_mut(x: Option<&mut i32>) {} +pub fn option_borrow_mut(_x: Option<&mut i32>) {} + +// Function that must NOT have `dereferenceable` or `align`. +#[rustc_layout_scalar_valid_range_start(16)] +pub struct RestrictedAddress(&'static i16); +enum E { + A(RestrictedAddress), + B, + C, +} +// If the `nonnull` ever goes missing, you might have to tweak the +// scalar_valid_range on `RestrictedAddress` to get it back. You +// might even have to add a `rustc_layout_scalar_valid_range_end`. +// CHECK: @nonnull_and_nondereferenceable(ptr noundef nonnull %_x) +#[no_mangle] +pub fn nonnull_and_nondereferenceable(_x: E) {} // CHECK: @raw_struct(ptr noundef %_1) #[no_mangle] -- cgit 1.4.1-3-g733a5 From 8fcc020a0c84d316509a48bf3fc97d08e790e83a Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Fri, 8 Nov 2024 10:41:16 -0500 Subject: try_question_mark_nop: update test for LLVM 20 llvm/llvm-project@dd116369f646d023a2e7e5c145a1bed5dcf9a45c changes the IR of this test in a way that I don't think is bad, but needs adjusting. r? @nikic @rustbot label: +llvm-main --- tests/codegen/try_question_mark_nop.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'tests/codegen') diff --git a/tests/codegen/try_question_mark_nop.rs b/tests/codegen/try_question_mark_nop.rs index bbab0d9eb1d..b68ecce869e 100644 --- a/tests/codegen/try_question_mark_nop.rs +++ b/tests/codegen/try_question_mark_nop.rs @@ -1,6 +1,10 @@ //@ compile-flags: -O -Z merge-functions=disabled --edition=2021 //@ only-x86_64 // FIXME: Remove the `min-llvm-version`. +//@ revisions: NINETEEN TWENTY +//@[NINETEEN] min-llvm-version: 19 +//@[NINETEEN] ignore-llvm-version: 20-99 +//@[TWENTY] min-llvm-version: 20 //@ min-llvm-version: 19 #![crate_type = "lib"] @@ -13,8 +17,11 @@ use std::ptr::NonNull; #[no_mangle] pub fn option_nop_match_32(x: Option) -> Option { // CHECK: start: + // TWENTY-NEXT: %trunc = trunc nuw i32 %0 to i1 + // TWENTY-NEXT: %.2 = select i1 %trunc, i32 %1, i32 undef // CHECK-NEXT: [[REG1:%.*]] = insertvalue { i32, i32 } poison, i32 %0, 0 - // CHECK-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } [[REG1]], i32 %1, 1 + // NINETEEN-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } [[REG1]], i32 %1, 1 + // TWENTY-NEXT: [[REG2:%.*]] = insertvalue { i32, i32 } [[REG1]], i32 %.2, 1 // CHECK-NEXT: ret { i32, i32 } [[REG2]] match x { Some(x) => Some(x), @@ -26,6 +33,8 @@ pub fn option_nop_match_32(x: Option) -> Option { #[no_mangle] pub fn option_nop_traits_32(x: Option) -> Option { // CHECK: start: + // TWENTY-NEXT: %trunc = trunc nuw i32 %0 to i1 + // TWENTY-NEXT: %.1 = select i1 %trunc, i32 %1, i32 undef // CHECK-NEXT: insertvalue { i32, i32 } // CHECK-NEXT: insertvalue { i32, i32 } // CHECK-NEXT: ret { i32, i32 } -- cgit 1.4.1-3-g733a5 From 1dc106121b62562ead6e7d612fa136dc4b35cd5d Mon Sep 17 00:00:00 2001 From: Kyle Huey Date: Mon, 4 Nov 2024 11:38:14 -0800 Subject: Add discriminators to DILocations when multiple functions are inlined into a single point. LLVM does not expect to ever see multiple dbg_declares for the same variable at the same location with different values. proc-macros make it possible for arbitrary code, including multiple calls that get inlined, to happen at any given location in the source code. Add discriminators when that happens so these locations are different to LLVM. This may interfere with the AddDiscriminators pass in LLVM, which is added by the unstable flag -Zdebug-info-for-profiling. Fixes #131944 --- .../src/debuginfo/create_scope_map.rs | 60 ++++++++++++++++++++-- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 4 ++ compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp | 8 +++ .../debuginfo-proc-macro/auxiliary/macro_def.rs | 11 ++++ .../mir_inlined_twice_var_locs.rs | 53 +++++++++++++++++++ 5 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 tests/codegen/debuginfo-proc-macro/auxiliary/macro_def.rs create mode 100644 tests/codegen/debuginfo-proc-macro/mir_inlined_twice_var_locs.rs (limited to 'tests/codegen') diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs index ac6c2fb1b83..0f1909486ec 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/create_scope_map.rs @@ -1,11 +1,15 @@ +use std::collections::hash_map::Entry; + use rustc_codegen_ssa::mir::debuginfo::{DebugScope, FunctionDebugContext}; use rustc_codegen_ssa::traits::*; +use rustc_data_structures::fx::FxHashMap; use rustc_index::Idx; use rustc_index::bit_set::BitSet; use rustc_middle::mir::{Body, SourceScope}; use rustc_middle::ty::layout::FnAbiOf; use rustc_middle::ty::{self, Instance}; use rustc_session::config::DebugInfo; +use rustc_span::BytePos; use super::metadata::file_metadata; use super::utils::DIB; @@ -37,10 +41,20 @@ pub(crate) fn compute_mir_scopes<'ll, 'tcx>( None }; let mut instantiated = BitSet::new_empty(mir.source_scopes.len()); + let mut discriminators = FxHashMap::default(); // Instantiate all scopes. for idx in 0..mir.source_scopes.len() { let scope = SourceScope::new(idx); - make_mir_scope(cx, instance, mir, &variables, debug_context, &mut instantiated, scope); + make_mir_scope( + cx, + instance, + mir, + &variables, + debug_context, + &mut instantiated, + &mut discriminators, + scope, + ); } assert!(instantiated.count() == mir.source_scopes.len()); } @@ -52,6 +66,7 @@ fn make_mir_scope<'ll, 'tcx>( variables: &Option>, debug_context: &mut FunctionDebugContext<'tcx, &'ll DIScope, &'ll DILocation>, instantiated: &mut BitSet, + discriminators: &mut FxHashMap, scope: SourceScope, ) { if instantiated.contains(scope) { @@ -60,7 +75,16 @@ fn make_mir_scope<'ll, 'tcx>( let scope_data = &mir.source_scopes[scope]; let parent_scope = if let Some(parent) = scope_data.parent_scope { - make_mir_scope(cx, instance, mir, variables, debug_context, instantiated, parent); + make_mir_scope( + cx, + instance, + mir, + variables, + debug_context, + instantiated, + discriminators, + parent, + ); debug_context.scopes[parent] } else { // The root is the function itself. @@ -117,7 +141,37 @@ fn make_mir_scope<'ll, 'tcx>( // FIXME(eddyb) this doesn't account for the macro-related // `Span` fixups that `rustc_codegen_ssa::mir::debuginfo` does. let callsite_scope = parent_scope.adjust_dbg_scope_for_span(cx, callsite_span); - cx.dbg_loc(callsite_scope, parent_scope.inlined_at, callsite_span) + let loc = cx.dbg_loc(callsite_scope, parent_scope.inlined_at, callsite_span); + + // NB: In order to produce proper debug info for variables (particularly + // arguments) in multiply-inline functions, LLVM expects to see a single + // DILocalVariable with multiple different DILocations in the IR. While + // the source information for each DILocation would be identical, their + // inlinedAt attributes will be unique to the particular callsite. + // + // We generate DILocations here based on the callsite's location in the + // source code. A single location in the source code usually can't + // produce multiple distinct calls so this mostly works, until + // proc-macros get involved. A proc-macro can generate multiple calls + // at the same span, which breaks the assumption that we're going to + // produce a unique DILocation for every scope we process here. We + // have to explicitly add discriminators if we see inlines into the + // same source code location. + // + // Note further that we can't key this hashtable on the span itself, + // because these spans could have distinct SyntaxContexts. We have + // to key on exactly what we're giving to LLVM. + match discriminators.entry(callsite_span.lo()) { + Entry::Occupied(mut o) => { + *o.get_mut() += 1; + unsafe { llvm::LLVMRustDILocationCloneWithBaseDiscriminator(loc, *o.get()) } + .expect("Failed to encode discriminator in DILocation") + } + Entry::Vacant(v) => { + v.insert(0); + loc + } + } }); debug_context.scopes[scope] = DebugScope { diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 3d2e270a386..75a5ec44c22 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -2174,6 +2174,10 @@ unsafe extern "C" { Scope: &'a DIScope, InlinedAt: Option<&'a DILocation>, ) -> &'a DILocation; + pub fn LLVMRustDILocationCloneWithBaseDiscriminator<'a>( + Location: &'a DILocation, + BD: c_uint, + ) -> Option<&'a DILocation>; pub fn LLVMRustDIBuilderCreateOpDeref() -> u64; pub fn LLVMRustDIBuilderCreateOpPlusUconst() -> u64; pub fn LLVMRustDIBuilderCreateOpLLVMFragment() -> u64; diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 9330c83b7f2..cd70c3f2669 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1305,6 +1305,14 @@ LLVMRustDIBuilderCreateDebugLocation(unsigned Line, unsigned Column, return wrap(Loc); } +extern "C" LLVMMetadataRef +LLVMRustDILocationCloneWithBaseDiscriminator(LLVMMetadataRef Location, + unsigned BD) { + DILocation *Loc = unwrapDIPtr(Location); + auto NewLoc = Loc->cloneWithBaseDiscriminator(BD); + return wrap(NewLoc.has_value() ? NewLoc.value() : nullptr); +} + extern "C" uint64_t LLVMRustDIBuilderCreateOpDeref() { return dwarf::DW_OP_deref; } diff --git a/tests/codegen/debuginfo-proc-macro/auxiliary/macro_def.rs b/tests/codegen/debuginfo-proc-macro/auxiliary/macro_def.rs new file mode 100644 index 00000000000..159ecfd0974 --- /dev/null +++ b/tests/codegen/debuginfo-proc-macro/auxiliary/macro_def.rs @@ -0,0 +1,11 @@ +//@ force-host +//@ no-prefer-dynamic +#![crate_type = "proc-macro"] + +extern crate proc_macro; +use proc_macro::*; + +#[proc_macro] +pub fn square_twice(_item: TokenStream) -> TokenStream { + "(square(env::vars().count() as i32), square(env::vars().count() as i32))".parse().unwrap() +} diff --git a/tests/codegen/debuginfo-proc-macro/mir_inlined_twice_var_locs.rs b/tests/codegen/debuginfo-proc-macro/mir_inlined_twice_var_locs.rs new file mode 100644 index 00000000000..c3858044c0c --- /dev/null +++ b/tests/codegen/debuginfo-proc-macro/mir_inlined_twice_var_locs.rs @@ -0,0 +1,53 @@ +//@ min-llvm-version: 19 +//@ compile-flags: -Cdebuginfo=2 -Copt-level=0 -Zmir-enable-passes=+Inline +// MSVC is different because of the individual allocas. +//@ ignore-msvc + +//@ aux-build:macro_def.rs + +// Find the variable. +// CHECK-DAG: ![[#var_dbg:]] = !DILocalVariable(name: "n",{{( arg: 1,)?}} scope: ![[#var_scope:]] + +// Find both dbg_declares. These will proceed the variable metadata, of course, so we're looking +// backwards. +// CHECK-DAG: dbg_declare(ptr %n.dbg.spill{{[0-9]}}, ![[#var_dbg]], !DIExpression(), ![[#var_loc2:]]) +// CHECK-DAG: dbg_declare(ptr %n.dbg.spill, ![[#var_dbg]], !DIExpression(), ![[#var_loc1:]]) + +// Find the first location definition, looking forwards again. +// CHECK: ![[#var_loc1]] = !DILocation +// CHECK-SAME: scope: ![[#var_scope:]], inlinedAt: ![[#var_inlinedAt1:]] + +// Find the first location's inlinedAt +// NB: If we fail here it's *probably* because we failed to produce two +// different locations and ended up reusing an earlier one. +// CHECK: ![[#var_inlinedAt1]] = !DILocation +// CHECK-SAME: scope: ![[var_inlinedAt1_scope:]] + +// Find the second location definition, still looking forwards. +// NB: If we failed to produce two different locations, the test will +// definitely fail by this point (if it hasn't already) because we won't +// be able to find the same line again. +// CHECK: ![[#var_loc2]] = !DILocation +// CHECK-SAME: scope: ![[#var_scope]], inlinedAt: ![[#var_inlinedAt2:]] + +// Find the second location's inlinedAt. +// CHECK: ![[#var_inlinedAt2]] = !DILocation +// CHECK-SAME: scope: ![[#var_inlinedAt2_scope:]] + +// Finally, check that a discriminator was emitted for the second scope. +// FIXMEkhuey ideally we would check that *either* scope has a discriminator +// but I don't know that it's possible to check that with FileCheck. +// CHECK: ![[#var_inlinedAt2_scope]] = !DILexicalBlockFile +// CHECK-SAME: discriminator: [[#]] +extern crate macro_def; + +use std::env; + +fn square(n: i32) -> i32 { + n * n +} + +fn main() { + let (z1, z2) = macro_def::square_twice!(); + println!("{z1} == {z2}"); +} -- cgit 1.4.1-3-g733a5