Skip to content

Commit

Permalink
Auto merge of #87082 - michaelwoerister:const-in-debuginfo-type-names…
Browse files Browse the repository at this point in the history 
…-fix, r=oli-obk,wesleywiser

Handle non-integer const generic parameters in debuginfo type names.

This PR fixes an ICE introduced by #85269 which started emitting const generic arguments for debuginfo names but did not cover the case where such an argument could not be evaluated to a flat string of bits.

The fix implemented in this PR is very basic: If `try_eval_bits()` fails for the constant in question, we fall back to generating a stable hash of the constant and emit that instead. This way we get a (virtually) unique name and side step the problem of generating a string representation of a potentially complex value.

The downside is that the generated name will be rather opaque. E.g. the regression test adds a function `const_generic_fn_non_int<()>` which is then rendered as `const_generic_fn_non_int<{CONST#fe3cfa0214ac55c7}>`. I think it's an open question how to deal with this more gracefully.

I'd be interested in ideas on how to do this better.

r? `@wesleywiser`

cc `@dpaoliello` (do you see any problems with this approach?)
cc `@Mark-Simulacrum` & `@nagisa` (who I've seen comment on debuginfo issues recently -- anyone else?)

Fixes #86893
  • Loading branch information
@bors
bors committed Jul 14, 2021
2 parents ee5ed4a + 28343be commit 4f0c568
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 20 deletions.
81 changes: 63 additions & 18 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@
// * `"` is treated as the start of a string.

use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::definitions::{DefPathData, DefPathDataName, DisambiguatedDefPathData};
use rustc_middle::ich::NodeIdHashingMode;
use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::subst::{GenericArgKind, SubstsRef};
use rustc_middle::ty::{self, AdtDef, Ty, TyCtxt};
use rustc_target::abi::{TagEncoding, Variants};
use rustc_target::abi::{Integer, TagEncoding, Variants};

use std::fmt::Write;

Expand Down Expand Up @@ -47,7 +50,7 @@ pub fn push_debuginfo_type_name<'tcx>(
) {
// When targeting MSVC, emit C++ style type names for compatibility with
// .natvis visualizers (and perhaps other existing native debuggers?)
let cpp_like_names = tcx.sess.target.is_like_msvc;
let cpp_like_names = cpp_like_names(tcx);

match *t.kind() {
ty::Bool => output.push_str("bool"),
Expand Down Expand Up @@ -424,16 +427,14 @@ fn push_unqualified_item_name(
disambiguated_data: DisambiguatedDefPathData,
output: &mut String,
) {
let cpp_like_names = tcx.sess.target.is_like_msvc;

match disambiguated_data.data {
DefPathData::CrateRoot => {
output.push_str(&tcx.crate_name(def_id.krate).as_str());
}
DefPathData::ClosureExpr if tcx.generator_kind(def_id).is_some() => {
// Generators look like closures, but we want to treat them differently
// in the debug info.
if cpp_like_names {
if cpp_like_names(tcx) {
write!(output, "generator${}", disambiguated_data.disambiguator).unwrap();
} else {
write!(output, "{{generator#{}}}", disambiguated_data.disambiguator).unwrap();
Expand All @@ -444,7 +445,7 @@ fn push_unqualified_item_name(
output.push_str(&name.as_str());
}
DefPathDataName::Anon { namespace } => {
if cpp_like_names {
if cpp_like_names(tcx) {
write!(output, "{}${}", namespace, disambiguated_data.disambiguator).unwrap();
} else {
write!(output, "{{{}#{}}}", namespace, disambiguated_data.disambiguator)
Expand Down Expand Up @@ -478,19 +479,14 @@ fn push_generic_params_internal<'tcx>(
match type_parameter {
GenericArgKind::Type(type_parameter) => {
push_debuginfo_type_name(tcx, type_parameter, true, output, visited);
output.push_str(", ");
}
GenericArgKind::Const(const_parameter) => match const_parameter.val {
ty::ConstKind::Param(param) => write!(output, "{}, ", param.name).unwrap(),
_ => write!(
output,
"0x{:x}, ",
const_parameter.eval_bits(tcx, ty::ParamEnv::reveal_all(), const_parameter.ty)
)
.unwrap(),
},
}
GenericArgKind::Const(ct) => {
push_const_param(tcx, ct, output);
}
other => bug!("Unexpected non-erasable generic: {:?}", other),
}

output.push_str(", ");
}

output.pop();
Expand All @@ -499,6 +495,51 @@ fn push_generic_params_internal<'tcx>(
push_close_angle_bracket(tcx, output);
}

fn push_const_param<'tcx>(tcx: TyCtxt<'tcx>, ct: &'tcx ty::Const<'tcx>, output: &mut String) {
match ct.val {
ty::ConstKind::Param(param) => {
write!(output, "{}", param.name)
}
_ => match ct.ty.kind() {
ty::Int(ity) => {
let bits = ct.eval_bits(tcx, ty::ParamEnv::reveal_all(), ct.ty);
let val = Integer::from_int_ty(&tcx, *ity).size().sign_extend(bits) as i128;
write!(output, "{}", val)
}
ty::Uint(_) => {
let val = ct.eval_bits(tcx, ty::ParamEnv::reveal_all(), ct.ty);
write!(output, "{}", val)
}
ty::Bool => {
let val = ct.try_eval_bool(tcx, ty::ParamEnv::reveal_all()).unwrap();
write!(output, "{}", val)
}
_ => {
// If we cannot evaluate the constant to a known type, we fall back
// to emitting a stable hash value of the constant. This isn't very pretty
// but we get a deterministic, virtually unique value for the constant.
let hcx = &mut tcx.create_stable_hashing_context();
let mut hasher = StableHasher::new();
hcx.while_hashing_spans(false, |hcx| {
hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| {
ct.val.hash_stable(hcx, &mut hasher);
});
});
// Let's only emit 64 bits of the hash value. That should be plenty for
// avoiding collisions and will make the emitted type names shorter.
let hash: u64 = hasher.finish();

if cpp_like_names(tcx) {
write!(output, "CONST${:x}", hash)
} else {
write!(output, "{{CONST#{:x}}}", hash)
}
}
},
}
.unwrap();
}

pub fn push_generic_params<'tcx>(tcx: TyCtxt<'tcx>, substs: SubstsRef<'tcx>, output: &mut String) {
let mut visited = FxHashSet::default();
push_generic_params_internal(tcx, substs, output, &mut visited);
Expand All @@ -507,9 +548,13 @@ pub fn push_generic_params<'tcx>(tcx: TyCtxt<'tcx>, substs: SubstsRef<'tcx>, out
fn push_close_angle_bracket<'tcx>(tcx: TyCtxt<'tcx>, output: &mut String) {
// MSVC debugger always treats `>>` as a shift, even when parsing templates,
// so add a space to avoid confusion.
if tcx.sess.target.is_like_msvc && output.ends_with('>') {
if cpp_like_names(tcx) && output.ends_with('>') {
output.push(' ')
};

output.push('>');
}

fn cpp_like_names(tcx: TyCtxt<'_>) -> bool {
tcx.sess.target.is_like_msvc
}
30 changes: 28 additions & 2 deletions src/test/debuginfo/function-names.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@
// Generator
// Generators don't seem to appear in GDB's symbol table.

// Const generic parameter
// gdb-command:info functions -q function_names::const_generic_fn.*
// gdb-check:[...]static fn function_names::const_generic_fn_bool();
// gdb-check:[...]static fn function_names::const_generic_fn_non_int();
// gdb-check:[...]static fn function_names::const_generic_fn_signed_int();
// gdb-check:[...]static fn function_names::const_generic_fn_unsigned_int();

// === CDB TESTS ===================================================================================

// Top-level function
Expand All @@ -49,9 +56,9 @@

// Trait implementations
// cdb-command:x a!function_names::*::trait_function*
// cdb-check:[...] a!function_names::impl$6::trait_function<i32, 0x1> (void)
// cdb-check:[...] a!function_names::impl$3::trait_function<i32> (void)
// cdb-check:[...] a!function_names::impl$1::trait_function (void)
// cdb-check:[...] a!function_names::impl$6::trait_function<i32, 1> (void)
// cdb-check:[...] a!function_names::impl$5::trait_function3<function_names::TestStruct1> (void)
// cdb-check:[...] a!function_names::Mod1::impl$1::trait_function (void)

Expand All @@ -65,10 +72,18 @@
// cdb-command:x a!function_names::*::generator*
// cdb-check:[...] a!function_names::main::generator$1 (void)

// Const generic parameter
// cdb-command:x a!function_names::const_generic_fn*
// cdb-check:[...] a!function_names::const_generic_fn_bool<false> (void)
// cdb-check:[...] a!function_names::const_generic_fn_non_int<CONST$fe3cfa0214ac55c7> (void)
// cdb-check:[...] a!function_names::const_generic_fn_unsigned_int<14> (void)
// cdb-check:[...] a!function_names::const_generic_fn_signed_int<-7> (void)

#![allow(unused_variables)]
#![feature(omit_gdb_pretty_printer_section)]
#![omit_gdb_pretty_printer_section]
#![feature(generators, generator_trait)]
#![feature(const_generics, generators, generator_trait)]
#![allow(incomplete_features)] // for const_generics

use Mod1::TestTrait2;
use std::ops::Generator;
Expand Down Expand Up @@ -97,6 +112,12 @@ fn main() {
// Generator
let mut generator = || { yield; return; };
Pin::new(&mut generator).resume(());

// Const generic functions
const_generic_fn_bool::<false>();
const_generic_fn_non_int::<{()}>();
const_generic_fn_signed_int::<-7>();
const_generic_fn_unsigned_int::<14>();
}

struct TestStruct1;
Expand Down Expand Up @@ -173,3 +194,8 @@ fn generic_func<T>(value: T) -> T {

value
}

fn const_generic_fn_bool<const C: bool>() {}
fn const_generic_fn_non_int<const C: ()>() {}
fn const_generic_fn_signed_int<const C: i64>() {}
fn const_generic_fn_unsigned_int<const C: u32>() {}

0 comments on commit 4f0c568

Please sign in to comment.