Hi!
When I've tried to backport recent LTO changes of mine, I've ran into
FAIL: g++.dg/ubsan/align-3.C -O2 -flto -fno-use-linker-plugin
-flto-partition=none output pattern test
FAIL: g++.dg/ubsan/align-3.C -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects output pattern test
regressions that don't for some reason show up on the trunk.
I've tracked it down to input_location_and_block being called recursively,
first on some UBSAN_NULL ifn call's location which needs to stream a BLOCK
that hasn't been streamed yet, which in turn needs to stream some locations
for the decls in the BLOCK.
Now, the problem is that both lto_output_location_1 and
input_location_and_block use static variables and/or class members to track
what the "current" location is (so that we don't stream everything all the
time), but on the writer side, lto_output_tree is pretty much the last thing
the function does, so it doesn't matter if the recursion updates those,
but on the reader side, the code relies on the stream_* static vars
and current_* class members not to change across the stream_read_tree call.
I believe we shouldn't be streaming any BLOCKs in the recursive calls,
as we only stream blocks for gimple_location, PHI locations and goto_locus.
Anyway, the following patch fixes it by forcing all those values that might
change across the call into automatic variables, so the code handles
recursion well. The lto-streamer-out.c change is actually not strictly
necessary because BLOCKs shouldn't be streamed out, and if they would be,
it is unclear how it could be handled, because on stream in we can't set
the "current" block until it is streamed in. So, maybe it is more correct
to leave that hunk out.
Tested so far with make check RUNTESTFLAGS=lto.exp, lto bootstrap in
progress, ok for trunk if it succeeds? With or without the
lto-streamer-out.c change?
2020-09-10 Jakub Jelinek <ja...@redhat.com>
* lto-streamer-in.c (lto_location_cache::input_location_and_block):
Don't use static variables nor class members across the
stream_read_tree call, as it might recurse into the
input_location_and_block method.
* lto-streamer-out.c (lto_output_location_1): Write ob->current_block
before calling lto_output_tree, as it might recurse into
lto_output_location_1.
--- gcc/lto-streamer-in.c.jj 2020-09-10 14:29:17.925466837 +0200
+++ gcc/lto-streamer-in.c 2020-09-10 15:09:57.408311917 +0200
@@ -298,7 +298,29 @@ lto_location_cache::input_location_and_b
if (column_change)
stream_col = bp_unpack_var_len_unsigned (bp);
+ /* stream_read_tree below might recurse into input_location_and_block,
+ e.g. when streaming locations of the decls in the BLOCK, although it
+ shouldn't recurse with ib non-NULL. So, avoid using the static
+ variables as well as class members across it. */
+ const char *file = stream_file;
+ int line = stream_line;
+ int col = stream_col;
+ bool sysp = stream_sysp;
tree block = NULL_TREE;
+ bool use_current_loc = false;
+
+ /* This optimization saves location cache operations during gimple
+ streaming. */
+
+ if (current_file == file
+ && current_line == line
+ && current_col == col
+ && current_sysp == sysp)
+ {
+ use_current_loc = true;
+ *loc = current_loc;
+ }
+
if (ib)
{
bool block_change = bp_unpack_value (bp, 1);
@@ -307,25 +329,19 @@ lto_location_cache::input_location_and_b
block = stream_block;
}
- /* This optimization saves location cache operations during gimple
- streaming. */
-
- if (current_file == stream_file
- && current_line == stream_line
- && current_col == stream_col
- && current_sysp == stream_sysp)
+ if (use_current_loc)
{
- if (current_block == block)
- *loc = current_loc;
- else if (block)
- *loc = set_block (current_loc, block);
- else
- *loc = LOCATION_LOCUS (current_loc);
+ if (LOCATION_BLOCK (*loc) != block)
+ {
+ if (block)
+ *loc = set_block (*loc, block);
+ else
+ *loc = LOCATION_LOCUS (*loc);
+ }
return;
}
- struct cached_location entry
- = {stream_file, loc, stream_line, stream_col, stream_sysp, block};
+ struct cached_location entry = { file, loc, line, col, sysp, block };
loc_cache.safe_push (entry);
}
--- gcc/lto-streamer-out.c.jj 2020-09-10 14:29:17.939466635 +0200
+++ gcc/lto-streamer-out.c 2020-09-10 15:12:00.948535171 +0200
@@ -231,8 +231,12 @@ lto_output_location_1 (struct output_blo
bp_pack_value (bp, ob->current_block != block, 1);
streamer_write_bitpack (bp);
if (ob->current_block != block)
- lto_output_tree (ob, block, true, true);
- ob->current_block = block;
+ {
+ ob->current_block = block;
+ /* lto_output_tree might recurse into lto_output_location_1,
+ so update ob->current_block before calling it. */
+ lto_output_tree (ob, block, true, true);
+ }
}
}
Jakub
