Auto merge of #25013 - pnkfelix:span_to_lines-oflo, r=huonw

Guard against overflow in `codemap::span_to_lines`.

(Revised/expanded version of PR #24976)

Make `span_to_lines` to return a `Result`.

In `diagnostic`, catch `Err` from `span_to_lines` and print `"(unprintable span)"` instead.

----

There a number of recent issues that report the bug here.  See e.g. #24761 and #24954.

This change *might* fix them. However, that is *not* its main goal. The main goals are:

 1. Make it possible for callers to recover from an error here, and

 2. Insert a more conservative check, in that we are also checking that the files match up.

----

As a drive-by, fix #24997 , which was causing my attempts to `make check-stage1` on an `--enable-debug` build to fail.

Author: bors

src/libcore/slice.rs

@@ -631,8 +631,14 @@ fn size_from_ptr<T>(_: *const T) -> usize {
 }
 
 
-// Use macro to be generic over const/mut
-macro_rules! slice_offset {
+// Use macros to be generic over const/mut
+//
+// They require non-negative `$by` because otherwise the expression
+// `(ptr as usize + $by)` would interpret `-1` as `usize::MAX` (and
+// thus trigger a panic when overflow checks are on).
+
+// Use this to do `$ptr + $by`, where `$by` is non-negative.
+macro_rules! slice_add_offset {
     ($ptr:expr, $by:expr) => {{
         let ptr = $ptr;
         if size_from_ptr(ptr) == 0 {
@@ -643,6 +649,18 @@ macro_rules! slice_offset {
     }};
 }
 
+// Use this to do `$ptr - $by`, where `$by` is non-negative.
+macro_rules! slice_sub_offset {
+    ($ptr:expr, $by:expr) => {{
+        let ptr = $ptr;
+        if size_from_ptr(ptr) == 0 {
+            transmute(ptr as usize - $by)
+        } else {
+            ptr.offset(-$by)
+        }
+    }};
+}
+
 macro_rules! slice_ref {
     ($ptr:expr) => {{
         let ptr = $ptr;
@@ -672,7 +690,7 @@ macro_rules! iterator {
                         None
                     } else {
                         let old = self.ptr;
-                        self.ptr = slice_offset!(self.ptr, 1);
+                        self.ptr = slice_add_offset!(self.ptr, 1);
                         Some(slice_ref!(old))
                     }
                 }
@@ -714,7 +732,7 @@ macro_rules! iterator {
                     if self.end == self.ptr {
                         None
                     } else {
-                        self.end = slice_offset!(self.end, -1);
+                        self.end = slice_sub_offset!(self.end, 1);
                         Some(slice_ref!(self.end))
                     }
                 }
@@ -776,7 +794,7 @@ impl<'a, T> Iter<'a, T> {
     fn iter_nth(&mut self, n: usize) -> Option<&'a T> {
         match self.as_slice().get(n) {
             Some(elem_ref) => unsafe {
-                self.ptr = slice_offset!(elem_ref as *const _, 1);
+                self.ptr = slice_add_offset!(elem_ref as *const _, 1);
                 Some(slice_ref!(elem_ref))
             },
             None => {
@@ -849,7 +867,7 @@ impl<'a, T> IterMut<'a, T> {
     fn iter_nth(&mut self, n: usize) -> Option<&'a mut T> {
         match make_mut_slice!(T => &'a mut [T]: self.ptr, self.end).get_mut(n) {
             Some(elem_ref) => unsafe {
-                self.ptr = slice_offset!(elem_ref as *mut _, 1);
+                self.ptr = slice_add_offset!(elem_ref as *mut _, 1);
                 Some(slice_ref!(elem_ref))
             },
             None => {

src/libsyntax/codemap.rs

@@ -663,9 +663,22 @@ impl CodeMap {
         self.lookup_char_pos(sp.lo).file.name.to_string()
     }
 
-    pub fn span_to_lines(&self, sp: Span) -> FileLines {
+    pub fn span_to_lines(&self, sp: Span) -> FileLinesResult {
+        if sp.lo > sp.hi {
+            return Err(SpanLinesError::IllFormedSpan(sp));
+        }
+
         let lo = self.lookup_char_pos(sp.lo);
         let hi = self.lookup_char_pos(sp.hi);
+
+        if lo.file.start_pos != hi.file.start_pos {
+            return Err(SpanLinesError::DistinctSources(DistinctSources {
+                begin: (lo.file.name.clone(), lo.file.start_pos),
+                end: (hi.file.name.clone(), hi.file.start_pos),
+            }));
+        }
+        assert!(hi.line >= lo.line);
+
         let mut lines = Vec::with_capacity(hi.line - lo.line + 1);
 
         // The span starts partway through the first line,
@@ -689,7 +702,7 @@ impl CodeMap {
                               start_col: start_col,
                               end_col: hi.col });
 
-        FileLines {file: lo.file, lines: lines}
+        Ok(FileLines {file: lo.file, lines: lines})
     }
 
     pub fn span_to_snippet(&self, sp: Span) -> Result<String, SpanSnippetError> {
@@ -914,9 +927,17 @@ impl CodeMap {
 }
 
 // _____________________________________________________________________________
-// SpanSnippetError, DistinctSources, MalformedCodemapPositions
+// SpanLinesError, SpanSnippetError, DistinctSources, MalformedCodemapPositions
 //
 
+pub type FileLinesResult = Result<FileLines, SpanLinesError>;
+
+#[derive(Clone, PartialEq, Eq, Debug)]
+pub enum SpanLinesError {
+    IllFormedSpan(Span),
+    DistinctSources(DistinctSources),
+}
+
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum SpanSnippetError {
     IllFormedSpan(Span),
@@ -1082,7 +1103,7 @@ mod tests {
         // Test span_to_lines for a span ending at the end of filemap
         let cm = init_code_map();
         let span = Span {lo: BytePos(12), hi: BytePos(23), expn_id: NO_EXPANSION};
-        let file_lines = cm.span_to_lines(span);
+        let file_lines = cm.span_to_lines(span).unwrap();
 
         assert_eq!(file_lines.file.name, "blork.rs");
         assert_eq!(file_lines.lines.len(), 1);
@@ -1127,7 +1148,7 @@ mod tests {
         assert_eq!(&cm.span_to_snippet(span).unwrap(), "BB\nCCC\nDDDDD");
 
         // check that span_to_lines gives us the complete result with the lines/cols we expected
-        let lines = cm.span_to_lines(span);
+        let lines = cm.span_to_lines(span).unwrap();
         let expected = vec![
             LineInfo { line_index: 1, start_col: CharPos(4), end_col: CharPos(6) },
             LineInfo { line_index: 2, start_col: CharPos(0), end_col: CharPos(3) },

src/libsyntax/diagnostic.rs

@@ -522,7 +522,7 @@ fn highlight_suggestion(err: &mut EmitterWriter,
                         suggestion: &str)
                         -> io::Result<()>
 {
-    let lines = cm.span_to_lines(sp);
+    let lines = cm.span_to_lines(sp).unwrap();
     assert!(!lines.lines.is_empty());
 
     // To build up the result, we want to take the snippet from the first
@@ -567,9 +567,17 @@ fn highlight_lines(err: &mut EmitterWriter,
                    cm: &codemap::CodeMap,
                    sp: Span,
                    lvl: Level,
-                   lines: codemap::FileLines)
+                   lines: codemap::FileLinesResult)
                    -> io::Result<()>
 {
+    let lines = match lines {
+        Ok(lines) => lines,
+        Err(_) => {
+            try!(write!(&mut err.dst, "(internal compiler error: unprintable span)\n"));
+            return Ok(());
+        }
+    };
+
     let fm = &*lines.file;
 
     let line_strings: Option<Vec<&str>> =
@@ -690,8 +698,16 @@ fn end_highlight_lines(w: &mut EmitterWriter,
                           cm: &codemap::CodeMap,
                           sp: Span,
                           lvl: Level,
-                          lines: codemap::FileLines)
+                          lines: codemap::FileLinesResult)
                           -> io::Result<()> {
+    let lines = match lines {
+        Ok(lines) => lines,
+        Err(_) => {
+            try!(write!(&mut w.dst, "(internal compiler error: unprintable span)\n"));
+            return Ok(());
+        }
+    };
+
     let fm = &*lines.file;
 
     let lines = &lines.lines[..];

src/test/run-pass/import-crate-with-invalid-spans.rs

@@ -18,5 +18,6 @@ fn main() {
     // The AST of `exported_generic` stored in crate_with_invalid_spans's
     // metadata should contain an invalid span where span.lo > span.hi.
     // Let's make sure the compiler doesn't crash when encountering this.
-    let _ = crate_with_invalid_spans::exported_generic(32u32, 7u32);
+    let (a, b) = crate_with_invalid_spans::exported_generic(32u32, 7u32);
+    assert_eq!((a, b), (32u32, 8 + 8 - 1));
 }