[PATCH] lto, testsuite: Fix ICE in -Wodr (PR lto/83121)

Richard Biener richard.guenther@gmail.com
Mon Jan 15 10:09:00 GMT 2018


On Mon, Jan 8, 2018 at 8:36 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Sat, 2018-01-06 at 08:44 +0100, Richard Biener wrote:
>> On January 5, 2018 11:55:11 PM GMT+01:00, David Malcolm <dmalcolm@red
>> hat.com> wrote:
>> > On Fri, 2018-01-05 at 10:36 +0100, Richard Biener wrote:
>> > > On Thu, Jan 4, 2018 at 10:52 PM, David Malcolm <dmalcolm@redhat.c
>> > > om>
>> > > wrote:
>> > > > 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.
>> > > >
>> > > > (I wonder if we should instead have a INVALID_LOCATION value to
>> > > > handle
>> > > > this case more explicitly?  e.g. 0xffffffff?  or reserve 2 in
>> > > > libcpp for
>> > > > that purpose, and have the non-reserved locations start at
>> > > > 3?  Either
>> > > > would be more invasive, though)
>> > > >
>> > > > 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".
>> > > >
>> > > > Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
>> > > >
>> > > > OK for trunk?
>> > >
>> > > Hmm, but we do this in warn_odr already?  How's that not enough?
>> > >
>> > > At least it seems the place you add this isn't ideal (not at the
>> > > "root cause").
>> > >
>> > > Richard.
>> >
>> > [CCing Honza]
>> >
>> > Yes, warn_odr does apply the cache, but this warning is coming from
>> > warn_types_mismatch.
>> >
>> > Of the various calls to warn_types_mismatch, most are immediately
>> > after
>> > a warn_odr guarded by if (warn && warned) or if (warned), and if
>> > warn_odr writes back true to *warned, it has definitely applied the
>> > cache.
>> >
>> > However, the 7-argument overload of odr_types_equivalent_p can
>> > also call warn_types_mismatch, passing in the location_t values it
>> > received.
>> >
>> > The problem occurs with this call stack, when:
>> >
>> >  add_type_duplicate, passes DECL_SOURCE_LOCATIONs to:
>> >    odr_types_equivalent_p (7-argument overload) - which calls:
>> >      warn_types_mismatch, which calls:
>> >        warning_at with the given location_t
>> >
>> > where the DECL_SOURCE_LOCATION might not yet have been fixed up
>> > yet.
>> > My patch adds the fixup there.
>> >
>> > This behavior seems to have been introduced by r224248 (aka
>> > 379ca7f842e5b13109b689b3c732741cab3d178e), Honza's fix for "PR
>> > lto/65378" (though that seems to have been a typo, as that bug is
>> > unrelated to the changes in that commit); the relevant ML post was:
>> >  https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00523.html
>> >
>> >
>> > Would you prefer this if I reworked the two-liner into something
>> > like
>> > an "lto_location_cache::ensure_cache_applied" function?
>>
>> No, but put it in warn_types_mismatch?
>
> Hmm... putting it in warn_types_mismatch dosn't fix it, for the same
> reason as in the earlier description (or maybe I'm misunderstanding
> you?).
>
> As I understand it, the lto_location_cache effectively contains a vec
> of location_t * and expanded locations, and writes back to the
> location_t * once those "expanded" locations can be turned back into
> valid location_t values.
>
> The issue is that add_type_duplicate takes the DECL_SOURCE_LOCATION of
> a decl before the cache is applied, and that location_t gets passed
> around as a parameter.  At that point, even if the cache is applied and
> the DECL_SOURCE_LOCATION is fixed, the local location_t values being
> passed around aren't going to be touched (they're not in the lto
> location cache - they're on the stack or in registers).
>
> FWIW, the backtrace at the point of failure looks like as follows; the
> invalid loc=2 values can be seen there.  Note how the invalid
> location_t value of 2 is read in frame 17, and is passed around as a
> param in frames 16 down to 10, leading to the crash:
>
> (gdb) bt
> #0  fancy_abort (file=0x2135f00 "../../src/libcpp/line-map.c", line=990,
>     function=0x2136390 <linemap_ordinary_map_lookup(line_maps*, unsigned int)::__FUNCTION__> "linemap_ordinary_map_lookup") at ../../src/gcc/diagnostic.c:1500
> #1  0x0000000001c262c7 in linemap_ordinary_map_lookup (line=2, set=<optimized out>)
>     at ../../src/libcpp/line-map.c:990
> #2  linemap_lookup (set=set@entry=0x7ffff7ffb000, line=<optimized out>, line@entry=2)
>     at ../../src/libcpp/line-map.c:943
> #3  0x0000000001c27294 in linemap_macro_loc_to_def_point (original_map=0x7fffffffd2e0, location=2,
>     set=0x7ffff7ffb000) at ../../src/libcpp/line-map.c:1448
> #4  linemap_resolve_location (set=0x7ffff7ffb000, loc=<optimized out>, lrk=<optimized out>, map=0x7fffffffd2e0)
>     at ../../src/libcpp/line-map.c:1580
> #5  0x0000000001bdc4f2 in diagnostic_report_current_module (context=0x288bce0 <global_diagnostic_context>, where=2)
>     at ../../src/gcc/diagnostic.c:581
> #6  0x0000000000f36ae0 in diagnostic_report_current_function (context=0x288bce0 <global_diagnostic_context>,
>     diagnostic=0x7fffffffd3f0) at ../../src/gcc/tree-diagnostic.c:39
> #7  0x0000000000f36b44 in default_tree_diagnostic_starter (context=0x288bce0 <global_diagnostic_context>,
>     diagnostic=0x7fffffffd3f0) at ../../src/gcc/tree-diagnostic.c:48
> #8  0x0000000001bdd3b4 in diagnostic_report_diagnostic (context=0x288bce0 <global_diagnostic_context>,
>     diagnostic=0x7fffffffd3f0) at ../../src/gcc/diagnostic.c:985
> #9  0x0000000001bdd90e in diagnostic_impl (richloc=0x7fffffffd450, opt=-1,
>     gmsgid=0x1cdcd40 "array types have different bounds", ap=0x7fffffffd4d8, kind=DK_NOTE)
>     at ../../src/gcc/diagnostic.c:1108
> #10 0x0000000001bddbfa in inform (location=2, gmsgid=0x1cdcd40 "array types have different bounds")
>     at ../../src/gcc/diagnostic.c:1160
> #11 0x0000000000bb2dd0 in warn_types_mismatch (t1=<array_type 0x7ffff1aa6dc8>, t2=<array_type 0x7ffff1aa69d8>,
>     loc1=2, loc2=4448) at ../../src/gcc/ipa-devirt.c:1176
> #12 0x0000000000bb5245 in odr_types_equivalent_p (t1=<record_type 0x7ffff1aa6e70 AsyncHooks>,
>     t2=<record_type 0x7ffff1aa6b28 AsyncHooks>, warn=true, warned=0x7fffffffd7e3, visited=0x7fffffffd7b0, loc1=2,
>     loc2=4448) at ../../src/gcc/ipa-devirt.c:1569
> #13 0x0000000000bb6ebb in add_type_duplicate (val=0x7ffff18ab570, type=<record_type 0x7ffff1aa6b28 AsyncHooks>)
>     at ../../src/gcc/ipa-devirt.c:1848
> #14 0x0000000000bb77f2 in get_odr_type (type=<record_type 0x7ffff1aa6b28 AsyncHooks>, insert=true)
>     at ../../src/gcc/ipa-devirt.c:2028
> #15 0x0000000000bb0ae0 in odr_subtypes_equivalent_p (t1=<record_type 0x7ffff1aa6b28 AsyncHooks>,
>     t2=<record_type 0x7ffff1aa6e70 AsyncHooks>, visited=0x7fffffffda10, loc1=288, loc2=2)
>     at ../../src/gcc/ipa-devirt.c:689
> #16 0x0000000000bb5151 in odr_types_equivalent_p (t1=<record_type 0x7ffff1aa6a80 Environment>,
>     t2=<record_type 0x7ffff1aa6f18 Environment>, warn=true, warned=0x7fffffffda43, visited=0x7fffffffda10, loc1=288,
>     loc2=2) at ../../src/gcc/ipa-devirt.c:1556
> #17 0x0000000000bb6ebb in add_type_duplicate (val=0x7ffff18ab4e0, type=<record_type 0x7ffff1aa6f18 Environment>)
>     at ../../src/gcc/ipa-devirt.c:1848
> #18 0x0000000000bb77f2 in get_odr_type (type=<record_type 0x7ffff1aa6f18 Environment>, insert=true)
>     at ../../src/gcc/ipa-devirt.c:2028
> #19 0x0000000000bb7e42 in register_odr_type (type=<record_type 0x7ffff1aa6f18 Environment>)
>     at ../../src/gcc/ipa-devirt.c:2111
> #20 0x00000000007eae64 in lto_read_decls (decl_data=0x7ffff1696000, data=0x296c040, resolutions=...)
>     at ../../src/gcc/lto/lto.c:1755
> #21 0x00000000007eb9c3 in lto_file_finalize (file_data=0x7ffff1696000, file=0x2968cc0)
>     at ../../src/gcc/lto/lto.c:2055
> #22 0x00000000007eba1b in lto_create_files_from_ids (file=0x2968cc0, file_data=0x7ffff1696000, count=0x7fffffffdd4c)
>     at ../../src/gcc/lto/lto.c:2065
> #23 0x00000000007ebb4f in lto_file_read (file=0x2968cc0, resolution_file=0x290aad0, count=0x7fffffffdd4c)
>     at ../../src/gcc/lto/lto.c:2106
> #24 0x00000000007ef930 in read_cgraph_and_symbols (nfiles=2, fnames=0x28e5c00) at ../../src/gcc/lto/lto.c:2818
> #25 0x00000000007f0c16 in lto_main () at ../../src/gcc/lto/lto.c:3323
> #26 0x0000000000eb3ae6 in compile_file () at ../../src/gcc/toplev.c:455
> #27 0x0000000000eb6de6 in do_compile () at ../../src/gcc/toplev.c:2076
> #28 0x0000000000eb715c in toplev::main (this=0x7fffffffdeae, argc=16, argv=0x28a0aa0) at ../../src/gcc/toplev.c:2211
> #29 0x0000000001bbd183 in main (argc=15, argv=0x7fffffffdfa8) at ../../src/gcc/main.c:39
>
> My patch put the apply_location_cache immediately before the read in
> add_type_duplicate, but maybe there's a better place?

Ugh, quite a maze ;)  So your original patch is ok unless Honza comes
up with something better.

Thanks,
Richard.

> Thanks
> Dave
>
>>
>> Richard.
>>
>> > Thanks
>> > Dave
>> >
>> > >
>> > > > 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.
>> > > > ---
>> > > >  gcc/ipa-devirt.c                     |   7 +-
>> > > >  gcc/testsuite/g++.dg/lto/pr83121_0.C |  12 +++
>> > > >  gcc/testsuite/g++.dg/lto/pr83121_1.C |  10 ++
>> > > >  gcc/testsuite/lib/lto.exp            | 199
>> > > > ++++++++++++++++++++++++++++++++++-
>> > > >  4 files changed, 222 insertions(+), 6 deletions(-)
>> > > >  create mode 100644 gcc/testsuite/g++.dg/lto/pr83121_0.C
>> > > >  create mode 100644 gcc/testsuite/g++.dg/lto/pr83121_1.C
>> > > >
>> > > > diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
>> > > > index 540f038..f3d2e4a 100644
>> > > > --- a/gcc/ipa-devirt.c
>> > > > +++ b/gcc/ipa-devirt.c
>> > > > @@ -1844,7 +1844,12 @@ add_type_duplicate (odr_type val, tree
>> > > > type)
>> > > >         }
>> > > >      }
>> > > >
>> > > > -  /* Next compare memory layout.  */
>> > > > +  /* Next compare memory layout.
>> > > > +     The DECL_SOURCE_LOCATIONs in this invocation came from
>> > > > LTO
>> > > > streaming.
>> > > > +     We must apply the location cache to ensure that they are
>> > > > valid
>> > > > +     before we can pass them to odr_types_equivalent_p (PR
>> > > > lto/83121).  */
>> > > > +  if (lto_location_cache::current_cache)
>> > > > +    lto_location_cache::current_cache->apply_location_cache
>> > > > ();
>> > > >    if (!odr_types_equivalent_p (val->type, type,
>> > > >                                !flag_ltrans && !val-
>> > > > >odr_violated
>> > > > && !warned,
>> > > >                                &warned, &visited,
>> > > > diff --git a/gcc/testsuite/g++.dg/lto/pr83121_0.C
>> > > > b/gcc/testsuite/g++.dg/lto/pr83121_0.C
>> > > > new file mode 100644
>> > > > index 0000000..ef894c7
>> > > > --- /dev/null
>> > > > +++ b/gcc/testsuite/g++.dg/lto/pr83121_0.C
>> > > > @@ -0,0 +1,12 @@
>> > > > +// { dg-lto-do link }
>> > > > +// { dg-lto-options {{-O0 -flto}} }
>> > > > +/* We need -O0 to avoid the "Environment" locals in the test
>> > > > functions
>> > > > +   from being optimized away.  */
>> > > > +
>> > > > +struct Environment { // { dg-lto-warning "8: type 'struct
>> > > > Environment' violates the C\\+\\+ One Definition Rule" }
>> > > > +  struct AsyncHooks {
>> > > > +    int providers_[2]; // { dg-lto-message "a field of same
>> > > > name
>> > > > but different type is defined in another translation unit" }
>> > > > +  };
>> > > > +  AsyncHooks async_hooks_;
>> > > > +};
>> > > > +void fn2() { Environment a; }
>> > > > diff --git a/gcc/testsuite/g++.dg/lto/pr83121_1.C
>> > > > b/gcc/testsuite/g++.dg/lto/pr83121_1.C
>> > > > new file mode 100644
>> > > > index 0000000..2aef1b5
>> > > > --- /dev/null
>> > > > +++ b/gcc/testsuite/g++.dg/lto/pr83121_1.C
>> > > > @@ -0,0 +1,10 @@
>> > > > +struct Environment {
>> > > > +  struct AsyncHooks { // { dg-lto-warning "10: type 'struct
>> > > > AsyncHooks' violates the C\\+\\+ One Definition Rule" }
>> > > > +    int providers_[1]; // { dg-lto-message "the first
>> > > > difference
>> > > > of corresponding definitions is field 'providers_'" }
>> > > > +  };
>> > > > +  AsyncHooks async_hooks_;
>> > > > +};
>> > > > +void fn1() { Environment a; }
>> > > > +int main ()
>> > > > +{
>> > > > +}
>> > > > diff --git a/gcc/testsuite/lib/lto.exp
>> > > > b/gcc/testsuite/lib/lto.exp
>> > > > index 477f683..e69d8d4 100644
>> > > > --- a/gcc/testsuite/lib/lto.exp
>> > > > +++ b/gcc/testsuite/lib/lto.exp
>> > > > @@ -16,6 +16,122 @@
>> > > >
>> > > >  # Contributed by Diego Novillo <dnovillo@google.com>
>> > > >
>> > > > +# A subroutine of lto_handle_diagnostics: check TEXT for the
>> > > > expected
>> > > > +# diagnostics for one specific source file, issuing PASS/FAIL
>> > > > results.
>> > > > +# Return TEXT, stripped of any diagnostics that were handled.
>> > > > +#
>> > > > +# NAME is the testcase name to use when reporting PASS/FAIL
>> > > > results.
>> > > > +# FILENAME is the name (with full path) of the file we're
>> > > > interested in.
>> > > > +# MESSAGES_FOR_FILE is a list of expected messages, akin to
>> > > > DejaGnu's
>> > > > +# "dg-messages" variable.
>> > > > +# TEXT is the textual output from the LTO link.
>> > > > +
>> > > > +proc lto_handle_diagnostics_for_file { name filename
>> > > > messages_for_file text } {
>> > > > +    global dg-linenum-format
>> > > > +
>> > > > +    set filename_without_path [file tail $filename]
>> > > > +
>> > > > +    # This loop is adapted from the related part of DejaGnu's
>> > > > dg-
>> > > > test,
>> > > > +    # with changes as detailed below to cope with the LTO
>> > > > case.
>> > > > +
>> > > > +    foreach i ${messages_for_file} {
>> > > > +       verbose "Scanning for message: $i" 4
>> > > > +
>> > > > +       # Remove all error messages for the line [lindex $i 0]
>> > > > +       # in the source file.  If we find any, success!
>> > > > +       set line [lindex $i 0]
>> > > > +       set pattern [lindex $i 2]
>> > > > +       set comment [lindex $i 3]
>> > > > +       verbose "line: $line" 4
>> > > > +       verbose "pattern: $pattern" 4
>> > > > +       verbose "comment: $comment" 4
>> > > > +       #send_user "Before:\n$text\n"
>> > > > +
>> > > > +       # Unlike dg-test, we use $filename_without_path in this
>> > > > pattern.
>> > > > +       # This is to ensure that we have the correct file/line
>> > > > combination.
>> > > > +       # This imposes the restriction that the filename can't
>> > > > contain
>> > > > +       # any regexp control characters.  We have to strip the
>> > > > path, since
>> > > > +       # e.g. the '+' in "g++.dg" wouldn't be valid.
>> > > > +       set pat
>> > > > "(^|\n)(\[^\n\]+$filename_without_path$line\[^\n\]*($pattern)\[
>> > > > ^\n\
>> > > > ]*\n?)+"
>> > > > +       if {[regsub -all $pat $text "\n" text]} {
>> > > > +           set text [string trimleft $text]
>> > > > +           set ok pass
>> > > > +           set uhoh fail
>> > > > +       } else {
>> > > > +           set ok fail
>> > > > +           set uhoh pass
>> > > > +       }
>> > > > +       #send_user "After:\n$text\n"
>> > > > +
>> > > > +       # $line will either be a formatted line number or a
>> > > > number
>> > > > all by
>> > > > +       # itself.  Delete the formatting.
>> > > > +       scan $line ${dg-linenum-format} line
>> > > > +
>> > > > +       # Unlike dg-test, add the filename to the PASS/FAIL
>> > > > message
>> > > > (rather
>> > > > +       # than just the line number) so that the user can
>> > > > identify
>> > > > the
>> > > > +       # pertinent directive.
>> > > > +       set describe_where "$filename_without_path line $line"
>> > > > +
>> > > > +       # Issue the PASS/FAIL, adding "LTO" to the messages
>> > > > (e.g.
>> > > > "LTO errors")
>> > > > +       # to distinguish them from the non-LTO case (in case we
>> > > > ever need to
>> > > > +       # support both).
>> > > > +       switch [lindex $i 1] {
>> > > > +           "ERROR" {
>> > > > +               $ok "$name $comment (test for LTO errors,
>> > > > $describe_where)"
>> > > > +           }
>> > > > +           "XERROR" {
>> > > > +               x$ok "$name $comment (test for LTO errors,
>> > > > $describe_where)"
>> > > > +           }
>> > > > +           "WARNING" {
>> > > > +               $ok "$name $comment (test for LTO warnings,
>> > > > $describe_where)"
>> > > > +           }
>> > > > +           "XWARNING" {
>> > > > +               x$ok "$name $comment (test for LTO warnings,
>> > > > $describe_where)"
>> > > > +           }
>> > > > +           "BOGUS" {
>> > > > +               $uhoh "$name $comment (test for LTO bogus
>> > > > messages,
>> > > > $describe_where)"
>> > > > +           }
>> > > > +           "XBOGUS" {
>> > > > +               x$uhoh "$name $comment (test for LTO bogus
>> > > > messages, $describe_where)"
>> > > > +           }
>> > > > +           "BUILD" {
>> > > > +               $uhoh "$name $comment (test for LTO build
>> > > > failure,
>> > > > $describe_where)"
>> > > > +           }
>> > > > +           "XBUILD" {
>> > > > +               x$uhoh "$name $comment (test for LTO build
>> > > > failure,
>> > > > $describe_where)"
>> > > > +           }
>> > > > +           "EXEC" { }
>> > > > +           "XEXEC" { }
>> > > > +       }
>> > > > +    }
>> > > > +    return $text
>> > > > +}
>> > > > +
>> > > > +# Support for checking for link-time diagnostics: check for
>> > > > +# the expected diagnostics within TEXT, issuing PASS/FAIL
>> > > > results.
>> > > > +# Return TEXT, stripped of any diagnostics that were handled.
>> > > > +#
>> > > > +# MESSAGES_BY_FILE is a dict; the keys are source files (with
>> > > > paths)
>> > > > +# the values are lists of expected messages, akin to DejaGnu's
>> > > > "dg-messages"
>> > > > +# variable.
>> > > > +# TEXT is the textual output from the LTO link.
>> > > > +
>> > > > +proc lto_handle_diagnostics { messages_by_file text } {
>> > > > +    global testcase
>> > > > +
>> > > > +    verbose "lto_handle_diagnostics: entry: $text" 2
>> > > > +    verbose "  messages_by_file $messages_by_file" 3
>> > > > +
>> > > > +    dict for {src dg-messages} $messages_by_file {
>> > > > +       set text [lto_handle_diagnostics_for_file $testcase
>> > > > $src \
>> > > > +                     ${dg-messages} $text]
>> > > > +    }
>> > > > +
>> > > > +    verbose "lto_handle_diagnostics: exit: $text" 2
>> > > > +
>> > > > +    return $text
>> > > > +}
>> > > > +
>> > > >  # Prune messages that aren't useful.
>> > > >
>> > > >  proc lto_prune_warns { text } {
>> > > > @@ -39,6 +155,9 @@ proc lto_prune_warns { text } {
>> > > >      regsub -all "(^|\n)\[ \t\]*\[\(\]file \[^\n\]*
>> > > > value=\[^\n\]*;
>> > > > file \[^\n\]* value=\[^\n\]*\[)\];" $text "" text
>> > > >      regsub -all "(^|\n)\[ \t\]*\[^\n\]* definition taken"
>> > > > $text ""
>> > > > text
>> > > >
>> > > > +    # Ignore informational notes.
>> > > > +    regsub -all "(^|\n)\[^\n\]*: note: \[^\n\]*" $text "" text
>> > > > +
>> > > >      verbose "lto_prune_warns: exit: $text" 2
>> > > >
>> > > >      return $text
>> > > > @@ -175,12 +294,17 @@ proc lto-obj { source dest optall optfile
>> > > > optstr xfaildata } {
>> > > >  # OPTALL is a list of compiler and linker options to use for
>> > > > all
>> > > > tests
>> > > >  # OPTFILE is a list of compiler and linker options to use for
>> > > > this
>> > > > test
>> > > >  # OPTSTR is the list of options to list in messages
>> > > > -proc lto-link-and-maybe-run { testname objlist dest optall
>> > > > optfile
>> > > > optstr } {
>> > > > +# MESSAGES_BY_FILE is a dict of (src, dg-messages)
>> > > > +proc lto-link-and-maybe-run { testname objlist dest optall
>> > > > optfile
>> > > > optstr \
>> > > > +                             messages_by_file } {
>> > > >      global testcase
>> > > >      global tool
>> > > >      global compile_type
>> > > >      global board_info
>> > > >
>> > > > +    verbose "lto-link-and-maybe-run" 2
>> > > > +    verbose "  messages_by_file $messages_by_file" 3
>> > > > +
>> > > >      # Check that all of the objects were built successfully.
>> > > >      foreach obj [split $objlist] {
>> > > >         if ![file_on_host exists $obj] then {
>> > > > @@ -217,12 +341,17 @@ proc lto-link-and-maybe-run { testname
>> > > > objlist dest optall optfile optstr } {
>> > > >         set board_info($target_board,ldscript) $saved_ldscript
>> > > >      }
>> > > >
>> > > > +    # Check for diagnostics specified by directives
>> > > > +    set comp_output [lto_handle_diagnostics $messages_by_file
>> > > > $comp_output]
>> > > > +
>> > > >      # Prune unimportant visibility warnings before checking
>> > > > output.
>> > > >      set comp_output [lto_prune_warns $comp_output]
>> > > >
>> > > >      if ![${tool}_check_compile "$testcase $testname link"
>> > > > $optstr
>> > > > \
>> > > >          $dest $comp_output] then {
>> > > > -       unresolved "$testcase $testname execute $optstr"
>> > > > +       if { ![string compare "execute" $compile_type] } {
>> > > > +           unresolved "$testcase $testname execute $optstr"
>> > > > +       }
>> > > >         return
>> > > >      }
>> > > >
>> > > > @@ -243,6 +372,51 @@ proc lto-link-and-maybe-run { testname
>> > > > objlist
>> > > > dest optall optfile optstr } {
>> > > >      $status "$testcase $testname execute $optstr"
>> > > >  }
>> > > >
>> > > > +# Potentially handle the given dg- directive (a list)
>> > > > +# Return true is the directive was handled, false otherwise.
>> > > > +
>> > > > +proc lto-can-handle-directive { op } {
>> > > > +    set cmd [lindex $op 0]
>> > > > +
>> > > > +    # dg-warning and dg-message append to dg-messages.
>> > > > +    upvar dg-messages dg-messages
>> > > > +
>> > > > +    # A list of directives to recognize, and a list of
>> > > > directives
>> > > > +    # to remap them to.
>> > > > +    # For example, "dg-lto-warning" is implemented by calling
>> > > > "dg-
>> > > > warning".
>> > > > +    set directives { dg-lto-warning dg-lto-message }
>> > > > +    set remapped_directives { dg-warning dg-message }
>> > > > +
>> > > > +    set idx [lsearch -exact $directives $cmd]
>> > > > +    if { $idx != -1 } {
>> > > > +       verbose "remapping from: $op" 4
>> > > > +
>> > > > +       set remapped_cmd [lindex $remapped_directives $idx]
>> > > > +       set op [lreplace $op 0 0 $remapped_cmd]
>> > > > +
>> > > > +       verbose "remapped to: $op" 4
>> > > > +
>> > > > +       set status [catch "$op" errmsg]
>> > > > +       if { $status != 0 } {
>> > > > +           if { 0 && [info exists errorInfo] } {
>> > > > +               # This also prints a backtrace which will just
>> > > > confuse
>> > > > +               # testcase writers, so it's disabled.
>> > > > +               perror "$name: $errorInfo\n"
>> > > > +           } else {
>> > > > +               perror "$name: $errmsg for \"$op\"\n"
>> > > > +           }
>> > > > +           # ??? The call to unresolved here is necessary to
>> > > > clear
>> > > > `errcnt'.
>> > > > +           # What we really need is a proc like perror that
>> > > > doesn't set errcnt.
>> > > > +           # It should also set exit_status to 1.
>> > > > +           unresolved "$name: $errmsg for \"$op\""
>> > > > +       }
>> > > > +
>> > > > +       return true
>> > > > +    }
>> > > > +
>> > > > +    return false
>> > > > +}
>> > > > +
>> > > >  # lto-get-options-main -- get target requirements for a test
>> > > > and
>> > > >  # options for the primary source file and the test as a whole
>> > > >  #
>> > > > @@ -266,6 +440,10 @@ proc lto-get-options-main { src } {
>> > > >      upvar dg-final-code dg-final-code
>> > > >      set dg-final-code ""
>> > > >
>> > > > +    # dg-warning and dg-message append to dg-messages.
>> > > > +    upvar dg-messages-by-file dg-messages-by-file
>> > > > +    set dg-messages ""
>> > > > +
>> > > >      set tmp [dg-get-options $src]
>> > > >      verbose "getting options for $src: $tmp"
>> > > >      foreach op $tmp {
>> > > > @@ -342,12 +520,15 @@ proc lto-get-options-main { src } {
>> > > >             } else {
>> > > >                 append dg-final-code "[lindex $op 2]\n"
>> > > >             }
>> > > > -       } else {
>> > > > +       } elseif { ![lto-can-handle-directive $op] } {
>> > > >             # Ignore unrecognized dg- commands, but warn about
>> > > > them.
>> > > >             warning "lto.exp does not support $cmd"
>> > > >         }
>> > > >      }
>> > > >
>> > > > +    verbose "dg-messages: ${dg-messages}" 3
>> > > > +    dict append dg-messages-by-file $src ${dg-messages}
>> > > > +
>> > > >      # Return flags to use for compiling the primary source
>> > > > file
>> > > > and for
>> > > >      # linking.
>> > > >      verbose "dg-extra-tool-flags for main is ${dg-extra-tool-
>> > > > flags}"
>> > > > @@ -373,6 +554,10 @@ proc lto-get-options { src } {
>> > > >      # dg-xfail-if needs access to dg-do-what.
>> > > >      upvar dg-do-what dg-do-what
>> > > >
>> > > > +    # dg-warning appends to dg-messages.
>> > > > +    upvar dg-messages-by-file dg-messages-by-file
>> > > > +    set dg-messages ""
>> > > > +
>> > > >      set tmp [dg-get-options $src]
>> > > >      foreach op $tmp {
>> > > >         set cmd [lindex $op 0]
>> > > > @@ -386,12 +571,15 @@ proc lto-get-options { src } {
>> > > >             }
>> > > >         } elseif { [string match "dg-require-*" $cmd] } {
>> > > >             warning "lto.exp does not support $cmd in secondary
>> > > > source files"
>> > > > -       } else {
>> > > > +       } elseif { ![lto-can-handle-directive $op] } {
>> > > >             # Ignore unrecognized dg- commands, but warn about
>> > > > them.
>> > > >             warning "lto.exp does not support $cmd in secondary
>> > > > source files"
>> > > >         }
>> > > >      }
>> > > >
>> > > > +    verbose "dg-messages: ${dg-messages}" 3
>> > > > +    dict append dg-messages-by-file $src ${dg-messages}
>> > > > +
>> > > >      return ${dg-extra-tool-flags}
>> > > >  }
>> > > >
>> > > > @@ -421,6 +609,7 @@ proc lto-execute { src1 sid } {
>> > > >      verbose "lto-execute: $src1" 1
>> > > >      set compile_type "run"
>> > > >      set dg-do-what [list ${dg-do-what-default} "" P]
>> > > > +    set dg-messages-by-file [dict create]
>> > > >      set extra_flags(0) [lto-get-options-main $src1]
>> > > >      set compile_xfail(0) ""
>> > > >
>> > > > @@ -544,7 +733,7 @@ proc lto-execute { src1 sid } {
>> > > >             lto-link-and-maybe-run \
>> > > >                     "[lindex $obj_list 0]-[lindex $obj_list
>> > > > end]" \
>> > > >                     $obj_list $execname $filtered ${dg-extra-
>> > > > ld-
>> > > > options} \
>> > > > -                   $filtered
>> > > > +                   $filtered ${dg-messages-by-file}
>> > > >         }
>> > > >
>> > > >
>> > > > --
>> > > > 1.8.5.3
>> > > >
>>
>>



More information about the Gcc-patches mailing list