From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8782 invoked by alias); 23 Aug 2018 00:03:27 -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 8600 invoked by uid 89); 23 Aug 2018 00:03:26 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham version=3.3.2 spammy=371, owned, Ping, sk:check_a X-HELO: sessmg23.ericsson.net Received: from sessmg23.ericsson.net (HELO sessmg23.ericsson.net) (193.180.251.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Aug 2018 00:03:22 +0000 DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/simple; q=dns/txt; i=@ericsson.com; t=1534982600; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:CC:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=lpatWbB9bnz4e1pfzdLCa5J9GjnvvOxozwsI3bWpK9k=; b=Ulrt88I+IVTtLzSkM6w1PzcbjE3iJKWqlnJp5RNq7XUUiAETWkbwXqm5QUIgGDQW bLR9yqlOtlwbjGfWe8rUW/NJ5CLVFbv93+53aGhdRY2VQeZjCwwAgX2sIURk7/yw /8r0H3KTDmvpGVtnPZi9Alvlvihpk2dBW8wwU2AOVV8=; Received: from ESESBMB504.ericsson.se (Unknown_Domain [153.88.183.117]) by sessmg23.ericsson.net (Symantec Mail Security) with SMTP id 31.E9.22015.8C9FD7B5; Thu, 23 Aug 2018 02:03:20 +0200 (CEST) Received: from ESESSMB504.ericsson.se (153.88.183.165) by ESESBMB504.ericsson.se (153.88.183.171) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Thu, 23 Aug 2018 02:03:20 +0200 Received: from NAM04-CO1-obe.outbound.protection.outlook.com (153.88.183.157) by ESESSMB504.ericsson.se (153.88.183.165) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Thu, 23 Aug 2018 02:03:19 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=GNeL7G2tqSpjt7xlJ9dlywZZW8YgMZWrm5IOmzRilz0=; b=lYveoPTXyjYWcAoAYFiOw3fvEPaeQQaeovF/J9THTEi7JhjunBSnefPU3/z3oxPKVyG+Xu5hZTS07sEztHD/Mc31VT5cpcwWRfb9xTq79DBTiqzwRREHTYhCe2c/LQqjQkzfuDYKOGTlBpgOKjGEqNeWzHkVuizLoDcwryI7po0= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from [10.0.0.110] (192.222.164.54) by BYAPR15MB2389.namprd15.prod.outlook.com (2603:10b6:a02:8c::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1059.19; Thu, 23 Aug 2018 00:03:15 +0000 Subject: Re: [PATCH] Handle dprintf argument evaluation errors better (PR gdb/16551) To: Simon Marchi CC: , Eli Zaretskii References: <1528920116-26170-1-git-send-email-simon.marchi@ericsson.com> From: Simon Marchi Message-ID: <8284ef0e-7f4b-b29e-0a55-2342ad261632@ericsson.com> Date: Thu, 23 Aug 2018 00:03:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-Path: simon.marchi@ericsson.com Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00559.txt.bz2 On 2018-06-27 10:31 AM, Simon Marchi wrote: > 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 Eli, would it be possible to take a look at least at the NEWS/doc part of this patch? If there are no comments on the behavior change, I would be ready to merge this patch, but I'd like to make sure that NEWS/doc is ok first. Thanks, Simon