[Bug lto/83121] [8 Regression] ICE: in linemap_ordinary_map_lookup, at libcpp/line-map.c:995

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83121

--- Comment #7 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
Author: dmalcolm
Date: Wed Jan 17 16:56:56 2018
New Revision: 256801

URL: https://gcc.gnu.org/viewcvs?rev=256801&root=gcc&view=rev
Log:
lto, testsuite: Fix ICE in -Wodr (PR lto/83121)

PR lto/83121 reports an ICE deep inside the linemap code when -Wodr
reports on a type mismatch.

The root cause is that the warning can access the DECL_SOURCE_LOCATION
of a streamed-in decl before the lto_location_cache has been applied.

lto_location_cache::input_location stores RESERVED_LOCATION_COUNT (==2)
as a poison value until the cache is applied:
250       /* Keep value RESERVED_LOCATION_COUNT in *loc as linemap lookups will
251          ICE on it.  */

The fix is relatively simple: apply the cache before reading the
DECL_SOURCE_LOCATION.

Triggering the ICE was fiddly: it seems to be affected by many things,
including the order of files, and (I think) by filenames.  My theory is
that it's affected by the ordering of the tree nodes in the LTO stream:
for the ICE to occur, the types in question need to be compared before
some other operation flushes the lto_location_cache.  This ordering
is affected by the hash-based ordering in DFS in lto-streamer-out.c, which
might explain why r255066 seemed to trigger the bug; the only relevant
change to LTO there seemed to be:
  * lto-streamer-out.c (hash_tree): Hash TYPE_EMPTY_P and DECL_PADDING_P.
If so, then the bug was presumably already present, but hidden.

The patch also adds regression test coverage for the ICE, which is more
involved - as far as I can tell, we don't have an existing way to verify
diagnostics emitted during link-time optimization.

Hence the patch adds some machinery to lib/lto.exp to support two new
directives: dg-lto-warning and dg-lto-message, corresponding to
dg-warning and dg-message respectively, where the diagnostics are
expected to be emitted at link-time.

The test case includes examples of LTO warnings and notes in both the
primary and secondary source files

Doing so required reusing the logic from DejaGnu for handling diagnostics.
Unfortunately the pertinent code is a 50 line loop within a ~200 line Tcl
function in dg.exp (dg-test), so I had to copy it from DejaGnu, making
various changes as necessary (see lto_handle_diagnostics_for_file in the
patch; for example the LTO version supports multiple source files,
identifying which source file emitted a diagnostic).

For non-LTO diagnostics we currently ignore surplus "note" diagnostics.
This patch updates lto_prune_warns to follow this behavior (since
otherwise we'd need numerous dg-lto-message directives for the motivating
test case).

The patch adds these PASS results to g++.sum:

PASS: g++.dg/lto/pr83121 cp_lto_pr83121_0.o assemble, -O0 -flto
PASS: g++.dg/lto/pr83121 cp_lto_pr83121_1.o assemble, -O0 -flto
PASS: g++.dg/lto/pr83121  (test for LTO warnings, pr83121_0.C line 6)
PASS: g++.dg/lto/pr83121  (test for LTO warnings, pr83121_0.C line 8)
PASS: g++.dg/lto/pr83121  (test for LTO warnings, pr83121_1.C line 2)
PASS: g++.dg/lto/pr83121  (test for LTO warnings, pr83121_1.C line 3)
PASS: g++.dg/lto/pr83121 cp_lto_pr83121_0.o-cp_lto_pr83121_1.o link, -O0 -flto

The output for dg-lto-message above refers to "warnings", rather than
"messages" but that's the same as for the non-LTO case, where dg-message
also refers to "warnings".

gcc/ChangeLog:
        PR lto/83121
        * ipa-devirt.c (add_type_duplicate): When comparing memory layout,
        call the lto_location_cache before reading the
        DECL_SOURCE_LOCATION of the types.

gcc/testsuite/ChangeLog:
        PR lto/83121
        * g++.dg/lto/pr83121_0.C: New test case.
        * g++.dg/lto/pr83121_1.C: New test case.
        * lib/lto.exp (lto_handle_diagnostics_for_file): New procedure,
        adapted from DejaGnu's dg-test.
        (lto_handle_diagnostics): New procedure.
        (lto_prune_warns): Ignore informational notes.
        (lto-link-and-maybe-run): Add "messages_by_file" param.
        Call lto_handle_diagnostics.  Avoid issuing "unresolved" for
        "execute" when "link" fails if "execute" was not specified.
        (lto-can-handle-directive): New procedure.
        (lto-get-options-main): Call lto-can-handle-directive.  Add a
        dg-messages local, using it to set the caller's
        dg-messages-by-file for the given source file.
        (lto-get-options): Likewise.
        (lto-execute): Add dg-messages-by-file local, and pass it to
        lto-link-and-maybe-run.


Added:
    trunk/gcc/testsuite/g++.dg/lto/pr83121_0.C
    trunk/gcc/testsuite/g++.dg/lto/pr83121_1.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-devirt.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/lib/lto.exp