Re: [PATCH][gdb/testsuite] Fix PATH error in gdb_file_cmd

[Pedro Alves <palves@redhat.com>]

On 6/4/20 12:51 PM, Tom de Vries wrote:
> On 04-06-2020 13:43, Pedro Alves wrote:
>> On 6/4/20 11:24 AM, Tom de Vries wrote:
>>> @@ -1800,7 +1801,7 @@ proc gdb_file_cmd { arg } {
>>>  	    return -1
>>>          }
>>>  	-re "A problem internal to GDB has been detected" {
>>> -	    fail "($arg) (GDB internal error)"
>>> +	    fail "($basename) (GDB internal error)"
>>>  	    gdb_internal_error_resync
>>>  	    return -1
>>>  	}
>>
>> The -re above has the exact same issue in the ERROR call.
> 
> Right, but the PATH detection is only for testnames, not for error messages.

That seems like a bug in the PATH detection stuff to me:

Running /home/pedro/brno/pedro/gdb/binutils-gdb-2/build/../src/gdb/testsuite/gdb.ada/O2_float_param.exp ...
ERROR: (/home/pedro/brno/pedro/gdb/binutils-gdb-2/build/gdb/testsuite/outputs/gdb.ada/O2_float_param/foo) (GDB internal error)

This makes is as hard to diff gdb.sum files as a FAIL/PASS with a build path.

> 
>> I have no idea why this one is a fail while everywhere else
>> in the procedure perror is used.  Seems inconsistent.
> 
> It is locally inconsistent indeed, but it is consistent with all other
> handling of "A problem internal to GDB has been detected" in gdb.exp.

If you look at those other spots, they are using fail/pass in the other
surrounding -re's, and thus that's just another fail among all those.
I.e., those cases are internally consistent.  Like e.g.:

		    -re "${sentinel}" {
			fail "${test} (pattern ${index} + sentinel)"
			set ok 0
		    }
		    -re ".*A problem internal to GDB has been detected" {
			fail "${test} (GDB internal error)"
			set ok 0
			gdb_internal_error_resync
		    }
		    timeout {
			fail "${test} (pattern ${index} + sentinel) (timeout)"
			set ok 0
		    }

The gdb_file_cmd one started out with a message string that didn't
even include the parens around the "GDB internal error" string in 04e7407c59a:

+       -re "A problem internal to GDB has been detected" {
+           fail "($arg) GDB internal error"
+           gdb_internal_error_resync
+           return -1
+       }

The parens were later added in 5b7d00507b9:

        -re "A problem internal to GDB has been detected" {
-           fail "($arg) GDB internal error"
+           fail "($arg) (GDB internal error)"

Arguably that patch should have switched to perror instead.
A FAIL message with only text wrapped in parens is odd,
since the trailing "(foo)" text is not supposed to count as
test message.  So in effect, you could say that this fail has
no test message, the same as if you wrote:

  fail ""

Pedro Alves