From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by sourceware.org (Postfix) with ESMTPS id 1ADB13857C6E for ; Mon, 27 Sep 2021 18:34:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1ADB13857C6E 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-f49.google.com with SMTP id b192-20020a1c1bc9000000b0030cfaf18864so709525wmb.4 for ; Mon, 27 Sep 2021 11:34:07 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:from:to:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=oP/M5SmfrnDGb5ryn79uH61Xnu1B4ZnVo/jx1NDGqy4=; b=nkVLKX7JnRlxws2yAPoCuEjmBy0eT/KBwGUptsWKWyQWNKXGcp1KwsYXqsiyCpfZxO bm/7tR9Q/rtJ/hV/m2MB1Rkhgz6leXn2bOzkiPmhd6VH7kgdx3G56c42R5e5RFQhapDY 8yhYsBV1i90zzR+m9oD1oeS/9TXIlzrvQ8ovQvqQVJ2uVHDo9cDoBUyk/5ZPK0UK0VRp YPXShkbRMS0fdbeSsQGs14aHUOqoRrr6CzJ+ryqy+P2z/Xl5LytWn98LRrUl+GvUtUNK XJ54+YBLvwcRWrZdTLeETJ8z+gB0rcuNbaP0j4MfEyaM3gSU89LfNcGUbs/i1nFujtv3 jUAA== X-Gm-Message-State: AOAM53016vkxuiKOndjbo+sZHvclMyrqTlW1u/Y4VAZwiQ/8gSOM/tfE vjMTSP9aIF9xnfibp/FZP/uRVeHbCUg= X-Google-Smtp-Source: ABdhPJz+Ks6SKQfZ3R08p2phdsXEluyeeDkU+wO8NDA/O00MaSzD/qMrV6GdJtIIbbM99P3E4bWzng== X-Received: by 2002:a05:600c:154f:: with SMTP id f15mr579320wmg.92.1632767645619; Mon, 27 Sep 2021 11:34:05 -0700 (PDT) Received: from ?IPv6:2001:8a0:f932:6a00:46bc:d03b:7b3a:2227? ([2001:8a0:f932:6a00:46bc:d03b:7b3a:2227]) by smtp.gmail.com with ESMTPSA id j4sm17723256wrt.67.2021.09.27.11.34.04 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 27 Sep 2021 11:34:04 -0700 (PDT) Subject: Re: [PATCH v2 6/6] QUIT processing w/ explicit sync_quit_force_run check From: Pedro Alves To: Kevin Buettner , gdb-patches@sourceware.org References: <20210822231959.184061-1-kevinb@redhat.com> <20210822231959.184061-7-kevinb@redhat.com> Message-ID: <01edd34b-db26-ac34-10df-4b15ed66ea8c@palves.net> Date: Mon, 27 Sep 2021 19:34:03 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <20210822231959.184061-7-kevinb@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.3 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 autolearn=ham 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: Mon, 27 Sep 2021 18:34:08 -0000 On 2021-08-23 12:20 a.m., Kevin Buettner wrote: > Aside from the extension language updates, I found two other > cases which required explicit checks of sync_quit_force_run... > > 1) remote_fileio_request in gdb/remote-fileio.c: > > The try block calls do_remote_fileio_request which can (in turn) > call one of the functions in remote_fio_func_map[]. Taking the > first one, remote_fileio_func_open(), we have the following call > path to maybe_quit(): > > "_ZL23remote_fileio_func_openP13remote_targetPc" > -> "_Z18target_read_memorymPhl" > -> "_Z11target_readP10target_ops13target_objectPKcPhml" > -> "_Z10maybe_quitv"; These would be easier to read if you pass them through c++filt. $ cat /tmp/foo | c++filt "remote_fileio_func_open(remote_target*, char*)" -> "target_read_memory(unsigned long, unsigned char*, long)" -> "target_read(target_ops*, target_object, char const*, unsigned char*, unsigned long, long)" -> "maybe_quit()"; > > Since there is a path to maybe_quit(), we must ensure that the > catch block is not permitted to swallow a QUIT representing a > SIGTERM. > > However, for this case, we must take care not to change the way > that Ctrl-C / SIGINT is handled; we want to send a suitable > EINTR reply to the remote target should that happen. That > being the case, I added an explicit check for sync_quit_force_run; > if set, it rethrows the exception. Again makes me wonder whether a separate exception type for sigterm wouldn't address this whole thing better. > > 2) mi_execute_command in gdb/mi/mi-main.c: > > The try block calls captured_mi_execute_command(); there exists > a call path to maybe_quit(): > > "_ZL27captured_mi_execute_commandP6ui_outP8mi_parse" > -> "_ZL14mi_cmd_executeP8mi_parse" > -> "_Z17get_current_framev" > -> "_ZL23get_prev_frame_always_1P10frame_info" > -> "_ZL30frame_register_unwind_locationP10frame_infoiPiP9lval_typePmS1_" > -> "_Z21frame_register_unwindP10frame_infoiPiS1_P9lval_typePmS1_Ph" > -> "_Z24value_entirely_availableP5value" > -> "_Z16value_fetch_lazyP5value" > -> "_ZL23value_fetch_lazy_memoryP5value" > -> "_Z17read_value_memoryP5valuelimPhm" > -> "_Z10maybe_quitv"; > > That being the case, we can't allow the exception handler (catch block) > to swallow a gdb_exception_quit for SIGTERM. However, it does seem > reasonable to output the exception via the mi interface so that some > suitable message regarding SIGTERM might be printed; therefore, I > do the check of sync_quit_force_run and the potential throw at the > end of the catch block. > > (While doing all of this, it's occurred to me that these checks of the > global could be avoided by introducing a new exception for SIGTERM. > I haven't explored the implications of doing this though.) Ah-ha! Yeah. If we don't do that, then this patch seems fine.