From 60122dbef4eb01b634019bd71ef47dec8faab274 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Mon, 24 Aug 2020 19:44:53 -0400 Subject: [PATCH] gdb/testsuite: make runto always emit a FAIL on internal error I noticed that when a test uses `runto_main` and a GDB internal error happens while running to main, no error or fail is emitted. This is because `runto_main` uses the `no-message` option of `runto`. As a result, if a test fails to run to main and exits, no sign that something went wrong is emitted. For example, add this always-false assertion to compute_frame_id: --- a/gdb/frame.c +++ b/gdb/frame.c @@ -545,6 +545,7 @@ static void compute_frame_id (struct frame_info *fi) { gdb_assert (!fi->this_id.p); + gdb_assert (false); if (frame_debug) fprintf_unfiltered (gdb_stdlog, "{ compute_frame_id (fi=%d) ", ... and run gdb.dwarf2/dw2-align.exp. No fail or sign that something went wrong is shown. It just appears as if the test gets skipped. A developer introducing such a regression in this test today would likely notice it, because we are used to diff-ing test results. So we would see some PASSes dispappear for no good reason and look into it. But I find it worrysome for two reasons: 1. Scripts that analyze regressions (such as the one on the buildbot) may only look for new FAILs or new ERRORs. It would probably miss this. 2. Imagine that we one day have a testsuite that runs cleanly (some people might already run subsets of the testsuite and expect it to all pass), we would just run the testsuite and check that there are no fails. It would be easy to miss something like this. In case of internal error, I suggest making `runto` emit a FAIL even if `no-message` was passed. This is different from other failure modes that might be expected (whchi rightfully cause the test to simply be skipped). An internal error is always bad, so if it happens it should noisily fail. gdb/testsuite/ChangeLog: * lib/gdb.exp (runto): Always emit fail on internal error. Change-Id: I6e6faed4868ea821541a23042b2d01c30058b0d3 --- gdb/testsuite/ChangeLog | 4 ++++ gdb/testsuite/lib/gdb.exp | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 61a37abb109..b976a54cc51 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2020-08-24 Simon Marchi + + * lib/gdb.exp (runto): Always emit fail on internal error. + 2020-08-24 Simon Marchi * gdb.base/gdb-sigterm.exp (do_test): Update expected regexp. diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 5bdeed70f30..8ff2025b6af 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -652,9 +652,9 @@ proc runto { function args } { return 0 } -re ".*A problem internal to GDB has been detected" { - if { $print_fail } { - fail "$test_name (GDB internal error)" - } + # Always emit a FAIL if we encounter an internal error: internal + # errors are never expected. + fail "$test_name (GDB internal error)" gdb_internal_error_resync return 0 } -- 2.30.2