From b71e627b26a5ef932c9e1c10813a500a70211810 Mon Sep 17 00:00:00 2001 From: Tyson Nottingham Date: Tue, 1 Sep 2020 21:50:07 -0700 Subject: [PATCH 1/3] incr-comp: hash span end line/column 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 --- compiler/rustc_span/src/lib.rs | 31 ++++++++++++++++--- .../incr-prev-body-beyond-eof/Makefile | 16 ++++++++++ .../run-make/incr-prev-body-beyond-eof/a.rs | 14 +++++++++ .../run-make/incr-prev-body-beyond-eof/b.rs | 10 ++++++ 4 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 src/test/run-make/incr-prev-body-beyond-eof/Makefile create mode 100644 src/test/run-make/incr-prev-body-beyond-eof/a.rs create mode 100644 src/test/run-make/incr-prev-body-beyond-eof/b.rs diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index d036c078049..480c08291b4 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -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); } } diff --git a/src/test/run-make/incr-prev-body-beyond-eof/Makefile b/src/test/run-make/incr-prev-body-beyond-eof/Makefile new file mode 100644 index 00000000000..428f5f17f91 --- /dev/null +++ b/src/test/run-make/incr-prev-body-beyond-eof/Makefile @@ -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 diff --git a/src/test/run-make/incr-prev-body-beyond-eof/a.rs b/src/test/run-make/incr-prev-body-beyond-eof/a.rs new file mode 100644 index 00000000000..a12bb96ba5a --- /dev/null +++ b/src/test/run-make/incr-prev-body-beyond-eof/a.rs @@ -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); + + + + +} diff --git a/src/test/run-make/incr-prev-body-beyond-eof/b.rs b/src/test/run-make/incr-prev-body-beyond-eof/b.rs new file mode 100644 index 00000000000..23e007d0f1c --- /dev/null +++ b/src/test/run-make/incr-prev-body-beyond-eof/b.rs @@ -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);//// +} From 73351109f573d0311f5aafcbeebdabac8c9e5db2 Mon Sep 17 00:00:00 2001 From: Tyson Nottingham Date: Sun, 8 Nov 2020 20:28:03 -0800 Subject: [PATCH 2/3] incr-comp: add clarifying comments to incr-prev-body-beyond-eof test --- src/test/run-make/incr-prev-body-beyond-eof/a.rs | 6 ++++-- src/test/run-make/incr-prev-body-beyond-eof/b.rs | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/test/run-make/incr-prev-body-beyond-eof/a.rs b/src/test/run-make/incr-prev-body-beyond-eof/a.rs index a12bb96ba5a..ca70fb56334 100644 --- a/src/test/run-make/incr-prev-body-beyond-eof/a.rs +++ b/src/test/run-make/incr-prev-body-beyond-eof/a.rs @@ -3,9 +3,11 @@ fn main() { foo(); } +// For this test to operate correctly, foo's body must start on exactly the same +// line and column and have the exact same length in bytes in a.rs and b.rs. In +// a.rs, the body must end on a line number which does not exist in b.rs. +// Basically, avoid modifying this file, including adding or removing whitespace! 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); diff --git a/src/test/run-make/incr-prev-body-beyond-eof/b.rs b/src/test/run-make/incr-prev-body-beyond-eof/b.rs index 23e007d0f1c..a272e44a632 100644 --- a/src/test/run-make/incr-prev-body-beyond-eof/b.rs +++ b/src/test/run-make/incr-prev-body-beyond-eof/b.rs @@ -3,8 +3,10 @@ fn main() { foo(); } +// For this test to operate correctly, foo's body must start on exactly the same +// line and column and have the exact same length in bytes in a.rs and b.rs. In +// a.rs, the body must end on a line number which does not exist in b.rs. +// Basically, avoid modifying this file, including adding or removing whitespace! 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);//// } From dac57e67d6116bcad81f905b8e92be3e9d8e4d23 Mon Sep 17 00:00:00 2001 From: Tyson Nottingham Date: Sun, 8 Nov 2020 20:31:05 -0800 Subject: [PATCH 3/3] incr-comp: add ignore-32bit directive to incr-prev-body-beyond-eof test Co-authored-by: Jonas Schievink --- src/test/run-make/incr-prev-body-beyond-eof/Makefile | 5 ++++- src/test/run-make/issue-36710/Makefile | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/run-make/incr-prev-body-beyond-eof/Makefile b/src/test/run-make/incr-prev-body-beyond-eof/Makefile index 428f5f17f91..49a7ee5f900 100644 --- a/src/test/run-make/incr-prev-body-beyond-eof/Makefile +++ b/src/test/run-make/incr-prev-body-beyond-eof/Makefile @@ -1,4 +1,7 @@ --include ../../run-make-fulldeps/tools.mk +include ../../run-make-fulldeps/tools.mk + +# FIXME https://github.com/rust-lang/rust/issues/78911 +# ignore-32bit wrong/no cross compiler and sometimes we pass wrong gcc args (-m64) # 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 diff --git a/src/test/run-make/issue-36710/Makefile b/src/test/run-make/issue-36710/Makefile index b7bf366c918..b0e8451ff5d 100644 --- a/src/test/run-make/issue-36710/Makefile +++ b/src/test/run-make/issue-36710/Makefile @@ -1,5 +1,6 @@ include ../../run-make-fulldeps/tools.mk +# FIXME https://github.com/rust-lang/rust/issues/78911 # ignore-32bit wrong/no cross compiler and sometimes we pass wrong gcc args (-m64) all: foo