From 70d8d9bd93f7912e56a27e64abc9e1e895fe143a Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Fri, 4 Sep 2020 11:53:28 +0200 Subject: [PATCH] lto: Ensure we force a change for file/line/column after clear_line_info As discussed yesterday: On the streamer out side, we call clear_line_info in multiple spots which resets the current_* values to something, but on the reader side, we don't have corresponding resets in the same location, just have the stream_* static variables that keep the current values through the entire stream in (so across all the clear_line_info spots in a single LTO object but also across jumping from one LTO object to another one). Now, in an earlier version of my patch it actually broke LTO bootstrap (and a lot of LTO testcases), so for the BLOCK case I've solved it by clear_line_info setting current_block to something that should never appear, which means that in the LTO stream after the clear_line_info spots including the start of the LTO stream we force the block change bit to be set and thus BLOCK to be streamed and therefore stream_block from earlier to be ignored. But for the rest I think that is not the case, so I wonder if we don't sometimes end up with wrong line/column info because of that, or please tell me what prevents that. clear_line_info does: ob->current_file = NULL; ob->current_line = 0; ob->current_col = 0; ob->current_sysp = false; while I think NULL current_file is something that should likely be different from expanded_location (...).file (UNKNOWN_LOCATION/BUILTINS_LOCATION are handled separately and not go through the caching), I think line number 0 can sometimes occur and especially column 0 occurs frequently if we ran out of location_t with columns info. But then we do: bp_pack_value (bp, ob->current_file != xloc.file, 1); bp_pack_value (bp, ob->current_line != xloc.line, 1); bp_pack_value (bp, ob->current_col != xloc.column, 1); and stream the details only if the != is true. If that happens immediately after clear_line_info and e.g. xloc.column is 0, we would stream 0 bit and not stream the actual value, so on read-in it would reuse whatever stream_col etc. were before. Shouldn't we set some ob->current_* new bit that would signal we are immediately past clear_line_info which would force all these != checks to non-zero? Either by oring something into those tests, or perhaps: if (ob->current_reset) { if (xloc.file == NULL) ob->current_file = ""; if (xloc.line == 0) ob->current_line = 1; if (xloc.column == 0) ob->current_column = 1; ob->current_reset = false; } before doing those bp_pack_value calls with a comment, effectively forcing all 6 != comparisons to be true? 2020-09-04 Jakub Jelinek * lto-streamer.h (struct output_block): Add reset_locus member. * lto-streamer-out.c (clear_line_info): Set reset_locus to true. (lto_output_location_1): If reset_locus, clear it and ensure current_{file,line,col} is different from xloc members. --- gcc/lto-streamer-out.c | 12 ++++++++++++ gcc/lto-streamer.h | 1 + 2 files changed, 13 insertions(+) diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c index 914d5eb70b0..b2d58106c49 100644 --- a/gcc/lto-streamer-out.c +++ b/gcc/lto-streamer-out.c @@ -60,6 +60,7 @@ clear_line_info (struct output_block *ob) ob->current_line = 0; ob->current_col = 0; ob->current_sysp = false; + ob->reset_locus = true; /* Initialize to something that will never appear as block, so that the first location with block in a function etc. always streams a change_block bit and the first block. */ @@ -195,6 +196,17 @@ lto_output_location_1 (struct output_block *ob, struct bitpack_d *bp, { expanded_location xloc = expand_location (loc); + if (ob->reset_locus) + { + if (xloc.file == NULL) + ob->current_file = ""; + if (xloc.line == 0) + ob->current_line = 1; + if (xloc.column == 0) + ob->current_col = 1; + ob->reset_locus = false; + } + bp_pack_value (bp, ob->current_file != xloc.file, 1); bp_pack_value (bp, ob->current_line != xloc.line, 1); bp_pack_value (bp, ob->current_col != xloc.column, 1); diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h index 938298459eb..470f6fbe0b7 100644 --- a/gcc/lto-streamer.h +++ b/gcc/lto-streamer.h @@ -717,6 +717,7 @@ struct output_block int current_line; int current_col; bool current_sysp; + bool reset_locus; tree current_block; /* Cache of nodes written in this section. */ -- 2.30.2