Jan,
It appears that
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61250 is due to a problem
in linemap_macro_map_lookup. It is hard to debug as the pch test
harness doesn't produce a simple logged compile failure to re-execute
but I can look at it from a core file in gdb. Unfortunately your
proposed patch doesn't eliminate PR61250 but I am wondering if it is
another related corner case with the linemaps overflows.
Jack
On Wed, Mar 25, 2015 at 8:56 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
> Hi,
> I read linemap_line_start and I think I noticed few issues with respect
> to overflows and lines being added randomly.
>
> 1) line_delta is computed as to_line SOURCE_LINE (map, set->highest_line)
> I think the last inserted line is not very releavnt. What we care about is
> the base of the last location and to keep thing dense how much we are
> stretching the value range from highest inserted element (inserting into
> middle
> is cheap).
>
> For this reason I added base_line_delta and changed line_delta to be
> to_line - SOURCE_LINE (map, set->highest_location).
>
> Because things go in randomly, highest_line, which really is last inserted
> line, may be somewhere in between.
> 2) (line_delta > 10 && line_delta * ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)
> > 1000)
> ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) is in range 7... 15, so it never
> gets high enough to make this conditional trigger. I changed it to:
>
> || line_delta > 1000
> || (line_delta << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)) > 1000
>
> I.e. we do not want to skip more than 1000 unused entries since highest
> inserted location.
>
> 3) (max_column_hint <= 80 && ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) >= 10)
> seems to intend to reduce the column range when it is no longer needed.
> Again, this is not really good idea when line inserted is not last.
>
> 4) the code deciding whether to do reuse seems worng:
> if (line_delta < 0
> || last_line != ORDINARY_MAP_STARTING_LINE_NUMBER (map)
> || SOURCE_COLUMN (map, highest) >= (1U << column_bits))
>
> line_delta really should be base_line_delta, we do not need to give up
> when map's line is 1, SOURCE_LINE (map, set->highest_line) is 5
> and we are requested to switch to line 3.
>
> Second last_line != ORDINARY_MAP_STARTING_LINE_NUMBER (map) tests whether
> location has only one line that does not work (at least with my changes)
> because we may switch to next line and back.
>
> This conditoinal also seems to be completely missing hanlding of overflows.
>
> The following patch makes all line info and all but one carret to to be right
> on chromium warnings
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> * line-map.c (linemap_line_start): Correct overflow tests.
> Index: line-map.c
> ===================================================================
> --- line-map.c (revision 221568)
> +++ line-map.c (working copy)
> @@ -519,25 +519,38 @@ linemap_line_start (struct line_maps *se
> struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (set);
> source_location highest = set->highest_location;
> source_location r;
> - linenum_type last_line =
> - SOURCE_LINE (map, set->highest_line);
> - int line_delta = to_line - last_line;
> + int base_line_delta = to_line - ORDINARY_MAP_STARTING_LINE_NUMBER (map);
> + int line_delta = to_line - SOURCE_LINE (map, set->highest_location);
> bool add_map = false;
>
> - if (line_delta < 0
> - || (line_delta > 10
> - && line_delta * ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) > 1000)
> - || (max_column_hint >= (1U << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS
> (map)))
> + /* Single MAP entry can be used to encode multiple source lines.
> + Look for situations when this is impossible or undesriable. */
> + if (base_line_delta < 0
> + /* We want to keep maps resonably dense, so do not increase the range
> + of this linemap entry by more than 1000. */
> + || line_delta > 1000
> + || (line_delta << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)) > 1000
> + /* If the max column is out of range and we are still not dropping line
> + info. */
> + || (max_column_hint >= (1U << ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map))
> + && highest < 0x60000000)
> + /* If the prevoius line was long. Ignore this problem is we already
> + re-used the map for lines with greater indexes. */
> || (max_column_hint <= 80
> - && ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) >= 10)
> + && ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map) >= 10 && line_delta > 0)
> + /* If we are just started running out of locations (which makes us to
> drop
> + column info), but current line map still has column info, create
> fresh
> + one. */
> || (highest > 0x60000000
> - && (set->max_column_hint || highest > 0x70000000)))
> + && (ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)
> + || highest > 0x70000000)))
> add_map = true;
> else
> max_column_hint = set->max_column_hint;
> if (add_map)
> {
> int column_bits;
> + bool reuse_map = true;
> if (max_column_hint > 100000 || highest > 0x60000000)
> {
> /* If the column number is ridiculous or we've allocated a huge
> @@ -554,11 +567,38 @@ linemap_line_start (struct line_maps *se
> column_bits++;
> max_column_hint = 1U << column_bits;
> }
> +
> /* Allocate the new line_map. However, if the current map only has a
> single line we can sometimes just increase its column_bits instead.
> */
> - if (line_delta < 0
> - || last_line != ORDINARY_MAP_STARTING_LINE_NUMBER (map)
> - || SOURCE_COLUMN (map, highest) >= (1U << column_bits))
> + if (base_line_delta < 0 || base_line_delta != line_delta
> + /* If we just started running out of locators, but current map still
> + has column info, do not reuse it. */
> + || (highest > 0x60000000
> + && ORDINARY_MAP_NUMBER_OF_COLUMN_BITS (map)
> + && base_line_delta)
> + /* If the line delta is too large. */
> + || line_delta > 1000)
> + reuse_map = false;
> +
> + /* When reusing, we must be sure that column_bits is high enough
> + for already recorded locations. */
> + if (reuse_map)
> + {
> + unsigned int max_column = SOURCE_COLUMN (map, highest) + 1;
> + int combined_column_bits = column_bits;
> +
> + while (max_column >= (1U << combined_column_bits))
> + combined_column_bits++;
> +
> + if ((line_delta << combined_column_bits) > 1000)
> + reuse_map = false;
> + else
> + {
> + column_bits = combined_column_bits;
> + max_column_hint = 1U << combined_column_bits;
> + }
> + }
> + if (!reuse_map)
> map = (struct line_map *) linemap_add (set, LC_RENAME,
> ORDINARY_MAP_IN_SYSTEM_HEADER_P
> (map),
> @@ -609,6 +649,9 @@ linemap_position_for_column (struct line
> {
> struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (set);
> r = linemap_line_start (set, SOURCE_LINE (map, r), to_column + 50);
> + /* We just got to overflow; disable column numbers. */
> + if (to_column >= set->max_column_hint)
> + return r;
> }
> }
> r = r + to_column;
