Re: [RFA] When getting the locno of a bpstat, handle the case of bp with null locations.

[Simon Marchi <simark@simark.ca>]

On 11/20/22 12:30, Philippe Waroquiers via Gdb-patches wrote:
> The test py-objfile.exp unloads the current file while debugging the process.
> This results in bpstat bs->b->loc to become nullptr.
> Handle this case in breakpoint.c:bpstat_locno.
> 
> Note: GDB crashes on this problem with an internal error,
> but the end of gdb summary shows:
>   ...
>                   === gdb Summary ===
> 
>   # of expected passes		36
> 
> The output also does not contain a 'FAIL:'.
> After the dix, the nr of expeted passes increased.

dix->fix
expeted -> expected

> 
> In the gdb.log output, one can see:
>   ...
>   Fatal signal: Segmentation fault
>   ----- Backtrace -----
>   0x55698905c5b9 gdb_internal_backtrace_1
>           ../../binutils-gdb/gdb/bt-utils.c:122
>   0x55698905c5b9 _Z22gdb_internal_backtracev
>   ...
> 
>   ERROR: Couldn't send python print(objfile.filename) to GDB.
>   ERROR: : spawn id exp9 not open
>       while executing
>   "expect {
>   -i exp9 -timeout 10
>           -re ".*A problem internal to GDB has been detected" {
>               fail "$message (GDB internal error)"
>               gdb_internal_error..."
>       ("uplevel" body line 1)
>       invoked from within
>   ....
> 
> Wondering if it might be possible to improve gdb_test to have
>   gdb_test "python print(objfile.filename)" "None" \
>       "objfile.filename after objfile is unloaded"
> reporting a failed result instead of just producing the internal error.

I think an UNRESOLVED would be appropriate here.  Normally, it should do
that automatically.  The perror here:

https://gitlab.com/gnutools/binutils-gdb/-/blob/84f9fbe90e5429adb9dee68f04f44c92fa9e2345/gdb/testsuite/lib/gdb.exp#L1183

... should make it so the next pass/fail becomes an UNRESOLVED.
However, we don't even reach a pass / fail, as the expect call throws
the error you pasted above, about the spawn id not being open.  This
mechanism works if GDB hasn't crashed yet when entering
gdb_test_multiple, and it's the command gdb_test_multiple sends that
crashes GDB.  But here, what crashed GDB is the previous gdb_unload
call, which uses bare expect, leaving no trace of the crash (in terms of
test result).

I propose the following changes to handle this situation better:

  - make gdb_unload use gdb_test_multiple, to make it record a test
    result and handle the different failure modes
  - instead of calling perror, manually call unresolved as soone as
    send_gdb returns an error, and return -1 immediately, to handle
    more gracefully the case where GDB is already crashed on entry

But this is orthogonal with your patch, I will send a separate series.

Your patch LGTM:

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon