gdb/testsuite: fix regressions in break-main-file-remove-fail.exp
authorAndrew Burgess <aburgess@redhat.com>
Mon, 15 May 2023 16:54:55 +0000 (17:54 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Tue, 16 May 2023 09:35:41 +0000 (10:35 +0100)
After this commit:

  commit a68f7e9844208ad8cd498f89b5100084ece7d0f6
  Date:   Tue May 9 10:28:42 2023 +0100

      gdb/testsuite: extend special '^' handling to gdb_test_multiple

buildbot notified me of a regression on s390 in the test:

  gdb.base/break-main-file-remove-fail.exp

the failure looks like this:

  print /d ((int (*) (void *, size_t)) munmap) (16781312, 4096)
  warning: Error removing breakpoint 0
  $2 = 0
  (gdb) FAIL: gdb.base/break-main-file-remove-fail.exp: cmdline: get integer valueof "((int (*) (void *, size_t)) munmap) (16781312, 4096)"

On the mailing list it has been reported that this failure also
impacts arm, aarch64, and possibly ppc/ppc64 too.

The above commit changed get_integer_valueof so that no output is
expected between the command and the '$2 = 0' line.  In this case the
'warning: Error removing breakpoint 0' output is causing the
get_integer_valueof call to fail.

The reason for this warning is that this test deliberately calls
munmap on a page of the inferior's code.  The test is checking that
GDB can handle the situation where a s/w breakpoint can't be
removed (due to the page no longer being readable/writable).

The test that is supposed to trigger the warning is later in the test
script when we delete a breakpoint.

So why do some targets trigger the warning earlier during the inferior
call?

The impacted targets use AT_ENTRY_POINT as their strategy for handling
inferior calls, that is, the trampoline that calls the inferior
function is placed at the program's entry point, e.g. often the _start
label.

If this location happens to be on the same page as the page that the
test script unmaps then, when the inferior function call returns, GDB
will not be able to remove the temporary breakpoint that is inserted
to catch the inferior function call returning!  As a result we end up
seeing the warning earlier than expected.

I did wonder if this means I should relax the pattern in
get_integer_valueof - just accept that there might be additional
output from GDB which we should ignore.

However, I don't think this the right way to go.  With the change in
a68f7e984420 we are now stricter for GDB emitting additional,
unexpected, output, and I think that is a good thing.

So, I think, in this case, in order to handle the possible extra
output, we should implement something like get_integer_valueof
directly in the gdb.base/break-main-file-remove-fail.exp test script.
This local version will handle the possible warning output.

After this the test should pass again on the impacted targets.

gdb/testsuite/gdb.base/break-main-file-remove-fail.exp

index 66ccc60a21a3b376ad7564eef8a6955edbce7b47..c7cf4f3df00ff83f649f046fd63db3a7339073bc 100644 (file)
@@ -89,7 +89,22 @@ proc test_remove_bp { initial_load } {
        set align_addr [expr $bp_addr - $bp_addr % $pagesize]
        set munmap_prototype "int (*) (void *, size_t)"
        set munmap_expr "(($munmap_prototype) munmap) ($align_addr, $pagesize)"
-       set munmap [get_integer_valueof $munmap_expr -1]
+
+       # Use gdb_test_multiple here rather than get_integer_valueof.
+       # Targets that use the AT_ENTRY_POINT strategy for inferior
+       # function calls will place a breakpoint near the entry point
+       # to catch the return from the inferior function call, and
+       # this is likely on the page we are about to unmap.  As a
+       # consequence we will see the warning about being unable to
+       # remove the breakpoint here, which throws off
+       # get_integer_valueof.
+       set munmap -1
+       gdb_test_multiple "print /d ${munmap_expr}" "get result of munmap call" {
+           -re -wrap "^(?:warning: Error removing breakpoint $::decimal\r\n)?\\$\[0-9\]* = (\[-\]*\[0-9\]*).*" {
+               set munmap $expect_out(1,string)
+               pass $gdb_test_name
+           }
+       }
 
        if {$munmap != 0} {
            unsupported "can't munmap foo's page"