From 4aa48a32413e07e1475201c22a42d533b2417091 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 4 Jan 2018 11:26:47 +0100 Subject: [PATCH 1/2] Map invalid Spans to DUMMY_SP during crate metadata encoding. This mirrors what we for stabilizing the incr. comp. cache and is necessary for reproducible builds. --- src/librustc_metadata/decoder.rs | 34 +++++++++++---------------- src/librustc_metadata/encoder.rs | 40 +++++++++++++++++++++++++++++++- src/librustc_metadata/schema.rs | 4 ++++ 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index fb4a73891a3..91703425017 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -270,19 +270,17 @@ impl<'a, 'tcx> SpecializedDecoder for DecodeContext<'a, 'tcx> { impl<'a, 'tcx> SpecializedDecoder for DecodeContext<'a, 'tcx> { fn specialized_decode(&mut self) -> Result { + let tag = u8::decode(self)?; + + if tag == TAG_INVALID_SPAN { + return Ok(DUMMY_SP) + } + + debug_assert_eq!(tag, TAG_VALID_SPAN); + let lo = BytePos::decode(self)?; - let hi = BytePos::decode(self)?; - - if lo == BytePos(0) && hi == BytePos(0) { - // Don't try to rebase DUMMY_SP. Otherwise it will look like a valid - // Span again. - return Ok(DUMMY_SP) - } - - if hi < lo { - // Consistently map invalid spans to DUMMY_SP. - return Ok(DUMMY_SP) - } + let len = BytePos::decode(self)?; + let hi = lo + len; let sess = if let Some(sess) = self.sess { sess @@ -297,9 +295,7 @@ impl<'a, 'tcx> SpecializedDecoder for DecodeContext<'a, 'tcx> { let last_filemap = &imported_filemaps[self.last_filemap_index]; if lo >= last_filemap.original_start_pos && - lo <= last_filemap.original_end_pos && - hi >= last_filemap.original_start_pos && - hi <= last_filemap.original_end_pos { + lo <= last_filemap.original_end_pos { last_filemap } else { let mut a = 0; @@ -323,11 +319,9 @@ impl<'a, 'tcx> SpecializedDecoder for DecodeContext<'a, 'tcx> { debug_assert!(lo >= filemap.original_start_pos && lo <= filemap.original_end_pos); - if hi < filemap.original_start_pos || hi > filemap.original_end_pos { - // `hi` points to a different FileMap than `lo` which is invalid. - // Again, map invalid Spans to DUMMY_SP. - return Ok(DUMMY_SP) - } + // Make sure we correctly filtered out invalid spans during encoding + debug_assert!(hi >= filemap.original_start_pos && + hi <= filemap.original_end_pos); let lo = (lo + filemap.translated_filemap.start_pos) - filemap.original_start_pos; let hi = (hi + filemap.translated_filemap.start_pos) - filemap.original_start_pos; diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index 5ddbb18450e..efc39805a74 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -41,7 +41,7 @@ use syntax::ast::{self, CRATE_NODE_ID}; use syntax::codemap::Spanned; use syntax::attr; use syntax::symbol::Symbol; -use syntax_pos::{self, FileName}; +use syntax_pos::{self, FileName, FileMap, Span, DUMMY_SP}; use rustc::hir::{self, PatKind}; use rustc::hir::itemlikevisit::ItemLikeVisitor; @@ -57,6 +57,9 @@ pub struct EncodeContext<'a, 'tcx: 'a> { lazy_state: LazyState, type_shorthands: FxHashMap, usize>, predicate_shorthands: FxHashMap, usize>, + + // This is used to speed up Span encoding. + filemap_cache: Rc, } macro_rules! encoder_methods { @@ -140,6 +143,40 @@ impl<'a, 'tcx> SpecializedEncoder for EncodeContext<'a, 'tcx> { } } +impl<'a, 'tcx> SpecializedEncoder for EncodeContext<'a, 'tcx> { + fn specialized_encode(&mut self, span: &Span) -> Result<(), Self::Error> { + if *span == DUMMY_SP { + return TAG_INVALID_SPAN.encode(self) + } + + let span = span.data(); + + if span.lo > span.hi { + return TAG_INVALID_SPAN.encode(self) + } + + if !self.filemap_cache.contains(span.lo) { + let codemap = self.tcx.sess.codemap(); + let filemap_index = codemap.lookup_filemap_idx(span.lo); + self.filemap_cache = codemap.files()[filemap_index].clone(); + } + + if !self.filemap_cache.contains(span.hi) { + return TAG_INVALID_SPAN.encode(self) + } + + TAG_VALID_SPAN.encode(self)?; + span.lo.encode(self)?; + + // Encode length which is usually less than span.hi and profits more + // from the variable-length integer encoding that we use. + let len = span.hi - span.lo; + len.encode(self) + + // Don't encode the expansion context. + } +} + impl<'a, 'tcx> SpecializedEncoder> for EncodeContext<'a, 'tcx> { fn specialized_encode(&mut self, ty: &Ty<'tcx>) -> Result<(), Self::Error> { ty_codec::encode_with_shorthand(self, ty, |ecx| &mut ecx.type_shorthands) @@ -1648,6 +1685,7 @@ pub fn encode_metadata<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, lazy_state: LazyState::NoNode, type_shorthands: Default::default(), predicate_shorthands: Default::default(), + filemap_cache: tcx.sess.codemap().files()[0].clone(), }; // Encode the rustc version string in a predictable location. diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index 8ff32746391..5510d66b55b 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -521,3 +521,7 @@ pub struct GeneratorData<'tcx> { pub layout: mir::GeneratorLayout<'tcx>, } impl_stable_hash_for!(struct GeneratorData<'tcx> { layout }); + +// Tags used for encoding Spans: +pub const TAG_VALID_SPAN: u8 = 0; +pub const TAG_INVALID_SPAN: u8 = 1; From 30d921fb1e0ea920fc85af6fa46c2be3317568c3 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Thu, 4 Jan 2018 13:13:32 +0100 Subject: [PATCH 2/2] Span Encoding: Replace if with debug_assertion() and add some comments. --- src/librustc_metadata/encoder.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index efc39805a74..a42c8be9c20 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -151,9 +151,8 @@ impl<'a, 'tcx> SpecializedEncoder for EncodeContext<'a, 'tcx> { let span = span.data(); - if span.lo > span.hi { - return TAG_INVALID_SPAN.encode(self) - } + // The Span infrastructure should make sure that this invariant holds: + debug_assert!(span.lo <= span.hi); if !self.filemap_cache.contains(span.lo) { let codemap = self.tcx.sess.codemap(); @@ -162,6 +161,8 @@ impl<'a, 'tcx> SpecializedEncoder for EncodeContext<'a, 'tcx> { } if !self.filemap_cache.contains(span.hi) { + // Unfortunately, macro expansion still sometimes generates Spans + // that malformed in this way. return TAG_INVALID_SPAN.encode(self) }