On Mon, Aug 10, 2020 at 09:48:06AM +0100, Richard Sandiford wrote:
> Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> >> > > sure.
> >> > > * develop patch
> >> > > * run testsuite
> >> > > * observe unexpected ICEs
> >> > > * load g++.log into editor
> >> > > * ^sinternal comp
> >> > > * gets to first unexpected ICE
> >> > > * debug it.
> >> > > 
> >> > > What does '^sinternal comp' become?  As there could be many expected 
> >> > > ICEs
> >> > > it'll be painful to determine whether any particular utterance of 
> >> > > 'internal
> >> > > compiler' is expected or not.
> >> > 
> >> > That is a problem I don't know how to deal with.  I know how to pass
> >> > additional options to the compiler from dejagnu.  I thought maybe I 
> >> > could use
> >> > -pass-exit-codes, redirect stderr to /dev/null, and check if the exit 
> >> > code is
> >> > ICE_EXIT_CODE, but there seems to be no way to do that redirection.  So 
> >> > I'm
> >> > stuck.
> >> > 
> >> > Though, you could just grep for '^FAIL.*internal comp', which will find 
> >> > the
> >> > first unexpected ICE.  Contrary to the expected ICEs, the unexpected 
> >> > ICEs will
> >> > be shown in 'Excess errors:'.  Won't that work?
> >> 
> >> Read what I wrote:
> >> >> * load g++.log into editor
> >> >> * ^sinternal comp
> >> 
> >> are you telling me not to use an editor for this task?  the search is so 
> >> one
> >> can get the command line.  Grepping the log file will not do that.
> >
> > No, I'm saying that looking for '^FAIL.*internal comp' (in an editor) 
> > instead
> > of just 'internal comp' will get you to the ICEs you care about.  In vim,
> > '/^FAIL.*in' gets me there.
> 
> Yeah, this works.  I'm not sure whether Nathan's ^s is isearch
> or regexp-isearch :-)
> 
> > If that is still too much, we could maybe add a new diagnostic kind, like
> > DK_EXPECTED_ICE that has different wording than DK_ICE, and have dg-ice
> > use it via some option/envvar.
> 
> Seems unnecessary given the above.

Yeah, I would like to avoid adding hacks like that.

> >> while solveable, this seems to be the equivalent of putting stones in ones
> >> shoe.  an annoyance one could do without.  I'm sadly not convinced the goal
> >> is worth it.
> >
> > If this is an annoyance to people, I'll just go back to my own repo.
> 
> Please don't!  I'm sure we can find something for Nathan's use case,
> if your regexp search isn't enough.
> 
> How do things stand with the new directives?  Seems like there are no
> objections to dg-ice, but I lost track of the others.  Personally I'm
> not interested in any solution that replaces one of your new directives
> with two existing directives: one (sub)test should be one directive as
> far as possible.  So IMO we should add every directive that doesn't
> already have a one-for-one analogue.

Thanks a lot.  Here's the latest version of my patch, which only adds dg-ice
at this point.

I do agree with adding useful directives that don't have an existing
variant, or whose effect could only be achieved by a cumbersome combination
of existing directives.  Perhaps we'll realize we need another one down the
line.

Here's the patch.  Tested by adding two same foo.C and foo2.C tests that
ICE, only foo.C has { dg-ice "" }, and running

$ make check-c++ RUNTESTFLAGS=dg.exp=foo*.C

The result is

FAIL: g++.dg/foo2.C  -std=c++14 (internal compiler error)
FAIL: g++.dg/foo2.C  -std=c++14 (test for excess errors)

so only the unexpected ICE is displayed.

So, um, OK?  Mike, would you be able to take a look at this patch?

I guess it wouldn't hurt to mention this in
https://gcc.gnu.org/codingconventions.html#Testsuite

Thanks,

-- >8 --
This patch adds a new DejaGNU directive, dg-ice, as outlined in the
proposal here:
https://gcc.gnu.org/pipermail/gcc-patches/2020-July/550913.html

It means that it's expected that the compiler crashes with an internal
compiler error when compiling test with such a directive.

A minor optimization could be to use -pass-exit-codes and then check for
ICE_EXIT_CODE return code instead of using string match.

gcc/ChangeLog:

        * doc/sourcebuild.texi: Document dg-ice.

gcc/testsuite/ChangeLog:

        * lib/gcc-dg.exp (gcc-dg-test-1): Handle dg-ice.
        (cleanup-after-saved-dg-test): Reset expect_ice.
        * lib/prune.exp (prune_ices): New.
        * lib/target-supports-dg.exp (dg-ice): New.
---
 gcc/doc/sourcebuild.texi                 | 10 +++++++++
 gcc/testsuite/lib/gcc-dg.exp             | 20 +++++++++++++++--
 gcc/testsuite/lib/prune.exp              |  9 ++++++++
 gcc/testsuite/lib/target-supports-dg.exp | 28 ++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 63216a0daba..967cb135cb4 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -1172,6 +1172,16 @@ Expect the execute step of a test to fail if the 
conditions (which are
 the same as for @code{dg-skip-if}) are met.
 @end table
 
+@subsubsection Expect the compiler to crash
+
+@table @code
+@item  @{ dg-ice @var{comment} [@{ @var{selector} @} [@{ @var{include-opts} @} 
[@{ @var{exclude-opts} @}]]] @}
+Expect the compiler to crash with an internal compiler error and return
+a nonzero exit status if the conditions (which are the same as for
+@code{dg-skip-if}) are met.  Used for tests that test bugs that have not been
+fixed yet.
+@end table
+
 @subsubsection Expect the test executable to fail
 
 @table @code
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 45d97024883..e8ad3052657 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -308,13 +308,27 @@ proc gcc-dg-test-1 { target_compile prog do_what 
extra_tool_flags } {
     verbose "$target_compile $prog $output_file $compile_type $options" 4
     set comp_output [$target_compile "$prog" "$output_file" "$compile_type" 
$options]
 
+    global expect_ice
     # Look for an internal compiler error, which sometimes masks the fact
     # that we didn't get an expected error message.  XFAIL an ICE via
     # dg-xfail-if and use { dg-prune-output ".*internal compiler error.*" }
     # to avoid a second failure for excess errors.
-    if [string match "*internal compiler error*" $comp_output] {
+    # "Error reporting routines re-entered" ICE says "Internal" rather than
+    # "internal", so match that too.
+    if [string match {*[Ii]nternal compiler error*} $comp_output] {
        upvar 2 name name
-       fail "$name (internal compiler error)"
+       if { $expect_ice == 0 } {
+         fail "$name (internal compiler error)"
+       } else {
+         # We expected an ICE and we got it.
+         xfail "$name (internal compiler error)"
+         # Prune the ICE from the output.
+         set comp_output [prune_ices $comp_output]
+       }
+    } elseif { $expect_ice == 1 } {
+       upvar 2 name name
+       # We expected an ICE but we didn't get it.
+       xpass "$name (internal compiler error)"
     }
 
     if { $do_what == "repo" } {
@@ -939,6 +953,7 @@ if { [info procs saved-dg-test] == [list] } {
        global additional_prunes
        global compiler_conditional_xfail_data
        global shouldfail
+       global expect_ice
        global testname_with_flags
        global set_target_env_var
        global set_compiler_env_var
@@ -954,6 +969,7 @@ if { [info procs saved-dg-test] == [list] } {
        set additional_sources_used ""
        set additional_prunes ""
        set shouldfail 0
+       set expect_ice 0
        if [info exists set_target_env_var] {
            unset set_target_env_var
        }
diff --git a/gcc/testsuite/lib/prune.exp b/gcc/testsuite/lib/prune.exp
index 1c776249f1a..58a739684a5 100644
--- a/gcc/testsuite/lib/prune.exp
+++ b/gcc/testsuite/lib/prune.exp
@@ -118,6 +118,15 @@ proc prune_file_path { text } {
     return $text
 }
 
+# Prune internal compiler error messages, including the "Please submit..."
+# footnote.
+
+proc prune_ices { text } {
+  regsub -all "(^|\n)\[^\n\]*: internal compiler error:.*for 
instructions\[^\n\]*" $text "" text
+  regsub -all "(^|\n|')*Internal compiler error:.*for instructions\[^\n\]*" 
$text "" text
+  return $text
+}
+
 # Provide a definition of this if missing (delete after next dejagnu release).
 
 if { [info procs prune_warnings] == "" } then {
diff --git a/gcc/testsuite/lib/target-supports-dg.exp 
b/gcc/testsuite/lib/target-supports-dg.exp
index 2a21424b890..5bb99f4e8f9 100644
--- a/gcc/testsuite/lib/target-supports-dg.exp
+++ b/gcc/testsuite/lib/target-supports-dg.exp
@@ -495,6 +495,34 @@ proc dg-shouldfail { args } {
     }
 }
 
+# Record whether the compiler is expected (at the moment) to ICE.
+# Used for tests that test bugs that have not been fixed yet.
+
+set expect_ice 0
+
+proc dg-ice { args } {
+    # Don't bother if we're already skipping the test.
+    upvar dg-do-what dg-do-what
+    if { [lindex ${dg-do-what} 1] == "N" } {
+      return
+    }
+
+    global expect_ice
+
+    set args [lreplace $args 0 0]
+    if { [llength $args] > 1 } {
+       set selector [list target [lindex $args 1]]
+       if { [dg-process-target-1 $selector] == "S" } {
+           # The target matches, now check the flags.
+           if [check-flags $args] {
+               set expect_ice 1
+           }
+       }
+    } else {
+       set expect_ice 1
+    }
+}
+
 # Intercept the call to the DejaGnu version of dg-process-target to
 # support use of an effective-target keyword in place of a list of
 # target triplets to xfail or skip a test.

base-commit: e4ced0b60ccb4c944970304cf74f1ee9086e5553
-- 
2.26.2

Reply via email to