Hi David,
On 15 January 2018 at 11:09, Richard Biener <richard.guent...@gmail.com> wrote: > On Mon, Jan 8, 2018 at 8:36 PM, David Malcolm <dmalc...@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®rtested 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. I've noticed that the new tests fail on aarch64/arm: FAIL: g++.dg/lto/pr83121 (test for LTO warnings, pr83121_0.C line 8) FAIL: g++.dg/lto/pr83121 (test for LTO warnings, pr83121_1.C line 2) FAIL: g++.dg/lto/pr83121 (test for LTO warnings, pr83121_1.C line 3) on aarch64, the logs contain: /gcc/testsuite/g++.dg/lto/pr83121_0.C:6:8: warning: type 'struct Environment' violates the C++ One Definition Rule [-Wodr] /gcc/testsuite/g++.dg/lto/pr83121_1.C:1:8: note: a type with different size is defined in another translation unit on arm, the logs contain: /gcc/testsuite/g++.dg/lto/pr83121_0.C:7:10: warning: type 'struct AsyncHooks' violates the C++ One Definition Rule [-Wodr] /gcc/testsuite/g++.dg/lto/pr83121_1.C:2:10: note: a different type is defined in another translation unit /gcc/testsuite/g++.dg/lto/pr83121_0.C:8:21: note: the first difference of corresponding definitions is field 'providers_' /gcc/testsuite/g++.dg/lto/pr83121_1.C:3:21: note: a field of same name but different type is defined in another translation unit /gcc/testsuite/g++.dg/lto/pr83121_0.C:7:10: note: array types have different bounds /gcc/testsuite/g++.dg/lto/pr83121_0.C:6:8: warning: type 'struct Environment' violates the C++ One Definition Rule [-Wodr] /gcc/testsuite/g++.dg/lto/pr83121_1.C:1:8: note: a different type is defined in another translation unit /gcc/testsuite/g++.dg/lto/pr83121_0.C:10:14: note: the first difference of corresponding definitions is field 'async_hooks_' /gcc/testsuite/g++.dg/lto/pr83121_1.C:5:14: note: a field of same name but different type is defined in another translation unit /gcc/testsuite/g++.dg/lto/pr83121_0.C:7:10: note: type 'struct AsyncHooks' itself violates the C++ One Definition Rule /gcc/testsuite/g++.dg/lto/pr83121_1.C:2:10: note: the incompatible type is defined here and on arm only: FAIL: g++.dg/lto/pr83121 cp_lto_pr83121_0.o-cp_lto_pr83121_1.o link, -O0 -flto Christophe >>> > > > * 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 <dnovi...@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 >>> > > > >>> >>>
