Skip to content

Commit 1dff2c5

Browse files
committed
Guard against overflow in codemap::span_to_lines.
Make `span_to_lines` to return a `Result`. (This is better than just asserting internally, since it allows caller to decide if they can recover from the problem.) Added type alias for `FileLinesResult` returned by `span_to_lines`. Update embedded unit test to reflect `span_to_lines` signature change. 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. rust-lang#24761 and rust-lang#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.
1 parent 20a256a commit 1dff2c5

File tree

2 files changed

+45
-8
lines changed

2 files changed

+45
-8
lines changed

src/libsyntax/codemap.rs

+26-5
Original file line numberDiff line numberDiff line change
@@ -667,9 +667,22 @@ impl CodeMap {
667667
self.lookup_char_pos(sp.lo).file.name.to_string()
668668
}
669669

670-
pub fn span_to_lines(&self, sp: Span) -> FileLines {
670+
pub fn span_to_lines(&self, sp: Span) -> FileLinesResult {
671+
if sp.lo > sp.hi {
672+
return Err(SpanLinesError::IllFormedSpan(sp));
673+
}
674+
671675
let lo = self.lookup_char_pos(sp.lo);
672676
let hi = self.lookup_char_pos(sp.hi);
677+
678+
if lo.file.start_pos != hi.file.start_pos {
679+
return Err(SpanLinesError::DistinctSources(DistinctSources {
680+
begin: (lo.file.name.clone(), lo.file.start_pos),
681+
end: (hi.file.name.clone(), hi.file.start_pos),
682+
}));
683+
}
684+
assert!(hi.line >= lo.line);
685+
673686
let mut lines = Vec::with_capacity(hi.line - lo.line + 1);
674687

675688
// The span starts partway through the first line,
@@ -693,7 +706,7 @@ impl CodeMap {
693706
start_col: start_col,
694707
end_col: hi.col });
695708

696-
FileLines {file: lo.file, lines: lines}
709+
Ok(FileLines {file: lo.file, lines: lines})
697710
}
698711

699712
pub fn span_to_snippet(&self, sp: Span) -> Result<String, SpanSnippetError> {
@@ -918,9 +931,17 @@ impl CodeMap {
918931
}
919932

920933
// _____________________________________________________________________________
921-
// SpanSnippetError, DistinctSources, MalformedCodemapPositions
934+
// SpanLinesError, SpanSnippetError, DistinctSources, MalformedCodemapPositions
922935
//
923936

937+
pub type FileLinesResult = Result<FileLines, SpanLinesError>;
938+
939+
#[derive(Clone, PartialEq, Eq, Debug)]
940+
pub enum SpanLinesError {
941+
IllFormedSpan(Span),
942+
DistinctSources(DistinctSources),
943+
}
944+
924945
#[derive(Clone, PartialEq, Eq, Debug)]
925946
pub enum SpanSnippetError {
926947
IllFormedSpan(Span),
@@ -1086,7 +1107,7 @@ mod tests {
10861107
// Test span_to_lines for a span ending at the end of filemap
10871108
let cm = init_code_map();
10881109
let span = Span {lo: BytePos(12), hi: BytePos(23), expn_id: NO_EXPANSION};
1089-
let file_lines = cm.span_to_lines(span);
1110+
let file_lines = cm.span_to_lines(span).unwrap();
10901111

10911112
assert_eq!(file_lines.file.name, "blork.rs");
10921113
assert_eq!(file_lines.lines.len(), 1);
@@ -1131,7 +1152,7 @@ mod tests {
11311152
assert_eq!(&cm.span_to_snippet(span).unwrap(), "BB\nCCC\nDDDDD");
11321153

11331154
// check that span_to_lines gives us the complete result with the lines/cols we expected
1134-
let lines = cm.span_to_lines(span);
1155+
let lines = cm.span_to_lines(span).unwrap();
11351156
let expected = vec![
11361157
LineInfo { line_index: 1, start_col: CharPos(4), end_col: CharPos(6) },
11371158
LineInfo { line_index: 2, start_col: CharPos(0), end_col: CharPos(3) },

src/libsyntax/diagnostic.rs

+19-3
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ fn highlight_suggestion(err: &mut EmitterWriter,
522522
suggestion: &str)
523523
-> io::Result<()>
524524
{
525-
let lines = cm.span_to_lines(sp);
525+
let lines = cm.span_to_lines(sp).unwrap();
526526
assert!(!lines.lines.is_empty());
527527

528528
// To build up the result, we want to take the snippet from the first
@@ -567,9 +567,17 @@ fn highlight_lines(err: &mut EmitterWriter,
567567
cm: &codemap::CodeMap,
568568
sp: Span,
569569
lvl: Level,
570-
lines: codemap::FileLines)
570+
lines: codemap::FileLinesResult)
571571
-> io::Result<()>
572572
{
573+
let lines = match lines {
574+
Ok(lines) => lines,
575+
Err(_) => {
576+
try!(write!(&mut err.dst, "(unprintable span)\n"));
577+
return Ok(());
578+
}
579+
};
580+
573581
let fm = &*lines.file;
574582

575583
let line_strings: Option<Vec<&str>> =
@@ -690,8 +698,16 @@ fn end_highlight_lines(w: &mut EmitterWriter,
690698
cm: &codemap::CodeMap,
691699
sp: Span,
692700
lvl: Level,
693-
lines: codemap::FileLines)
701+
lines: codemap::FileLinesResult)
694702
-> io::Result<()> {
703+
let lines = match lines {
704+
Ok(lines) => lines,
705+
Err(_) => {
706+
try!(write!(&mut w.dst, "(unprintable span)\n"));
707+
return Ok(());
708+
}
709+
};
710+
695711
let fm = &*lines.file;
696712

697713
let lines = &lines.lines[..];

0 commit comments

Comments
 (0)