Skip to content

Commit

Permalink
incr-comp: hash span end line/column
Browse files Browse the repository at this point in the history
Hash both the length and the end location (line/column) of a span. If we
hash only the length, for example, then two otherwise equal spans with
different end locations will have the same hash. This can cause a
problem during incremental compilation wherein a previous result for a
query that depends on the end location of a span will be incorrectly
reused when the end location of the span it depends on has changed. A
similar analysis applies if some query depends specifically on the
length of the span, but we only hash the end location. So hash both.

Fix #46744, fix #59954, fix #63161, fix #73640, fix #73967, fix #74890, fix #75900
  • Loading branch information
tgnottingham committed Nov 4, 2020
1 parent 0dce3f6 commit b71e627
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 5 deletions.
31 changes: 26 additions & 5 deletions compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1878,16 +1878,37 @@ where
return;
}

let (_, line_hi, col_hi) = match ctx.byte_pos_to_line_and_col(span.hi) {
Some(pos) => pos,
None => {
Hash::hash(&TAG_INVALID_SPAN, hasher);
span.ctxt.hash_stable(ctx, hasher);
return;
}
};

Hash::hash(&TAG_VALID_SPAN, hasher);
// We truncate the stable ID hash and line and column numbers. The chances
// of causing a collision this way should be minimal.
Hash::hash(&(file_lo.name_hash as u64), hasher);

let col = (col_lo.0 as u64) & 0xFF;
let line = ((line_lo as u64) & 0xFF_FF_FF) << 8;
let len = ((span.hi - span.lo).0 as u64) << 32;
let line_col_len = col | line | len;
Hash::hash(&line_col_len, hasher);
// Hash both the length and the end location (line/column) of a span. If we
// hash only the length, for example, then two otherwise equal spans with
// different end locations will have the same hash. This can cause a problem
// during incremental compilation wherein a previous result for a query that
// depends on the end location of a span will be incorrectly reused when the
// end location of the span it depends on has changed (see issue #74890). A
// similar analysis applies if some query depends specifically on the length
// of the span, but we only hash the end location. So hash both.

let col_lo_trunc = (col_lo.0 as u64) & 0xFF;
let line_lo_trunc = ((line_lo as u64) & 0xFF_FF_FF) << 8;
let col_hi_trunc = (col_hi.0 as u64) & 0xFF << 32;
let line_hi_trunc = ((line_hi as u64) & 0xFF_FF_FF) << 40;
let col_line = col_lo_trunc | line_lo_trunc | col_hi_trunc | line_hi_trunc;
let len = (span.hi - span.lo).0;
Hash::hash(&col_line, hasher);
Hash::hash(&len, hasher);
span.ctxt.hash_stable(ctx, hasher);
}
}
Expand Down
16 changes: 16 additions & 0 deletions src/test/run-make/incr-prev-body-beyond-eof/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
-include ../../run-make-fulldeps/tools.mk

# Tests that we don't ICE during incremental compilation after modifying a
# function span such that its previous end line exceeds the number of lines
# in the new file, but its start line/column and length remain the same.

SRC=$(TMPDIR)/src
INCR=$(TMPDIR)/incr

all:
mkdir $(SRC)
mkdir $(INCR)
cp a.rs $(SRC)/main.rs
$(RUSTC) -C incremental=$(INCR) $(SRC)/main.rs
cp b.rs $(SRC)/main.rs
$(RUSTC) -C incremental=$(INCR) $(SRC)/main.rs
14 changes: 14 additions & 0 deletions src/test/run-make/incr-prev-body-beyond-eof/a.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
fn main() {
// foo must be used.
foo();
}

fn foo() {
// foo's span in a.rs and b.rs must be identical
// with respect to start line/column and length.
assert_eq!(1, 1);




}
10 changes: 10 additions & 0 deletions src/test/run-make/incr-prev-body-beyond-eof/b.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
fn main() {
// foo must be used.
foo();
}

fn foo() {
// foo's span in a.rs and b.rs must be identical
// with respect to start line/column and length.
assert_eq!(1, 1);////
}

0 comments on commit b71e627

Please sign in to comment.