Re: [PATCH] Handle dprintf argument evaluation errors better (PR gdb/16551)

[Pedro Alves <palves@redhat.com>]

On 06/13/2018 09:01 PM, Simon Marchi wrote:
> The current behavior when GDB fails to evaluate the arguments of a
> dynamic printf is not very good.
> 
> There was a previous attempt at fixing this here, but it did not went
> through:
>   https://sourceware.org/ml/gdb-patches/2015-02/msg00258.html
> 
> The issue can be reproduced by setting a dprintf referring to a variable
> that doesn't exist or that triggers a memory error:
> 
>   dprintf hello, "hello %d\n", *((int*)0)
>   dprintf hello, "hello %d\n", doesnt_exist
> 
> When an evaluation error occurs, an error is thrown at the stack trace
> shown below and is caught in start_event_loop.  This leaves things in a
> relatively bad shape:
> 
> - Printing the error in start_event_loop causes GDB to receive a SIGTTOU
>   signal, because the terminal is still owned by the inferior at that
>   point.
> - There is an error message (e.g. No symbol "foo" in current context)
>   and you are back to the GDB prompt, but nothing gives a clue about the
>   context of the error.
> - The thread that hit the dprintf is stopped.  If in all-stop mode on an
>   all-stop-on-top-of-non-stop target, you end up with a single thread
>   stopped and the others running, which is not good.
> - With MI, the thread(s) is/are stopped but no *stopped event is sent,
>   so frontends still show it as running.
> 
> I actually think it is nice that the program stops if there is an
> error, so you can notice the problem and fix it.  It just needs to be
> handled better.  

I'm not so sure about that.  Consider a dprintf like this:

  int *some_int_global;

  dprintf funct, "some_int_global: %d\n", *some_int_global

and during most of the run, some_int_global points somewhere
valid (or some more complex expression involving pointers,
like a->b->c->d, you get the idea).  However, at some point, maybe
after days of a long running hard-to-collect-data debug session,
the pointer points to NULL or to garbage.

If I understood correctly, that makes the dprintf stop the thread.
That doesn't seem desirable to me.

Also, if that dprintf is handled on the target side, then GDB is
not notified of the error either, and no thread stops, right?

It would seem to me better, in terms of the spirit of the
command (non-intrusive dynamic logging), to catch the error,
print something like "<error: xxxxx>" and let the thread/program
run.

Thanks,
Pedro Alves