From 16b37b50273e9b6a12e20fd5f00d2d9bf50a3775 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 26 Jun 2019 05:39:07 -0700 Subject: [PATCH 01/28] Update linked OpenSSL version This bumps our linked OpenSSL version from 1.1.1a to 1.1.1c, picking up some various bug fixes and minor security issue fixes. --- Cargo.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eaec52a7a150c..b82970ecb895c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1836,7 +1836,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "openssl-src" -version = "111.1.0+1.1.1a" +version = "111.3.0+1.1.1c" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "cc 1.0.35 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1849,7 +1849,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "cc 1.0.35 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.54 (registry+https://github.com/rust-lang/crates.io-index)", - "openssl-src 111.1.0+1.1.1a (registry+https://github.com/rust-lang/crates.io-index)", + "openssl-src 111.3.0+1.1.1c (registry+https://github.com/rust-lang/crates.io-index)", "pkg-config 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", "vcpkg 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -4398,7 +4398,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" "checksum opener 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "998c59e83d9474c01127a96e023b7a04bb061dd286bf8bb939d31dc8d31a7448" "checksum openssl 0.10.16 (registry+https://github.com/rust-lang/crates.io-index)" = "ec7bd7ca4cce6dbdc77e7c1230682740d307d1218a87fb0349a571272be749f9" "checksum openssl-probe 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "77af24da69f9d9341038eba93a073b1fdaaa1b788221b00a69bce9e762cb32de" -"checksum openssl-src 111.1.0+1.1.1a (registry+https://github.com/rust-lang/crates.io-index)" = "26bb632127731bf4ac49bf86a5dde12d2ca0918c2234fc39d79d4da2ccbc6da7" +"checksum openssl-src 111.3.0+1.1.1c (registry+https://github.com/rust-lang/crates.io-index)" = "53ed5f31d294bdf5f7a4ba0a206c2754b0f60e9a63b7e3076babc5317873c797" "checksum openssl-sys 0.9.43 (registry+https://github.com/rust-lang/crates.io-index)" = "33c86834957dd5b915623e94f2f4ab2c70dd8f6b70679824155d5ae21dbd495d" "checksum ordermap 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)" = "a86ed3f5f244b372d6b1a00b72ef7f8876d0bc6a78a4c9985c53614041512063" "checksum ordslice 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "dd20eec3dbe4376829cb7d80ae6ac45e0a766831dca50202ff2d40db46a8a024" From 05f4a574f6409c932475aea0f95b6d8adfe4222b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 23:49:37 +0200 Subject: [PATCH 02/28] forward read_c_str method from Memory to Alloc --- src/librustc_mir/interpret/memory.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index c3eec677a4850..f76107078807e 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -677,6 +677,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// Reading and writing. impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { + /// Reads the given number of bytes from memory. Returns them as a slice. + /// /// Performs appropriate bounds checks. pub fn read_bytes( &self, @@ -690,6 +692,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { self.get(ptr.alloc_id)?.get_bytes(self, ptr, size) } + /// Reads a 0-terminated sequence of bytes from memory. Returns them as a slice. + /// + /// Performs appropriate bounds checks. + pub fn read_c_str(&self, ptr: Scalar) -> InterpResult<'tcx, &[u8]> { + let ptr = self.force_ptr(ptr)?; // We need to read at least 1 byte, so we *need* a ptr. + self.get(ptr.alloc_id)?.read_c_str(self, ptr) + } + /// Performs appropriate bounds checks. pub fn copy( &mut self, From 2bad604587c5ca9f9fca25a803d00daa60a8d796 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 09:17:38 +0200 Subject: [PATCH 03/28] request at least ptr-size alignment from posix_memalign --- src/libstd/sys/unix/alloc.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libstd/sys/unix/alloc.rs b/src/libstd/sys/unix/alloc.rs index 8e8f5017da75a..c5b6a360dd344 100644 --- a/src/libstd/sys/unix/alloc.rs +++ b/src/libstd/sys/unix/alloc.rs @@ -1,11 +1,15 @@ use crate::ptr; use crate::sys_common::alloc::{MIN_ALIGN, realloc_fallback}; use crate::alloc::{GlobalAlloc, Layout, System}; +use crate::mem; #[stable(feature = "alloc_system_type", since = "1.28.0")] unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc(&self, layout: Layout) -> *mut u8 { + // jemalloc provides alignment less than MIN_ALIGN for small allocations. + // So only rely on MIN_ALIGN if size >= align. + // Also see . if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() { libc::malloc(layout.size()) as *mut u8 } else { @@ -21,6 +25,9 @@ unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { + // jemalloc provides alignment less than MIN_ALIGN for small allocations. + // So only rely on MIN_ALIGN if size >= align. + // Also see . if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() { libc::calloc(layout.size(), 1) as *mut u8 } else { @@ -80,7 +87,10 @@ unsafe fn aligned_malloc(layout: &Layout) -> *mut u8 { #[inline] unsafe fn aligned_malloc(layout: &Layout) -> *mut u8 { let mut out = ptr::null_mut(); - let ret = libc::posix_memalign(&mut out, layout.align(), layout.size()); + // posix_memalign requires that the alignment be a multiple of `sizeof(void*)`. + // Since these are all powers of 2, we can just use max. + let align = layout.align().max(mem::size_of::()); + let ret = libc::posix_memalign(&mut out, align, layout.size()); if ret != 0 { ptr::null_mut() } else { From 576369bfce92f98f78bdac31067d8cb8fee7b0a2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 12:51:00 +0200 Subject: [PATCH 04/28] improve and deduplicate comments --- src/libstd/sys/unix/alloc.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/libstd/sys/unix/alloc.rs b/src/libstd/sys/unix/alloc.rs index c5b6a360dd344..4ac66230e6ed7 100644 --- a/src/libstd/sys/unix/alloc.rs +++ b/src/libstd/sys/unix/alloc.rs @@ -9,7 +9,8 @@ unsafe impl GlobalAlloc for System { unsafe fn alloc(&self, layout: Layout) -> *mut u8 { // jemalloc provides alignment less than MIN_ALIGN for small allocations. // So only rely on MIN_ALIGN if size >= align. - // Also see . + // Also see and + // . if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() { libc::malloc(layout.size()) as *mut u8 } else { @@ -25,9 +26,7 @@ unsafe impl GlobalAlloc for System { #[inline] unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 { - // jemalloc provides alignment less than MIN_ALIGN for small allocations. - // So only rely on MIN_ALIGN if size >= align. - // Also see . + // See the comment above in `alloc` for why this check looks the way it does. if layout.align() <= MIN_ALIGN && layout.align() <= layout.size() { libc::calloc(layout.size(), 1) as *mut u8 } else { From 45e7ba96cb9e5cc018f213f15a2c71f4e1350424 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 12:56:51 +0200 Subject: [PATCH 05/28] test more possible overaligned requests --- src/liballoc/tests/heap.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/liballoc/tests/heap.rs b/src/liballoc/tests/heap.rs index c225ebfa96b91..904b3e7e1b07c 100644 --- a/src/liballoc/tests/heap.rs +++ b/src/liballoc/tests/heap.rs @@ -1,6 +1,6 @@ use std::alloc::{Global, Alloc, Layout, System}; -/// Issue #45955. +/// Issue #45955 and #62251. #[test] fn alloc_system_overaligned_request() { check_overalign_requests(System) @@ -12,21 +12,23 @@ fn std_heap_overaligned_request() { } fn check_overalign_requests(mut allocator: T) { - let size = 8; - let align = 16; // greater than size - let iterations = 100; - unsafe { - let pointers: Vec<_> = (0..iterations).map(|_| { - allocator.alloc(Layout::from_size_align(size, align).unwrap()).unwrap() - }).collect(); - for &ptr in &pointers { - assert_eq!((ptr.as_ptr() as usize) % align, 0, - "Got a pointer less aligned than requested") - } + for &align in &[4, 8, 16, 32] { // less than and bigger than `MIN_ALIGN` + for &size in &[align/2, align-1] { // size less than alignment + let iterations = 128; + unsafe { + let pointers: Vec<_> = (0..iterations).map(|_| { + allocator.alloc(Layout::from_size_align(size, align).unwrap()).unwrap() + }).collect(); + for &ptr in &pointers { + assert_eq!((ptr.as_ptr() as usize) % align, 0, + "Got a pointer less aligned than requested") + } - // Clean up - for &ptr in &pointers { - allocator.dealloc(ptr, Layout::from_size_align(size, align).unwrap()) + // Clean up + for &ptr in &pointers { + allocator.dealloc(ptr, Layout::from_size_align(size, align).unwrap()) + } + } } } } From 2e47fc3bcd46e548d7168e2ba631b3dfa8464d01 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 3 Jul 2019 22:45:27 +0200 Subject: [PATCH 06/28] fix unused-import error on android --- src/libstd/sys/unix/alloc.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libstd/sys/unix/alloc.rs b/src/libstd/sys/unix/alloc.rs index 4ac66230e6ed7..2c2dd3b77eae9 100644 --- a/src/libstd/sys/unix/alloc.rs +++ b/src/libstd/sys/unix/alloc.rs @@ -1,7 +1,6 @@ use crate::ptr; use crate::sys_common::alloc::{MIN_ALIGN, realloc_fallback}; use crate::alloc::{GlobalAlloc, Layout, System}; -use crate::mem; #[stable(feature = "alloc_system_type", since = "1.28.0")] unsafe impl GlobalAlloc for System { @@ -88,7 +87,7 @@ unsafe fn aligned_malloc(layout: &Layout) -> *mut u8 { let mut out = ptr::null_mut(); // posix_memalign requires that the alignment be a multiple of `sizeof(void*)`. // Since these are all powers of 2, we can just use max. - let align = layout.align().max(mem::size_of::()); + let align = layout.align().max(crate::mem::size_of::()); let ret = libc::posix_memalign(&mut out, align, layout.size()); if ret != 0 { ptr::null_mut() From 830ff4a592cf6a5adc0e5482d4294779d7a91177 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 2 Jul 2019 13:44:38 +0300 Subject: [PATCH 07/28] remove StringReader::peek The reader itself doesn't need ability to peek tokens, so it's better if clients implement this functionality. This hopefully becomes especially easy once we use iterator interface for lexer, but this is not too easy at the moment, because of buffered errors. --- src/librustdoc/html/highlight.rs | 29 ++++++++++++++++++++--------- src/libsyntax/parse/lexer/mod.rs | 4 ---- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 99ca8c43cfbe2..852c1e031de4a 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -79,6 +79,7 @@ pub fn render_with_highlighting( /// each span of text in sequence. struct Classifier<'a> { lexer: lexer::StringReader<'a>, + peek_token: Option, source_map: &'a SourceMap, // State of the classifier. @@ -178,6 +179,7 @@ impl<'a> Classifier<'a> { fn new(lexer: lexer::StringReader<'a>, source_map: &'a SourceMap) -> Classifier<'a> { Classifier { lexer, + peek_token: None, source_map, in_attribute: false, in_macro: false, @@ -187,10 +189,19 @@ impl<'a> Classifier<'a> { /// Gets the next token out of the lexer. fn try_next_token(&mut self) -> Result { - match self.lexer.try_next_token() { - Ok(token) => Ok(token), - Err(_) => Err(HighlightError::LexError), + if let Some(token) = self.peek_token.take() { + return Ok(token); } + self.lexer.try_next_token().map_err(|()| HighlightError::LexError) + } + + fn peek(&mut self) -> Result<&Token, HighlightError> { + if self.peek_token.is_none() { + self.peek_token = Some( + self.lexer.try_next_token().map_err(|()| HighlightError::LexError)? + ); + } + Ok(self.peek_token.as_ref().unwrap()) } /// Exhausts the `lexer` writing the output into `out`. @@ -234,7 +245,7 @@ impl<'a> Classifier<'a> { // reference or dereference operator or a reference or pointer type, instead of the // bit-and or multiplication operator. token::BinOp(token::And) | token::BinOp(token::Star) - if self.lexer.peek() != &token::Whitespace => Class::RefKeyWord, + if self.peek()? != &token::Whitespace => Class::RefKeyWord, // Consider this as part of a macro invocation if there was a // leading identifier. @@ -257,7 +268,7 @@ impl<'a> Classifier<'a> { token::Question => Class::QuestionMark, token::Dollar => { - if self.lexer.peek().is_ident() { + if self.peek()?.is_ident() { self.in_macro_nonterminal = true; Class::MacroNonTerminal } else { @@ -280,9 +291,9 @@ impl<'a> Classifier<'a> { // as an attribute. // Case 1: #![inner_attribute] - if self.lexer.peek() == &token::Not { + if self.peek()? == &token::Not { self.try_next_token()?; // NOTE: consumes `!` token! - if self.lexer.peek() == &token::OpenDelim(token::Bracket) { + if self.peek()? == &token::OpenDelim(token::Bracket) { self.in_attribute = true; out.enter_span(Class::Attribute)?; } @@ -292,7 +303,7 @@ impl<'a> Classifier<'a> { } // Case 2: #[outer_attribute] - if self.lexer.peek() == &token::OpenDelim(token::Bracket) { + if self.peek()? == &token::OpenDelim(token::Bracket) { self.in_attribute = true; out.enter_span(Class::Attribute)?; } @@ -341,7 +352,7 @@ impl<'a> Classifier<'a> { if self.in_macro_nonterminal { self.in_macro_nonterminal = false; Class::MacroNonTerminal - } else if self.lexer.peek() == &token::Not { + } else if self.peek()? == &token::Not { self.in_macro = true; Class::Macro } else { diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index 49f714e4e4654..021b623d509c7 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -142,10 +142,6 @@ impl<'a> StringReader<'a> { buffer } - pub fn peek(&self) -> &Token { - &self.peek_token - } - /// For comments.rs, which hackily pokes into next_pos and ch fn new_raw(sess: &'a ParseSess, source_file: Lrc, From e9dc95c86ecb296e0a2067ca5813043f380b9ea6 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 2 Jul 2019 17:08:11 +0300 Subject: [PATCH 08/28] remove peek_token from StringReader --- src/libsyntax/parse/lexer/comments.rs | 2 +- src/libsyntax/parse/lexer/mod.rs | 24 +++++++----------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/src/libsyntax/parse/lexer/comments.rs b/src/libsyntax/parse/lexer/comments.rs index 97d3fc002e9b0..2ab0bebf92927 100644 --- a/src/libsyntax/parse/lexer/comments.rs +++ b/src/libsyntax/parse/lexer/comments.rs @@ -268,7 +268,7 @@ fn read_block_comment(rdr: &mut StringReader<'_>, while level > 0 { debug!("=== block comment level {}", level); if rdr.is_eof() { - rdr.fatal("unterminated block comment").raise(); + rdr.fatal_span_(rdr.pos, rdr.pos, "unterminated block comment").raise(); } if rdr.ch_is('\n') { trim_whitespace_prefix_and_push_line(&mut lines, curr_line, col); diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index 021b623d509c7..a24c72ecc24f1 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -39,7 +39,6 @@ pub struct StringReader<'a> { /// Stop reading src at this index. crate end_src_index: usize, // cached: - peek_token: Token, peek_span_src_raw: Span, fatal_errs: Vec>, // cache a direct reference to the source text, so that we don't have to @@ -78,9 +77,7 @@ impl<'a> StringReader<'a> { /// Returns the next token. EFFECT: advances the string_reader. pub fn try_next_token(&mut self) -> Result { assert!(self.fatal_errs.is_empty()); - let ret_val = self.peek_token.take(); - self.advance_token()?; - Ok(ret_val) + self.advance_token() } fn try_real_token(&mut self) -> Result { @@ -120,10 +117,6 @@ impl<'a> StringReader<'a> { FatalError.raise(); } - fn fatal(&self, m: &str) -> FatalError { - self.fatal_span(self.peek_token.span, m) - } - crate fn emit_fatal_errors(&mut self) { for err in &mut self.fatal_errs { err.emit(); @@ -169,7 +162,6 @@ impl<'a> StringReader<'a> { ch: Some('\n'), source_file, end_src_index: src.len(), - peek_token: Token::dummy(), peek_span_src_raw: syntax_pos::DUMMY_SP, src, fatal_errs: Vec::new(), @@ -267,11 +259,11 @@ impl<'a> StringReader<'a> { /// Advance peek_token to refer to the next token, and /// possibly update the interner. - fn advance_token(&mut self) -> Result<(), ()> { + fn advance_token(&mut self) -> Result { match self.scan_whitespace_or_comment() { Some(comment) => { self.peek_span_src_raw = comment.span; - self.peek_token = comment; + Ok(comment) } None => { let (kind, start_pos, end_pos) = if self.is_eof() { @@ -281,12 +273,10 @@ impl<'a> StringReader<'a> { (self.next_token_inner()?, start_pos, self.pos) }; let (real, raw) = self.mk_sp_and_raw(start_pos, end_pos); - self.peek_token = Token::new(kind, real); self.peek_span_src_raw = raw; + Ok(Token::new(kind, real)) } } - - Ok(()) } #[inline] @@ -1484,17 +1474,17 @@ mod tests { assert_eq!(tok1.kind, tok2.kind); assert_eq!(tok1.span, tok2.span); assert_eq!(string_reader.next_token(), token::Whitespace); - // the 'main' id is already read: - assert_eq!(string_reader.pos.clone(), BytePos(28)); // read another token: let tok3 = string_reader.next_token(); + assert_eq!(string_reader.pos.clone(), BytePos(28)); let tok4 = Token::new( mk_ident("main"), Span::new(BytePos(24), BytePos(28), NO_EXPANSION), ); assert_eq!(tok3.kind, tok4.kind); assert_eq!(tok3.span, tok4.span); - // the lparen is already read: + + assert_eq!(string_reader.next_token(), token::OpenDelim(token::Paren)); assert_eq!(string_reader.pos.clone(), BytePos(29)) }) } From 256df83f642ff3cfff82b266edc7d9bbe3fd2ecc Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 3 Jul 2019 12:52:22 +0300 Subject: [PATCH 09/28] remove peek_span_src_raw from StringReader --- src/librustc_save_analysis/span_utils.rs | 2 +- src/libsyntax/parse/lexer/mod.rs | 98 +++++++++--------------- src/libsyntax/parse/lexer/tokentrees.rs | 15 ++-- 3 files changed, 46 insertions(+), 69 deletions(-) diff --git a/src/librustc_save_analysis/span_utils.rs b/src/librustc_save_analysis/span_utils.rs index 8905f475647ba..fb9919d777db1 100644 --- a/src/librustc_save_analysis/span_utils.rs +++ b/src/librustc_save_analysis/span_utils.rs @@ -53,7 +53,7 @@ impl<'a> SpanUtils<'a> { pub fn sub_span_of_token(&self, span: Span, tok: TokenKind) -> Option { let mut toks = self.retokenise_span(span); loop { - let next = toks.real_token(); + let next = toks.next_token(); if next == token::Eof { return None; } diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index a24c72ecc24f1..8b43b88fbac9a 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -38,8 +38,6 @@ pub struct StringReader<'a> { crate source_file: Lrc, /// Stop reading src at this index. crate end_src_index: usize, - // cached: - peek_span_src_raw: Span, fatal_errs: Vec>, // cache a direct reference to the source text, so that we don't have to // retrieve it via `self.source_file.src.as_ref().unwrap()` all the time. @@ -59,7 +57,7 @@ impl<'a> StringReader<'a> { (real, raw) } - fn unwrap_or_abort(&mut self, res: Result) -> Token { + fn unwrap_or_abort(&mut self, res: Result) -> T { match res { Ok(tok) => tok, Err(_) => { @@ -69,36 +67,52 @@ impl<'a> StringReader<'a> { } } - fn next_token(&mut self) -> Token where Self: Sized { - let res = self.try_next_token(); - self.unwrap_or_abort(res) - } - /// Returns the next token. EFFECT: advances the string_reader. pub fn try_next_token(&mut self) -> Result { - assert!(self.fatal_errs.is_empty()); - self.advance_token() + let (token, _raw_span) = self.try_next_token_with_raw_span()?; + Ok(token) } - fn try_real_token(&mut self) -> Result { - let mut t = self.try_next_token()?; + pub fn next_token(&mut self) -> Token { + let res = self.try_next_token(); + self.unwrap_or_abort(res) + } + + fn try_real_token(&mut self) -> Result<(Token, Span), ()> { loop { - match t.kind { - token::Whitespace | token::Comment | token::Shebang(_) => { - t = self.try_next_token()?; - } - _ => break, + let t = self.try_next_token_with_raw_span()?; + match t.0.kind { + token::Whitespace | token::Comment | token::Shebang(_) => continue, + _ => return Ok(t), } } - - Ok(t) } - pub fn real_token(&mut self) -> Token { + fn real_token(&mut self) -> (Token, Span) { let res = self.try_real_token(); self.unwrap_or_abort(res) } + fn try_next_token_with_raw_span(&mut self) -> Result<(Token, Span), ()> { + assert!(self.fatal_errs.is_empty()); + match self.scan_whitespace_or_comment() { + Some(comment) => { + let raw_span = comment.span; + Ok((comment, raw_span)) + } + None => { + let (kind, start_pos, end_pos) = if self.is_eof() { + (token::Eof, self.source_file.end_pos, self.source_file.end_pos) + } else { + let start_pos = self.pos; + (self.next_token_inner()?, start_pos, self.pos) + }; + let (real, raw) = self.mk_sp_and_raw(start_pos, end_pos); + Ok((Token::new(kind, real), raw)) + } + } + } + #[inline] fn is_eof(&self) -> bool { self.ch.is_none() @@ -141,7 +155,6 @@ impl<'a> StringReader<'a> { override_span: Option) -> Self { let mut sr = StringReader::new_raw_internal(sess, source_file, override_span); sr.bump(); - sr } @@ -162,7 +175,6 @@ impl<'a> StringReader<'a> { ch: Some('\n'), source_file, end_src_index: src.len(), - peek_span_src_raw: syntax_pos::DUMMY_SP, src, fatal_errs: Vec::new(), override_span, @@ -172,12 +184,8 @@ impl<'a> StringReader<'a> { pub fn new_or_buffered_errs(sess: &'a ParseSess, source_file: Lrc, override_span: Option) -> Result> { - let mut sr = StringReader::new_raw(sess, source_file, override_span); - if sr.advance_token().is_err() { - Err(sr.buffer_fatal_errors()) - } else { - Ok(sr) - } + let sr = StringReader::new_raw(sess, source_file, override_span); + Ok(sr) } pub fn retokenize(sess: &'a ParseSess, mut span: Span) -> Self { @@ -197,11 +205,6 @@ impl<'a> StringReader<'a> { sr.bump(); - if sr.advance_token().is_err() { - sr.emit_fatal_errors(); - FatalError.raise(); - } - sr } @@ -257,28 +260,6 @@ impl<'a> StringReader<'a> { self.sess.span_diagnostic.struct_span_fatal(self.mk_sp(from_pos, to_pos), &m[..]) } - /// Advance peek_token to refer to the next token, and - /// possibly update the interner. - fn advance_token(&mut self) -> Result { - match self.scan_whitespace_or_comment() { - Some(comment) => { - self.peek_span_src_raw = comment.span; - Ok(comment) - } - None => { - let (kind, start_pos, end_pos) = if self.is_eof() { - (token::Eof, self.source_file.end_pos, self.source_file.end_pos) - } else { - let start_pos = self.pos; - (self.next_token_inner()?, start_pos, self.pos) - }; - let (real, raw) = self.mk_sp_and_raw(start_pos, end_pos); - self.peek_span_src_raw = raw; - Ok(Token::new(kind, real)) - } - } - } - #[inline] fn src_index(&self, pos: BytePos) -> usize { (pos - self.source_file.start_pos).to_usize() @@ -1447,12 +1428,7 @@ mod tests { teststr: String) -> StringReader<'a> { let sf = sm.new_source_file(PathBuf::from(teststr.clone()).into(), teststr); - let mut sr = StringReader::new_raw(sess, sf, None); - if sr.advance_token().is_err() { - sr.emit_fatal_errors(); - FatalError.raise(); - } - sr + StringReader::new_raw(sess, sf, None) } #[test] diff --git a/src/libsyntax/parse/lexer/tokentrees.rs b/src/libsyntax/parse/lexer/tokentrees.rs index 99d9d40a45b93..9593a50bdd2a1 100644 --- a/src/libsyntax/parse/lexer/tokentrees.rs +++ b/src/libsyntax/parse/lexer/tokentrees.rs @@ -1,4 +1,4 @@ -use syntax_pos::Span; +use syntax_pos::{Span, DUMMY_SP}; use crate::print::pprust::token_to_string; use crate::parse::lexer::{StringReader, UnmatchedBrace}; @@ -11,6 +11,7 @@ impl<'a> StringReader<'a> { let mut tt_reader = TokenTreesReader { string_reader: self, token: Token::dummy(), + raw_span: DUMMY_SP, open_braces: Vec::new(), unmatched_braces: Vec::new(), matching_delim_spans: Vec::new(), @@ -24,6 +25,7 @@ impl<'a> StringReader<'a> { struct TokenTreesReader<'a> { string_reader: StringReader<'a>, token: Token, + raw_span: Span, /// Stack of open delimiters and their spans. Used for error message. open_braces: Vec<(token::DelimToken, Span)>, unmatched_braces: Vec, @@ -206,18 +208,17 @@ impl<'a> TokenTreesReader<'a> { // Note that testing for joint-ness here is done via the raw // source span as the joint-ness is a property of the raw source // rather than wanting to take `override_span` into account. - // Additionally, we actually check if the *next* pair of tokens - // is joint, but this is equivalent to checking the current pair. - let raw = self.string_reader.peek_span_src_raw; + let raw_span = self.raw_span; self.real_token(); - let is_joint = raw.hi() == self.string_reader.peek_span_src_raw.lo() - && self.token.is_op(); + let is_joint = raw_span.hi() == self.raw_span.lo() && self.token.is_op(); Ok((tt, if is_joint { Joint } else { NonJoint })) } } } fn real_token(&mut self) { - self.token = self.string_reader.real_token(); + let (token, raw_span) = self.string_reader.real_token(); + self.token = token; + self.raw_span = raw_span; } } From 601bad86b227a73970a6912d1efea48553728b3d Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 3 Jul 2019 13:30:12 +0300 Subject: [PATCH 10/28] cleanup lexer constructors --- src/librustdoc/html/highlight.rs | 22 +++++++++---------- .../passes/check_code_block_syntax.rs | 5 +++-- src/libsyntax/parse/lexer/comments.rs | 2 +- src/libsyntax/parse/lexer/mod.rs | 18 +++++---------- src/libsyntax/parse/mod.rs | 2 +- 5 files changed, 21 insertions(+), 28 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 852c1e031de4a..8132074d6e0e7 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -38,17 +38,17 @@ pub fn render_with_highlighting( FileName::Custom(String::from("rustdoc-highlighting")), src.to_owned(), ); - let highlight_result = - lexer::StringReader::new_or_buffered_errs(&sess, fm, None).and_then(|lexer| { - let mut classifier = Classifier::new(lexer, sess.source_map()); - - let mut highlighted_source = vec![]; - if classifier.write_source(&mut highlighted_source).is_err() { - Err(classifier.lexer.buffer_fatal_errors()) - } else { - Ok(String::from_utf8_lossy(&highlighted_source).into_owned()) - } - }); + let highlight_result = { + let lexer = lexer::StringReader::new(&sess, fm, None); + let mut classifier = Classifier::new(lexer, sess.source_map()); + + let mut highlighted_source = vec![]; + if classifier.write_source(&mut highlighted_source).is_err() { + Err(classifier.lexer.buffer_fatal_errors()) + } else { + Ok(String::from_utf8_lossy(&highlighted_source).into_owned()) + } + }; match highlight_result { Ok(highlighted_source) => { diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index f6ab1290da37c..0488153e7cb73 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -32,7 +32,8 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> { dox[code_block.code].to_owned(), ); - let errors = Lexer::new_or_buffered_errs(&sess, source_file, None).and_then(|mut lexer| { + let errors = { + let mut lexer = Lexer::new(&sess, source_file, None); while let Ok(token::Token { kind, .. }) = lexer.try_next_token() { if kind == token::Eof { break; @@ -46,7 +47,7 @@ impl<'a, 'tcx> SyntaxChecker<'a, 'tcx> { } else { Ok(()) } - }); + }; if let Err(errors) = errors { let mut diag = if let Some(sp) = diff --git a/src/libsyntax/parse/lexer/comments.rs b/src/libsyntax/parse/lexer/comments.rs index 2ab0bebf92927..988f1aa38d926 100644 --- a/src/libsyntax/parse/lexer/comments.rs +++ b/src/libsyntax/parse/lexer/comments.rs @@ -346,7 +346,7 @@ pub fn gather_comments(sess: &ParseSess, path: FileName, srdr: &mut dyn Read) -> srdr.read_to_string(&mut src).unwrap(); let cm = SourceMap::new(sess.source_map().path_mapping().clone()); let source_file = cm.new_source_file(path, src); - let mut rdr = lexer::StringReader::new_raw(sess, source_file, None); + let mut rdr = lexer::StringReader::new(sess, source_file, None); let mut comments: Vec = Vec::new(); let mut code_to_the_left = false; // Only code diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index 8b43b88fbac9a..fd593fb0d090a 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -149,16 +149,15 @@ impl<'a> StringReader<'a> { buffer } - /// For comments.rs, which hackily pokes into next_pos and ch - fn new_raw(sess: &'a ParseSess, + pub fn new(sess: &'a ParseSess, source_file: Lrc, override_span: Option) -> Self { - let mut sr = StringReader::new_raw_internal(sess, source_file, override_span); + let mut sr = StringReader::new_internal(sess, source_file, override_span); sr.bump(); sr } - fn new_raw_internal(sess: &'a ParseSess, source_file: Lrc, + fn new_internal(sess: &'a ParseSess, source_file: Lrc, override_span: Option) -> Self { if source_file.src.is_none() { @@ -181,13 +180,6 @@ impl<'a> StringReader<'a> { } } - pub fn new_or_buffered_errs(sess: &'a ParseSess, - source_file: Lrc, - override_span: Option) -> Result> { - let sr = StringReader::new_raw(sess, source_file, override_span); - Ok(sr) - } - pub fn retokenize(sess: &'a ParseSess, mut span: Span) -> Self { let begin = sess.source_map().lookup_byte_offset(span.lo()); let end = sess.source_map().lookup_byte_offset(span.hi()); @@ -197,7 +189,7 @@ impl<'a> StringReader<'a> { span = span.shrink_to_lo(); } - let mut sr = StringReader::new_raw_internal(sess, begin.sf, None); + let mut sr = StringReader::new_internal(sess, begin.sf, None); // Seek the lexer to the right byte range. sr.next_pos = span.lo(); @@ -1428,7 +1420,7 @@ mod tests { teststr: String) -> StringReader<'a> { let sf = sm.new_source_file(PathBuf::from(teststr.clone()).into(), teststr); - StringReader::new_raw(sess, sf, None) + StringReader::new(sess, sf, None) } #[test] diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs index e19eab371f44e..ff2275ca348bc 100644 --- a/src/libsyntax/parse/mod.rs +++ b/src/libsyntax/parse/mod.rs @@ -305,7 +305,7 @@ pub fn maybe_file_to_stream( source_file: Lrc, override_span: Option, ) -> Result<(TokenStream, Vec), Vec> { - let srdr = lexer::StringReader::new_or_buffered_errs(sess, source_file, override_span)?; + let srdr = lexer::StringReader::new(sess, source_file, override_span); let (token_trees, unmatched_braces) = srdr.into_token_trees(); match token_trees { From 30fa99e5b8c89df2c27d10a5d38a7c0d50f155a7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 3 Jul 2019 13:31:52 +0300 Subject: [PATCH 11/28] move constructors to top --- src/libsyntax/parse/lexer/mod.rs | 102 +++++++++++++++---------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index fd593fb0d090a..b764f9678c5b4 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -46,6 +46,57 @@ pub struct StringReader<'a> { } impl<'a> StringReader<'a> { + pub fn new(sess: &'a ParseSess, + source_file: Lrc, + override_span: Option) -> Self { + let mut sr = StringReader::new_internal(sess, source_file, override_span); + sr.bump(); + sr + } + + pub fn retokenize(sess: &'a ParseSess, mut span: Span) -> Self { + let begin = sess.source_map().lookup_byte_offset(span.lo()); + let end = sess.source_map().lookup_byte_offset(span.hi()); + + // Make the range zero-length if the span is invalid. + if span.lo() > span.hi() || begin.sf.start_pos != end.sf.start_pos { + span = span.shrink_to_lo(); + } + + let mut sr = StringReader::new_internal(sess, begin.sf, None); + + // Seek the lexer to the right byte range. + sr.next_pos = span.lo(); + sr.end_src_index = sr.src_index(span.hi()); + + sr.bump(); + + sr + } + + fn new_internal(sess: &'a ParseSess, source_file: Lrc, + override_span: Option) -> Self + { + if source_file.src.is_none() { + sess.span_diagnostic.bug(&format!("Cannot lex source_file without source: {}", + source_file.name)); + } + + let src = (*source_file.src.as_ref().unwrap()).clone(); + + StringReader { + sess, + next_pos: source_file.start_pos, + pos: source_file.start_pos, + ch: Some('\n'), + source_file, + end_src_index: src.len(), + src, + fatal_errs: Vec::new(), + override_span, + } + } + fn mk_sp(&self, lo: BytePos, hi: BytePos) -> Span { self.mk_sp_and_raw(lo, hi).0 } @@ -149,57 +200,6 @@ impl<'a> StringReader<'a> { buffer } - pub fn new(sess: &'a ParseSess, - source_file: Lrc, - override_span: Option) -> Self { - let mut sr = StringReader::new_internal(sess, source_file, override_span); - sr.bump(); - sr - } - - fn new_internal(sess: &'a ParseSess, source_file: Lrc, - override_span: Option) -> Self - { - if source_file.src.is_none() { - sess.span_diagnostic.bug(&format!("Cannot lex source_file without source: {}", - source_file.name)); - } - - let src = (*source_file.src.as_ref().unwrap()).clone(); - - StringReader { - sess, - next_pos: source_file.start_pos, - pos: source_file.start_pos, - ch: Some('\n'), - source_file, - end_src_index: src.len(), - src, - fatal_errs: Vec::new(), - override_span, - } - } - - pub fn retokenize(sess: &'a ParseSess, mut span: Span) -> Self { - let begin = sess.source_map().lookup_byte_offset(span.lo()); - let end = sess.source_map().lookup_byte_offset(span.hi()); - - // Make the range zero-length if the span is invalid. - if span.lo() > span.hi() || begin.sf.start_pos != end.sf.start_pos { - span = span.shrink_to_lo(); - } - - let mut sr = StringReader::new_internal(sess, begin.sf, None); - - // Seek the lexer to the right byte range. - sr.next_pos = span.lo(); - sr.end_src_index = sr.src_index(span.hi()); - - sr.bump(); - - sr - } - #[inline] fn ch_is(&self, c: char) -> bool { self.ch == Some(c) From 1c6eb19d2fd9be130b6265f6bdbf8da3ba49c513 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 3 Jul 2019 14:06:10 +0300 Subject: [PATCH 12/28] slightly comment lexer API --- src/libsyntax/parse/lexer/mod.rs | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index b764f9678c5b4..829083fe4f7d0 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -118,29 +118,36 @@ impl<'a> StringReader<'a> { } } - /// Returns the next token. EFFECT: advances the string_reader. + /// Returns the next token, including trivia like whitespace or comments. + /// + /// `Err(())` means that some errors were encountered, which can be + /// retrieved using `buffer_fatal_errors`. pub fn try_next_token(&mut self) -> Result { let (token, _raw_span) = self.try_next_token_with_raw_span()?; Ok(token) } + /// Returns the next token, including trivia like whitespace or comments. + /// + /// Aborts in case of an error. pub fn next_token(&mut self) -> Token { let res = self.try_next_token(); self.unwrap_or_abort(res) } - fn try_real_token(&mut self) -> Result<(Token, Span), ()> { - loop { - let t = self.try_next_token_with_raw_span()?; - match t.0.kind { - token::Whitespace | token::Comment | token::Shebang(_) => continue, - _ => return Ok(t), + /// Returns the next token, skipping over trivia. + /// Also returns an unoverriden span which can be used to check tokens + fn real_token(&mut self) -> (Token, Span) { + let res = try { + loop { + let t = self.try_next_token_with_raw_span()?; + match t.0.kind { + token::Whitespace | token::Comment | token::Shebang(_) => continue, + _ => break t, + } } - } - } + }; - fn real_token(&mut self) -> (Token, Span) { - let res = self.try_real_token(); self.unwrap_or_abort(res) } From 8bea334a266dcf439ca2f61f448a15770a3766b7 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 3 Jul 2019 15:07:41 +0300 Subject: [PATCH 13/28] don't rely on spans when checking tokens for jointness --- src/libsyntax/parse/lexer/mod.rs | 46 ++++++------------------- src/libsyntax/parse/lexer/tokentrees.rs | 30 +++++++++------- 2 files changed, 29 insertions(+), 47 deletions(-) diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index 829083fe4f7d0..f9b9c85fb5602 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -123,41 +123,9 @@ impl<'a> StringReader<'a> { /// `Err(())` means that some errors were encountered, which can be /// retrieved using `buffer_fatal_errors`. pub fn try_next_token(&mut self) -> Result { - let (token, _raw_span) = self.try_next_token_with_raw_span()?; - Ok(token) - } - - /// Returns the next token, including trivia like whitespace or comments. - /// - /// Aborts in case of an error. - pub fn next_token(&mut self) -> Token { - let res = self.try_next_token(); - self.unwrap_or_abort(res) - } - - /// Returns the next token, skipping over trivia. - /// Also returns an unoverriden span which can be used to check tokens - fn real_token(&mut self) -> (Token, Span) { - let res = try { - loop { - let t = self.try_next_token_with_raw_span()?; - match t.0.kind { - token::Whitespace | token::Comment | token::Shebang(_) => continue, - _ => break t, - } - } - }; - - self.unwrap_or_abort(res) - } - - fn try_next_token_with_raw_span(&mut self) -> Result<(Token, Span), ()> { assert!(self.fatal_errs.is_empty()); match self.scan_whitespace_or_comment() { - Some(comment) => { - let raw_span = comment.span; - Ok((comment, raw_span)) - } + Some(comment) => Ok(comment), None => { let (kind, start_pos, end_pos) = if self.is_eof() { (token::Eof, self.source_file.end_pos, self.source_file.end_pos) @@ -165,12 +133,20 @@ impl<'a> StringReader<'a> { let start_pos = self.pos; (self.next_token_inner()?, start_pos, self.pos) }; - let (real, raw) = self.mk_sp_and_raw(start_pos, end_pos); - Ok((Token::new(kind, real), raw)) + let (real, _raw) = self.mk_sp_and_raw(start_pos, end_pos); + Ok(Token::new(kind, real)) } } } + /// Returns the next token, including trivia like whitespace or comments. + /// + /// Aborts in case of an error. + pub fn next_token(&mut self) -> Token { + let res = self.try_next_token(); + self.unwrap_or_abort(res) + } + #[inline] fn is_eof(&self) -> bool { self.ch.is_none() diff --git a/src/libsyntax/parse/lexer/tokentrees.rs b/src/libsyntax/parse/lexer/tokentrees.rs index 9593a50bdd2a1..830fbec58ded9 100644 --- a/src/libsyntax/parse/lexer/tokentrees.rs +++ b/src/libsyntax/parse/lexer/tokentrees.rs @@ -1,17 +1,17 @@ -use syntax_pos::{Span, DUMMY_SP}; +use syntax_pos::Span; use crate::print::pprust::token_to_string; use crate::parse::lexer::{StringReader, UnmatchedBrace}; use crate::parse::token::{self, Token}; use crate::parse::PResult; -use crate::tokenstream::{DelimSpan, IsJoint::*, TokenStream, TokenTree, TreeAndJoint}; +use crate::tokenstream::{DelimSpan, IsJoint::{self, *}, TokenStream, TokenTree, TreeAndJoint}; impl<'a> StringReader<'a> { crate fn into_token_trees(self) -> (PResult<'a, TokenStream>, Vec) { let mut tt_reader = TokenTreesReader { string_reader: self, token: Token::dummy(), - raw_span: DUMMY_SP, + joint_to_prev: Joint, open_braces: Vec::new(), unmatched_braces: Vec::new(), matching_delim_spans: Vec::new(), @@ -25,7 +25,7 @@ impl<'a> StringReader<'a> { struct TokenTreesReader<'a> { string_reader: StringReader<'a>, token: Token, - raw_span: Span, + joint_to_prev: IsJoint, /// Stack of open delimiters and their spans. Used for error message. open_braces: Vec<(token::DelimToken, Span)>, unmatched_braces: Vec, @@ -205,20 +205,26 @@ impl<'a> TokenTreesReader<'a> { }, _ => { let tt = TokenTree::Token(self.token.take()); - // Note that testing for joint-ness here is done via the raw - // source span as the joint-ness is a property of the raw source - // rather than wanting to take `override_span` into account. - let raw_span = self.raw_span; self.real_token(); - let is_joint = raw_span.hi() == self.raw_span.lo() && self.token.is_op(); + let is_joint = self.joint_to_prev == Joint && self.token.is_op(); Ok((tt, if is_joint { Joint } else { NonJoint })) } } } fn real_token(&mut self) { - let (token, raw_span) = self.string_reader.real_token(); - self.token = token; - self.raw_span = raw_span; + self.joint_to_prev = Joint; + loop { + let token = self.string_reader.next_token(); + match token.kind { + token::Whitespace | token::Comment | token::Shebang(_) => { + self.joint_to_prev = NonJoint; + } + _ => { + self.token = token; + return; + }, + } + } } } From 3035a05a74db3a1a7f95e139c4d683cc7be51159 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 3 Jul 2019 15:09:06 +0300 Subject: [PATCH 14/28] remove unused mk_sp_and_raw --- src/libsyntax/parse/lexer/mod.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index f9b9c85fb5602..a56a4ce097e20 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -98,14 +98,7 @@ impl<'a> StringReader<'a> { } fn mk_sp(&self, lo: BytePos, hi: BytePos) -> Span { - self.mk_sp_and_raw(lo, hi).0 - } - - fn mk_sp_and_raw(&self, lo: BytePos, hi: BytePos) -> (Span, Span) { - let raw = Span::new(lo, hi, NO_EXPANSION); - let real = self.override_span.unwrap_or(raw); - - (real, raw) + self.override_span.unwrap_or_else(|| Span::new(lo, hi, NO_EXPANSION)) } fn unwrap_or_abort(&mut self, res: Result) -> T { @@ -133,8 +126,8 @@ impl<'a> StringReader<'a> { let start_pos = self.pos; (self.next_token_inner()?, start_pos, self.pos) }; - let (real, _raw) = self.mk_sp_and_raw(start_pos, end_pos); - Ok(Token::new(kind, real)) + let span = self.mk_sp(start_pos, end_pos); + Ok(Token::new(kind, span)) } } } From 3e362a4800932186c7351972753ecdf715050983 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 3 Jul 2019 15:31:59 +0300 Subject: [PATCH 15/28] make unwrap_or_abort non-generic again --- src/libsyntax/parse/lexer/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsyntax/parse/lexer/mod.rs b/src/libsyntax/parse/lexer/mod.rs index a56a4ce097e20..2ad562485eb7c 100644 --- a/src/libsyntax/parse/lexer/mod.rs +++ b/src/libsyntax/parse/lexer/mod.rs @@ -101,7 +101,7 @@ impl<'a> StringReader<'a> { self.override_span.unwrap_or_else(|| Span::new(lo, hi, NO_EXPANSION)) } - fn unwrap_or_abort(&mut self, res: Result) -> T { + fn unwrap_or_abort(&mut self, res: Result) -> Token { match res { Ok(tok) => tok, Err(_) => { From 8ad28cd2cb77758cc0a77555db5c03648fc8cd3f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 12:19:58 +0200 Subject: [PATCH 16/28] Machine: make self-like parameters come first --- src/librustc_mir/const_eval.rs | 8 ++++---- src/librustc_mir/interpret/machine.rs | 12 ++++++------ src/librustc_mir/interpret/memory.rs | 12 ++++++------ 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index aa264bbd4bb5c..4a4bc7e04c1b9 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -11,7 +11,7 @@ use rustc::hir::def::DefKind; use rustc::hir::def_id::DefId; use rustc::mir::interpret::{ConstEvalErr, ErrorHandled, ScalarMaybeUndef}; use rustc::mir; -use rustc::ty::{self, TyCtxt, query::TyCtxtAt}; +use rustc::ty::{self, TyCtxt}; use rustc::ty::layout::{self, LayoutOf, VariantIdx}; use rustc::ty::subst::Subst; use rustc::traits::Reveal; @@ -398,18 +398,18 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } fn find_foreign_static( + _tcx: TyCtxt<'tcx>, _def_id: DefId, - _tcx: TyCtxtAt<'tcx>, ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { err!(ReadForeignStatic) } #[inline(always)] fn tag_allocation<'b>( + _memory: &Memory<'mir, 'tcx, Self>, _id: AllocId, alloc: Cow<'b, Allocation>, _kind: Option>, - _memory: &Memory<'mir, 'tcx, Self>, ) -> (Cow<'b, Allocation>, Self::PointerTag) { // We do not use a tag so we can just cheaply forward the allocation (alloc, ()) @@ -417,8 +417,8 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, #[inline(always)] fn tag_static_base_pointer( - _id: AllocId, _memory: &Memory<'mir, 'tcx, Self>, + _id: AllocId, ) -> Self::PointerTag { () } diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index f16c21857b987..42163d0c0889a 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -7,7 +7,7 @@ use std::hash::Hash; use rustc::hir::def_id::DefId; use rustc::mir; -use rustc::ty::{self, query::TyCtxtAt}; +use rustc::ty::{self, TyCtxt}; use super::{ Allocation, AllocId, InterpResult, Scalar, AllocationExtra, @@ -136,8 +136,8 @@ pub trait Machine<'mir, 'tcx>: Sized { /// /// This allocation will then be fed to `tag_allocation` to initialize the "extra" state. fn find_foreign_static( + tcx: TyCtxt<'tcx>, def_id: DefId, - tcx: TyCtxtAt<'tcx>, ) -> InterpResult<'tcx, Cow<'tcx, Allocation>>; /// Called for all binary operations on integer(-like) types when one operand is a pointer @@ -174,10 +174,10 @@ pub trait Machine<'mir, 'tcx>: Sized { /// For static allocations, the tag returned must be the same as the one returned by /// `tag_static_base_pointer`. fn tag_allocation<'b>( + memory: &Memory<'mir, 'tcx, Self>, id: AllocId, alloc: Cow<'b, Allocation>, kind: Option>, - memory: &Memory<'mir, 'tcx, Self>, ) -> (Cow<'b, Allocation>, Self::PointerTag); /// Return the "base" tag for the given static allocation: the one that is used for direct @@ -186,8 +186,8 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Be aware that requesting the `Allocation` for that `id` will lead to cycles /// for cyclic statics! fn tag_static_base_pointer( - id: AllocId, memory: &Memory<'mir, 'tcx, Self>, + id: AllocId, ) -> Self::PointerTag; /// Executes a retagging operation @@ -210,8 +210,8 @@ pub trait Machine<'mir, 'tcx>: Sized { ) -> InterpResult<'tcx>; fn int_to_ptr( - int: u64, _mem: &Memory<'mir, 'tcx, Self>, + int: u64, ) -> InterpResult<'tcx, Pointer> { if int == 0 { err!(InvalidNullPointerUsage) @@ -221,8 +221,8 @@ pub trait Machine<'mir, 'tcx>: Sized { } fn ptr_to_int( - _ptr: Pointer, _mem: &Memory<'mir, 'tcx, Self>, + _ptr: Pointer, ) -> InterpResult<'tcx, u64> { err!(ReadPointerAsBytes) } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 5b177d05bb862..30fe6d0df17fb 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -117,7 +117,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { #[inline] pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { - ptr.with_tag(M::tag_static_base_pointer(ptr.alloc_id, &self)) + ptr.with_tag(M::tag_static_base_pointer(&self, ptr.alloc_id)) } pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { @@ -150,7 +150,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { kind: MemoryKind, ) -> Pointer { let id = self.tcx.alloc_map.lock().reserve(); - let (alloc, tag) = M::tag_allocation(id, Cow::Owned(alloc), Some(kind), &self); + let (alloc, tag) = M::tag_allocation(&self, id, Cow::Owned(alloc), Some(kind)); self.alloc_map.insert(id, (kind, alloc.into_owned())); Pointer::from(id).with_tag(tag) } @@ -384,7 +384,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // We got a "lazy" static that has not been computed yet. if tcx.is_foreign_item(def_id) { trace!("static_alloc: foreign item {:?}", def_id); - M::find_foreign_static(def_id, tcx)? + M::find_foreign_static(tcx.tcx, def_id)? } else { trace!("static_alloc: Need to compute {:?}", def_id); let instance = Instance::mono(tcx.tcx, def_id); @@ -414,10 +414,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // We got tcx memory. Let the machine figure out whether and how to // turn that into memory with the right pointer tag. Ok(M::tag_allocation( + memory, id, // always use the ID we got as input, not the "hidden" one. alloc, M::STATIC_KIND.map(MemoryKind::Machine), - memory ).0) } @@ -890,7 +890,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ) -> InterpResult<'tcx, Pointer> { match scalar { Scalar::Ptr(ptr) => Ok(ptr), - _ => M::int_to_ptr(scalar.to_usize(self)?, self) + _ => M::int_to_ptr(&self, scalar.to_usize(self)?) } } @@ -901,7 +901,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ) -> InterpResult<'tcx, u128> { match scalar.to_bits_or_ptr(size, self) { Ok(bits) => Ok(bits), - Err(ptr) => Ok(M::ptr_to_int(ptr, self)? as u128) + Err(ptr) => Ok(M::ptr_to_int(&self, ptr)? as u128) } } } From 127610b7c4d4c4bd2c2e4dd4b62640a2cdc41f14 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 12:31:17 +0200 Subject: [PATCH 17/28] Go back to just passing MemoryExtra to the machine-level allocation hooks This is needed to avoid doing unnecessary global alloc_map lookups --- src/librustc_mir/const_eval.rs | 6 +++--- src/librustc_mir/interpret/machine.rs | 4 ++-- src/librustc_mir/interpret/memory.rs | 16 ++++++++-------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 4a4bc7e04c1b9..2d07103d5ee30 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -23,7 +23,7 @@ use crate::interpret::{self, PlaceTy, MPlaceTy, OpTy, ImmTy, Immediate, Scalar, RawConst, ConstValue, InterpResult, InterpErrorInfo, InterpError, GlobalId, InterpCx, StackPopCleanup, - Allocation, AllocId, MemoryKind, Memory, + Allocation, AllocId, MemoryKind, snapshot, RefTracking, intern_const_alloc_recursive, }; @@ -406,7 +406,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, #[inline(always)] fn tag_allocation<'b>( - _memory: &Memory<'mir, 'tcx, Self>, + _memory_extra: &(), _id: AllocId, alloc: Cow<'b, Allocation>, _kind: Option>, @@ -417,7 +417,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, #[inline(always)] fn tag_static_base_pointer( - _memory: &Memory<'mir, 'tcx, Self>, + _memory_extra: &(), _id: AllocId, ) -> Self::PointerTag { () diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 42163d0c0889a..f283cacb792d3 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -174,7 +174,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// For static allocations, the tag returned must be the same as the one returned by /// `tag_static_base_pointer`. fn tag_allocation<'b>( - memory: &Memory<'mir, 'tcx, Self>, + memory_extra: &Self::MemoryExtra, id: AllocId, alloc: Cow<'b, Allocation>, kind: Option>, @@ -186,7 +186,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Be aware that requesting the `Allocation` for that `id` will lead to cycles /// for cyclic statics! fn tag_static_base_pointer( - memory: &Memory<'mir, 'tcx, Self>, + memory_extra: &Self::MemoryExtra, id: AllocId, ) -> Self::PointerTag; diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 30fe6d0df17fb..0171325e5e3a4 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -117,7 +117,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { #[inline] pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { - ptr.with_tag(M::tag_static_base_pointer(&self, ptr.alloc_id)) + ptr.with_tag(M::tag_static_base_pointer(&self.extra, ptr.alloc_id)) } pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { @@ -150,7 +150,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { kind: MemoryKind, ) -> Pointer { let id = self.tcx.alloc_map.lock().reserve(); - let (alloc, tag) = M::tag_allocation(&self, id, Cow::Owned(alloc), Some(kind)); + let (alloc, tag) = M::tag_allocation(&self.extra, id, Cow::Owned(alloc), Some(kind)); self.alloc_map.insert(id, (kind, alloc.into_owned())); Pointer::from(id).with_tag(tag) } @@ -368,9 +368,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { /// contains a reference to memory that was created during its evaluation (i.e., not to /// another static), those inner references only exist in "resolved" form. fn get_static_alloc( - id: AllocId, + memory_extra: &M::MemoryExtra, tcx: TyCtxtAt<'tcx>, - memory: &Memory<'mir, 'tcx, M>, + id: AllocId, ) -> InterpResult<'tcx, Cow<'tcx, Allocation>> { let alloc = tcx.alloc_map.lock().get(id); let alloc = match alloc { @@ -414,7 +414,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // We got tcx memory. Let the machine figure out whether and how to // turn that into memory with the right pointer tag. Ok(M::tag_allocation( - memory, + memory_extra, id, // always use the ID we got as input, not the "hidden" one. alloc, M::STATIC_KIND.map(MemoryKind::Machine), @@ -430,7 +430,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { // `get_static_alloc` that we can actually use directly without inserting anything anywhere. // So the error type is `InterpResult<'tcx, &Allocation>`. let a = self.alloc_map.get_or(id, || { - let alloc = Self::get_static_alloc(id, self.tcx, &self).map_err(Err)?; + let alloc = Self::get_static_alloc(&self.extra, self.tcx, id).map_err(Err)?; match alloc { Cow::Borrowed(alloc) => { // We got a ref, cheaply return that as an "error" so that the @@ -459,11 +459,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { id: AllocId, ) -> InterpResult<'tcx, &mut Allocation> { let tcx = self.tcx; - let alloc = Self::get_static_alloc(id, tcx, &self); + let memory_extra = &self.extra; let a = self.alloc_map.get_mut_or(id, || { // Need to make a copy, even if `get_static_alloc` is able // to give us a cheap reference. - let alloc = alloc?; + let alloc = Self::get_static_alloc(memory_extra, tcx, id)?; if alloc.mutability == Mutability::Immutable { return err!(ModifiedConstantMemory); } From 1297a274a37f6e7c48ad8cc8a402c28d83cb98b5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 13:51:18 +0200 Subject: [PATCH 18/28] Add basic support for "other" kinds of values for function pointers, determined by the machine instance. So far, however, calling such a function will fail. --- src/librustc_mir/const_eval.rs | 1 + src/librustc_mir/interpret/cast.rs | 6 +- src/librustc_mir/interpret/machine.rs | 5 + src/librustc_mir/interpret/memory.rs | 121 ++++++++++++++--------- src/librustc_mir/interpret/mod.rs | 2 +- src/librustc_mir/interpret/terminator.rs | 26 ++--- src/librustc_mir/interpret/traits.rs | 10 +- 7 files changed, 107 insertions(+), 64 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index aa264bbd4bb5c..a5c52324c89c2 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -316,6 +316,7 @@ impl interpret::MayLeak for ! { impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, 'tcx> { type MemoryKinds = !; type PointerTag = (); + type ExtraFnVal = !; type FrameExtra = (); type MemoryExtra = (); diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index d61fb87336ccf..3ef525979f8c9 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -11,7 +11,7 @@ use rustc::mir::interpret::{ }; use rustc::mir::CastKind; -use super::{InterpCx, Machine, PlaceTy, OpTy, Immediate}; +use super::{InterpCx, Machine, PlaceTy, OpTy, Immediate, FnVal}; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { fn type_is_fat_ptr(&self, ty: Ty<'tcx>) -> bool { @@ -86,7 +86,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { def_id, substs, ).ok_or_else(|| InterpError::TooGeneric.into()); - let fn_ptr = self.memory.create_fn_alloc(instance?); + let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance?)); self.write_scalar(Scalar::Ptr(fn_ptr.into()), dest)?; } _ => bug!("reify fn pointer on {:?}", src.layout.ty), @@ -115,7 +115,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { substs, ty::ClosureKind::FnOnce, ); - let fn_ptr = self.memory.create_fn_alloc(instance); + let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance)); let val = Immediate::Scalar(Scalar::Ptr(fn_ptr.into()).into()); self.write_immediate(val, dest)?; } diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index f16c21857b987..02ba00508441c 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -67,6 +67,11 @@ pub trait Machine<'mir, 'tcx>: Sized { /// The `default()` is used for pointers to consts, statics, vtables and functions. type PointerTag: ::std::fmt::Debug + Copy + Eq + Hash + 'static; + /// Machines can define extra (non-instance) things that represent values of function pointers. + /// For example, Miri uses this to return a fucntion pointer from `dlsym` + /// that can later be called to execute the right thing. + type ExtraFnVal: ::std::fmt::Debug + Copy; + /// Extra data stored in every call frame. type FrameExtra; diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 5b177d05bb862..fab559bf39db4 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -10,7 +10,7 @@ use std::collections::VecDeque; use std::ptr; use std::borrow::Cow; -use rustc::ty::{self, Instance, ParamEnv, query::TyCtxtAt}; +use rustc::ty::{self, Instance, query::TyCtxtAt}; use rustc::ty::layout::{Align, TargetDataLayout, Size, HasDataLayout}; use rustc_data_structures::fx::{FxHashSet, FxHashMap}; @@ -54,6 +54,26 @@ pub enum AllocCheck { MaybeDead, } +/// The value of a function pointer. +#[derive(Debug, Copy, Clone)] +pub enum FnVal<'tcx, Other> { + Instance(Instance<'tcx>), + Other(Other), +} + +impl<'tcx, Other> FnVal<'tcx, Other> { + pub fn as_instance(self) -> InterpResult<'tcx, Instance<'tcx>> { + match self { + FnVal::Instance(instance) => + Ok(instance), + FnVal::Other(_) => + err!(MachineError( + format!("Expected instance function pointer, got 'other' pointer") + )), + } + } +} + // `Memory` has to depend on the `Machine` because some of its operations // (e.g., `get`) call a `Machine` hook. pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { @@ -69,16 +89,20 @@ pub struct Memory<'mir, 'tcx, M: Machine<'mir, 'tcx>> { // FIXME: this should not be public, but interning currently needs access to it pub(super) alloc_map: M::MemoryMap, + /// Map for "extra" function pointers. + extra_fn_ptr_map: FxHashMap, + /// To be able to compare pointers with NULL, and to check alignment for accesses /// to ZSTs (where pointers may dangle), we keep track of the size even for allocations /// that do not exist any more. + // FIXME: this should not be public, but interning currently needs access to it pub(super) dead_alloc_map: FxHashMap, /// Extra data added by the machine. pub extra: M::MemoryExtra, /// Lets us implement `HasDataLayout`, which is awfully convenient. - pub(super) tcx: TyCtxtAt<'tcx>, + pub tcx: TyCtxtAt<'tcx>, } impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for Memory<'mir, 'tcx, M> { @@ -98,6 +122,7 @@ where fn clone(&self) -> Self { Memory { alloc_map: self.alloc_map.clone(), + extra_fn_ptr_map: self.extra_fn_ptr_map.clone(), dead_alloc_map: self.dead_alloc_map.clone(), extra: (), tcx: self.tcx, @@ -109,6 +134,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { pub fn new(tcx: TyCtxtAt<'tcx>) -> Self { Memory { alloc_map: M::MemoryMap::default(), + extra_fn_ptr_map: FxHashMap::default(), dead_alloc_map: FxHashMap::default(), extra: M::MemoryExtra::default(), tcx, @@ -120,8 +146,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ptr.with_tag(M::tag_static_base_pointer(ptr.alloc_id, &self)) } - pub fn create_fn_alloc(&mut self, instance: Instance<'tcx>) -> Pointer { - let id = self.tcx.alloc_map.lock().create_fn_alloc(instance); + pub fn create_fn_alloc( + &mut self, + fn_val: FnVal<'tcx, M::ExtraFnVal>, + ) -> Pointer + { + let id = match fn_val { + FnVal::Instance(instance) => self.tcx.alloc_map.lock().create_fn_alloc(instance), + FnVal::Other(extra) => { + // TODO: Should we have a cache here? + let id = self.tcx.alloc_map.lock().reserve(); + let old = self.extra_fn_ptr_map.insert(id, extra); + assert!(old.is_none()); + id + } + }; self.tag_static_base_pointer(Pointer::from(id)) } @@ -495,56 +534,50 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { id: AllocId, liveness: AllocCheck, ) -> InterpResult<'static, (Size, Align)> { + // Regular allocations. if let Ok(alloc) = self.get(id) { - return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); + Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)) } - // can't do this in the match argument, we may get cycle errors since the lock would get - // dropped after the match. - let alloc = self.tcx.alloc_map.lock().get(id); - // Could also be a fn ptr or extern static - match alloc { - Some(GlobalAlloc::Function(..)) => { - if let AllocCheck::Dereferencable = liveness { - // The caller requested no function pointers. - err!(DerefFunctionPointer) - } else { - Ok((Size::ZERO, Align::from_bytes(1).unwrap())) - } + // Function pointers. + else if let Ok(_) = self.get_fn_alloc(id) { + if let AllocCheck::Dereferencable = liveness { + // The caller requested no function pointers. + err!(DerefFunctionPointer) + } else { + Ok((Size::ZERO, Align::from_bytes(1).unwrap())) } - // `self.get` would also work, but can cause cycles if a static refers to itself - Some(GlobalAlloc::Static(did)) => { - // The only way `get` couldn't have worked here is if this is an extern static - assert!(self.tcx.is_foreign_item(did)); - // Use size and align of the type - let ty = self.tcx.type_of(did); - let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap(); - Ok((layout.size, layout.align.abi)) + } + // The rest must be dead. + else if let AllocCheck::MaybeDead = liveness { + // Deallocated pointers are allowed, we should be able to find + // them in the map. + Ok(*self.dead_alloc_map.get(&id) + .expect("deallocated pointers should all be recorded in `dead_alloc_map`")) + } else { + err!(DanglingPointerDeref) + } + } + + fn get_fn_alloc(&self, id: AllocId) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> { + trace!("reading fn ptr: {}", id); + if let Some(extra) = self.extra_fn_ptr_map.get(&id) { + Ok(FnVal::Other(*extra)) + } else { + match self.tcx.alloc_map.lock().get(id) { + Some(GlobalAlloc::Function(instance)) => Ok(FnVal::Instance(instance)), + _ => Err(InterpError::ExecuteMemory.into()), } - _ => { - if let Ok(alloc) = self.get(id) { - Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)) - } - else if let AllocCheck::MaybeDead = liveness { - // Deallocated pointers are allowed, we should be able to find - // them in the map. - Ok(*self.dead_alloc_map.get(&id) - .expect("deallocated pointers should all be recorded in `dead_alloc_map`")) - } else { - err!(DanglingPointerDeref) - } - }, } } - pub fn get_fn(&self, ptr: Pointer) -> InterpResult<'tcx, Instance<'tcx>> { + pub fn get_fn( + &self, + ptr: Pointer, + ) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> { if ptr.offset.bytes() != 0 { return err!(InvalidFunctionPointer); } - trace!("reading fn ptr: {}", ptr.alloc_id); - match self.tcx.alloc_map.lock().get(ptr.alloc_id) { - Some(GlobalAlloc::Function(instance)) => Ok(instance), - _ => Err(InterpError::ExecuteMemory.into()), - } + self.get_fn_alloc(ptr.alloc_id) } pub fn mark_immutable(&mut self, id: AllocId) -> InterpResult<'tcx> { diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index 2b20f9df53837..45d24347e4efd 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -24,7 +24,7 @@ pub use self::eval_context::{ pub use self::place::{Place, PlaceTy, MemPlace, MPlaceTy}; -pub use self::memory::{Memory, MemoryKind, AllocCheck}; +pub use self::memory::{Memory, MemoryKind, AllocCheck, FnVal}; pub use self::machine::{Machine, AllocMap, MayLeak}; diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index d6f3de02ec918..4440d677a41d4 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -6,9 +6,9 @@ use rustc::ty::layout::{self, TyLayout, LayoutOf}; use syntax::source_map::Span; use rustc_target::spec::abi::Abi; -use rustc::mir::interpret::{InterpResult, PointerArithmetic, InterpError, Scalar}; use super::{ - InterpCx, Machine, Immediate, OpTy, ImmTy, PlaceTy, MPlaceTy, StackPopCleanup + InterpResult, PointerArithmetic, InterpError, Scalar, + InterpCx, Machine, Immediate, OpTy, ImmTy, PlaceTy, MPlaceTy, StackPopCleanup, FnVal, }; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { @@ -76,16 +76,16 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; let func = self.eval_operand(func, None)?; - let (fn_def, abi) = match func.layout.ty.sty { + let (fn_val, abi) = match func.layout.ty.sty { ty::FnPtr(sig) => { let caller_abi = sig.abi(); let fn_ptr = self.force_ptr(self.read_scalar(func)?.not_undef()?)?; - let instance = self.memory.get_fn(fn_ptr)?; - (instance, caller_abi) + let fn_val = self.memory.get_fn(fn_ptr)?; + (fn_val, caller_abi) } ty::FnDef(def_id, substs) => { let sig = func.layout.ty.fn_sig(*self.tcx); - (self.resolve(def_id, substs)?, sig.abi()) + (FnVal::Instance(self.resolve(def_id, substs)?), sig.abi()) }, _ => { let msg = format!("can't handle callee of type {:?}", func.layout.ty); @@ -94,7 +94,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }; let args = self.eval_operands(args)?; self.eval_fn_call( - fn_def, + fn_val, terminator.source_info.span, abi, &args[..], @@ -228,14 +228,16 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Call this function -- pushing the stack frame and initializing the arguments. fn eval_fn_call( &mut self, - instance: ty::Instance<'tcx>, + fn_val: FnVal<'tcx, M::ExtraFnVal>, span: Span, caller_abi: Abi, args: &[OpTy<'tcx, M::PointerTag>], dest: Option>, ret: Option, ) -> InterpResult<'tcx> { - trace!("eval_fn_call: {:#?}", instance); + trace!("eval_fn_call: {:#?}", fn_val); + + let instance = fn_val.as_instance()?; match instance.def { ty::InstanceDef::Intrinsic(..) => { @@ -432,7 +434,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { )?.expect("cannot be a ZST"); let fn_ptr = self.memory.get(vtable_slot.alloc_id)? .read_ptr_sized(self, vtable_slot)?.to_ptr()?; - let instance = self.memory.get_fn(fn_ptr)?; + let drop_fn = self.memory.get_fn(fn_ptr)?; // `*mut receiver_place.layout.ty` is almost the layout that we // want for args[0]: We have to project to field 0 because we want @@ -447,7 +449,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { }); trace!("Patched self operand to {:#?}", args[0]); // recurse with concrete function - self.eval_fn_call(instance, span, caller_abi, &args, dest, ret) + self.eval_fn_call(drop_fn, span, caller_abi, &args, dest, ret) } } } @@ -482,7 +484,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let dest = MPlaceTy::dangling(self.layout_of(ty)?, self); self.eval_fn_call( - instance, + FnVal::Instance(instance), span, Abi::Rust, &[arg.into()], diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 27d127514229c..35eb01392f4d5 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -2,7 +2,7 @@ use rustc::ty::{self, Ty, Instance}; use rustc::ty::layout::{Size, Align, LayoutOf}; use rustc::mir::interpret::{Scalar, Pointer, InterpResult, PointerArithmetic}; -use super::{InterpCx, InterpError, Machine, MemoryKind}; +use super::{InterpCx, InterpError, Machine, MemoryKind, FnVal}; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Creates a dynamic vtable for the given type and vtable origin. This is used only for @@ -56,7 +56,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let tcx = &*self.tcx; let drop = Instance::resolve_drop_in_place(*tcx, ty); - let drop = self.memory.create_fn_alloc(drop); + let drop = self.memory.create_fn_alloc(FnVal::Instance(drop)); // no need to do any alignment checks on the memory accesses below, because we know the // allocation is correctly aligned as we created it above. Also we're only offsetting by @@ -84,7 +84,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { def_id, substs, ).ok_or_else(|| InterpError::TooGeneric)?; - let fn_ptr = self.memory.create_fn_alloc(instance); + let fn_ptr = self.memory.create_fn_alloc(FnVal::Instance(instance)); let method_ptr = vtable.offset(ptr_size * (3 + i as u64), self)?; self.memory .get_mut(method_ptr.alloc_id)? @@ -113,7 +113,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { .get(vtable.alloc_id)? .read_ptr_sized(self, vtable)? .to_ptr()?; - let drop_instance = self.memory.get_fn(drop_fn)?; + // We *need* an instance here, no other kind of function value, to be able + // to determine the type. + let drop_instance = self.memory.get_fn(drop_fn)?.as_instance()?; trace!("Found drop fn: {:?}", drop_instance); let fn_sig = drop_instance.ty(*self.tcx).fn_sig(*self.tcx); let fn_sig = self.tcx.normalize_erasing_late_bound_regions(self.param_env, &fn_sig); From 5612feb5133d24caaf68f18e9ae8812ecbaa0ba3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 15:06:13 +0200 Subject: [PATCH 19/28] add machine hook to handle calls to 'extra' function values --- src/librustc_mir/const_eval.rs | 10 ++++++++++ src/librustc_mir/interpret/machine.rs | 10 ++++++++++ src/librustc_mir/interpret/terminator.rs | 7 ++++++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index a5c52324c89c2..74df2898db4c5 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -371,6 +371,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, })) } + fn call_extra_fn( + _ecx: &mut InterpretCx<'mir, 'tcx, Self>, + fn_val: !, + _args: &[OpTy<'tcx>], + _dest: Option>, + _ret: Option, + ) -> InterpResult<'tcx> { + match fn_val {} + } + fn call_intrinsic( ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 02ba00508441c..b35d468c49a91 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -124,6 +124,16 @@ pub trait Machine<'mir, 'tcx>: Sized { ret: Option, ) -> InterpResult<'tcx, Option<&'mir mir::Body<'tcx>>>; + /// Execute `fn_val`. it is the hook's responsibility to advance the instruction + /// pointer as appropriate. + fn call_extra_fn( + ecx: &mut InterpretCx<'mir, 'tcx, Self>, + fn_val: Self::ExtraFnVal, + args: &[OpTy<'tcx, Self::PointerTag>], + dest: Option>, + ret: Option, + ) -> InterpResult<'tcx>; + /// Directly process an intrinsic without pushing a stack frame. /// If this returns successfully, the engine will take care of jumping to the next block. fn call_intrinsic( diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 4440d677a41d4..7978e8c36c772 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -237,7 +237,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx> { trace!("eval_fn_call: {:#?}", fn_val); - let instance = fn_val.as_instance()?; + let instance = match fn_val { + FnVal::Instance(instance) => instance, + FnVal::Other(extra) => { + return M::call_extra_fn(self, extra, args, dest, ret); + } + }; match instance.def { ty::InstanceDef::Intrinsic(..) => { From 486720f0800fd53cb0466f20bbba840798fe5da3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 15:29:04 +0200 Subject: [PATCH 20/28] fix determinig the size of foreign static allocations --- src/librustc_mir/interpret/memory.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index fab559bf39db4..bed531504d3e4 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -10,7 +10,7 @@ use std::collections::VecDeque; use std::ptr; use std::borrow::Cow; -use rustc::ty::{self, Instance, query::TyCtxtAt}; +use rustc::ty::{self, Instance, ParamEnv, query::TyCtxtAt}; use rustc::ty::layout::{Align, TargetDataLayout, Size, HasDataLayout}; use rustc_data_structures::fx::{FxHashSet, FxHashMap}; @@ -536,19 +536,33 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ) -> InterpResult<'static, (Size, Align)> { // Regular allocations. if let Ok(alloc) = self.get(id) { - Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)) + return Ok((Size::from_bytes(alloc.bytes.len() as u64), alloc.align)); } // Function pointers. - else if let Ok(_) = self.get_fn_alloc(id) { - if let AllocCheck::Dereferencable = liveness { + if let Ok(_) = self.get_fn_alloc(id) { + return if let AllocCheck::Dereferencable = liveness { // The caller requested no function pointers. err!(DerefFunctionPointer) } else { Ok((Size::ZERO, Align::from_bytes(1).unwrap())) + }; + } + // Foreign statics. + // Can't do this in the match argument, we may get cycle errors since the lock would + // be held throughout the match. + let alloc = self.tcx.alloc_map.lock().get(id); + match alloc { + Some(GlobalAlloc::Static(did)) => { + assert!(self.tcx.is_foreign_item(did)); + // Use size and align of the type + let ty = self.tcx.type_of(did); + let layout = self.tcx.layout_of(ParamEnv::empty().and(ty)).unwrap(); + return Ok((layout.size, layout.align.abi)); } + _ => {} } // The rest must be dead. - else if let AllocCheck::MaybeDead = liveness { + if let AllocCheck::MaybeDead = liveness { // Deallocated pointers are allowed, we should be able to find // them in the map. Ok(*self.dead_alloc_map.get(&id) From b4be08a66692e5f9aa15b14b233d88e626f5c605 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jun 2019 15:35:39 +0200 Subject: [PATCH 21/28] fix for tidy --- src/librustc_mir/interpret/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index bed531504d3e4..0bd732eb31d6d 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -154,7 +154,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { let id = match fn_val { FnVal::Instance(instance) => self.tcx.alloc_map.lock().create_fn_alloc(instance), FnVal::Other(extra) => { - // TODO: Should we have a cache here? + // FIXME(RalfJung): Should we have a cache here? let id = self.tcx.alloc_map.lock().reserve(); let old = self.extra_fn_ptr_map.insert(id, extra); assert!(old.is_none()); From 842bbd2764797cbb7a8364cb17c0bb4b4a5c8432 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 11:16:18 +0200 Subject: [PATCH 22/28] make Memory::get_fn take a Scalar like most of the Memory API surface --- src/librustc_mir/interpret/memory.rs | 3 ++- src/librustc_mir/interpret/terminator.rs | 4 ++-- src/librustc_mir/interpret/traits.rs | 2 +- src/librustc_mir/interpret/validity.rs | 10 +++++----- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 0bd732eb31d6d..1cdfa4c63b475 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -586,8 +586,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { pub fn get_fn( &self, - ptr: Pointer, + ptr: Scalar, ) -> InterpResult<'tcx, FnVal<'tcx, M::ExtraFnVal>> { + let ptr = self.force_ptr(ptr)?; // We definitely need a pointer value. if ptr.offset.bytes() != 0 { return err!(InvalidFunctionPointer); } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 7978e8c36c772..0ab428628de68 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -79,7 +79,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let (fn_val, abi) = match func.layout.ty.sty { ty::FnPtr(sig) => { let caller_abi = sig.abi(); - let fn_ptr = self.force_ptr(self.read_scalar(func)?.not_undef()?)?; + let fn_ptr = self.read_scalar(func)?.not_undef()?; let fn_val = self.memory.get_fn(fn_ptr)?; (fn_val, caller_abi) } @@ -438,7 +438,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.tcx.data_layout.pointer_align.abi, )?.expect("cannot be a ZST"); let fn_ptr = self.memory.get(vtable_slot.alloc_id)? - .read_ptr_sized(self, vtable_slot)?.to_ptr()?; + .read_ptr_sized(self, vtable_slot)?.not_undef()?; let drop_fn = self.memory.get_fn(fn_ptr)?; // `*mut receiver_place.layout.ty` is almost the layout that we diff --git a/src/librustc_mir/interpret/traits.rs b/src/librustc_mir/interpret/traits.rs index 35eb01392f4d5..e7363f6876c28 100644 --- a/src/librustc_mir/interpret/traits.rs +++ b/src/librustc_mir/interpret/traits.rs @@ -112,7 +112,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let drop_fn = self.memory .get(vtable.alloc_id)? .read_ptr_sized(self, vtable)? - .to_ptr()?; + .not_undef()?; // We *need* an instance here, no other kind of function value, to be able // to determine the type. let drop_instance = self.memory.get_fn(drop_fn)?.as_instance()?; diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 374f42261bf62..5049c50004a95 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -457,10 +457,10 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> } ty::FnPtr(_sig) => { let value = value.to_scalar_or_undef(); - let ptr = try_validation!(value.to_ptr(), - value, self.path, "a pointer"); - let _fn = try_validation!(self.ecx.memory.get_fn(ptr), - value, self.path, "a function pointer"); + let _fn = try_validation!( + value.not_undef().and_then(|ptr| self.ecx.memory.get_fn(ptr)), + value, self.path, "a function pointer" + ); // FIXME: Check if the signature matches } // This should be all the primitive types @@ -508,7 +508,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // differentiate between null pointers and dangling pointers if self.ref_tracking_for_consts.is_some() && self.ecx.memory.get(ptr.alloc_id).is_err() && - self.ecx.memory.get_fn(ptr).is_err() { + self.ecx.memory.get_fn(ptr.into()).is_err() { return validation_failure!( "encountered dangling pointer", self.path ); From 956a3ef3bb64dd75bd9e680c199bee013c00fe4a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 10:48:51 +0200 Subject: [PATCH 23/28] more inlining --- src/librustc_mir/interpret/machine.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index f283cacb792d3..24d882b8b4640 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -10,8 +10,8 @@ use rustc::mir; use rustc::ty::{self, TyCtxt}; use super::{ - Allocation, AllocId, InterpResult, Scalar, AllocationExtra, - InterpCx, PlaceTy, OpTy, ImmTy, MemoryKind, Pointer, Memory + Allocation, AllocId, InterpResult, InterpError, Scalar, AllocationExtra, + InterpCx, PlaceTy, OpTy, ImmTy, MemoryKind, Pointer, Memory, }; /// Whether this kind of memory is allowed to leak @@ -209,17 +209,19 @@ pub trait Machine<'mir, 'tcx>: Sized { extra: Self::FrameExtra, ) -> InterpResult<'tcx>; + #[inline(always)] fn int_to_ptr( _mem: &Memory<'mir, 'tcx, Self>, int: u64, ) -> InterpResult<'tcx, Pointer> { - if int == 0 { - err!(InvalidNullPointerUsage) + Err((if int == 0 { + InterpError::InvalidNullPointerUsage } else { - err!(ReadBytesAsPointer) - } + InterpError::ReadBytesAsPointer + }).into()) } + #[inline(always)] fn ptr_to_int( _mem: &Memory<'mir, 'tcx, Self>, _ptr: Pointer, From 52e6f859f176e860ce8fbe479c991c8037e8a05a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 2 Jul 2019 10:49:02 +0200 Subject: [PATCH 24/28] organize methods a bit better --- src/librustc_mir/interpret/eval_context.rs | 74 +++++++++++----------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index e24fa6351e5df..16be6476cfc4f 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -217,6 +217,23 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &mut self.memory } + #[inline(always)] + pub fn force_ptr( + &self, + scalar: Scalar, + ) -> InterpResult<'tcx, Pointer> { + self.memory.force_ptr(scalar) + } + + #[inline(always)] + pub fn force_bits( + &self, + scalar: Scalar, + size: Size + ) -> InterpResult<'tcx, u128> { + self.memory.force_bits(scalar, size) + } + #[inline(always)] pub fn tag_static_base_pointer(&self, ptr: Pointer) -> Pointer { self.memory.tag_static_base_pointer(ptr) @@ -248,6 +265,27 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.frame().body } + #[inline(always)] + pub fn sign_extend(&self, value: u128, ty: TyLayout<'_>) -> u128 { + assert!(ty.abi.is_signed()); + sign_extend(value, ty.size) + } + + #[inline(always)] + pub fn truncate(&self, value: u128, ty: TyLayout<'_>) -> u128 { + truncate(value, ty.size) + } + + #[inline] + pub fn type_is_sized(&self, ty: Ty<'tcx>) -> bool { + ty.is_sized(self.tcx, self.param_env) + } + + #[inline] + pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool { + ty.is_freeze(*self.tcx, self.param_env, DUMMY_SP) + } + pub(super) fn subst_and_normalize_erasing_regions>( &self, substs: T, @@ -283,14 +321,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ).ok_or_else(|| InterpError::TooGeneric.into()) } - pub fn type_is_sized(&self, ty: Ty<'tcx>) -> bool { - ty.is_sized(self.tcx, self.param_env) - } - - pub fn type_is_freeze(&self, ty: Ty<'tcx>) -> bool { - ty.is_freeze(*self.tcx, self.param_env, DUMMY_SP) - } - pub fn load_mir( &self, instance: ty::InstanceDef<'tcx>, @@ -761,32 +791,4 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { trace!("generate stacktrace: {:#?}, {:?}", frames, explicit_span); frames } - - #[inline(always)] - pub fn sign_extend(&self, value: u128, ty: TyLayout<'_>) -> u128 { - assert!(ty.abi.is_signed()); - sign_extend(value, ty.size) - } - - #[inline(always)] - pub fn truncate(&self, value: u128, ty: TyLayout<'_>) -> u128 { - truncate(value, ty.size) - } - - #[inline(always)] - pub fn force_ptr( - &self, - scalar: Scalar, - ) -> InterpResult<'tcx, Pointer> { - self.memory.force_ptr(scalar) - } - - #[inline(always)] - pub fn force_bits( - &self, - scalar: Scalar, - size: Size - ) -> InterpResult<'tcx, u128> { - self.memory.force_bits(scalar, size) - } } From 317c6ac12935046b04c349f6151148600d4c1512 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 11:26:28 +0200 Subject: [PATCH 25/28] use get_size_and_align to test if an allocation is live --- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/interpret/machine.rs | 2 +- src/librustc_mir/interpret/validity.rs | 17 ++++++++--------- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 74df2898db4c5..10dbe1799a5dc 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -372,7 +372,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, } fn call_extra_fn( - _ecx: &mut InterpretCx<'mir, 'tcx, Self>, + _ecx: &mut InterpCx<'mir, 'tcx, Self>, fn_val: !, _args: &[OpTy<'tcx>], _dest: Option>, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index b35d468c49a91..5d88274eb968f 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -127,7 +127,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// Execute `fn_val`. it is the hook's responsibility to advance the instruction /// pointer as appropriate. fn call_extra_fn( - ecx: &mut InterpretCx<'mir, 'tcx, Self>, + ecx: &mut InterpCx<'mir, 'tcx, Self>, fn_val: Self::ExtraFnVal, args: &[OpTy<'tcx, Self::PointerTag>], dest: Option>, diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 5049c50004a95..642227fe29c16 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -6,14 +6,12 @@ use rustc::hir; use rustc::ty::layout::{self, TyLayout, LayoutOf, VariantIdx}; use rustc::ty; use rustc_data_structures::fx::FxHashSet; -use rustc::mir::interpret::{ - GlobalAlloc, InterpResult, InterpError, -}; use std::hash::Hash; use super::{ - OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, + GlobalAlloc, InterpResult, InterpError, + OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, AllocCheck, }; macro_rules! validation_failure { @@ -505,19 +503,20 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> // Only NULL is the niche. So make sure the ptr is NOT NULL. if self.ecx.memory.ptr_may_be_null(ptr) { // These conditions are just here to improve the diagnostics so we can - // differentiate between null pointers and dangling pointers + // differentiate between null pointers and dangling pointers. if self.ref_tracking_for_consts.is_some() && - self.ecx.memory.get(ptr.alloc_id).is_err() && - self.ecx.memory.get_fn(ptr.into()).is_err() { + self.ecx.memory.get_size_and_align(ptr.alloc_id, AllocCheck::Live) + .is_err() + { return validation_failure!( - "encountered dangling pointer", self.path + "a dangling pointer", self.path ); } return validation_failure!("a potentially NULL pointer", self.path); } return Ok(()); } else { - // Conservatively, we reject, because the pointer *could* have this + // Conservatively, we reject, because the pointer *could* have a bad // value. return validation_failure!( "a pointer", From d9d6b3bb28a0a1e94486f066f38f2923dc0cd64d Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 19:11:32 +0200 Subject: [PATCH 26/28] turns out that dangling pointer branch is dead code; remove it and improve the error that actually gets shown a bit --- src/librustc_mir/interpret/validity.rs | 21 +++---- src/test/ui/consts/const-eval/ub-enum.rs | 32 +++++++---- src/test/ui/consts/const-eval/ub-enum.stderr | 56 ++++++++++++------- src/test/ui/consts/const-eval/ub-nonnull.rs | 10 ++++ .../ui/consts/const-eval/ub-nonnull.stderr | 27 ++++++--- 5 files changed, 97 insertions(+), 49 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 642227fe29c16..4a8906338bc6a 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -11,7 +11,7 @@ use std::hash::Hash; use super::{ GlobalAlloc, InterpResult, InterpError, - OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, AllocCheck, + OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, }; macro_rules! validation_failure { @@ -502,17 +502,14 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> if lo == 1 && hi == max_hi { // Only NULL is the niche. So make sure the ptr is NOT NULL. if self.ecx.memory.ptr_may_be_null(ptr) { - // These conditions are just here to improve the diagnostics so we can - // differentiate between null pointers and dangling pointers. - if self.ref_tracking_for_consts.is_some() && - self.ecx.memory.get_size_and_align(ptr.alloc_id, AllocCheck::Live) - .is_err() - { - return validation_failure!( - "a dangling pointer", self.path - ); - } - return validation_failure!("a potentially NULL pointer", self.path); + return validation_failure!( + "a potentially NULL pointer", + self.path, + format!( + "something that cannot possibly fail to be {}", + wrapping_range_format(&layout.valid_range, max_hi) + ) + ); } return Ok(()); } else { diff --git a/src/test/ui/consts/const-eval/ub-enum.rs b/src/test/ui/consts/const-eval/ub-enum.rs index fe8675d326eee..d4b2220695102 100644 --- a/src/test/ui/consts/const-eval/ub-enum.rs +++ b/src/test/ui/consts/const-eval/ub-enum.rs @@ -1,5 +1,10 @@ #![allow(const_err)] // make sure we cannot allow away the errors tested here + +#[repr(transparent)] +#[derive(Copy, Clone)] +struct Wrap(T); + #[repr(usize)] #[derive(Copy, Clone)] enum Enum { @@ -7,11 +12,20 @@ enum Enum { } union TransmuteEnum { in1: &'static u8, + in2: usize, out1: Enum, + out2: Wrap, } -// A pointer is guaranteed non-null -const BAD_ENUM: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; +const GOOD_ENUM: Enum = unsafe { TransmuteEnum { in2: 0 }.out1 }; + +const BAD_ENUM: Enum = unsafe { TransmuteEnum { in2: 1 }.out1 }; +//~^ ERROR is undefined behavior + +const BAD_ENUM_PTR: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; +//~^ ERROR is undefined behavior + +const BAD_ENUM_WRAPPED: Wrap = unsafe { TransmuteEnum { in1: &1 }.out2 }; //~^ ERROR is undefined behavior // (Potentially) invalid enum discriminant @@ -20,9 +34,7 @@ const BAD_ENUM: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; enum Enum2 { A = 2, } -#[repr(transparent)] -#[derive(Copy, Clone)] -struct Wrap(T); + union TransmuteEnum2 { in1: usize, in2: &'static u8, @@ -33,17 +45,17 @@ union TransmuteEnum2 { } const BAD_ENUM2: Enum2 = unsafe { TransmuteEnum2 { in1: 0 }.out1 }; //~^ ERROR is undefined behavior -const BAD_ENUM3: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; +const BAD_ENUM2_PTR: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; //~^ ERROR is undefined behavior -const BAD_ENUM4: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; +const BAD_ENUM2_WRAPPED: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; //~^ ERROR is undefined behavior // Undef enum discriminant. -const BAD_ENUM_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; +const BAD_ENUM2_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; //~^ ERROR is undefined behavior // Pointer value in an enum with a niche that is not just 0. -const BAD_ENUM_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; +const BAD_ENUM2_OPTION_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; //~^ ERROR is undefined behavior // Invalid enum field content (mostly to test printing of paths for enum tuple @@ -53,7 +65,7 @@ union TransmuteChar { b: char, } // Need to create something which does not clash with enum layout optimizations. -const BAD_ENUM_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); +const BAD_OPTION_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); //~^ ERROR is undefined behavior fn main() { diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 2bf4cf9bfd65b..38374e7fbad88 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -1,13 +1,29 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:14:1 + --> $DIR/ub-enum.rs:22:1 | -LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant +LL | const BAD_ENUM: Enum = unsafe { TransmuteEnum { in2: 1 }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 1, but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:34:1 + --> $DIR/ub-enum.rs:25:1 + | +LL | const BAD_ENUM_PTR: Enum = unsafe { TransmuteEnum { in1: &1 }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior + +error[E0080]: it is undefined behavior to use this value + --> $DIR/ub-enum.rs:28:1 + | +LL | const BAD_ENUM_WRAPPED: Wrap = unsafe { TransmuteEnum { in1: &1 }.out2 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be less or equal to 0 + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior + +error[E0080]: it is undefined behavior to use this value + --> $DIR/ub-enum.rs:46:1 | LL | const BAD_ENUM2: Enum2 = unsafe { TransmuteEnum2 { in1: 0 }.out1 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected a valid enum discriminant @@ -15,45 +31,45 @@ LL | const BAD_ENUM2: Enum2 = unsafe { TransmuteEnum2 { in1: 0 }.out1 }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:36:1 + --> $DIR/ub-enum.rs:48:1 | -LL | const BAD_ENUM3: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant +LL | const BAD_ENUM2_PTR: Enum2 = unsafe { TransmuteEnum2 { in2: &0 }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:38:1 + --> $DIR/ub-enum.rs:50:1 | -LL | const BAD_ENUM4: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be in the range 2..=2 +LL | const BAD_ENUM2_WRAPPED: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be in the range 2..=2 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:42:1 + --> $DIR/ub-enum.rs:54:1 | -LL | const BAD_ENUM_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid enum discriminant +LL | const BAD_ENUM2_UNDEF : Enum2 = unsafe { TransmuteEnum2 { in3: () }.out1 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:46:1 + --> $DIR/ub-enum.rs:58:1 | -LL | const BAD_ENUM_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant +LL | const BAD_ENUM2_OPTION_PTR: Option = unsafe { TransmuteEnum2 { in2: &0 }.out3 }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected a valid enum discriminant | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-enum.rs:56:1 + --> $DIR/ub-enum.rs:68:1 | -LL | const BAD_ENUM_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at ..0.1, but expected something less or equal to 1114111 +LL | const BAD_OPTION_CHAR: Option<(char, char)> = Some(('x', unsafe { TransmuteChar { a: !0 }.b })); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 4294967295 at ..0.1, but expected something less or equal to 1114111 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior -error: aborting due to 7 previous errors +error: aborting due to 9 previous errors For more information about this error, try `rustc --explain E0080`. diff --git a/src/test/ui/consts/const-eval/ub-nonnull.rs b/src/test/ui/consts/const-eval/ub-nonnull.rs index 3e0b0948ef3c3..bcbb4358aec03 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.rs +++ b/src/test/ui/consts/const-eval/ub-nonnull.rs @@ -5,9 +5,19 @@ use std::mem; use std::ptr::NonNull; use std::num::{NonZeroU8, NonZeroUsize}; +const NON_NULL: NonNull = unsafe { mem::transmute(1usize) }; +const NON_NULL_PTR: NonNull = unsafe { mem::transmute(&1) }; + const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; //~^ ERROR it is undefined behavior to use this value +const OUT_OF_BOUNDS_PTR: NonNull = { unsafe { +//~^ ERROR it is undefined behavior to use this value + let ptr: &(u8, u8, u8) = mem::transmute(&0u8); // &0 gets promoted so it does not dangle + let out_of_bounds_ptr = &ptr.2; // use address-of-field for pointer arithmetic + mem::transmute(out_of_bounds_ptr) +} }; + const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; //~^ ERROR it is undefined behavior to use this value const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; diff --git a/src/test/ui/consts/const-eval/ub-nonnull.stderr b/src/test/ui/consts/const-eval/ub-nonnull.stderr index 6230712ad6f23..2f9423fed3530 100644 --- a/src/test/ui/consts/const-eval/ub-nonnull.stderr +++ b/src/test/ui/consts/const-eval/ub-nonnull.stderr @@ -1,5 +1,5 @@ error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:8:1 + --> $DIR/ub-nonnull.rs:11:1 | LL | const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 @@ -7,7 +7,20 @@ LL | const NULL_PTR: NonNull = unsafe { mem::transmute(0usize) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:11:1 + --> $DIR/ub-nonnull.rs:14:1 + | +LL | / const OUT_OF_BOUNDS_PTR: NonNull = { unsafe { +LL | | +LL | | let ptr: &(u8, u8, u8) = mem::transmute(&0u8); // &0 gets promoted so it does not dangle +LL | | let out_of_bounds_ptr = &ptr.2; // use address-of-field for pointer arithmetic +LL | | mem::transmute(out_of_bounds_ptr) +LL | | } }; + | |____^ type validation failed: encountered a potentially NULL pointer, but expected something that cannot possibly fail to be greater or equal to 1 + | + = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior + +error[E0080]: it is undefined behavior to use this value + --> $DIR/ub-nonnull.rs:21:1 | LL | const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 @@ -15,7 +28,7 @@ LL | const NULL_U8: NonZeroU8 = unsafe { mem::transmute(0u8) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:13:1 + --> $DIR/ub-nonnull.rs:23:1 | LL | const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 0, but expected something greater or equal to 1 @@ -23,7 +36,7 @@ LL | const NULL_USIZE: NonZeroUsize = unsafe { mem::transmute(0usize) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:20:1 + --> $DIR/ub-nonnull.rs:30:1 | LL | const UNINIT: NonZeroU8 = unsafe { Transmute { uninit: () }.out }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered uninitialized bytes, but expected something greater or equal to 1 @@ -31,7 +44,7 @@ LL | const UNINIT: NonZeroU8 = unsafe { Transmute { uninit: () }.out }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:28:1 + --> $DIR/ub-nonnull.rs:38:1 | LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 42, but expected something in the range 10..=30 @@ -39,13 +52,13 @@ LL | const BAD_RANGE1: RestrictedRange1 = unsafe { RestrictedRange1(42) }; = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior error[E0080]: it is undefined behavior to use this value - --> $DIR/ub-nonnull.rs:34:1 + --> $DIR/ub-nonnull.rs:44:1 | LL | const BAD_RANGE2: RestrictedRange2 = unsafe { RestrictedRange2(20) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered 20, but expected something less or equal to 10, or greater or equal to 30 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior -error: aborting due to 6 previous errors +error: aborting due to 7 previous errors For more information about this error, try `rustc --explain E0080`. From ceb496cf59ea28d98146cdcfc072cede4f7c6585 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 1 Jul 2019 19:19:19 +0200 Subject: [PATCH 27/28] improve validity error range printing for singleton ranges --- src/librustc_mir/interpret/validity.rs | 17 +++++++++-------- src/test/ui/consts/const-eval/ub-enum.stderr | 4 ++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 4a8906338bc6a..34892f5b8ca01 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -151,15 +151,16 @@ fn wrapping_range_format(r: &RangeInclusive, max_hi: u128) -> String { debug_assert!(hi <= max_hi); if lo > hi { format!("less or equal to {}, or greater or equal to {}", hi, lo) + } else if lo == hi { + format!("equal to {}", lo) + } else if lo == 0 { + debug_assert!(hi < max_hi, "should not be printing if the range covers everything"); + format!("less or equal to {}", hi) + } else if hi == max_hi { + debug_assert!(lo > 0, "should not be printing if the range covers everything"); + format!("greater or equal to {}", lo) } else { - if lo == 0 { - debug_assert!(hi < max_hi, "should not be printing if the range covers everything"); - format!("less or equal to {}", hi) - } else if hi == max_hi { - format!("greater or equal to {}", lo) - } else { - format!("in the range {:?}", r) - } + format!("in the range {:?}", r) } } diff --git a/src/test/ui/consts/const-eval/ub-enum.stderr b/src/test/ui/consts/const-eval/ub-enum.stderr index 38374e7fbad88..8ecb1aabdd0f7 100644 --- a/src/test/ui/consts/const-eval/ub-enum.stderr +++ b/src/test/ui/consts/const-eval/ub-enum.stderr @@ -18,7 +18,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:28:1 | LL | const BAD_ENUM_WRAPPED: Wrap = unsafe { TransmuteEnum { in1: &1 }.out2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be less or equal to 0 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be equal to 0 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior @@ -42,7 +42,7 @@ error[E0080]: it is undefined behavior to use this value --> $DIR/ub-enum.rs:50:1 | LL | const BAD_ENUM2_WRAPPED: Wrap = unsafe { TransmuteEnum2 { in2: &0 }.out2 }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be in the range 2..=2 + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected something that cannot possibly fail to be equal to 2 | = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior From 12672e205348a343c9fd51d7be360039a4cfef77 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 4 Jul 2019 10:48:47 -0400 Subject: [PATCH 28/28] Add test for ICE #62375 Fixes #62375 --- src/test/ui/issues/issue-62375.rs | 9 +++++++++ src/test/ui/issues/issue-62375.stderr | 13 +++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 src/test/ui/issues/issue-62375.rs create mode 100644 src/test/ui/issues/issue-62375.stderr diff --git a/src/test/ui/issues/issue-62375.rs b/src/test/ui/issues/issue-62375.rs new file mode 100644 index 0000000000000..a2c8fe551bb03 --- /dev/null +++ b/src/test/ui/issues/issue-62375.rs @@ -0,0 +1,9 @@ +enum A { + Value(()) +} + +fn main() { + let a = A::Value(()); + a == A::Value; + //~^ ERROR binary operation `==` cannot be applied to type `A` +} diff --git a/src/test/ui/issues/issue-62375.stderr b/src/test/ui/issues/issue-62375.stderr new file mode 100644 index 0000000000000..6db45630b9437 --- /dev/null +++ b/src/test/ui/issues/issue-62375.stderr @@ -0,0 +1,13 @@ +error[E0369]: binary operation `==` cannot be applied to type `A` + --> $DIR/issue-62375.rs:7:7 + | +LL | a == A::Value; + | - ^^ -------- fn(()) -> A {A::Value} + | | + | A + | + = note: an implementation of `std::cmp::PartialEq` might be missing for `A` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0369`.