Skip to content

Commit

Permalink
avoid leaking host details in proc macro metadata decoding
Browse files Browse the repository at this point in the history
proc macro crates are essentially implemented as dynamic libraries using
a dlopen-based ABI. They are also Rust crates, so they have 2 worlds -
the "host" world in which they are defined, and the "target" world in
which they are used.

For all the "target" world knows, the proc macro crate might not even
be implemented in Rust, so leaks of details from the host to the target
must be avoided for correctness.

Because the "host" DefId space is different from the "target" DefId
space, any leak involving a DefId will have a nonsensical or
out-of-bounds DefKey, and will cause all sorts of crashes.

This PR fixes all leaks I have found in `decoder`. In particular, #54059
was caused by host native libraries leaking into the target, which feels
like it might even be a correctness issue if it doesn't cause an ICE.

Fixes #54059
  • Loading branch information
arielb1 committed Sep 15, 2018
1 parent cba0fdf commit 0d840e1
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 24 deletions.
6 changes: 0 additions & 6 deletions src/librustc_metadata/cstore_impl.rs
Expand Up @@ -259,12 +259,6 @@ provide! { <'tcx> tcx, def_id, other, cdata,
let cnum = cdata.cnum;
assert!(cnum != LOCAL_CRATE);

// If this crate is a custom derive crate, then we're not even going to
// link those in so we skip those crates.
if cdata.root.macro_derive_registrar.is_some() {
return Arc::new(Vec::new())
}

Arc::new(cdata.exported_symbols(tcx))
}
}
Expand Down
71 changes: 53 additions & 18 deletions src/librustc_metadata/decoder.rs
Expand Up @@ -692,6 +692,8 @@ impl<'a, 'tcx> CrateMetadata {

/// Iterates over all the stability attributes in the given crate.
pub fn get_lib_features(&self) -> Vec<(ast::Name, Option<ast::Name>)> {
// FIXME: For a proc macro crate, not sure whether we should return the "host"
// features or an empty Vec. Both don't cause ICEs.
self.root
.lib_features
.decode(self)
Expand All @@ -700,11 +702,16 @@ impl<'a, 'tcx> CrateMetadata {

/// Iterates over the language items in the given crate.
pub fn get_lang_items(&self) -> Vec<(DefId, usize)> {
self.root
.lang_items
.decode(self)
.map(|(def_index, index)| (self.local_def_id(def_index), index))
.collect()
if self.proc_macros.is_some() {
// Proc macro crates do not export any lang-items to the target.
vec![]
} else {
self.root
.lang_items
.decode(self)
.map(|(def_index, index)| (self.local_def_id(def_index), index))
.collect()
}
}

/// Iterates over each child of the given item.
Expand Down Expand Up @@ -978,12 +985,16 @@ impl<'a, 'tcx> CrateMetadata {
pub fn get_implementations_for_trait(&self,
filter: Option<DefId>,
result: &mut Vec<DefId>) {
if self.proc_macros.is_some() {
// proc-macro crates export no trait impls.
return
}

// Do a reverse lookup beforehand to avoid touching the crate_num
// hash map in the loop below.
let filter = match filter.map(|def_id| self.reverse_translate_def_id(def_id)) {
Some(Some(def_id)) => Some((def_id.krate.as_u32(), def_id.index)),
Some(None) => return,
None if self.proc_macros.is_some() => return,
None => None,
};

Expand Down Expand Up @@ -1016,11 +1027,21 @@ impl<'a, 'tcx> CrateMetadata {


pub fn get_native_libraries(&self, sess: &Session) -> Vec<NativeLibrary> {
self.root.native_libraries.decode((self, sess)).collect()
if self.proc_macros.is_some() {
// Proc macro crates do not have any *target* native libraries.
vec![]
} else {
self.root.native_libraries.decode((self, sess)).collect()
}
}

pub fn get_foreign_modules(&self, sess: &Session) -> Vec<ForeignModule> {
self.root.foreign_modules.decode((self, sess)).collect()
if self.proc_macros.is_some() {
// Proc macro crates do not have any *target* foreign modules.
vec![]
} else {
self.root.foreign_modules.decode((self, sess)).collect()
}
}

pub fn get_dylib_dependency_formats(&self) -> Vec<(CrateNum, LinkagePreference)> {
Expand All @@ -1036,10 +1057,15 @@ impl<'a, 'tcx> CrateMetadata {
}

pub fn get_missing_lang_items(&self) -> Vec<lang_items::LangItem> {
self.root
.lang_items_missing
.decode(self)
.collect()
if self.proc_macros.is_some() {
// Proc macro crates do not depend on any target weak lang-items.
vec![]
} else {
self.root
.lang_items_missing
.decode(self)
.collect()
}
}

pub fn get_fn_arg_names(&self, id: DefIndex) -> Vec<ast::Name> {
Expand All @@ -1055,10 +1081,16 @@ impl<'a, 'tcx> CrateMetadata {
pub fn exported_symbols(&self,
tcx: TyCtxt<'a, 'tcx, 'tcx>)
-> Vec<(ExportedSymbol<'tcx>, SymbolExportLevel)> {
let lazy_seq: LazySeq<(ExportedSymbol<'tcx>, SymbolExportLevel)> =
LazySeq::with_position_and_length(self.root.exported_symbols.position,
self.root.exported_symbols.len);
lazy_seq.decode((self, tcx)).collect()
if self.proc_macros.is_some() {
// If this crate is a custom derive crate, then we're not even going to
// link those in so we skip those crates.
vec![]
} else {
let lazy_seq: LazySeq<(ExportedSymbol<'tcx>, SymbolExportLevel)> =
LazySeq::with_position_and_length(self.root.exported_symbols.position,
self.root.exported_symbols.len);
lazy_seq.decode((self, tcx)).collect()
}
}

pub fn get_rendered_const(&self, id: DefIndex) -> String {
Expand Down Expand Up @@ -1149,9 +1181,12 @@ impl<'a, 'tcx> CrateMetadata {
/// file they represent, just information about length, line breaks, and
/// multibyte characters. This information is enough to generate valid debuginfo
/// for items inlined from other crates.
///
/// Proc macro crates don't currently export spans, so this function does not have
/// to work for them.
pub fn imported_source_files(&'a self,
local_source_map: &source_map::SourceMap)
-> ReadGuard<'a, Vec<cstore::ImportedSourceFile>> {
local_source_map: &source_map::SourceMap)
-> ReadGuard<'a, Vec<cstore::ImportedSourceFile>> {
{
let source_files = self.source_map_import_info.borrow();
if !source_files.is_empty() {
Expand Down
59 changes: 59 additions & 0 deletions src/test/incremental-fulldeps/auxiliary/issue_54059.rs
@@ -0,0 +1,59 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// no-prefer-dynamic

// check that having extern "C" functions in a proc macro doesn't crash.

#![crate_type="proc-macro"]
#![allow(non_snake_case)]

extern crate proc_macro;

macro_rules! proc_macro_tokenstream {
() => {
::proc_macro::TokenStream
};
}

macro_rules! proc_macro_expr_impl {
($(
$( #[$attr:meta] )*
pub fn $func:ident($input:ident: &str) -> String $body:block
)+) => {
$(
// Parses an input that looks like:
//
// ```
// #[allow(unused)]
// enum ProcMacroHack {
// Input = (stringify!(ARGS), 0).1,
// }
// ```
$( #[$attr] )*
#[proc_macro_derive($func)]
pub fn $func(input: proc_macro_tokenstream!()) -> proc_macro_tokenstream!() {
unsafe { rust_dbg_extern_identity_u64(0); }
panic!()
}
)+
};
}

proc_macro_expr_impl! {
pub fn base2_impl(input: &str) -> String {
panic!()
}
}

#[link(name="rust_test_helpers")]
extern "C" {
pub fn rust_dbg_extern_identity_u64(v: u64) -> u64;
}
18 changes: 18 additions & 0 deletions src/test/incremental-fulldeps/issue-54059.rs
@@ -0,0 +1,18 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// aux-build:issue_54059.rs
// ignore-stage1
// ignore-wasm32-bare no libc for ffi testing
// revisions: rpass1

extern crate issue_54059;

fn main() {}

0 comments on commit 0d840e1

Please sign in to comment.