From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by sourceware.org (Postfix) with ESMTPS id 841F93858D39 for ; Thu, 3 Mar 2022 21:11:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 841F93858D39 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f43.google.com with SMTP id bi14-20020a05600c3d8e00b00386f2897400so310583wmb.5 for ; Thu, 03 Mar 2022 13:11:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:references:from:in-reply-to :content-transfer-encoding; bh=ieB1KWwmfn3eccHaqpFMph16Rey1PFzmez+wZxlm2MI=; b=RcAtz2PF8Tf95Grye3EUYJOFizhczBweeDhq17pX7bHP6QmOBH0b4rBXbaTM3N6sEB ritCSO1dofFXz8p9FZKWtU0nG085e7l9eME6hEiwdGlAjNcTrjAVxFYVaULs8T1I8XRO 5wfIUFDUAd5dh+l8Ux8VcTsm6gk/LOaEBUCJu6bmDPoMMZyjSn6nOhV5jIEXFid3gDVQ GKr3cT/UWnDWRrPpmEQ3flnSIj1Pp/qY8Rcd0VtguM9pjqFLD6MH08uozHK+5IajKmQ6 XkKTBtOTErPwmdG/MsbzC3+L2Ygw1kDRDbWfaXB+ysbvSSypYIkAscJ99EHYpk1tLLa6 68uw== X-Gm-Message-State: AOAM530hp4EV6hRgGXxCTYWohLggdYP0OUOgEioDJv23oUxXpszHwy+9 oFuFnPnNErmK6MI3mr45D6IYZXwhOA4= X-Google-Smtp-Source: ABdhPJxYwZMpj2HAXcRw3XkGNDB4mhTUfR43Fq+vJWk+0/eGIsLpv/Hh6HphxM8+1HhTahFvvD4DkQ== X-Received: by 2002:a1c:f315:0:b0:381:1f6d:6ca6 with SMTP id q21-20020a1cf315000000b003811f6d6ca6mr5235837wmq.25.1646341916236; Thu, 03 Mar 2022 13:11:56 -0800 (PST) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id v124-20020a1cac82000000b0037c3d08e0e7sm9897040wme.29.2022.03.03.13.11.54 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 03 Mar 2022 13:11:55 -0800 (PST) Message-ID: Date: Thu, 3 Mar 2022 21:11:50 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH v3 3/7] Catch gdb_exception_error instead of gdb_exception (in many places) Content-Language: en-US To: Kevin Buettner , gdb-patches@sourceware.org References: <20220227000051.3336149-1-kevinb@redhat.com> <20220227000051.3336149-4-kevinb@redhat.com> From: Pedro Alves In-Reply-To: <20220227000051.3336149-4-kevinb@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Mar 2022 21:11:58 -0000 Hi Kevin, I've started giving the series an initial skim, but haven't read it all yet. Meanwhile, here's one spot I noticed isn't correct. On 2022-02-27 00:00, Kevin Buettner wrote: > --- a/gdb/cli/cli-interp.c > +++ b/gdb/cli/cli-interp.c > @@ -368,7 +368,7 @@ safe_execute_command (struct ui_out *command_uiout, const char *command, > { > execute_command (command, from_tty); > } > - catch (gdb_exception &exception) > + catch (gdb_exception_error &exception) > { > e = std::move (exception); > } This one here, as is, can leave things in a messed up state. Note that the function returns the exception. And then the caller does this: gdb_exception cli_interp::exec (const char *command_str) { struct cli_interp *cli = this; struct ui_file *old_stream; struct gdb_exception result; /* gdb_stdout could change between the time cli_uiout was initialized and now. Since we're probably using a different interpreter which has a new ui_file for gdb_stdout, use that one instead of the default. It is important that it gets reset everytime, since the user could set gdb to use a different interpreter. */ old_stream = cli->cli_uiout->set_stream (gdb_stdout); result = safe_execute_command (cli->cli_uiout, command_str, 1); cli->cli_uiout->set_stream (old_stream); <<<<<<<<<<<< return result; } Note the "<<<<" line. If a QUIT exception propagates, the line doesn't execute. Looking at the caller higher up the in chain, we get to interpreter_exec_cmd, which does: for (i = 1; i < nrules; i++) { struct gdb_exception e = interp_exec (interp_to_use, prules[i]); if (e.reason < 0) { interp_set (old_interp, 0); error (_("error in command: \"%s\"."), prules[i]); } } That throws plain error, so it won't be sufficient to just revert your hunk, as that would convert a quit to a plain error. I don't know off hand why we need to have safe_execute_command and interp_exec catch the exception and return it via the normal return path. That's normally needed when we need to cross C code, like readline or ncurses, but that's not the case here. Maybe we can just eliminate safe_execute_command and let exceptions propagate normally.