Re: [PATCH 5/5] Make gdb_test's question non-optional if specified

[Pedro Alves <pedro@palves.net>]

On 2022-04-07 21:31, Bruno Larsen wrote:
> On 3/30/22 16:29, Pedro Alves wrote:
>> gdb_test supports handling scenarios where GDB asks a question before
>> finishing handling some command.  The full prototype of gdb_test is:
>>
>>    # gdb_test COMMAND PATTERN MESSAGE QUESTION RESPONSE
>>
>> However, QUESTION is a question that GDB _may_ ask, not one that GDB
>> _must_ ask:
>>
>>   # QUESTION is a question GDB may ask in response to COMMAND, like
>>   #   "are you sure?"
>>   # RESPONSE is the response to send if QUESTION appears.
>>
>> If GDB doesn't raise the question, the test still passes.
>>
>> I think that this is a misfeature.  If GDB regresses and stops asking
>> a question, the testsuite won't notice.  So I think that if a QUESTION
>> is specified, gdb_test should ensure it comes out of GDB.
> 
> I just ran into a neat use of this, or possibly a mis-use, depending on how much you like it. gdb.base/skip.exp uses:
> 
> gdb_test "step" "desired spot" "go to desired spot" "undesirable spot" "step"
> 
> as a simple way to handle a gcc 9.2.0 bug (misfeature) where gdb could stop in an undesirable spot without actually causing a failure in the test. This feels like a neat way to deal with compiler problems, as it would introduce a simple way to deal with clang's lack of epilogue, for instance.

This is:

    # With gcc 9.2.0 we jump once back to main before entering foo here.
    # If that happens try to step a second time.
    gdb_test "step" "foo \\(\\) at.*" "step 3" \
	"main \\(\\) at .*\r\n$gdb_prompt " "step"

I think this is a misuse.  That case doesn't really handle a GDB confirmation question.
What if you need to issue more steps, or the program may stop elsewhere for
different ports, and thus you need more than one regexp?  For the latter you can
use (REGEX1|REGEX1), but it isn't as nice as separate -re entries, IMHO.

IMHO, writing it with gdb_test_multiple in these cases is OK:

gdb_test_multiple "step" "step 3" {
  -re -wrap "foo \\(\\) at.*" {
     pass $gdb_test_multiple
  }
  -re -wrap "main \\(\\) at .*" {
    # With gcc 9.2.0 we jump once back to main before entering foo.
    # If that happens try another step.
    send_gdb "step\n"
    exp_continue
  }
}

It's a standard pattern we use in many places, I don't think it's bad enough that we need
to hide it.

> 
> I'm not suggesting that this patch be scrapped, but maybe this could be implemented on purpose, something like gdb_test_optional. The code itself LGTM, but I am not able to approve patches.
>