Don't rely on symbol names in is_blocklisted_fn. (#265)

* Don't rely on symbol names in is_blocklisted_fn.

* is_blocklisted_fn: move "fmt_decimal" to symbols.
This commit is contained in:
Eduard-Mihai Burtescu 2020-11-20 13:43:31 +02:00 committed by GitHub
parent a49acc1709
commit 3eebcd789f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 51 additions and 12 deletions

View File

@ -58,7 +58,6 @@ impl<'tcx> CodegenCx<'tcx> {
// MiscMethods::get_fn_addr -> get_fn_ext -> declare_fn_ext
// PreDefineMethods::predefine_fn -> declare_fn_ext
fn declare_fn_ext(&self, instance: Instance<'tcx>, linkage: Option<LinkageType>) -> SpirvValue {
let symbol_name = self.tcx.symbol_name(instance).name;
let control = attrs_to_spirv(self.tcx.codegen_fn_attrs(instance.def_id()));
let fn_abi = FnAbi::of_instance(self, instance, &[]);
let function_type = fn_abi.spirv_type(self);
@ -70,7 +69,7 @@ impl<'tcx> CodegenCx<'tcx> {
other => bug!("fn_abi type {}", other.debug(function_type, self)),
};
if crate::is_blocklisted_fn(symbol_name) {
if crate::is_blocklisted_fn(self.tcx, &self.sym, instance) {
// This can happen if we call a blocklisted function in another crate.
let result = self.undef(function_type);
// TODO: Span info here
@ -96,6 +95,7 @@ impl<'tcx> CodegenCx<'tcx> {
emit.name(fn_id, &human_name);
drop(emit); // set_linkage uses emit
if let Some(linkage) = linkage {
let symbol_name = self.tcx.symbol_name(instance).name;
self.set_linkage(fn_id, symbol_name.to_owned(), linkage);
}

View File

@ -112,11 +112,11 @@ use rustc_middle::middle::cstore::{EncodedMetadata, MetadataLoader, MetadataLoad
use rustc_middle::mir::mono::{Linkage, MonoItem, Visibility};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{InstanceDef, TyCtxt};
use rustc_middle::ty::{self, DefIdTree, Instance, InstanceDef, TyCtxt};
use rustc_mir::util::write_mir_pretty;
use rustc_session::config::{self, OptLevel, OutputFilenames, OutputType};
use rustc_session::Session;
use rustc_span::Symbol;
use rustc_span::symbol::{sym, Symbol};
use rustc_target::spec::abi::Abi;
use rustc_target::spec::{LinkerFlavor, PanicStrategy, Target, TargetOptions, TargetTriple};
use std::any::Any;
@ -146,9 +146,41 @@ fn dump_mir<'tcx>(
}
}
fn is_blocklisted_fn(symbol_name: &str) -> bool {
fn is_blocklisted_fn<'tcx>(
tcx: TyCtxt<'tcx>,
sym: &symbols::Symbols,
instance: Instance<'tcx>,
) -> bool {
// TODO: These sometimes have a constant value of an enum variant with a hole
symbol_name.contains("core..fmt..Debug")
if let InstanceDef::Item(def) = instance.def {
if let Some(debug_trait_def_id) = tcx.get_diagnostic_item(sym::debug_trait) {
// Helper for detecting `<_ as core::fmt::Debug>::fmt` (in impls).
let is_debug_fmt_method = |def_id| match tcx.opt_associated_item(def_id) {
Some(assoc) if assoc.ident.name == sym::fmt => match assoc.container {
ty::ImplContainer(impl_def_id) => {
tcx.impl_trait_ref(impl_def_id).map(|tr| tr.def_id)
== Some(debug_trait_def_id)
}
ty::TraitContainer(_) => false,
},
_ => false,
};
if is_debug_fmt_method(def.did) {
return true;
}
if tcx.opt_item_name(def.did).map(|i| i.name) == Some(sym.fmt_decimal) {
if let Some(parent_def_id) = tcx.parent(def.did) {
if is_debug_fmt_method(parent_def_id) {
return true;
}
}
}
}
}
false
}
fn target_options() -> Target {
@ -489,18 +521,20 @@ impl ExtraBackendMethods for SpirvCodegenBackend {
}
for &(mono_item, (linkage, visibility)) in mono_items.iter() {
let name = mono_item.symbol_name(cx.tcx).name;
if is_blocklisted_fn(name) {
continue;
if let MonoItem::Fn(instance) = mono_item {
if is_blocklisted_fn(cx.tcx, &cx.sym, instance) {
continue;
}
}
mono_item.predefine::<Builder<'_, '_>>(&cx, linkage, visibility);
}
// ... and now that we have everything pre-defined, fill out those definitions.
for &(mono_item, _) in mono_items.iter() {
let name = mono_item.symbol_name(cx.tcx).name;
if is_blocklisted_fn(name) {
continue;
if let MonoItem::Fn(instance) = mono_item {
if is_blocklisted_fn(cx.tcx, &cx.sym, instance) {
continue;
}
}
mono_item.define::<Builder<'_, '_>>(&cx);
}

View File

@ -10,6 +10,9 @@ use std::collections::HashMap;
/// already exists, deduplicating it if so. This makes things like comparison and cloning really cheap. So, this struct
/// is to allocate all our keywords up front and intern them all, so we can do comparisons really easily and fast.
pub struct Symbols {
// Used by `is_blocklisted_fn`.
pub fmt_decimal: Symbol,
pub spirv: Symbol,
pub spirv_std: Symbol,
pub kernel: Symbol,
@ -331,6 +334,8 @@ impl Symbols {
assert!(old.is_none());
});
Self {
fmt_decimal: Symbol::intern("fmt_decimal"),
spirv: Symbol::intern("spirv"),
spirv_std: Symbol::intern("spirv_std"),
kernel: Symbol::intern("kernel"),