From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 64711 invoked by alias); 27 Jun 2018 14:32:02 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 62960 invoked by uid 89); 27 Jun 2018 14:32:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=1099, 1090, 1064 X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 27 Jun 2018 14:31:59 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w5REVqtM005819 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 27 Jun 2018 10:31:57 -0400 Received: by simark.ca (Postfix, from userid 112) id 982711EF28; Wed, 27 Jun 2018 10:31:52 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 7F6A91E059; Wed, 27 Jun 2018 10:31:51 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 27 Jun 2018 14:32:00 -0000 From: Simon Marchi To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Handle dprintf argument evaluation errors better (PR gdb/16551) In-Reply-To: <1528920116-26170-1-git-send-email-simon.marchi@ericsson.com> References: <1528920116-26170-1-git-send-email-simon.marchi@ericsson.com> Message-ID: X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00650.txt.bz2 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