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

[Simon Marchi]

On 2018-06-13 16:01, 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.  This patch makes GDB catch the evaluation error in the
> dprintf_after_condition_true function, and sets bpstat::stop to 1/true,
> so that the dprintf will cause a stop similar to a breakpoint hit.  The
> dprintf_print_it function defines how a "dprintf error hit" is printed.
> The result looks like this:
> 
>   (gdb) c
>   Continuing.
> 
>   Dprintf 2, failed to evaluate: No symbol "lalala" in current context.
>   foo (n=1) at
> /home/emaisin/src/binutils-gdb/gdb/testsuite/gdb.base/dprintf-error.c:30
> 
> When using MI, the stop is communicated using a new reason
> "dprintf-error", and the error-message field gives the text of the
> exception:
> 
>   *stopped,reason="dprintf-error",bkptno="2",error-message="No symbol
> \"lalala\" in current context.",frame=...
> 
>   #0  error (fmt=0x12bad88 "No symbol \"%s\" in current context.") at
> /home/emaisin/src/binutils-gdb/gdb/common/errors.c:39
>   #1  0x00000000006f5d49 in c_yyparse () at
> /home/emaisin/src/binutils-gdb/gdb/c-exp.y:1090
>   #2  0x00000000006fc082 in c_parse (par_state=0x7ffd3ed91c50) at
> /home/emaisin/src/binutils-gdb/gdb/c-exp.y:3273
>   #3  0x0000000000979d88 in parse_exp_in_context_1
> (stringptr=0x7ffd3ed91e00, pc=0, block=0x0, comma=1, void_context_p=0,
> out_subexp=0x0) at /home/emaisin/src/binutils-gdb/gdb/parse.c:1205
>   #4  0x0000000000979aae in parse_exp_in_context
> (stringptr=0x7ffd3ed91e00, pc=0, block=0x0, comma=1, void_context_p=0,
> out_subexp=0x0) at /home/emaisin/src/binutils-gdb/gdb/parse.c:1108
>   #5  0x0000000000979a2d in parse_exp_1 (stringptr=0x7ffd3ed91e00,
> pc=0, block=0x0, comma=1) at
> /home/emaisin/src/binutils-gdb/gdb/parse.c:1099
>   #6  0x000000000089ec1d in parse_to_comma_and_eval
> (expp=0x7ffd3ed91e00) at /home/emaisin/src/binutils-gdb/gdb/eval.c:126
>   #7  0x0000000000981a84 in ui_printf (arg=0x307f377 "\"hello %d\\n\",
> lalala", stream=0x2f34b90) at
> /home/emaisin/src/binutils-gdb/gdb/printcmd.c:2464
>   #8  0x000000000098212a in printf_command (arg=0x307f377 "\"hello
> %d\\n\", lalala", from_tty=0) at
> /home/emaisin/src/binutils-gdb/gdb/printcmd.c:2580
>   #9  0x0000000000603808 in do_const_cfunc (c=0x2ee6b00,
> args=0x307f377 "\"hello %d\\n\", lalala", from_tty=0) at
> /home/emaisin/src/binutils-gdb/gdb/cli/cli-decode.c:106
>   #10 0x0000000000606900 in cmd_func (cmd=0x2ee6b00, args=0x307f377
> "\"hello %d\\n\", lalala", from_tty=0) at
> /home/emaisin/src/binutils-gdb/gdb/cli/cli-decode.c:1857
>   #11 0x0000000000a61415 in execute_command (p=0x307f38a "a",
> from_tty=0) at /home/emaisin/src/binutils-gdb/gdb/top.c:630
>   #12 0x00000000006106f6 in execute_control_command_1 (cmd=0x2fd6f30,
> from_tty=0) at /home/emaisin/src/binutils-gdb/gdb/cli/cli-script.c:525
>   #13 0x0000000000610d2a in execute_control_command (cmd=0x2fd6f30,
> from_tty=0) at /home/emaisin/src/binutils-gdb/gdb/cli/cli-script.c:694
>   #14 0x000000000077e83f in bpstat_do_actions_1 (bsp=0x7ffd3ed922e8)
> at /home/emaisin/src/binutils-gdb/gdb/breakpoint.c:4433
>   #15 0x00000000007920e1 in dprintf_after_condition_true
> (bs=0x2fe9260) at
> /home/emaisin/src/binutils-gdb/gdb/breakpoint.c:13035
>   #16 0x000000000078057c in bpstat_stop_status (aspace=0x2f169e0,
> bp_addr=4195559, ptid=..., ws=0x7ffd3ed927a0, stop_chain=0x2fe9260) at
> /home/emaisin/src/binutils-gdb/gdb/breakpoint.c:5460
>   #17 0x0000000000908760 in handle_signal_stop (ecs=0x7ffd3ed92780) at
> /home/emaisin/src/binutils-gdb/gdb/infrun.c:5946
>   #18 0x0000000000907463 in handle_inferior_event_1
> (ecs=0x7ffd3ed92780) at
> /home/emaisin/src/binutils-gdb/gdb/infrun.c:5375
>   #19 0x00000000009075aa in handle_inferior_event (ecs=0x7ffd3ed92780)
> at /home/emaisin/src/binutils-gdb/gdb/infrun.c:5410
>   #20 0x000000000090449a in fetch_inferior_event (client_data=0x0) at
> /home/emaisin/src/binutils-gdb/gdb/infrun.c:3924
>   #21 0x00000000008efe47 in inferior_event_handler
> (event_type=INF_REG_EVENT, client_data=0x0) at
> /home/emaisin/src/binutils-gdb/gdb/inf-loop.c:43
>   #22 0x000000000090ef4b in infrun_async_inferior_event_handler
> (data=0x0) at /home/emaisin/src/binutils-gdb/gdb/infrun.c:9164
>   #23 0x00000000008aaa43 in check_async_event_handlers () at
> /home/emaisin/src/binutils-gdb/gdb/event-loop.c:1064
>   #24 0x00000000008a9375 in gdb_do_one_event () at
> /home/emaisin/src/binutils-gdb/gdb/event-loop.c:326
>   #25 0x00000000008a9426 in start_event_loop () at
> /home/emaisin/src/binutils-gdb/gdb/event-loop.c:371
>   #26 0x000000000093d38f in captured_command_loop () at
> /home/emaisin/src/binutils-gdb/gdb/main.c:330
>   #27 0x000000000093e87a in captured_main (data=0x7ffd3ed929e0) at
> /home/emaisin/src/binutils-gdb/gdb/main.c:1157
>   #28 0x000000000093e945 in gdb_main (args=0x7ffd3ed929e0) at
> /home/emaisin/src/binutils-gdb/gdb/main.c:1173
>   #29 0x0000000000411f5c in main (argc=10, argv=0x7ffd3ed92ae8) at
> /home/emaisin/src/binutils-gdb/gdb/gdb.c:32

Ping.  Since there would be multiple ways of handling this, I'd like to 
get a second opinion to know if this makes sense.

Simon